feat(backend): Add support for JWTs in OAuth token type #7308
Conversation
🦋 Changeset detectedLatest commit: 150bb33 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds JWT detection and verification for OAuth access tokens across backend: new JWT helpers and fixtures, header-type assertion and VerifyJwt option, IdPOAuthAccessToken.fromJwtPayload factory, routing JWT-format machine tokens into OAuth verification flow, tests, and a new token verification failure code. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Req as Request
participant Auth as AuthHandler
participant Machine as MachineUtils
participant Verify as TokenVerifier
participant JWT as JWTUtils
participant Model as IdPModel
participant Err as ErrorHandler
Req->>Auth: authenticateRequest(token)
Auth->>Machine: isMachineToken(token)
alt machine token (JWT-format)
Machine-->>Auth: true
Auth->>Verify: verifyMachineAuthToken(token)
Verify->>Machine: isOAuthJwt(token)?
alt OAuth JWT
Machine-->>Verify: true
Verify->>JWT: decodeJwt(token)
JWT->>JWT: assertHeaderType(typ, allowedTypes)
JWT-->>Verify: payload
Verify->>Model: IdPOAuthAccessToken.fromJwtPayload(payload, clockSkewInMs)
Model-->>Verify: token instance
Verify-->>Auth: success
else Not OAuth JWT
Machine-->>Verify: false
Verify->>Verify: prefix-based verification (JWK/PEM)
end
else not machine token
Machine-->>Auth: false
Auth->>Verify: session / API-key verification
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
packages/backend/src/api/resources/IdPOAuthAccessToken.ts (2)
47-63: Consider validating required JWT claims before construction.The
jticlaim is used as the tokenid. Ifjtiis missing or empty, the resulting token will have an emptyid, which could cause issues in downstream logic that relies on token identification. Consider throwing aTokenVerificationErrorwhen required claims are missing rather than silently defaulting to empty strings.static fromJwtPayload(payload: JwtPayload, clockSkewInMs = 5000): IdPOAuthAccessToken { const oauthPayload = payload as OAuthJwtPayload; + if (!oauthPayload.jti) { + throw new TokenVerificationError({ + reason: TokenVerificationErrorReason.TokenVerificationFailed, + message: 'Invalid JWT jti claim. Expected a non-empty string.', + }); + } + // Map JWT claims to IdPOAuthAccessToken fields return new IdPOAuthAccessToken( - oauthPayload.jti ?? '', + oauthPayload.jti, oauthPayload.client_id ?? '',Note: The import for
TokenVerificationErrorandTokenVerificationErrorReasonwould need to be added from../errors.
5-10: Add JSDoc documentation for theOAuthJwtPayloadtype.Per coding guidelines, public APIs and types should be documented with JSDoc. This type extends
JwtPayloadwith OAuth-specific claims and would benefit from documentation explaining each field.+/** + * Extended JWT payload for OAuth 2.0 access tokens per RFC 9068. + * @see https://www.rfc-editor.org/rfc/rfc9068.html + */ type OAuthJwtPayload = JwtPayload & { + /** JWT ID - unique identifier for the token */ jti?: string; + /** OAuth 2.0 client identifier */ client_id?: string; + /** Space-delimited scope string */ scope?: string; + /** Array of scope values (alternative to space-delimited scope) */ scp?: string[]; };packages/backend/src/tokens/__tests__/verify.test.ts (2)
436-451: Consider removing verbose implementation comments.These comments explain the testing rationale in detail, which is useful during development but adds noise to the test file. The test description and assertions should be self-explanatory.
it('verifies JWT with typ application/at+jwt', async () => { server.use( http.get( 'https://api.clerk.test/v1/jwks', validateHeaders(() => { return HttpResponse.json(mockJwks); }), ), ); const validJtiPayload = { ...mockOAuthAccessTokenJwtPayload, jti: 'oat_12345678901234567890123456789012', }; - // We mock verifyJwt to ensure we don't hit the real signature check failure (since we use createJwt which signs with a dummy key/algo) - // But we want to test headerType validation logic. The real verifyJwt does signature check LAST. - // assertHeaderType is called BEFORE signature check. - // So if we mock verifyJwt, we skip assertHeaderType inside verifyJwt unless we spy on assertions? - // No, VerifyJwtModule.verifyJwt IS the function we call. - // If we mock it, we bypass the real logic entirely. - - // To test that verifyOAuthToken accepts 'application/at+jwt', we should let it call verifyJwt. - // But verifyJwt will fail signature if we don't provide correct key/signature. - // However, if we Mock verifyJwt, we are just testing verifyMachineAuthToken -> verifyOAuthToken flow, not verifyJwt internals. - - // Let's rely on the fact that verifyOAuthToken calls verifyJwt with headerType: ['at+jwt', 'application/at+jwt']. - // We verified that call in the first test ("verifies a valid OAuth JWT"). - - // But we can add a test case where we simulate a successful verifyJwt call with application/at+jwt. - const spy = vi.spyOn(VerifyJwtModule, 'verifyJwt').mockResolvedValueOnce({
477-491: Add more specific error assertions for invalid JWT format test.The test only checks that
errorsis defined. Consider asserting on the specific error code or message to ensure the correct error path is exercised.it('handles invalid JWT format', async () => { const invalidJwt = 'invalid.jwt.token'; - // Mock isJwt to return true so it enters verifyOAuthToken, - // OR rely on regex. Regex requires 3 parts. - // 'invalid.jwt.token' matches 3 parts. - - // But decodeJwt will fail if base64 is bad. const result = await verifyMachineAuthToken(invalidJwt, { apiUrl: 'https://api.clerk.test', secretKey: 'a-valid-key', }); expect(result.errors).toBeDefined(); + expect(result.errors?.[0].code).toBeDefined(); });packages/backend/src/tokens/machine.ts (2)
18-22: Consider exportingOAUTH_ACCESS_TOKEN_TYPESfor external consumers.This constant defines RFC 9068 compliant OAuth access token types and could be useful for consumers who need to validate or generate OAuth JWTs outside of this module.
/** * Valid OAuth 2.0 JWT access token type values per RFC 9068. * @see https://www.rfc-editor.org/rfc/rfc9068.html#section-2.1 */ -const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt'] as const; +export const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt'] as const;
24-34: Optimize redundant JWT decoding in machine token detection flow.Verified:
isMachineToken()(request.ts:105) callsisOAuthJwt()which decodes the JWT, then immediatelygetMachineTokenType()(request.ts:106) callsisOAuthJwt()again on the same token, causing redundant parsing. Consider memoizing the decoded JWT result, combining detection and type extraction, or refactoring to avoid dual decoding in the authentication hot path.packages/backend/src/tokens/verify.ts (3)
213-227: Redundant try-catch arounddecodeJwt.
decodeJwtreturns a result object with{ data, errors }rather than throwing exceptions. The outer try-catch on lines 215-227 is unnecessary since errors are already handled via the result pattern on lines 229-240.Consider simplifying:
- if (isJwt(accessToken)) { - let decoded: JwtReturnType<Jwt, TokenVerificationError>; - try { - decoded = decodeJwt(accessToken); - } catch (e) { - return { - tokenType: TokenType.OAuthToken, - errors: [ - new MachineTokenVerificationError({ - code: MachineTokenVerificationErrorCode.TokenInvalid, - message: (e as Error).message, - }), - ], - }; - } - - const { data: decodedResult, errors } = decoded; + if (isJwt(accessToken)) { + const { data: decodedResult, errors } = decodeJwt(accessToken);
282-294: Hardcoded jti format validation.The regex
/^oat_[0-9A-Za-z]{32}$/is hardcoded here. Consider extracting this pattern to a shared constant alongsideOAUTH_TOKEN_PREFIXinmachine.tsfor consistency and maintainability.
350-361: Remove duplicateisJwtand reuse exportedisJwtFormatfrom machine.ts.Both
isJwtin verify.ts (lines 359-361) andisJwtFormatin machine.ts are identical—same regex pattern and logic. Replace the duplicate function and regex with an import ofisJwtFormat:-import { API_KEY_PREFIX, M2M_TOKEN_PREFIX, OAUTH_TOKEN_PREFIX } from './machine'; +import { API_KEY_PREFIX, isJwtFormat, M2M_TOKEN_PREFIX, OAUTH_TOKEN_PREFIX } from './machine'; ... -const JwtFormatRegExp = /^[a-zA-Z0-9\-_]+\.[a-zA-Z0-9\-_]+\.[a-zA-Z0-9\-_]+$/; - -function isJwt(token: string): boolean { - return JwtFormatRegExp.test(token); -}Then update lines 213 and 350 to use
isJwtFormat(...)instead ofisJwt(...).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
.changeset/swift-sheep-notice.md(1 hunks)packages/backend/src/api/resources/IdPOAuthAccessToken.ts(2 hunks)packages/backend/src/errors.ts(2 hunks)packages/backend/src/fixtures/index.ts(1 hunks)packages/backend/src/jwt/assertions.ts(1 hunks)packages/backend/src/jwt/verifyJwt.ts(2 hunks)packages/backend/src/tokens/__tests__/machine.test.ts(2 hunks)packages/backend/src/tokens/__tests__/verify.test.ts(2 hunks)packages/backend/src/tokens/machine.ts(4 hunks)packages/backend/src/tokens/request.ts(4 hunks)packages/backend/src/tokens/verify.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/jwt/assertions.tspackages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/errors.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/request.tspackages/backend/src/tokens/machine.tspackages/backend/src/jwt/verifyJwt.tspackages/backend/src/api/resources/IdPOAuthAccessToken.tspackages/backend/src/tokens/verify.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/jwt/assertions.tspackages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/errors.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/request.tspackages/backend/src/tokens/machine.tspackages/backend/src/jwt/verifyJwt.tspackages/backend/src/api/resources/IdPOAuthAccessToken.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/jwt/assertions.tspackages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/errors.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/request.tspackages/backend/src/tokens/machine.tspackages/backend/src/jwt/verifyJwt.tspackages/backend/src/api/resources/IdPOAuthAccessToken.tspackages/backend/src/tokens/verify.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/jwt/assertions.tspackages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/errors.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/request.tspackages/backend/src/tokens/machine.tspackages/backend/src/jwt/verifyJwt.tspackages/backend/src/api/resources/IdPOAuthAccessToken.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/jwt/assertions.tspackages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/errors.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/request.tspackages/backend/src/tokens/machine.tspackages/backend/src/jwt/verifyJwt.tspackages/backend/src/api/resources/IdPOAuthAccessToken.tspackages/backend/src/tokens/verify.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/jwt/assertions.tspackages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/errors.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/request.tspackages/backend/src/tokens/machine.tspackages/backend/src/jwt/verifyJwt.tspackages/backend/src/api/resources/IdPOAuthAccessToken.tspackages/backend/src/tokens/verify.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/jwt/assertions.tspackages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/errors.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/request.tspackages/backend/src/tokens/machine.tspackages/backend/src/jwt/verifyJwt.tspackages/backend/src/api/resources/IdPOAuthAccessToken.tspackages/backend/src/tokens/verify.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/machine.test.tspackages/backend/src/tokens/__tests__/verify.test.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/backend/src/fixtures/index.ts
🧬 Code graph analysis (5)
packages/backend/src/jwt/assertions.ts (1)
packages/backend/src/errors.ts (5)
TokenVerificationError(41-69)TokenVerificationErrorAction(30-36)TokenVerificationErrorAction(38-39)TokenVerificationErrorReason(9-25)TokenVerificationErrorReason(27-28)
packages/backend/src/tokens/__tests__/verify.test.ts (6)
packages/backend/src/mock-server.ts (1)
server(6-6)packages/backend/src/fixtures/index.ts (2)
mockOAuthAccessTokenJwtPayload(36-46)createJwt(49-60).typedoc/custom-theme.mjs (1)
result(171-171)packages/backend/src/tokens/verify.ts (1)
verifyMachineAuthToken(340-355)packages/backend/src/internal.ts (1)
verifyMachineAuthToken(57-57)packages/backend/src/api/resources/IdPOAuthAccessToken.ts (1)
IdPOAuthAccessToken(12-65)
packages/backend/src/tokens/request.ts (1)
packages/backend/src/tokens/machine.ts (1)
isMachineToken(56-58)
packages/backend/src/tokens/machine.ts (2)
packages/backend/src/jwt/verifyJwt.ts (1)
decodeJwt(45-95)packages/backend/src/internal.ts (1)
isMachineTokenByPrefix(59-59)
packages/backend/src/jwt/verifyJwt.ts (3)
packages/backend/src/jwt/types.ts (1)
JwtReturnType(3-11)packages/backend/src/errors.ts (1)
TokenVerificationError(41-69)packages/backend/src/jwt/assertions.ts (1)
assertHeaderType(50-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (33)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (12)
packages/backend/src/tokens/__tests__/machine.test.ts (1)
98-139: Good test coverage for new JWT utilities.The tests comprehensively cover:
- Valid/invalid JWT format detection with boundary cases
- OAuth JWT type detection for both RFC 9068 compliant
typvalues (at+jwtandapplication/at+jwt)- Edge cases like malformed JWT strings
packages/backend/src/jwt/assertions.ts (1)
50-63: LGTM!The enhanced
assertHeaderTypefunction maintains backward compatibility with the default'JWT'value while supporting multiple allowed header types for OAuth JWT verification. The implementation correctly normalizes the input to an array and provides a clear error message.packages/backend/src/jwt/verifyJwt.ts (1)
122-127: LGTM!The
headerTypeoption is well-documented with an RFC reference, properly typed, and cleanly integrated into the verification flow.packages/backend/src/errors.ts (1)
73-110: LGTM!The error handling extensions are well-structured:
- New
TokenVerificationFailedcode for OAuth JWT verification errors- Optional
statusandactionfields maintain backward compatibility- The
getFullMessage()gracefully handles missing status with'n/a'packages/backend/src/tokens/machine.ts (1)
50-58: Good implementation of combined machine token detection.The
isMachineTokenfunction cleanly combines prefix-based detection with OAuth JWT recognition, properly extending machine token handling to support JWT-format OAuth tokens.packages/backend/src/tokens/request.ts (4)
17-17: LGTM!The import update correctly reflects the new
isMachineTokenpredicate which now includes OAuth JWT detection alongside prefix-based machine token detection.
101-113: LGTM!The function correctly uses
isMachineTokento determine token type, which now properly handles both prefix-based machine tokens and OAuth JWTs.
699-732: LGTM!The guard at line 707 correctly rejects non-machine tokens when the endpoint expects machine tokens. The updated predicate ensures OAuth JWTs are now recognized as valid machine tokens and routed to
verifyMachineAuthToken.
734-774: LGTM!The
authenticateAnyRequestWithTokenInHeaderfunction correctly usesisMachineTokenat line 742 to route OAuth JWTs through the machine token verification path, while regular session JWTs continue throughverifyToken.packages/backend/src/tokens/verify.ts (1)
264-268: Good addition: JWT header type validation for OAuth tokens.The
headerType: ['at+jwt', 'application/at+jwt']option correctly validates that the JWT header type conforms to RFC 9068 for access tokens.packages/backend/src/fixtures/index.ts (2)
48-60: LGTM!The
createJwthelper is well-designed with sensible defaults and proper base64url encoding. The type definition allows flexible override of header, payload, and signature.
62-65: LGTM!The
mockOAuthAccessTokenJwtcorrectly usestyp: 'at+jwt'header which aligns with the RFC 9068 validation inverifyOAuthToken.
wobsoriano
changed the title
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/backend/src/tokens/__tests__/verify.test.ts (2)
402-421: Consider mockingverifyJwtfor deterministic type validation testing.Unlike other tests in this block, this test doesn't mock
verifyJwt. The test relies on the real verification flow, but the mock JWT signature likely won't pass cryptographic verification, potentially failing before theheaderTypecheck. Consider mockingverifyJwtto return an error specifically for the type mismatch to ensure this test validates the intended behavior.it('fails if JWT type is not at+jwt or application/at+jwt', async () => { - server.use( - http.get( - 'https://api.clerk.test/v1/jwks', - validateHeaders(() => { - return HttpResponse.json(mockJwks); - }), - ), - ); - + vi.spyOn(VerifyJwtModule, 'verifyJwt').mockResolvedValueOnce({ + errors: [{ message: 'Invalid JWT type "JWT". Expected "at+jwt" or "application/at+jwt".' }], + }); + const oauthJwt = createOAuthJwt(mockOAuthAccessTokenJwtPayload, 'JWT'); // Wrong type const result = await verifyMachineAuthToken(oauthJwt, { apiUrl: 'https://api.clerk.test', secretKey: 'a-valid-key', }); expect(result.errors).toBeDefined(); expect(result.errors?.[0].message).toContain('Invalid JWT type'); });
460-469: Strengthen the assertion to verify the specific error.The current assertion only checks that
errorsis defined. Consider validating the error code and/or message for more robust testing, similar to other error cases in this file.it('handles invalid JWT format', async () => { const invalidJwt = 'invalid.jwt.token'; const result = await verifyMachineAuthToken(invalidJwt, { apiUrl: 'https://api.clerk.test', secretKey: 'a-valid-key', }); expect(result.errors).toBeDefined(); + expect(result.errors?.[0].code).toBe('token-invalid'); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/swift-sheep-notice.md(1 hunks)packages/backend/src/tokens/__tests__/verify.test.ts(2 hunks)packages/backend/src/tokens/verify.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/swift-sheep-notice.md
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/__tests__/verify.test.ts (4)
packages/backend/src/fixtures/index.ts (2)
mockOAuthAccessTokenJwtPayload(36-46)createJwt(49-60)packages/backend/src/mock-server.ts (2)
server(6-6)validateHeaders(9-47)packages/backend/src/tokens/verify.ts (1)
verifyMachineAuthToken(340-355)packages/backend/src/api/resources/IdPOAuthAccessToken.ts (1)
IdPOAuthAccessToken(12-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (32)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (10)
packages/backend/src/tokens/__tests__/verify.test.ts (4)
11-20: LGTM! Well-structured test helper.The helper function is appropriately scoped to the test file and provides a clean API for creating test JWTs with configurable
typheader values.
330-373: LGTM! Comprehensive test for valid OAuth JWT verification.The test correctly validates the token structure, data fields, and ensures
verifyJwtis called with the expectedheaderTypevalues.
375-400: LGTM!Correctly tests the jti validation logic with a mock that bypasses signature verification.
423-458: LGTM!Good coverage for the alternative
application/at+jwttyp value.packages/backend/src/tokens/verify.ts (6)
3-5: LGTM!Import changes correctly add the
Jwttype and adjust import structure forIdPOAuthAccessToken(now imported as a value for its static method usage).
213-240: LGTM!The JWT decoding and error handling follows the established pattern. The try-catch provides a defensive layer even though
decodeJwtreturns a result object.
242-262: LGTM!JWK resolution logic correctly follows the established pattern from
verifyToken.
264-280: LGTM!Correctly validates both
at+jwtandapplication/at+jwttyp values per RFC 9068.
296-309: LGTM!Token creation and error handling are correct. The
clockSkewInMsparameter is properly passed to align expiration calculations.
350-352: LGTM!JWT-format tokens are correctly routed to OAuth verification as a fallback after prefix checks.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/backend/src/tokens/verify.ts (1)
281-295: Confirm OAuth JWTjtiformat assumptions before finalizing the regexThe
jtivalidation currently accepts anyoat_-prefixed alphanumeric string:if (!jti || !/^oat_[0-9A-Za-z]+$/.test(jti)) { // ... }A previous review noted that production OAuth access token IDs may be fixed-length (e.g. exactly 32 chars after
oat_), and recommended tightening this regex accordingly, along with adjusting the default fixture value. Before merging, please double‑check the actual IdP OAuth access token ID format and decide whether this regex should enforce a fixed length (e.g.{32}) to match real IDs, or if variable length is intentional.
🧹 Nitpick comments (2)
packages/backend/src/tokens/__tests__/verify.test.ts (1)
329-460: JWT OAuth tests cover key paths; consider tightening the invalid-format assertionThe new
verifyOAuthToken with JWTsuite nicely exercises:
- happy path (including
headerTypepropagation),- invalid
jti,- invalid
typ,- alternate
typ(application/at+jwt),- and structurally invalid JWTs.
For the
"handles invalid JWT format"test, you currently only assert thatresult.errorsis defined. If you want stronger regression protection, you could also assert the machine errorcode(e.g.token-invalid) or part of the message, so future changes to error mapping don’t silently weaken guarantees.packages/backend/src/tokens/verify.ts (1)
212-308: Ensure error returns in the JWT OAuth path consistently populatedataThe new JWT-specific branch in
verifyOAuthTokenis structurally sound: it:
- guards with
isJwtFormat,- decodes the JWT with a defensive
try/catch,- resolves the JWK using
jwtKeyorsecretKey,- reuses
verifyJwtwithheaderType: ['at+jwt', 'application/at+jwt'],- validates
jtibefore mapping viaIdPOAuthAccessToken.fromJwtPayload.One small inconsistency: several error paths in this branch return only
{ tokenType, errors }withoutdata, while other helpers (likehandleClerkAPIError) always setdata: undefinedon failures. For a discriminatedMachineTokenReturnType, consistently includingdata: undefinedin all error cases makes the runtime shape predictable and avoids subtle checks on the presence of thedataproperty.You can align these branches by populating
data: undefinedwherever you return errors, e.g.:- return { - tokenType: TokenType.OAuthToken, - errors: [ + return { + data: undefined, + tokenType: TokenType.OAuthToken, + errors: [ new MachineTokenVerificationError({ code: MachineTokenVerificationErrorCode.TokenInvalid, message: (e as Error).message, }), ], };and similarly for the other error returns in this JWT block (
errorsfromdecodeJwt, JWK resolution failures,verifyJwtfailures, invalidjti, and the outercatch).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/backend/src/tokens/__tests__/verify.test.ts(2 hunks)packages/backend/src/tokens/verify.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/verify.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/verify.ts (4)
packages/backend/src/tokens/machine.ts (1)
isJwtFormat(14-16)packages/backend/src/jwt/verifyJwt.ts (2)
decodeJwt(45-95)verifyJwt(129-187)packages/backend/src/tokens/tokenTypes.ts (2)
TokenType(1-6)TokenType(11-11)packages/backend/src/tokens/keys.ts (2)
loadClerkJwkFromPem(52-86)loadClerkJWKFromRemote(131-173)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/backend/src/tokens/__tests__/verify.test.ts (1)
5-20: JWT fixtures import andcreateOAuthJwthelper look goodLocalizing the OAuth JWT builder in the test file and parameterizing
typgives you flexible coverage without coupling tests too tightly to fixtures. Defaulting the payload tomockOAuthAccessTokenJwtPayloadis reasonable here since the tests don’t mutate it.packages/backend/src/tokens/verify.ts (2)
2-5: Type imports and value imports are aligned with shared types usageSwitching to
@clerk/shared/typesforJwt,JwtPayload, andSimplify, and importingIdPOAuthAccessTokenas a value from../api, matches the guideline to prefer shared types and keeps tree‑shaking friendly exports.Also applies to: 18-18
349-351: JWT-format routing inverifyMachineAuthTokenis consistent with OAuth JWT supportAdding the
isJwtFormatbranch here:if (isJwtFormat(token)) { return verifyOAuthToken(token, options); }makes
verifyMachineAuthTokenhandle both opaque OAuth IDs (oat_…) and bare OAuth JWTs. Given the earlier prefix checks for M2M, OAuth, and API keys, this shouldn’t interfere with existing machine token flows and keeps the new JWT behavior nicely encapsulated inverifyOAuthToken.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/backend/src/fixtures/index.ts (1)
48-66: TightenCreateJwtoption types to avoidanyand clarify API surfaceThe
createJwtlogic is good, but the current signature usesanyforheaderandpayload, which goes against the repo’s TypeScript guidelines and reduces type safety. You can keep the same runtime behavior while improving typings with a small refactor, e.g.:-type CreateJwt = (opts?: { header?: any; payload?: any; signature?: string }) => string; -export const createJwt: CreateJwt = ({ header, payload, signature = mockJwtSignature } = {}) => { +type CreateJwtOptions = { + header?: Record<string, unknown>; + payload?: Record<string, unknown>; + signature?: string; +}; + +type CreateJwt = (opts?: CreateJwtOptions) => string; + +export const createJwt: CreateJwt = ({ header, payload, signature = mockJwtSignature } = {}) => { const encoder = new TextEncoder();This keeps the helper flexible for tests while avoiding
anyand making the API a bit clearer. As per coding guidelines, this better aligns with the “noanytypes” and “use proper type annotations” recommendations.After adjusting the types, please run the existing TypeScript build/tests (e.g.,
pnpm buildorpnpm test) to confirm there are no downstream type errors wherecreateJwtis used.packages/backend/src/tokens/verify.ts (1)
214-226: Remove redundant try-catch arounddecodeJwt.The
decodeJwtfunction returns errors viaJwtReturnType<Jwt, TokenVerificationError>and doesn't throw exceptions (seepackages/backend/src/jwt/verifyJwt.ts). The error handling at lines 228-239 already covers the error case. The try-catch here only catches unexpected runtime errors, adding unnecessary nesting.Apply this diff to simplify the error handling:
- let decoded: JwtReturnType<Jwt, TokenVerificationError>; - try { - decoded = decodeJwt(accessToken); - } catch (e) { - return { - tokenType: TokenType.OAuthToken, - errors: [ - new MachineTokenVerificationError({ - code: MachineTokenVerificationErrorCode.TokenInvalid, - message: (e as Error).message, - }), - ], - }; - } - - const { data: decodedResult, errors } = decoded; + const { data: decodedResult, errors } = decodeJwt(accessToken);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/backend/src/fixtures/index.ts(1 hunks)packages/backend/src/tokens/verify.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/fixtures/index.tspackages/backend/src/tokens/verify.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/fixtures/index.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/fixtures/index.tspackages/backend/src/tokens/verify.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/fixtures/index.tspackages/backend/src/tokens/verify.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/fixtures/index.tspackages/backend/src/tokens/verify.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/fixtures/index.tspackages/backend/src/tokens/verify.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/fixtures/index.tspackages/backend/src/tokens/verify.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/backend/src/fixtures/index.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/verify.ts (5)
packages/backend/src/tokens/machine.ts (2)
isJwtFormat(14-16)OAUTH_TOKEN_PREFIX(7-7)packages/backend/src/jwt/types.ts (1)
JwtReturnType(3-11)packages/backend/src/jwt/verifyJwt.ts (2)
decodeJwt(45-95)verifyJwt(129-187)packages/backend/src/tokens/tokenTypes.ts (2)
TokenType(1-6)TokenType(11-11)packages/backend/src/tokens/keys.ts (2)
loadClerkJwkFromPem(52-86)loadClerkJWKFromRemote(131-173)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (6)
packages/backend/src/fixtures/index.ts (1)
36-46: OAuth access token JWT payload fixture looks consistentThe payload structure looks solid for an OAuth access token JWT: it reuses the base claims, overrides
iss/sub, addsclient_id,scope, andjti, and keepsnbf < iat < expwith a short-lived TTL. Thejtivalue now conforms to the expectedoat_+ 32‑character suffix format used in verification logic.packages/backend/src/tokens/verify.ts (5)
2-2: LGTM: Import changes are correct.The import modifications appropriately support the JWT verification feature.
Jwtis correctly imported as a type,IdPOAuthAccessTokenis imported as a value (required for the.fromJwtPayload()call), andisJwtFormatis added for routing logic.Also applies to: 4-4, 18-18
245-279: LGTM: Key resolution and JWT verification logic.The key resolution follows the same pattern as
verifyToken(lines 117-132), ensuring consistency. TheheaderTypevalidation with['at+jwt', 'application/at+jwt']correctly enforces RFC 9068 OAuth 2.0 Access Token JWT format requirements. Error handling appropriately wraps verification failures withMachineTokenVerificationError.
295-297: LGTM: Token creation from JWT payload.The call to
IdPOAuthAccessToken.fromJwtPayload(payload, options.clockSkewInMs)correctly maps the verified JWT payload to the token structure. The timestamp unit issue mentioned in past reviews was addressed in commit bc57e96.
343-343: LGTM: Enhanced routing logic for JWT-format OAuth tokens.Adding
isJwtFormat(token)to the routing condition correctly handles OAuth tokens in JWT format (not just prefix-based tokens). This enables the new JWT verification path while maintaining backward compatibility with existing prefix-based OAuth tokens.
212-309: Comprehensive error handling in JWT verification flow.The error handling throughout the new JWT verification logic is well-structured with specific error codes for each failure scenario (decode, key resolution, verification, jti validation). The catch-all handler at lines 298-308 appropriately wraps unexpected errors, and all error returns maintain a consistent structure matching existing token verification functions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/backend/src/tokens/__tests__/verify.test.ts (2)
5-7: Consider restoringverifyJwtspies between tests to avoid cross-test leakageEach test creates a new
vi.spyOn(VerifyJwtModule, 'verifyJwt'), but the spies are never restored. While usingmockResolvedValueOncemeans later calls still hit the real implementation, keeping the spy installed can leak into other test files sharing the same module cache. Addingvi.restoreAllMocks()in anafterEachfor this file (orspy.mockRestore()in each test) would make the isolation explicit without changing behavior.Please re-run the suite with
vi.restoreAllMocks()enabled to confirm no hidden coupling on the spied function.
11-20: Helper is solid; consider reusing the existing KID constant instead of duplicating it
createOAuthJwtis a nice focused helper and thetypunion matches the test cases. To avoid a hard-coded KID string that can drift frommockJwtHeader.kid, you could read it frommockJwtHeader(or omit it entirely and letcreateJwt’s default header provide it). This keeps fixtures consistent if the header KID ever changes.packages/backend/src/fixtures/index.ts (1)
48-60: TightenCreateJwtparameter types instead of usingany
createJwtis a handy shared helper, but theheader?: any; payload?: any;inCreateJwtare looser than necessary. You can avoidanywhile still keeping the fixture flexible by using a simple index signature type, e.g.:-type CreateJwt = (opts?: { header?: any; payload?: any; signature?: string }) => string; +interface JwtFixtureHeader { + [key: string]: unknown; +} + +interface JwtFixturePayload { + [key: string]: unknown; +} + +type CreateJwt = (opts?: { + header?: JwtFixtureHeader; + payload?: JwtFixturePayload; + signature?: string; +}) => string;This keeps the helper ergonomics the same but respects the “no
anywithout justification” guideline.As per coding guidelines, avoiding
anyimproves type safety even in test fixtures.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/backend/src/fixtures/index.ts(1 hunks)packages/backend/src/tokens/__tests__/verify.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/fixtures/index.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/fixtures/index.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/fixtures/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/fixtures/index.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/fixtures/index.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/fixtures/index.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/fixtures/index.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/backend/src/fixtures/index.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/__tests__/verify.test.ts (4)
packages/backend/src/fixtures/index.ts (3)
mockOAuthAccessTokenJwtPayload(36-46)createJwt(49-60)mockJwks(78-80)packages/backend/src/mock-server.ts (2)
server(6-6)validateHeaders(9-47)packages/backend/src/tokens/verify.ts (1)
verifyMachineAuthToken(339-351)packages/backend/src/api/resources/IdPOAuthAccessToken.ts (1)
IdPOAuthAccessToken(12-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 15, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/backend/src/tokens/__tests__/verify.test.ts (1)
329-460: New OAuth JWT tests give good coverage of the JWT-based OAuth verification flowThe
verifyOAuthToken with JWTsuite cleanly exercises:
- happy-path JWT verification (including
IdPOAuthAccessTokenmapping),- invalid
jtihandling,- invalid
typrejection,- support for both
at+jwtandapplication/at+jwt,- and malformed JWTs.
The use of
vi.spyOn(VerifyJwtModule, 'verifyJwt')to isolate headerType wiring and payload mapping is appropriate, and the expectations onheaderTypealign with the new behavior.packages/backend/src/fixtures/index.ts (2)
36-46: OAuth JWT payload fixture matches the verification and mapping requirements
mockOAuthAccessTokenJwtPayloadcleanly extendsmockJwtPayloadwithiss,sub,client_id,scope, and a well-formedjti, plus adjustedexp/iat/nbf. This lines up with the expectations inIdPOAuthAccessToken.fromJwtPayload(includingscope→ scopes array splitting) and the jti format validation used in the OAuth token verification logic.
62-65:mockOAuthAccessTokenJwthelper is a good reusable fixture for OAuth JWT testsExporting
mockOAuthAccessTokenJwtbuilt viacreateJwtwithtyp: 'at+jwt'and the shared OAuth payload is useful for tests that don’t need to vary header or claims. It keeps JWT construction consistent with other fixtures and centralizes the payload setup.
jfoshee
left a comment
jfoshee
left a comment
There was a problem hiding this comment.
Overall this looks good to me. Personally I would move the new block of code in verifyOAuthToken to a separate function like
if (isJwtFormat(accessToken)) {
return verifyJwtAccessToken(...);
}
For approval I defer to the JS team. Thanks for the quick work! 🚀
Co-authored-by: Jacob Foshee <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
packages/backend/src/tokens/machine.ts (1)
22-22: Addas constassertion for literal type safety.The
OAUTH_ACCESS_TOKEN_TYPESarray should be declared withas constto create a readonly tuple with literal types. This enables better type inference and eliminates the need for type assertions at usage sites (like line 30).Apply this diff:
-export const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt']; +export const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt'] as const;Based on coding guidelines: "Use
const assertionswithas constfor literal types" and "Declare readonly arrays and objects for immutable data structures."packages/backend/src/tokens/__tests__/verify.test.ts (5)
11-19: HelpercreateOAuthJwtreads well; consider explicit return typeThe helper nicely centralizes construction of OAuth JWTs and constrains
typto allowed values. For consistency with the TS guidelines, you could optionally add an explicit: stringreturn type.-function createOAuthJwt( +function createOAuthJwt( payload = mockOAuthAccessTokenJwtPayload, typ: 'at+jwt' | 'application/at+jwt' | 'JWT' = 'at+jwt', -) { +): string { return createJwt({ header: { typ, kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' }, payload, }); }
328-367: Valid OAuth JWT test andverifyJwtspy give good coverage; consider spying without stubbingThis test does a good job of verifying the JWT→
IdPOAuthAccessTokenmapping and asserting thatheaderTypeincludes both'at+jwt'and'application/at+jwt'. Since JWKS responses are already mocked, you might consider usingvi.spyOn(VerifyJwtModule, 'verifyJwt')withoutmockResolvedValueOnce(...)so the real implementation runs while still asserting onheaderType, giving a slightly stronger integration test.const spy = vi.spyOn(VerifyJwtModule, 'verifyJwt'); // no mockResolvedValueOnce call, rely on real verifyJwt + existing JWKS mockPlease verify that running the real
verifyJwthere doesn’t introduce unwanted flakiness or test runtime issues in your environment.
369-388: Invalidtyptest is clear; optionally assert on error code as wellThe test correctly ensures JWTs with
typ: 'JWT'are rejected and surfaces an “Invalid JWT type” message. For parity with other error-path tests in this file, you could also assert on an errorcode(if one is set for this condition) to stabilize the contract beyond message text.expect(result.errors?.[0].code).toBe('token-invalid'); // or the specific code used in implementationPlease confirm the actual error code used in the implementation before adding this assertion.
390-420:application/at+jwtcoverage looks good; minor duplication onlyThis test nicely mirrors the
at+jwthappy path and confirms that both header types route throughverifyJwtwith the expectedheaderTypearray. The duplication with the first test is small and acceptable; if this pattern grows, you might later refactor into a table-driven/parameterized test.
422-431: Invalid JWT format test covers a key edge case; consider tightening expectationsAsserting that
result.errorsis defined for structurally invalid JWTs is useful. If the implementation exposes a specific error message or code for malformed tokens, you could optionally assert on that as well to avoid false positives from unrelated failures (e.g., network issues).expect(result.errors?.[0].message).toContain('Invalid JWT'); // or more precise text // and/or expect(result.errors?.[0].code).toBe('token-invalid'); // depending on implementationVerify the actual shape of errors for malformed JWTs before tightening these assertions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/backend/src/tokens/__tests__/verify.test.ts(2 hunks)packages/backend/src/tokens/machine.ts(4 hunks)packages/backend/src/tokens/verify.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/backend/src/tokens/verify.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/machine.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/machine.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/machine.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/machine.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/machine.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/machine.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/tokens/__tests__/verify.test.tspackages/backend/src/tokens/machine.ts
🧬 Code graph analysis (2)
packages/backend/src/tokens/__tests__/verify.test.ts (4)
packages/backend/src/fixtures/index.ts (2)
mockOAuthAccessTokenJwtPayload(36-46)createJwt(49-60)packages/backend/src/mock-server.ts (2)
server(6-6)validateHeaders(9-47)packages/backend/src/tokens/verify.ts (1)
verifyMachineAuthToken(336-348)packages/backend/src/api/resources/IdPOAuthAccessToken.ts (1)
IdPOAuthAccessToken(12-65)
packages/backend/src/tokens/machine.ts (2)
packages/backend/src/jwt/verifyJwt.ts (1)
decodeJwt(45-95)packages/backend/src/internal.ts (1)
isMachineTokenByPrefix(59-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
packages/backend/src/tokens/machine.ts (2)
1-1: LGTM! Import correctly added for JWT decoding.The
decodeJwtimport is appropriately added to support the new OAuth JWT verification functionality.
76-78: LGTM! OAuth token detection correctly enhanced.The condition properly handles both prefix-based OAuth tokens (
oat_prefix) and JWT-formatted OAuth access tokens, maintaining backwards compatibility while adding the new JWT support.packages/backend/src/tokens/__tests__/verify.test.ts (3)
5-7: New JWT fixture and verifyJwt imports are appropriateUsing
createJwtandmockOAuthAccessTokenJwtPayloadfrom../../fixturesplus a module import forVerifyJwtModuleis consistent with existing patterns and enables clean spying onverifyJwtwithout affecting type-only imports. No changes needed here.
433-455:alg: "none"rejection test is solidThis test clearly verifies that JWTs using
alg: 'none'are rejected and that an “Invalid JWT algorithm” message is surfaced. This is an important security check and is well-covered here.
457-481: Expired OAuth JWT behavior appropriately validatedThe test for expired JWTs correctly manipulates
exprelative toiatand asserts that verification produces an error mentioning expiration. This provides good coverage for time-based validation in the OAuth JWT path.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/backend/src/tokens/machine.ts (1)
29-30: Add explicitdatacheck for type safety.The
decodeJwtreturn type is a discriminated union wheredatamay be undefined whenerrorsis present. While the short-circuit logic handles this at runtime, adding an explicit!!datacheck improves type safety and makes the defensive coding intention clear.const { data, errors } = decodeJwt(token); - return !errors && OAUTH_ACCESS_TOKEN_TYPES.includes(data.header.typ as (typeof OAUTH_ACCESS_TOKEN_TYPES)[number]); + return !errors && !!data && OAUTH_ACCESS_TOKEN_TYPES.includes(data.header.typ as (typeof OAUTH_ACCESS_TOKEN_TYPES)[number]);
🧹 Nitpick comments (1)
packages/backend/src/tokens/machine.ts (1)
22-22: Addas constfor proper type inference.Without
as const, the array type is inferred asstring[], making the type cast on line 30 effectivelystringrather than the intended union'at+jwt' | 'application/at+jwt'.-export const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt']; +export const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt'] as const;As per coding guidelines: "Use
const assertionswithas constfor literal types".
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/backend/src/tokens/machine.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/tokens/machine.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/machine.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/machine.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/tokens/machine.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/tokens/machine.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/machine.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/tokens/machine.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/machine.ts (2)
packages/backend/src/jwt/verifyJwt.ts (1)
decodeJwt(45-95)packages/backend/src/internal.ts (1)
isMachineTokenByPrefix(59-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/backend/src/tokens/machine.ts (3)
12-16: Looks good.The regex correctly matches the JWT structure (three non-empty base64url-encoded segments separated by dots), and the helper function is clear and concise.
50-58: LGTM!The updated JSDoc accurately reflects the implementation, and the logic correctly combines prefix-based and OAuth JWT detection.
76-78: LGTM!Correctly extends OAuth token detection to include JWT-format OAuth access tokens alongside prefix-based tokens.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/backend/src/fixtures/machine.ts (1)
69-81: OAuth JWT fixtures look consistent; consider documenting/allowlisting for secret scanners.The two new JWT fixtures appear coherent with the rest of the fixtures:
- Header
typvalues ("at+jwt"and"application/at+jwt") match the new test expectations.- Payload fields (
client_id,sub,jti,iat,exp,scope) align withmockOAuthAccessTokenJwtPayloadand the assertions inverify.test.ts.Given that these are full JWT strings, tools like gitleaks will flag them. Since they’re signed with the existing test keys in this repo and target example/test issuers, that’s fine from a security perspective, but you may want to:
- Add a brief comment that these are non-production test tokens only, and/or
- Add them to your secret-scanning allowlist if CI is noisy on these literals.
packages/backend/src/tokens/__tests__/verify.test.ts (2)
14-22: Helper correctly builds OAuth JWTs; optional explicit return type.
createOAuthJwtcleanly wrapscreateJwtwith the correcttypandkidand defaults the payload to the OAuth-access-token fixture. For readability and stricter typing, you could optionally annotate the return type asstring:function createOAuthJwt( payload = mockOAuthAccessTokenJwtPayload, typ: 'at+jwt' | 'application/at+jwt' | 'JWT' = 'at+jwt', ): string { return createJwt({ header: { typ, kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' }, payload }); }
331-468: JWT OAuth verification tests are thorough and aligned with fixtures.This block exercises the key JWT paths well:
- Valid OAuth JWT (typ
at+jwt) with full field mapping checks (idfromjti,clientId,subject, scopes).- Negative cases for wrong
typ, invalid JWT structure,alg: "none", and expiry.- Additional positive for
typ: "application/at+jwt".Timer setup and JWKS mocking are consistent with the rest of the suite, and routing through
verifyMachineAuthTokenvalidates the complete machine-token flow.If you want even stronger guarantees, you could also assert the error
codes (in addition to messages) for the failure cases, but that’s optional.packages/backend/src/fixtures/index.ts (1)
36-60: OAuth-access-token payload and JWT builder are coherent; consider tightening CreateJwt types.
mockOAuthAccessTokenJwtPayloadcleanly overlays OAuth-specific claims onmockJwtPayload(issuer, subject, client_id, scope, jti, and coherentexp/iat/nbf), and matches what the JWT-based tests assert against.createJwtcorrectly merges custom header/payload into the defaults and emits a compact base64url-encoded JWT using the shared mock signature, which keeps test JWT construction centralized.For slightly stronger typing, you could replace the
anyfields inCreateJwtwithunknownor a minimal header/payload shape, but given this is a fixtures module used only in tests, the current form is acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/backend/src/fixtures/index.ts(1 hunks)packages/backend/src/fixtures/machine.ts(1 hunks)packages/backend/src/tokens/__tests__/verify.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/fixtures/machine.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/__tests__/verify.test.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/fixtures/machine.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/__tests__/verify.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/fixtures/machine.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/__tests__/verify.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/fixtures/machine.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/__tests__/verify.test.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/fixtures/machine.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/__tests__/verify.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/fixtures/machine.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/__tests__/verify.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/fixtures/machine.tspackages/backend/src/fixtures/index.tspackages/backend/src/tokens/__tests__/verify.test.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/backend/src/fixtures/index.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/tokens/__tests__/verify.test.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/__tests__/verify.test.ts (3)
packages/backend/src/fixtures/index.ts (3)
mockOAuthAccessTokenJwtPayload(36-46)createJwt(49-60)mockJwtPayload(26-34)packages/backend/src/tokens/verify.ts (1)
verifyMachineAuthToken(336-348)packages/backend/src/fixtures/machine.ts (2)
mockSignedOAuthAccessTokenJwt(73-74)mockSignedOAuthAccessTokenJwtApplicationTyp(80-81)
🪛 Gitleaks (8.29.0)
packages/backend/src/fixtures/machine.ts
[high] 74-74: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 81-81: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/backend/src/tokens/__tests__/verify.test.ts (1)
5-10: Imports are appropriate and minimal.The new fixture imports are directly used by the JWT tests and keep dependencies localized to the existing fixtures modules. No issues here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/backend/src/jwt/__tests__/verifyJwt.test.ts (2)
136-177: Header-type success-path coverage for JWT and OAuth JWTs looks solidThe new tests exercising:
- default headerType (implicit
"JWT"),- explicit
headerType: 'JWT', and- OAuth access-token JWTs with
headerType: ['at+jwt', 'application/at+jwt']for bothat+jwtandapplication/at+jwtheaders,nicely cover the primary “happy paths” for both Clerk JWTs and OAuth access tokens. Assertions on
subfor the OAuth tokens are a good sanity check that the correct payload is being returned. No changes needed here.You could optionally de-duplicate the “default headerType = JWT” case with the earlier “returns the valid JWT payload...” test by either:
- repurposing the earlier test’s description to explicitly mention the default header type, or
- combining expectations if you want a single canonical “default JWT” test.
No diff suggested since this is purely about test organization/readability.
179-219: Negative-path headerType tests are correct; consider asserting error metadata tooThe three negative tests:
- JWT with
headerType: 'at+jwt',- OAuth JWT with
headerType: 'JWT', and- JWT with custom
typagainst['at+jwt', 'application/at+jwt'],correctly validate that:
verifyJwtsurfaces theInvalid JWT typeerror fromassertHeaderType, and- the error message reflects the configured allowed types (
Expected "at+jwt",Expected "JWT",Expected "at+jwt, application/at+jwt"), matching the implementation that joinsallowedTypes.To make these tests a bit more robust against message-text changes and to assert contract-level semantics, you might also check the error’s
reason(and optionallyaction) where available, similar to earlier tests usinginvalidTokenError. For example:it('rejects JWT when headerType does not match', async () => { @@ - const { errors: [error] = [] } = await verifyJwt(mockJwt, inputVerifyJwtOptions); - expect(error).toBeDefined(); - expect(error?.message).toContain('Invalid JWT type'); - expect(error?.message).toContain('Expected "at+jwt"'); + const { errors: [error] = [] } = await verifyJwt(mockJwt, inputVerifyJwtOptions); + expect(error).toBeDefined(); + expect(error?.reason).toBe('token-invalid'); + expect(error?.message).toContain('Invalid JWT type'); + expect(error?.message).toContain('Expected "at+jwt"'); }); @@ it('rejects OAuth JWT when headerType does not match', async () => { @@ - const { errors: [error] = [] } = await verifyJwt(mockSignedOAuthAccessTokenJwt, inputVerifyJwtOptions); - expect(error).toBeDefined(); - expect(error?.message).toContain('Invalid JWT type'); - expect(error?.message).toContain('Expected "JWT"'); + const { errors: [error] = [] } = await verifyJwt(mockSignedOAuthAccessTokenJwt, inputVerifyJwtOptions); + expect(error).toBeDefined(); + expect(error?.reason).toBe('token-invalid'); + expect(error?.message).toContain('Invalid JWT type'); + expect(error?.message).toContain('Expected "JWT"'); }); @@ it('rejects JWT when headerType array does not include the token type', async () => { @@ - const { errors: [error] = [] } = await verifyJwt(jwtWithCustomTyp, inputVerifyJwtOptions); - expect(error).toBeDefined(); - expect(error?.message).toContain('Invalid JWT type'); - expect(error?.message).toContain('Expected "at+jwt, application/at+jwt"'); + const { errors: [error] = [] } = await verifyJwt(jwtWithCustomTyp, inputVerifyJwtOptions); + expect(error).toBeDefined(); + expect(error?.reason).toBe('token-invalid'); + expect(error?.message).toContain('Invalid JWT type'); + expect(error?.message).toContain('Expected "at+jwt, application/at+jwt"'); });This keeps the new headerType behavior well-covered while also locking in the error’s semantic shape.
packages/backend/src/jwt/__tests__/assertions.test.ts (1)
109-150: assertHeaderType tests accurately mirror the new API and error messagesThe new
describe('assertHeaderType(typ?, allowedTypes?)')block thoroughly exercises:
- Missing
typwith default and customallowedTypes(no throw).- Default behavior for
"JWT"vs other values.- Single custom allowed type (e.g.,
'at+jwt','application/at+jwt'), including mismatch cases.- Multiple allowed types via array, ensuring both matches and mismatches.
- Exact error-message formatting (
Expected "JWT"andExpected "at+jwt, application/at+jwt"), which matches the implementation that joinsallowedTypeswith', '.This gives strong coverage for the new
allowedTypesparameter and its defaulting behavior.If you want to push edge-case coverage further, you could add a couple of tests for non-string
typvalues (e.g.,null, numbers) to lock in howJSON.stringify(typ)is rendered in the error message. Not necessary for correctness, but would fully document current behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/backend/src/jwt/__tests__/assertions.test.ts(1 hunks)packages/backend/src/jwt/__tests__/verifyJwt.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/jwt/__tests__/verifyJwt.test.tspackages/backend/src/jwt/__tests__/assertions.test.ts
🧬 Code graph analysis (2)
packages/backend/src/jwt/__tests__/verifyJwt.test.ts (2)
packages/backend/src/fixtures/index.ts (4)
mockJwks(73-75)mockJwtPayload(26-34)createJwt(49-60)mockOAuthAccessTokenJwtPayload(36-46)packages/backend/src/fixtures/machine.ts (2)
mockSignedOAuthAccessTokenJwt(73-74)mockSignedOAuthAccessTokenJwtApplicationTyp(80-81)
packages/backend/src/jwt/__tests__/assertions.test.ts (1)
packages/backend/src/jwt/assertions.ts (1)
assertHeaderType(50-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Unit Tests (22, shared, clerk-js, RQ)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/backend/src/jwt/__tests__/verifyJwt.test.ts (1)
3-15: New JWT/OAuth fixtures imports are correct and fully utilized
createJwt,mockOAuthAccessTokenJwtPayload,mockSignedOAuthAccessTokenJwt, andmockSignedOAuthAccessTokenJwtApplicationTypare all used in the new tests and match the intended OAuth access-token JWT scenarios. Import structure and usage look consistent with existing fixture patterns; no issues from ESLint/Prettier or test-design perspectives.
wobsoriano
changed the title
wobsoriano
changed the title
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/backend/src/tokens/machine.ts (2)
18-23: Consider markingOAUTH_ACCESS_TOKEN_TYPESas readonly withas constSince these are fixed RFC-defined values, you can tighten the type and prevent accidental mutation with a const assertion:
-export const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt']; +export const OAUTH_ACCESS_TOKEN_TYPES = ['at+jwt', 'application/at+jwt'] as const;This also gives you a literal union type for
(typeof OAUTH_ACCESS_TOKEN_TYPES)[number], aligning with the guidelines onreadonlydata andas const.
24-42:isOAuthJwtlogic is sound; optional refinement for header.typ type‑guardThe control flow (format check → safe decode with try/catch → header
typmembership) correctly identifies RFC 9068 OAuth JWT access tokens without attempting verification here.For slightly clearer typing and input validation, you could explicitly guard the header type before the
includescall:export function isOAuthJwt(token: string): boolean { if (!isJwtFormat(token)) { return false; } try { - const { data, errors } = decodeJwt(token); - return !errors && !!data && OAUTH_ACCESS_TOKEN_TYPES.includes(data.header.typ as (typeof OAUTH_ACCESS_TOKEN_TYPES)[number]); + const { data, errors } = decodeJwt(token); + if (errors || !data) { + return false; + } + const typ = data.header?.typ; + if (typeof typ !== 'string') { + return false; + } + return OAUTH_ACCESS_TOKEN_TYPES.includes(typ as (typeof OAUTH_ACCESS_TOKEN_TYPES)[number]); } catch { return false; } }This makes the intent explicit and avoids relying on assertions around
data.header.typ.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/backend/src/fixtures/index.ts(1 hunks)packages/backend/src/tokens/machine.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/backend/src/fixtures/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/backend/src/tokens/machine.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/backend/src/tokens/machine.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/machine.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/backend/src/tokens/machine.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/backend/src/tokens/machine.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/machine.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/backend/src/tokens/machine.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/machine.ts (1)
packages/backend/src/jwt/verifyJwt.ts (1)
decodeJwt(45-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/backend/src/tokens/machine.ts (4)
1-1: Reuse ofdecodeJwthere is appropriateImporting
decodeJwtdirectly into this module to inspect the header for classification keeps JWT parsing logic centralized and avoids duplicating decode logic. No issues here.
12-16: JWT format helper looks correct and narrowly scopedThe regexp and
isJwtFormathelper correctly implement a quick shape check for three base64url-ish segments before doing a full decode. This is a good, cheap guard in front ofdecodeJwt.
58-66: Machine token detection now correctly includes OAuth JWTsExtending
isMachineTokento short‑circuit on known prefixes and then fall back toisOAuthJwtis a clean way to treat bare RFC 9068 access tokens as machine tokens while preserving existing prefix behavior. JSDoc is accurate and matches the implementation.
84-84: Mapping OAuth JWTs toTokenType.OAuthTokenis consistent withisMachineTokenIncluding
isOAuthJwt(token)in the OAuth branch ofgetMachineTokenTypekeeps type resolution aligned withisMachineToken, so prefix‑less OAuth JWTs are consistently classified asoauth_token. This matches the PR objective and looks correct.
Description
This PR adds JWT verification support for OAuth access tokens (
acceptsToken: 'oauth_token')verifyJwtwith a headerType parameter to support OAuth access token JWTs (RFC 9068)Resolves USER-4076
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.