Skip to content

Update LogtoClient.php#30

Open
digood-dev wants to merge 1 commit into
logto-io:masterfrom
digood-dev:patch-1
Open

Update LogtoClient.php#30
digood-dev wants to merge 1 commit into
logto-io:masterfrom
digood-dev:patch-1

Conversation

@digood-dev
Copy link
Copy Markdown

修复用户资料中姓名为中文名(例如“测试”两个字)时,响应的base64文本格式标准不一的问题
Fix the problem where the response base64 text format is inconsistent when the user's name in the profile is in Chinese.

修复用户资料中姓名为中文名时,响应的base64文本格式标准不一的问题
Copy link
Copy Markdown

@darcyYe darcyYe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. The base64url handling change makes sense, but two issues still need to be addressed before this is ready:

  1. The fix only applies to getIdTokenClaims(). getAccessTokenClaims() still decodes the JWT payload with raw base64_decode(), so valid access tokens that require base64url normalization or padding will still fail. getOrganizationTokenClaims() inherits the same path.

  2. The test suite still only covers ASCII-only payloads that already worked before this patch. Please add a regression test whose payload actually requires base64url normalization or padding, and cover both the ID-token and access-token claim helpers so this bug class cannot regress silently.

Comment thread src/LogtoClient.php

$playloadDecode = base64_decode($data, true);

return new IdTokenClaims(...json_decode($playloadDecode, true));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only fixes base64url decoding for ID tokens. getAccessTokenClaims() still uses raw base64_decode() on the JWT payload, so valid access tokens that need URL-safe normalization or padding will still fail. Please extract this decode logic into a shared helper and reuse it for both token-claim paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants