chore(backend,nextjs): Refactor unauthenticated auth object fallback to respect intent #6112
Conversation
🦋 Changeset detectedLatest commit: e672cd3 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 Git ↗︎ 1 Skipped Deployment
|
|
Warning Rate limit exceeded@wobsoriano has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 Walkthrough## Walkthrough
The changes refactor machine token handling by replacing `isMachineToken` with `isMachineTokenByPrefix` and introducing `isMachineTokenType`. Related exports and imports are updated throughout the backend and Next.js packages. Authentication logic and tests are revised to reflect these changes, with improved handling of accepted token types and expanded test coverage.
## Changes
| Files/Groups | Change Summary |
|------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `packages/backend/src/internal.ts`, `packages/backend/src/__tests__/exports.test.ts` | Updated exports: replaced `isMachineToken` with `isMachineTokenByPrefix` and `isMachineTokenType`. Adjusted tests to reflect new exports. |
| `packages/backend/src/tokens/machine.ts`, `packages/backend/src/tokens/__tests__/machine.test.ts` | Renamed `isMachineToken` to `isMachineTokenByPrefix`. Added new function `isMachineTokenType` and corresponding tests. Updated imports and test usage to match new function names. |
| `packages/backend/src/tokens/authObjects.ts`, `packages/backend/src/tokens/__tests__/authObjects.test.ts` | Refactored `getAuthObjectForAcceptedToken` for clearer handling of accepted token types, distinguishing between array and single-value cases. Added `isAuthenticated` property to auth object types. Added new `InvalidTokenAuthObject` type and factory. Updated and expanded related tests for new logic. |
| `packages/backend/src/tokens/request.ts` | Replaced usage of `isMachineToken` with `isMachineTokenByPrefix` in authentication logic. No changes to public exports. |
| `packages/nextjs/src/server/data/getAuthDataFromRequest.ts` | Refactored `getAuthDataFromRequestAsync` to use new helpers for machine token handling and intent-based logic. Improved handling of token type acceptance. No changes to public exports. |
| `packages/nextjs/src/server/clerkMiddleware.ts` | Updated import and usage from `isMachineToken` to `isMachineTokenByPrefix` in middleware logic. No changes to public exports. |
| `packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts` | Added tests for new token type acceptance scenarios and updated type assertions in existing tests. |
| `packages/express/src/__tests__/getAuth.test.ts`, `packages/fastify/src/__tests__/getAuth.test.ts` | Updated tests to expect the accepted token type instead of the actual token type in unauthenticated auth objects when token types do not match. |
| `packages/backend/src/index.ts`, `packages/backend/src/tokens/types.ts`, `packages/nextjs/src/app-router/server/auth.ts` | Updated exports and type signatures to include `InvalidTokenAuthObject` in public API and authentication function return types. |
| `packages/nextjs/src/server/protect.ts` | Added explicit check to handle `null` tokenType by calling `handleUnauthorized()` early in `createProtect`. |
| `.changeset/open-swans-count.md` | Added patch update note describing changes to respect `acceptsToken` flag in unauthenticated session or machine object returns. |
## Suggested labels
`nextjs`, `backend`
## Suggested reviewers
- panteliselef
- tmilewski✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@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: 4
🔭 Outside diff range comments (1)
packages/backend/src/tokens/request.ts (1)
656-663:⚠️ Potential issue
acceptsTokenmay be an array – leads to wrongtokenTypein signed-out state
acceptsTokenis cast toMachineTokenType, but when the caller supplied an array (e.g.['api_key','machine_token']) this branch is still reached andsignedOut()will receive an array instead of a string, violating the expected contract and potentially blowing up downstream logic that pattern-matches ontokenType.- return signedOut({ - tokenType: acceptsToken as MachineTokenType, + const fallbackTokenType = + Array.isArray(acceptsToken) ? TokenType.MachineToken : (acceptsToken as MachineTokenType); + return signedOut({ + tokenType: fallbackTokenType,At minimum guard with
Array.isArray, or refactor the control-flow earlier so thatauthenticateMachineRequestWithTokenInHeaderis never called whenacceptsTokenis an array.
🧹 Nitpick comments (14)
packages/nextjs/src/server/clerkMiddleware.ts (1)
286-297: Minor: make “Bearer ” stripping case-insensitive
replace('Bearer ', '')misses the common lowercase form (bearer) and variants with extra spacing, which would causeisMachineTokenByPrefixto receive an untrimmed string and returnfalse.-const authHeader = getHeader(request, constants.Headers.Authorization)?.replace('Bearer ', '') ?? ''; +const authHeader = + getHeader(request, constants.Headers.Authorization) + // Strip the bearer scheme in a case-insensitive way and any leading spaces + ?.replace(/^Bearer\s+/i, '') ?? '';packages/backend/src/tokens/request.ts (1)
691-709: (optional) avoid duplicate prefix scanning
isMachineTokenByPrefixis executed, and on the success pathgetMachineTokenTypescans the prefixes again. Caching the mapping (prefix → type) removes the extra work and keeps the two helpers in sync.packages/backend/src/tokens/machine.ts (2)
21-23: Unify prefix → type logic & avoid double iterationBoth
isMachineTokenByPrefixandgetMachineTokenTypeperform separate prefix scans.
Creating one canonical lookup avoids divergence and improves perf (especially if custom prefixes grow).-const MACHINE_TOKEN_PREFIXES = [M2M_TOKEN_PREFIX, OAUTH_TOKEN_PREFIX, API_KEY_PREFIX] as const; +const PREFIX_TO_TYPE: Record<string, MachineTokenType> = { + [M2M_TOKEN_PREFIX]: TokenType.MachineToken, + [OAUTH_TOKEN_PREFIX]: TokenType.OAuthToken, + [API_KEY_PREFIX]: TokenType.ApiKey, +} as const; + +const MACHINE_TOKEN_PREFIXES = Object.keys(PREFIX_TO_TYPE); export function isMachineTokenByPrefix(token: string): boolean { - return MACHINE_TOKEN_PREFIXES.some(prefix => token.startsWith(prefix)); + return MACHINE_TOKEN_PREFIXES.some(prefix => token.startsWith(prefix)); } export function getMachineTokenType(token: string): MachineTokenType { - if (token.startsWith(M2M_TOKEN_PREFIX)) { - return TokenType.MachineToken; - } - ... - throw new Error('Unknown machine token type'); + for (const [prefix, type] of Object.entries(PREFIX_TO_TYPE)) { + if (token.startsWith(prefix)) { + return type; + } + } + throw new Error('Unknown machine token type'); }Also applies to: 36-50, 71-79
77-79: Nit: rename parameter for clarityUsing the identifier
typefor the function parameter shadows its semantic meaning.tokenTypeis more explicit and avoids confusion.-export function isMachineTokenType(type: string): type is MachineTokenType { - return type === TokenType.ApiKey || type === TokenType.MachineToken || type === TokenType.OAuthToken; +export function isMachineTokenType(tokenType: string): tokenType is MachineTokenType { + return ( + tokenType === TokenType.ApiKey || + tokenType === TokenType.MachineToken || + tokenType === TokenType.OAuthToken + ); }packages/backend/src/tokens/__tests__/machine.test.ts (2)
13-13: Rename describe block to match the renamed helper.The suite is still titled
isMachineToken, but the implementation under test is nowisMachineTokenByPrefix.
Keeping the names in sync avoids confusion when reading test output.-describe('isMachineToken', () => { +describe('isMachineTokenByPrefix', () => {
82-93: Broaden negative-case coverage forisMachineTokenType.Consider adding non-machine strings with similar shapes (
'api_Key','API_KEY', etc.) to ensure the predicate remains case- and value-sensitive.it('returns false for non-machine token types', () => { expect(isMachineTokenType('session_token')).toBe(false); + expect(isMachineTokenType('api_Key')).toBe(false); // mixed case + expect(isMachineTokenType('API_KEY')).toBe(false); // upper case });packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts (3)
45-47: Use the correct unauthenticated type in assertions.
authis expected to be unauthenticated, but the cast usesAuthenticatedMachineObject.
Switching toUnauthenticatedMachineObjectprevents accidental misuse of authenticated-only members in future edits.- expect((auth as AuthenticatedMachineObject<'machine_token'>).machineId).toBeNull(); + expect( + (auth as UnauthenticatedMachineObject<'machine_token'>).machineId, + ).toBeNull();
50-63: Same casting nit – unauthenticated object.Apply the same type-accurate cast here.
- expect((auth as AuthenticatedMachineObject<'api_key'>).userId).toBeNull(); + expect((auth as UnauthenticatedMachineObject<'api_key'>).userId).toBeNull();
65-71: Reset mocked verification between tests to avoid leakage.
verifyMachineAuthTokenis globally mocked and mutated in multiple tests.
Add anafterEach(() => vi.resetAllMocks())(orrestoreAllMocks) to ensure each test starts with a clean slate.32 describe('getAuthDataFromRequestAsync', () => { + afterEach(() => { + vi.resetAllMocks(); + });packages/nextjs/src/server/data/getAuthDataFromRequest.ts (2)
171-173: Redundant null-check ontokenType.
handleMachineTokenis only invoked whentokenTypeis already defined.
The early guard can be removed, simplifying the flow.- if (!tokenType) { - return signedOutAuthObject(options); - }
188-223: Nested conditional complexity ‑ consider flattening for readability.
handleIntentBasednow has three levels of branching plus early returns.
A small refactor to guard-clauses / separate helpers would improve maintainability, especially as fallback rules evolve.(No diff provided – structural suggestion.)
packages/backend/src/tokens/__tests__/authObjects.test.ts (3)
393-428: Consider table-driven tests to reduce duplicationAll five scenarios are valuable, but the structure is repetitive and could be collapsed into a parameterised (table-driven) test:
const cases = [ { acceptsToken: 'any', auth: machineAuth, expectedType: 'api_key' }, { acceptsToken: 'api_key', auth: machineAuth, expectedType: 'api_key' }, { acceptsToken: ['machine_token','oauth_token'], auth: machineAuth, expectedType: 'api_key' }, { acceptsToken: ['api_key','oauth_token'], auth: sessionAuth, expectedType: 'session_token' }, { acceptsToken: 'machine_token', auth: machineAuth, expectedType: 'machine_token' }, ] as const; cases.forEach(({acceptsToken, auth, expectedType}) => it(`returns correct object for acceptsToken=${JSON.stringify(acceptsToken)}`, () => { const result = getAuthObjectForAcceptedToken({authObject: auth, acceptsToken}); expect(result.tokenType).toBe(expectedType); }), );This eliminates boilerplate, makes future additions trivial, and keeps intent crystal-clear.
408-415: Broaden assertions for stronger guaranteesOnly
tokenTypeandidare asserted. If downstream logic relies on other properties (name,subject,scopes, etc.), they could silently regress without detection. Suggest asserting the entire object shape for the unauthenticated machine variant:expect(result).toMatchObject({ tokenType: 'api_key', id: null, subject: null, scopes: null, claims: null, });Strengthens the safety net with negligible overhead.
423-427: Minor semantic nit – variable naming
machineAuthactually represents an API-key auth object. Renaming toapiKeyAuthmakes the mismatch between the provided token (api_key) and the requested one (machine_token) more explicit and self-documenting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/backend/src/__tests__/exports.test.ts(1 hunks)packages/backend/src/internal.ts(1 hunks)packages/backend/src/tokens/__tests__/authObjects.test.ts(2 hunks)packages/backend/src/tokens/__tests__/machine.test.ts(2 hunks)packages/backend/src/tokens/authObjects.ts(3 hunks)packages/backend/src/tokens/machine.ts(2 hunks)packages/backend/src/tokens/request.ts(3 hunks)packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts(2 hunks)packages/nextjs/src/server/clerkMiddleware.ts(2 hunks)packages/nextjs/src/server/data/getAuthDataFromRequest.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts (5)
packages/nextjs/src/server/index.ts (1)
auth(44-44)packages/backend/src/tokens/authObjects.ts (1)
AuthenticatedMachineObject(115-125)packages/express/src/__tests__/helpers.ts (1)
mockRequest(25-27)packages/nextjs/src/server/data/getAuthDataFromRequest.ts (1)
getAuthDataFromRequestAsync(87-138)packages/backend/src/tokens/verify.ts (1)
verifyMachineAuthToken(248-260)
packages/backend/src/tokens/__tests__/machine.test.ts (1)
packages/backend/src/tokens/machine.ts (5)
isMachineTokenByPrefix(21-23)M2M_TOKEN_PREFIX(5-5)OAUTH_TOKEN_PREFIX(6-6)API_KEY_PREFIX(7-7)isMachineTokenType(77-79)
packages/backend/src/tokens/machine.ts (2)
packages/backend/src/internal.ts (4)
isMachineTokenByPrefix(56-56)isMachineTokenType(56-56)MachineTokenType(19-19)TokenType(18-18)packages/backend/src/tokens/tokenTypes.ts (3)
MachineTokenType(20-20)TokenType(1-6)TokenType(11-11)
packages/backend/src/tokens/request.ts (2)
packages/backend/src/internal.ts (1)
isMachineTokenByPrefix(56-56)packages/backend/src/tokens/machine.ts (1)
isMachineTokenByPrefix(21-23)
packages/nextjs/src/server/clerkMiddleware.ts (1)
packages/backend/src/tokens/machine.ts (1)
isMachineTokenByPrefix(21-23)
packages/backend/src/tokens/authObjects.ts (2)
packages/backend/src/internal.ts (4)
isTokenTypeAccepted(56-56)isMachineTokenType(56-56)unauthenticatedMachineObject(33-33)signedOutAuthObject(30-30)packages/backend/src/tokens/machine.ts (2)
isTokenTypeAccepted(59-69)isMachineTokenType(77-79)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/backend/src/__tests__/exports.test.ts (1)
53-55: Snapshot update looks correctThe new exports
isMachineTokenByPrefixandisMachineTokenTypeare alphabetically sorted and preserve the public surface.
No further action required.packages/backend/src/internal.ts (1)
56-56: Export list updated coherentlyThe re-exports now reflect the new helpers without breaking existing consumers. 👍
packages/backend/src/tokens/machine.ts (1)
21-23: Single-responsibility helper is fine
isMachineTokenByPrefixis concise and readable. 👍packages/backend/src/tokens/__tests__/authObjects.test.ts (2)
6-14: Type-only imports 👍Good move extracting
SignedOutAuthObject&UnauthenticatedMachineObjectas type-only imports – keeps runtime bundle clean while preserving strong typing.
417-421: Edge-case: legacy cookie-based session tokens
sessionAuthhere comes fromsignedOutAuthObject, which defaults tosession_token.
If the codebase still supports legacy__sessioncookies (no explicit token type), add a fixture & test to ensuregetAuthObjectForAcceptedTokenfalls back identically for that path, preventing accidental breakage during cookie deprecation work.
| if (!isTokenTypeAccepted(tokenType, acceptsToken ?? TokenType.SessionToken)) { | ||
| return unauthenticatedMachineObject(tokenType, options); | ||
| } | ||
| const { data, errors } = await verifyMachineAuthToken(bearerToken, options); | ||
| if (errors) { | ||
| return unauthenticatedMachineObject(tokenType, options); | ||
| } | ||
| return authenticatedMachineObject(tokenType, bearerToken, data); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pass only the expected shape to verifyMachineAuthToken.
verifyMachineAuthToken only needs { secretKey, publishableKey, apiUrl }.
Passing the full options (with unrelated fields) weakens type contracts.
- const { data, errors } = await verifyMachineAuthToken(bearerToken, options);
+ const { data, errors } = await verifyMachineAuthToken(bearerToken, {
+ secretKey: options.secretKey,
+ publishableKey: options.publishableKey,
+ apiUrl: options.apiUrl,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!isTokenTypeAccepted(tokenType, acceptsToken ?? TokenType.SessionToken)) { | |
| return unauthenticatedMachineObject(tokenType, options); | |
| } | |
| const { data, errors } = await verifyMachineAuthToken(bearerToken, options); | |
| if (errors) { | |
| return unauthenticatedMachineObject(tokenType, options); | |
| } | |
| return authenticatedMachineObject(tokenType, bearerToken, data); | |
| } | |
| if (!isTokenTypeAccepted(tokenType, acceptsToken ?? TokenType.SessionToken)) { | |
| return unauthenticatedMachineObject(tokenType, options); | |
| } | |
| - const { data, errors } = await verifyMachineAuthToken(bearerToken, options); | |
| + const { data, errors } = await verifyMachineAuthToken(bearerToken, { | |
| + secretKey: options.secretKey, | |
| + publishableKey: options.publishableKey, | |
| + apiUrl: options.apiUrl, | |
| + }); | |
| if (errors) { | |
| return unauthenticatedMachineObject(tokenType, options); | |
| } | |
| return authenticatedMachineObject(tokenType, bearerToken, data); | |
| } |
🤖 Prompt for AI Agents
In packages/nextjs/src/server/data/getAuthDataFromRequest.ts between lines 174
and 182, the function verifyMachineAuthToken is currently called with the entire
options object, which includes unrelated fields. To fix this, extract only the
expected properties secretKey, publishableKey, and apiUrl from options and pass
them to verifyMachineAuthToken. This ensures the function receives only the
necessary data, maintaining strong type contracts.
| const options = { | ||
| secretKey: opts?.secretKey || SECRET_KEY, | ||
| publishableKey: PUBLISHABLE_KEY, | ||
| apiUrl: API_URL, | ||
| authStatus, | ||
| authMessage, | ||
| authReason, | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
options deserves a typed interface instead of Record<string, any>.
Using Record<string, any> erodes type safety and allows accidental leakage of irrelevant keys throughout the call-chain.
Define a dedicated interface with the exact props consumed by downstream helpers and use it here & in helper signatures.
- const options = {
+ interface AuthOptions {
+ secretKey: string;
+ publishableKey: string;
+ apiUrl: string;
+ authStatus: string | null;
+ authMessage: string | null;
+ authReason: string | null;
+ }
+
+ const options: AuthOptions = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const options = { | |
| secretKey: opts?.secretKey || SECRET_KEY, | |
| publishableKey: PUBLISHABLE_KEY, | |
| apiUrl: API_URL, | |
| authStatus, | |
| authMessage, | |
| authReason, | |
| }; | |
| interface AuthOptions { | |
| secretKey: string; | |
| publishableKey: string; | |
| apiUrl: string; | |
| authStatus: string | null; | |
| authMessage: string | null; | |
| authReason: string | null; | |
| } | |
| const options: AuthOptions = { | |
| secretKey: opts?.secretKey || SECRET_KEY, | |
| publishableKey: PUBLISHABLE_KEY, | |
| apiUrl: API_URL, | |
| authStatus, | |
| authMessage, | |
| authReason, | |
| }; |
🤖 Prompt for AI Agents
In packages/nextjs/src/server/data/getAuthDataFromRequest.ts around lines 96 to
104, the options object is currently typed as Record<string, any>, which reduces
type safety. Define a dedicated TypeScript interface specifying the exact
properties (secretKey, publishableKey, apiUrl, authStatus, authMessage,
authReason) with their appropriate types, then use this interface to type the
options object and update any downstream helper function signatures accordingly
to ensure strict type checking and prevent accidental key leakage.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.changeset/open-swans-count.md (1)
6-6: Clarify the changeset summary for future readersA single-sentence summary is fine, but adding one more phrase about how the fallback now behaves (e.g., “returns an unauthenticated object of the accepted token type rather than the parsed token type”) would make the patch note self-contained and clearer in changelogs.
packages/fastify/src/__tests__/getAuth.test.ts (1)
37-43: Update the inline comment to match the new behaviourThe expectation is now
api_keybecause it mirrors the accepted token type, not the actual token found.
Leaving the old wording is misleading for future maintainers.- expect(result.tokenType).toBe('api_key'); // reflects the actual token found + expect(result.tokenType).toBe('api_key'); // mirrors the *accepted* token typepackages/express/src/__tests__/getAuth.test.ts (1)
40-46: Fix the stale comment to avoid confusionSame as in the Fastify test: the returned
tokenTypereflects the accepted token type, not the one on the incoming request. Adjust the comment accordingly.- expect(result.tokenType).toBe('api_key'); // reflects the actual token found + expect(result.tokenType).toBe('api_key'); // mirrors the *accepted* token type
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/open-swans-count.md(1 hunks)packages/express/src/__tests__/getAuth.test.ts(1 hunks)packages/fastify/src/__tests__/getAuth.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
|
!snapshot |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/nextjs/src/server/protect.ts (1)
144-151: Consider folding the newnullcheck into the existing guard for cleaner flowFunctionally this works, but we now have two adjacent early-return guards that both call
handleUnauthorized().
Combining them (or moving thenullcheck first) keeps the intent crystal-clear and avoids an unnecessary call toisTokenTypeAcceptedwhen the token is already known to be invalid.- if (!isTokenTypeAccepted(authObject.tokenType, requestedToken)) { - return handleUnauthorized(); - } - - if (authObject.tokenType === null) { - return handleUnauthorized(); - } + if ( + authObject.tokenType === null || + !isTokenTypeAccepted(authObject.tokenType, requestedToken) + ) { + return handleUnauthorized(); + }Same behaviour, fewer branches.
Use or ignore at your discretion.packages/nextjs/src/app-router/server/auth.ts (1)
59-65: Document the newInvalidTokenAuthObjectreturn pathThe overload now rightfully includes
InvalidTokenAuthObject, but the surrounding JSDoc (@examplelines 52-56) still states “returns the auth object” without mentioning this extra unauthenticated variant. Adding a short note (or an extra example) will prevent consumers from overlooking the possibility of the invalid-token path and improves IntelliSense docs.packages/backend/src/tokens/types.ts (1)
204-206: Synchronise docs & ‘any’ overload with the new invalid token unionThe array overload now unions
InvalidTokenAuthObject, but:
- The earlier JSDoc (
@examplelines 197-200) doesn’t mention that an invalid object may be returned.- The
'any'overload (line 225) still returns plainAuthObject; verify thatAuthObjectalready includesInvalidTokenAuthObject, otherwise callers using'any'won’t see this variant in their type checking.Updating the docs and, if needed, the
'any'overload will keep the public contract crystal-clear.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/backend/src/__tests__/exports.test.ts(1 hunks)packages/backend/src/index.ts(1 hunks)packages/backend/src/internal.ts(2 hunks)packages/backend/src/tokens/__tests__/authObjects.test.ts(2 hunks)packages/backend/src/tokens/__tests__/getAuth.test-d.ts(2 hunks)packages/backend/src/tokens/authObjects.ts(12 hunks)packages/backend/src/tokens/machine.ts(2 hunks)packages/backend/src/tokens/types.ts(2 hunks)packages/nextjs/src/app-router/server/auth.ts(2 hunks)packages/nextjs/src/server/protect.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/backend/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/backend/src/tests/exports.test.ts
- packages/backend/src/tokens/machine.ts
- packages/backend/src/internal.ts
- packages/backend/src/tokens/tests/authObjects.test.ts
- packages/backend/src/tokens/authObjects.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/backend/src/tokens/types.ts (4)
packages/backend/src/internal.ts (2)
InferAuthObjectFromTokenArray(14-14)SessionTokenType(19-19)packages/backend/src/index.ts (3)
SessionAuthObject(165-165)MachineAuthObject(165-165)InvalidTokenAuthObject(164-164)packages/backend/src/tokens/tokenTypes.ts (1)
SessionTokenType(16-16)packages/backend/src/tokens/authObjects.ts (1)
InvalidTokenAuthObject(151-157)
packages/nextjs/src/app-router/server/auth.ts (3)
packages/backend/src/tokens/types.ts (2)
InferAuthObjectFromTokenArray(160-168)MachineAuthObject(181-183)packages/backend/src/tokens/tokenTypes.ts (1)
SessionTokenType(16-16)packages/backend/src/tokens/authObjects.ts (1)
InvalidTokenAuthObject(151-157)
packages/backend/src/tokens/__tests__/getAuth.test-d.ts (2)
packages/backend/src/tokens/types.ts (2)
SessionAuthObject(180-180)MachineAuthObject(181-183)packages/backend/src/tokens/authObjects.ts (1)
InvalidTokenAuthObject(151-157)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/backend/src/tokens/__tests__/getAuth.test-d.ts (1)
24-29: LGTM – type expectations correctly expandedThe added
InvalidTokenAuthObjectin the unions accurately mirrors the new fallback behaviour and compiles as expected.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/backend/src/tokens/authObjects.ts (1)
455-462:⚠️ Potential issue
authObject.debugis still passed as data – identical issue flagged previously
unauthenticatedMachineObject/signedOutAuthObjectexpect raw debug data which they wrap withcreateDebug.
Passing the already-wrappedauthObject.debug(a function) leads tocreateDebugspreading a function and silently discarding the original context.Issue was highlighted in an earlier review and remains unfixed.
- return unauthenticatedMachineObject(acceptsToken, authObject.debug); + return unauthenticatedMachineObject(acceptsToken, authObject.debug?.()); - return signedOutAuthObject(authObject.debug); + return signedOutAuthObject(authObject.debug?.());Please apply the same fix everywhere this pattern occurs.
🧹 Nitpick comments (2)
packages/backend/src/tokens/authObjects.ts (2)
151-157: Consider surfacing minimal debug context inInvalidTokenAuthObject
debugcurrently returns an empty object, so consumers lose request-level hints (e.g. headers) that might aid investigation. Passing through sanitised context (like in other factory helpers) would keep parity across auth-object variants.
361-372:invalidTokenAuthObject()duplicates boilerplate – could reuse helpersImplementation is hand-rolled; reusing
unauthenticatedMachineObject& friends (or a shared base factory) would avoid future drift if common props change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/backend/src/tokens/authObjects.ts(12 hunks)packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts(2 hunks)packages/nextjs/src/server/data/getAuthDataFromRequest.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nextjs/src/server/tests/getAuthDataFromRequest.test.ts
- packages/nextjs/src/server/data/getAuthDataFromRequest.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/backend/src/tokens/authObjects.ts (1)
61-63: Nice explicitness withisAuthenticatedflagIntroducing a literal
true/falsediscriminator simplifies downstream type-narrowing and improves DX. No further action required.Also applies to: 82-83, 126-128, 147-148
|
!snapshot |
|
Hey @wobsoriano - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact |
tmilewski
left a comment
tmilewski
left a comment
There was a problem hiding this comment.
Reviewed this with @wobsoriano on Tuple. 🚀
Description
isAuthenticatedproperty to auth objectFor
acceptsTokenas a single value:This means that even if the result of
authenticateRequest()parses a different token type, the returned object will always match what the developer specified inacceptsTokenofauth()orgetAuth().Example:
For
acceptsTokenas an array:Example:
Note:
These changes only affect the behavior for unauthenticated requests and fallback logic. Authenticated flows and matching token types are unchanged.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor