feat(backend): Add support for JWTs in OAuth token type #7308
Conversation
🦋 Changeset detectedLatest commit: 4c11786 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.
|
WalkthroughAdds JWT detection, header-type flexibility, and verification for OAuth access tokens: new JWT utilities and fixtures, IdPOAuthAccessToken.fromJwtPayload factory, verifyJwt headerType option, routing JWT-formatted machine tokens into OAuth verification, error-code and error-class updates, and expanded tests. 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 IdPOAuthAccessToken
participant Err as MachineTokenVerificationError
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: fromJwtPayload(payload, clockSkewInMs)
Model-->>Verify: IdP token
Verify-->>Auth: verified token
else Not OAuth JWT
Machine-->>Verify: false
Verify->>Verify: prefix-based JWK/PEM verification
end
else not machine token
Machine-->>Auth: false
Auth->>Verify: session or API-key verification
end
alt verification error
Verify-->>Auth: throw MachineTokenVerificationError (TokenInvalid / TokenVerificationFailed)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 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.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/backend/src/tokens/machine.ts (1)
1-1: Consider tightening typing ofOAUTH_ACCESS_TOKEN_TYPES.Reusing
decodeJwtwith a guarded try/catch and validating the headertypagainst RFC 9068 values is a solid approach forisOAuthJwt. To make(typeof OAUTH_ACCESS_TOKEN_TYPES)[number]a useful string-literal union and better match the guidelines aboutas constfor literals, you can mark the array as a const tuple:-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 keeps runtime behavior the same while giving you the
'at+jwt' | 'application/at+jwt'union type for stronger typing and editor hints.Also applies to: 18-46
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 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 (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). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/backend/src/tokens/machine.ts (3)
12-16: JWT format helper is concise and correct.The regex cleanly matches three base64url-like segments separated by dots, and
isJwtFormatkeeps this structural check isolated from decoding logic. No changes needed.
62-70: Machine token detection correctly incorporates OAuth JWTs.Combining
isMachineTokenByPrefixwithisOAuthJwtcleanly extendsisMachineTokento support JWT-formatted OAuth access tokens without changing existing prefix-based behavior.
88-90:getMachineTokenTypeclassification for OAuth JWTs is consistent.Classifying tokens with the
oat_prefix or those recognized byisOAuthJwtasTokenType.OAuthTokenkeeps this helper in sync withisMachineTokenand the newoauth_tokensemantics.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/backend/src/tokens/__tests__/machine.test.ts (2)
99-113: NewgetMachineTokenTypetests correctly exercise OAuth JWT routingVerifying that all three OAuth JWT variants (two fixtures plus a
createJwt-generated token) return"oauth_token"solidly covers the new branch; you could optionally add a test that a malformed three-part JWT-like string still throws to pin down that error behavior, but it’s not required.
177-205:isOAuthJwttests validatetyp-based OAuth access token detectionBoth allowed
typvalues plus a non-OAuthtypand a malformed token are covered; you might optionally add a case where the header omitstypentirely to ensure it continues to be treated as non-OAuth, but the current coverage is already solid.
📜 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/__tests__/machine.test.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__/machine.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/tokens/__tests__/machine.test.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/tokens/__tests__/machine.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/tokens/__tests__/machine.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/tokens/__tests__/machine.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/tokens/__tests__/machine.test.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/backend/src/tokens/__tests__/machine.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.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__/machine.test.ts
🧬 Code graph analysis (1)
packages/backend/src/tokens/__tests__/machine.test.ts (3)
packages/backend/src/tokens/machine.ts (7)
isMachineToken(68-70)M2M_TOKEN_PREFIX(6-6)OAUTH_TOKEN_PREFIX(7-7)API_KEY_PREFIX(8-8)getMachineTokenType(83-97)isJwtFormat(14-16)isOAuthJwt(32-46)packages/backend/src/fixtures/machine.ts (2)
mockSignedOAuthAccessTokenJwt(73-74)mockSignedOAuthAccessTokenJwtApplicationTyp(80-81)packages/backend/src/fixtures/index.ts (2)
createJwt(167-178)mockOAuthAccessTokenJwtPayload(36-46)
⏰ 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: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/backend/src/tokens/__tests__/machine.test.ts (4)
3-16: Imports for JWT fixtures and helpers look correctAll newly added imports are used, paths match the described fixture locations, and they line up with the new token helper APIs with no redundancy.
18-40: Prefix-based machine token tests remain thoroughThe suite still covers all recognized prefixes plus unknown and empty-token cases, so prefix-only behavior is clearly specified and unchanged by the new JWT handling.
42-88: Comprehensive coverage forisMachineTokenacross prefixes and OAuth JWTsThese tests exercise all relevant paths: known prefixes, both OAuth JWT
typvariants, JWTs created viacreateJwt, regular/non-OAuth JWTs, unknown prefixes, and empty strings, giving good confidence in the newisOAuthJwtintegration.
164-175:isJwtFormattests clearly define the accepted JWT-like structurePositive and negative examples around the three-segment
header.payload.signaturepattern look correct and match the intended “format only” semantics without over-constraining base64 details.
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.