fix(backend): preserve custom claims when verifying JWT M2M tokens#8697
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 95eb4a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Snapi: no API changes detected in |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR reconstructs and preserves user-defined claims for JWT-format M2M tokens by adding M2M_RESERVED_JWT_CLAIMS and an extractCustomClaims(payload) helper that removes only structural fields (iss, sub, exp, nbf, iat, jti). M2MToken.fromJwtPayload now sets token.claims from extractCustomClaims(payload) (or null if none). Tests were updated and added to verify reserved fields are stripped, custom claims (e.g., aud, permissions, role, scopes) are retained, scopes are parsed into token.scopes, and an integration test asserts m2mApi.verify preserves embedded custom claims. A package changeset documents the behavior. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/backend/src/api/resources/M2MToken.ts (1)
20-34: 💤 Low valueConsider enhancing JSDoc with structured tags.
The function documentation is clear, but adding
@param,@returnstags would improve consistency with project JSDoc standards for better IDE tooling support.📝 Suggested JSDoc enhancement
/** * Reconstructs the custom claims that were attached at token creation by * stripping the reserved JWT/M2M claims from the verified payload. Returns * `null` when no custom claims are present, matching the opaque-token path * where a token created without claims verifies back to `claims: null`. + * + * `@param` payload - The verified JWT payload containing both reserved and custom claims + * `@returns` The custom claims object, or null if no custom claims are present */🤖 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/backend/src/api/resources/M2MToken.ts` around lines 20 - 34, Add structured JSDoc tags to the existing comment for function extractCustomClaims: include an `@param` tag describing the payload parameter (type M2MJwtPayload and that it contains the verified JWT claims) and an `@returns` tag describing the return type (Record<string, any> | null and when null is returned), and optionally mention that reserved keys in M2M_RESERVED_JWT_CLAIMS are stripped; keep the original summary but append these tags to match project JSDoc standards.
🤖 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.
Nitpick comments:
In `@packages/backend/src/api/resources/M2MToken.ts`:
- Around line 20-34: Add structured JSDoc tags to the existing comment for
function extractCustomClaims: include an `@param` tag describing the payload
parameter (type M2MJwtPayload and that it contains the verified JWT claims) and
an `@returns` tag describing the return type (Record<string, any> | null and when
null is returned), and optionally mention that reserved keys in
M2M_RESERVED_JWT_CLAIMS are stripped; keep the original summary but append these
tags to match project JSDoc standards.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5a1315be-178f-45d0-8537-991c39cf0553
📒 Files selected for processing (4)
.changeset/m2m-jwt-custom-claims.mdpackages/backend/src/api/__tests__/M2MTokenApi.test.tspackages/backend/src/api/resources/M2MToken.tspackages/backend/src/api/resources/__tests__/M2MToken.test.ts
|
@wobsoriano I pushed some additional hardening and logic changes.. can you re-review? |
thanks, I reviewed and it's good |
|
Thanks @wobsoriano ! 🏆 |
JWT-format M2M tokens came back from
client.m2m.verify()(and request-levelauth()) withclaims: nullno matter what was baked into the token.M2MToken.fromJwtPayloadhardcoded theclaimsconstructor slot tonull, so custom claims were dropped on the JWT path even though the opaque path returned them fine.The JWT path is fully local (decode + signature check), so the payload is the only source of claims. The fix reconstructs them by excluding the reserved JWT/M2M claims that already map to their own fields. That denylist is the load-bearing part worth a look:
A token with no custom claims still returns
claims: null, matching the opaque path.Fixes #8680.