feat(auth0-auth-js): add actor token support and act claim to Custom Token Exchange#175
Conversation
77b32f4 to
1ef94d5
Compare
|
|
||
| tokenResponse.tokenType = response.token_type; | ||
| tokenResponse.issuedTokenType = (response as typeof response & { issued_token_type?: string }).issued_token_type; | ||
| const atClaims = decodeJwt(response.access_token); |
There was a problem hiding this comment.
What will happen if Auth0 issues an opaque token.
There was a problem hiding this comment.
yes, this condition needs to be handled. Updated to catch the error when it's a opaque token and populate the act claim to be undefined.
| tokenResponse.tokenType = response.token_type; | ||
| tokenResponse.issuedTokenType = (response as typeof response & { issued_token_type?: string }).issued_token_type; | ||
| const atClaims = decodeJwt(response.access_token); | ||
| tokenResponse.act = (claims?.act ?? atClaims.act) as ActClaim | undefined; |
There was a problem hiding this comment.
Does that mean, tokenResponse would always have act ?
Because fromTokenEndpointResponse is called from multiple places.
There was a problem hiding this comment.
yes, tokenResponse from any exchange would populate the act claim, but it will be undefined except response from exchangeProfileToken.
I agree, the static method fromTokenEndpointResponse is called at many places. And none of them need the act claim population logic except exchangeProfileToken.
It seems, a better design choice will be to keep the act claim population logic in that specific method itself.
Update: The act claim population logic has been -
- Removed from
fromTokenEndpointResponsestatic method. - Added in
exchangeProfileTokenmethod, which is the only method that would ever populate it.
What do you suggest!
|
| * the acting party on whose behalf the subject token was exchanged. | ||
| * | ||
| * @see {@link https://www.rfc-editor.org/rfc/rfc8693#section-4.1 RFC 8693 Section 4.1} | ||
| */ |
There was a problem hiding this comment.
Same as the ActClaim interface above: act is defined in Section 4.4, not 4.1. Should be #section-4.4.
|
|
||
| validateSubjectToken(options.subjectToken); | ||
| validateTokenTypeUri(options.subjectTokenType, 'subjectTokenType'); | ||
|
|
There was a problem hiding this comment.
Two concerns here:
-
The team aligned that token-type URI validation stays server-side, no other SDK (Swift, Android, spa-js) does client-side URI validation. Adding it here for
subjectTokenType(which previously had none) breaks that alignment and is a behavior change for existing callers. -
new URL()is the wrong validator and risks false rejections. It can reject values the Auth0 server would actually accept, turning a server-side concern into a client-side breaking change on upgrade. (new URL('urn:acme:custom-token')happens to pass, but the general class of valid token-type identifiers isn't guaranteed to parse as a WHATWG URL.)
There was a problem hiding this comment.
Yes, that makes complete sense.
Removed the entire URI validation logic.
|
Warning Review limit reached
More reviews will be available in 6 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds RFC 8693 actor-token support: new types, request validation/wiring to include actor_token/actor_token_type, extraction of the act claim from id_token or JWT access_token, expanded tests, and example docs; several tests were refactored to use generated access tokens. ChangesActor Token Delegation Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/auth0-auth-js/src/auth-client.ts (1)
772-799:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate actor-token pairs symmetrically.
Line 772 treats
actorToken: ''as “provided”, but Line 796 drops it because the append branch is truthy-based. That silently downgrades a delegation exchange into a plain subject-token exchange instead of failing fast.actorTokenTypewithoutactorTokenis also accepted and then ignored, even though the public contract says the pair must be used together.Proposed fix
- if (options.actorToken !== undefined && options.actorTokenType === undefined) { - throw new TokenExchangeError('actorTokenType is required when actorToken is provided'); - } - if (options.actorTokenType !== undefined) { - validateTokenTypeUri(options.actorTokenType, 'actorTokenType'); + if ((options.actorToken === undefined) !== (options.actorTokenType === undefined)) { + throw new TokenExchangeError('actorToken and actorTokenType must be provided together'); + } + if (options.actorToken !== undefined) { + if (options.actorToken.trim().length === 0) { + throw new TokenExchangeError('actorToken cannot be blank or whitespace'); + } + validateTokenTypeUri(options.actorTokenType, 'actorTokenType'); } @@ - if (options.actorToken) { + if (options.actorToken !== undefined) { tokenRequestParams.append('actor_token', options.actorToken); tokenRequestParams.append('actor_token_type', options.actorTokenType!); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/auth0-auth-js/src/auth-client.ts` around lines 772 - 799, The code treats options.actorToken='' as provided but later ignores falsy values when appending, and allows actorTokenType without actorToken; update the validation in the token exchange branch so the actor pair is treated symmetrically: treat an empty string as "provided" (i.e. if actorToken is !== undefined but actorToken === '' or actorTokenType is missing, throw TokenExchangeError), and likewise if actorTokenType is provided without actorToken throw; only call validateTokenTypeUri('actorTokenType') and append actor_token/actor_token_type to tokenRequestParams when both options.actorToken and options.actorTokenType are present and non-empty (use explicit !== undefined and !== '' checks), and keep references to TokenExchangeError, validateTokenTypeUri, options.actorToken, options.actorTokenType, and tokenRequestParams to locate the changes.
🧹 Nitpick comments (1)
packages/auth0-auth-js/src/auth-client.spec.ts (1)
3036-3059: ⚡ Quick winConsider adding test coverage for opaque access tokens.
The test suite covers M2M delegation when the access token is a JWT, but there's no explicit test for opaque (non-JWT) access tokens. Based on the implementation (context snippet shows a try-catch around
decodeJwt), opaque tokens should be handled gracefully withactremaining undefined.🧪 Suggested test for opaque token handling
Add a test after line 3059:
+ test('should handle opaque access tokens gracefully (act undefined when decode fails)', async () => { + const authClient = new AuthClient({ domain, clientId: '<client_id>', clientSecret: '<client_secret>' }); + + const opaqueAccessToken = 'opaque-token-not-a-jwt'; + + server.use( + http.post(mockOpenIdConfiguration.token_endpoint, async () => { + return HttpResponse.json({ + access_token: opaqueAccessToken, + expires_in: 3600, + token_type: 'Bearer', + scope: 'read:data', + // no id_token — M2M flow + }); + }) + ); + + const result = await authClient.exchangeToken(baseOptions); + + expect(result.accessToken).toBe(opaqueAccessToken); + expect(result.act).toBeUndefined(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/auth0-auth-js/src/auth-client.spec.ts` around lines 3036 - 3059, Add a new unit test after the existing M2M JWT test that verifies opaque access tokens are handled: simulate the token endpoint returning a non-JWT opaque access_token (e.g., a random string like 'opaque-token-123') with no id_token, call AuthClient.exchangeToken (same baseOptions) and assert that the returned result.act is undefined; this exercises the decodeJwt error path and ensures no exception is thrown. Reference the existing test pattern and functions: AuthClient, exchangeToken, and the token endpoint mock used in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/auth0-auth-js/src/types.ts`:
- Around line 635-643: The docblock for the act property is inaccurate: update
the comment for the act?: ActClaim on TokenResponse to reflect runtime behavior
of `#exchangeProfileToken`() — it can be populated from the id_token or, if no
id_token is issued, from the JWT access_token. Mention both sources (id_token
and JWT access_token) and keep the reference to ActClaim and RFC 8693 for
context so consumers see the correct runtime semantics for TokenResponse.act.
---
Outside diff comments:
In `@packages/auth0-auth-js/src/auth-client.ts`:
- Around line 772-799: The code treats options.actorToken='' as provided but
later ignores falsy values when appending, and allows actorTokenType without
actorToken; update the validation in the token exchange branch so the actor pair
is treated symmetrically: treat an empty string as "provided" (i.e. if
actorToken is !== undefined but actorToken === '' or actorTokenType is missing,
throw TokenExchangeError), and likewise if actorTokenType is provided without
actorToken throw; only call validateTokenTypeUri('actorTokenType') and append
actor_token/actor_token_type to tokenRequestParams when both options.actorToken
and options.actorTokenType are present and non-empty (use explicit !== undefined
and !== '' checks), and keep references to TokenExchangeError,
validateTokenTypeUri, options.actorToken, options.actorTokenType, and
tokenRequestParams to locate the changes.
---
Nitpick comments:
In `@packages/auth0-auth-js/src/auth-client.spec.ts`:
- Around line 3036-3059: Add a new unit test after the existing M2M JWT test
that verifies opaque access tokens are handled: simulate the token endpoint
returning a non-JWT opaque access_token (e.g., a random string like
'opaque-token-123') with no id_token, call AuthClient.exchangeToken (same
baseOptions) and assert that the returned result.act is undefined; this
exercises the decodeJwt error path and ensures no exception is thrown. Reference
the existing test pattern and functions: AuthClient, exchangeToken, and the
token endpoint mock used in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6098b860-e4e0-41f3-969c-9809b79d5bef
📒 Files selected for processing (5)
packages/auth0-api-js/src/api-client.spec.tspackages/auth0-auth-js/src/auth-client.spec.tspackages/auth0-auth-js/src/auth-client.tspackages/auth0-auth-js/src/mfa/mfa-client.spec.tspackages/auth0-auth-js/src/types.ts
c44ba1d to
af8e6d6
Compare
- Added ActClaim interface (sub: string + open index signature for extra claims per RFC 8693)
- Added actorToken? and actorTokenType? to ExchangeProfileOptions
- Added act?: ActClaim property to TokenResponse
- Populated act from parsed ID token claims in fromTokenEndpointResponse()
auth-client.ts
- Removed actor_token and actor_token_type from PARAM_DENYLIST (they now have explicit typed params)
- Updated the denylist comment block to remove the stale bullet
- Added validateTokenTypeUri() — syntactic-only new URL() check, throws TokenExchangeError with a clear message
- In #exchangeProfileToken(): calls validateTokenTypeUri for both subjectTokenType and actorTokenType, then appends actor_token/actor_token_type to the request when present
auth-client.spec.ts
- New describe('exchangeToken — actor token support') block with 5 tests covering: params wired + act claim populated, act claim with extra fields, params absent when not provided, invalid URI validation (both params in one test), valid URI acceptance
… to profileexchangetoken method
…_toke and actor_token_type to param deny list
af8e6d6 to
ed6e9c8
Compare
…sumption, fix double claims() call, move passkey comments inside test bodies
Description
Extends the Custom Token Exchange (CTE) implementation in
auth0-auth-jsto support RFC 8693 delegation flows via actor tokens.actorTokenandactorTokenTypeoptional fields toExchangeProfileOptions, forwarded asactor_tokenandactor_token_typein the token request. Both remain inPARAM_DENYLISTto prevent injection viaextra— callers must use the typed fields.ActClaiminterface({ sub: string; [key: string]: unknown })per RFC 8693 Section 4.1. The index signature allows for additional server-provided fields (e.g. chained delegation,iss).act?: ActClaimtoTokenResponse, populated exclusively inside#exchangeProfileToken()(not in the sharedfromTokenEndpointResponse()). Prefers theactclaim from the verified ID token; falls back to decoding the JWT access token for M2M flows where no ID token is issued. Wrapped in try/catch to handle opaque access tokens gracefully.subjectTokenTypeoractorTokenType— invalid URIs are rejected by the Auth0 server, consistent with Swift, Android, and spa-js behaviour.Examples
Added a full "Custom Token Exchange" section to
EXAMPLES.mdcovering basic exchange, delegation with actor token, readingact, M2M fallback behaviour, and error handling.Testing
Unit Tests
result.actare correctly wirediss) onactare accessibleManual Tests
Tested end-to-end against a live Auth0 tenant using a locally built version.
The test covered a delegation exchange where a service actor exchanges a user's token on their behalf using
actorTokenandactorTokenTypeparameters. The following behaviours were verified:actclaim is present on theTokenResponse, correctly populated from the ID token with the actor identity set by the CTE Actionactor_tokenis present in the request, and the SDK handles the absence gracefullyChecklist