From 48b8917bc20882f44f70eafa4a282e88eaa5d856 Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 28 Apr 2025 21:29:10 -0500 Subject: [PATCH 01/13] feat(backend): handshake nonce --- packages/backend/src/constants.ts | 2 ++ packages/backend/src/tokens/authenticateContext.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index ee7adae57ef..08e90a3164f 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -21,6 +21,7 @@ const Cookies = { Handshake: '__clerk_handshake', DevBrowser: '__clerk_db_jwt', RedirectCount: '__clerk_redirect_count', + HandshakeNonce: '__clerk_handshake_nonce', } as const; const QueryParameters = { @@ -33,6 +34,7 @@ const QueryParameters = { HandshakeHelp: '__clerk_help', LegacyDevBrowser: '__dev_session', HandshakeReason: '__clerk_hs_reason', + HandshakeNonce: Cookies.HandshakeNonce, } as const; const Headers = { diff --git a/packages/backend/src/tokens/authenticateContext.ts b/packages/backend/src/tokens/authenticateContext.ts index 3f945c0a7c9..985c6a84f2a 100644 --- a/packages/backend/src/tokens/authenticateContext.ts +++ b/packages/backend/src/tokens/authenticateContext.ts @@ -27,6 +27,7 @@ interface AuthenticateContext extends AuthenticateRequestOptions { devBrowserToken: string | undefined; handshakeToken: string | undefined; handshakeRedirectLoopCounter: number; + handshakeNonce: string | undefined; // url derived from headers clerkUrl: URL; // enforce existence of the following props @@ -198,6 +199,7 @@ class AuthenticateContext implements AuthenticateContext { this.handshakeToken = this.getQueryParam(constants.QueryParameters.Handshake) || this.getCookie(constants.Cookies.Handshake); this.handshakeRedirectLoopCounter = Number(this.getCookie(constants.Cookies.RedirectCount)) || 0; + this.handshakeNonce = this.getQueryParam(constants.QueryParameters.HandshakeNonce) || undefined; } private getQueryParam(name: string) { From deb2d05a079624b4eaa4e18c720eade13db02e70 Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 28 Apr 2025 22:11:12 -0500 Subject: [PATCH 02/13] get nonce from cookie or query string --- packages/backend/src/tokens/authenticateContext.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/tokens/authenticateContext.ts b/packages/backend/src/tokens/authenticateContext.ts index 985c6a84f2a..db3765be5de 100644 --- a/packages/backend/src/tokens/authenticateContext.ts +++ b/packages/backend/src/tokens/authenticateContext.ts @@ -25,9 +25,10 @@ interface AuthenticateContext extends AuthenticateRequestOptions { clientUat: number; // handshake-related values devBrowserToken: string | undefined; + handshakeNonce: string | undefined; handshakeToken: string | undefined; handshakeRedirectLoopCounter: number; - handshakeNonce: string | undefined; + // url derived from headers clerkUrl: URL; // enforce existence of the following props @@ -199,7 +200,8 @@ class AuthenticateContext implements AuthenticateContext { this.handshakeToken = this.getQueryParam(constants.QueryParameters.Handshake) || this.getCookie(constants.Cookies.Handshake); this.handshakeRedirectLoopCounter = Number(this.getCookie(constants.Cookies.RedirectCount)) || 0; - this.handshakeNonce = this.getQueryParam(constants.QueryParameters.HandshakeNonce) || undefined; + this.handshakeNonce = + this.getQueryParam(constants.QueryParameters.HandshakeNonce) || this.getCookie(constants.Cookies.HandshakeNonce); } private getQueryParam(name: string) { From 816ebd4a37a21fec0660404e102c53e0d4fe0156 Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 28 Apr 2025 22:20:50 -0500 Subject: [PATCH 03/13] stub out handshake nonce handling --- packages/backend/src/tokens/request.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 9d714029618..864082527d5 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -156,9 +156,16 @@ export async function authenticateRequest( 'Access-Control-Allow-Credentials': 'true', }); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const handshakePayload = await verifyHandshakeToken(authenticateContext.handshakeToken!, authenticateContext); - const cookiesToSet = handshakePayload.handshake; + const cookiesToSet: string[] = []; + + if (authenticateContext.handshakeNonce) { + // TODO: implement handshake nonce handling, fetch handshake payload with nonce + + } else { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const handshakePayload = await verifyHandshakeToken(authenticateContext.handshakeToken!, authenticateContext); + cookiesToSet.push(...handshakePayload.handshake); + } let sessionToken = ''; cookiesToSet.forEach((x: string) => { From 37a160307ef54e57e64cb0b1d2b97dee198f8f47 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 08:16:06 -0500 Subject: [PATCH 04/13] handshake api --- .../src/api/endpoints/HandshakePayloadApi.ts | 18 ++++++++++++++++++ .../src/api/resources/HandshakePayload.ts | 15 +++++++++++++++ packages/backend/src/tokens/request.ts | 1 - 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 packages/backend/src/api/endpoints/HandshakePayloadApi.ts create mode 100644 packages/backend/src/api/resources/HandshakePayload.ts diff --git a/packages/backend/src/api/endpoints/HandshakePayloadApi.ts b/packages/backend/src/api/endpoints/HandshakePayloadApi.ts new file mode 100644 index 00000000000..5bdf9f24404 --- /dev/null +++ b/packages/backend/src/api/endpoints/HandshakePayloadApi.ts @@ -0,0 +1,18 @@ +import type { HandshakePayload } from '../resources/HandshakePayload'; +import { AbstractAPI } from './AbstractApi'; + +const BASE_PATH = '/handshake_payload'; + +type GetHandshakePayloadParams = { + nonce: string; +}; + +export class HandshakePayloadAPI extends AbstractAPI { + public async getHandshakePayload(queryParams: GetHandshakePayloadParams) { + return this.request({ + method: 'GET', + path: BASE_PATH, + queryParams, + }); + } +} diff --git a/packages/backend/src/api/resources/HandshakePayload.ts b/packages/backend/src/api/resources/HandshakePayload.ts new file mode 100644 index 00000000000..229acb92951 --- /dev/null +++ b/packages/backend/src/api/resources/HandshakePayload.ts @@ -0,0 +1,15 @@ +export type HandshakePayloadJSON = { + nonce: string; + payload: string; +}; + +export class HandshakePayload { + constructor( + readonly nonce: string, + readonly payload: string, + ) {} + + static fromJSON(data: HandshakePayloadJSON): HandshakePayload { + return new HandshakePayload(data.nonce, data.payload); + } +} diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 864082527d5..30bca5bbb1c 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -160,7 +160,6 @@ export async function authenticateRequest( if (authenticateContext.handshakeNonce) { // TODO: implement handshake nonce handling, fetch handshake payload with nonce - } else { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const handshakePayload = await verifyHandshakeToken(authenticateContext.handshakeToken!, authenticateContext); From d6cf587edf490b76183c030a6de9b26bad6c50ce Mon Sep 17 00:00:00 2001 From: Jacek Radko Date: Tue, 29 Apr 2025 10:24:43 -0500 Subject: [PATCH 05/13] refactor(backend): Introduce HandshakeService (#5754) --- .../src/tokens/__tests__/handshake.test.ts | 391 ++++++++++++++++++ packages/backend/src/tokens/handshake.ts | 283 +++++++++++++ packages/backend/src/tokens/request.ts | 216 +--------- 3 files changed, 681 insertions(+), 209 deletions(-) create mode 100644 packages/backend/src/tokens/__tests__/handshake.test.ts diff --git a/packages/backend/src/tokens/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts new file mode 100644 index 00000000000..11bf493f34e --- /dev/null +++ b/packages/backend/src/tokens/__tests__/handshake.test.ts @@ -0,0 +1,391 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { constants } from '../../constants'; +import { TokenVerificationError, TokenVerificationErrorReason } from '../../errors'; +import type { AuthenticateContext } from '../authenticateContext'; +import { AuthErrorReason, signedIn, signedOut } from '../authStatus'; +import type { OrganizationSyncTargetMatchers } from '../handshake'; +import { HandshakeService } from '../handshake'; + +vi.mock('../handshake.js', async importOriginal => { + const actual: any = await importOriginal(); + return { + ...actual, + verifyHandshakeToken: vi.fn().mockResolvedValue({ + handshake: ['cookie1=value1', 'session=session-token'], + }), + }; +}); + +vi.mock('../verify.js', async importOriginal => { + const actual: any = await importOriginal(); + return { + ...actual, + verifyToken: vi.fn(), + }; +}); + +vi.mock('../../jwt/verifyJwt.js', () => ({ + decodeJwt: vi.fn().mockReturnValue({ + data: { + header: { typ: 'JWT', alg: 'RS256', kid: 'test-kid' }, + payload: { + sub: 'user_123', + __raw: 'raw-token', + iss: 'issuer', + sid: 'session-id', + nbf: 1234567890, + exp: 1234567890, + iat: 1234567890, + v: 2 as const, + fea: undefined, + pla: undefined, + o: undefined, + org_permissions: undefined, + org_id: undefined, + org_slug: undefined, + org_role: undefined, + }, + signature: new Uint8Array([1, 2, 3]), + raw: { + header: 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9', + payload: 'eyJzdWIiOiJ1c2VyXzEyMyJ9', + signature: 'signature', + text: 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ1c2VyXzEyMyJ9.signature', + }, + }, + errors: undefined, + }), +})); + +describe('HandshakeService', () => { + let mockAuthenticateContext: AuthenticateContext; + let mockOrganizationSyncTargetMatchers: OrganizationSyncTargetMatchers; + let mockOptions: { + organizationSyncOptions?: { organizationPatterns?: string[]; personalAccountPatterns?: string[] }; + }; + let handshakeService: HandshakeService; + + beforeEach(() => { + vi.clearAllMocks(); + + mockAuthenticateContext = { + clerkUrl: new URL('https://example.com'), + frontendApi: 'api.clerk.com', + instanceType: 'production', + usesSuffixedCookies: () => true, + secFetchDest: 'document', + accept: 'text/html', + } as AuthenticateContext; + + mockOrganizationSyncTargetMatchers = { + OrganizationMatcher: null, + PersonalAccountMatcher: null, + }; + + mockOptions = { + organizationSyncOptions: { + organizationPatterns: ['/org/:id'], + personalAccountPatterns: ['/account'], + }, + }; + + handshakeService = new HandshakeService(mockAuthenticateContext, mockOrganizationSyncTargetMatchers, mockOptions); + }); + + describe('isRequestEligibleForHandshake', () => { + it('should return true for document secFetchDest', () => { + mockAuthenticateContext.secFetchDest = 'document'; + expect(handshakeService.isRequestEligibleForHandshake()).toBe(true); + }); + + it('should return true for iframe secFetchDest', () => { + mockAuthenticateContext.secFetchDest = 'iframe'; + expect(handshakeService.isRequestEligibleForHandshake()).toBe(true); + }); + + it('should return true for text/html accept header without secFetchDest', () => { + mockAuthenticateContext.secFetchDest = undefined; + mockAuthenticateContext.accept = 'text/html'; + expect(handshakeService.isRequestEligibleForHandshake()).toBe(true); + }); + + it('should return false for non-eligible requests', () => { + mockAuthenticateContext.secFetchDest = 'image'; + mockAuthenticateContext.accept = 'image/png'; + expect(handshakeService.isRequestEligibleForHandshake()).toBe(false); + }); + }); + + describe('buildRedirectToHandshake', () => { + it('should build redirect headers with basic parameters', () => { + const headers = handshakeService.buildRedirectToHandshake('test-reason'); + const location = headers.get(constants.Headers.Location); + if (!location) { + throw new Error('Location header is missing'); + } + const url = new URL(location); + + expect(url.hostname).toBe('api.clerk.com'); + expect(url.pathname).toBe('/v1/client/handshake'); + expect(url.searchParams.get('redirect_url')).toBe('https://example.com/'); + expect(url.searchParams.get(constants.QueryParameters.SuffixedCookies)).toBe('true'); + expect(url.searchParams.get(constants.QueryParameters.HandshakeReason)).toBe('test-reason'); + }); + + it('should include dev browser token in development mode', () => { + mockAuthenticateContext.instanceType = 'development'; + mockAuthenticateContext.devBrowserToken = 'dev-token'; + const headers = handshakeService.buildRedirectToHandshake('test-reason'); + const location = headers.get(constants.Headers.Location); + if (!location) { + throw new Error('Location header is missing'); + } + const url = new URL(location); + + expect(url.searchParams.get(constants.QueryParameters.DevBrowser)).toBe('dev-token'); + }); + + it('should throw error if clerkUrl is missing', () => { + mockAuthenticateContext.clerkUrl = undefined as any; + expect(() => handshakeService.buildRedirectToHandshake('test-reason')).toThrow( + 'Missing clerkUrl in authenticateContext', + ); + }); + }); + + describe.skip('resolveHandshake', () => { + it('should resolve handshake with valid token', async () => { + const mockJwt = { + header: { + typ: 'JWT', + alg: 'RS256', + kid: 'test-kid', + }, + payload: { + sub: 'user_123', + __raw: 'raw-token', + iss: 'issuer', + sid: 'session-id', + nbf: 1234567890, + exp: 1234567890, + iat: 1234567890, + v: 2 as const, + fea: undefined, + pla: undefined, + o: undefined, + org_permissions: undefined, + org_id: undefined, + org_slug: undefined, + org_role: undefined, + }, + signature: new Uint8Array([1, 2, 3]), + raw: { + header: 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9', + payload: 'eyJzdWIiOiJ1c2VyXzEyMyJ9', + signature: 'signature', + text: 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ1c2VyXzEyMyJ9.signature', + }, + }; + const mockHandshakePayload = { + handshake: ['cookie1=value1', 'session=session-token'], + }; + + const mockVerifyToken = vi.mocked(await import('../handshake.js')).verifyHandshakeToken; + mockVerifyToken.mockResolvedValue(mockHandshakePayload); + + const mockVerifyTokenResult = vi.mocked(await import('../verify.js')).verifyToken; + mockVerifyTokenResult.mockResolvedValue({ + data: mockJwt.payload, + errors: undefined, + }); + + const mockDecodeJwt = vi.mocked(await import('../../jwt/verifyJwt.js')).decodeJwt; + mockDecodeJwt.mockReturnValue({ + data: mockJwt, + errors: undefined, + }); + + vi.mocked(await import('../handshake.js')).verifyHandshakeToken.mockResolvedValue(mockHandshakePayload); + + mockAuthenticateContext.handshakeToken = 'any-token'; + const result = await handshakeService.resolveHandshake(); + + expect(result).toEqual( + signedIn( + mockAuthenticateContext, + { + sub: 'user_123', + __raw: 'raw-token', + iss: 'issuer', + sid: 'session-id', + nbf: 1234567890, + exp: 1234567890, + iat: 1234567890, + v: 2 as const, + fea: undefined, + pla: undefined, + o: undefined, + org_permissions: undefined, + org_id: undefined, + org_slug: undefined, + org_role: undefined, + }, + expect.any(Headers), + 'session-token', + ), + ); + }); + + it('should handle missing session token', async () => { + const mockHandshakePayload = { handshake: ['cookie1=value1'] }; + const mockVerifyToken = vi.mocked(await import('../handshake.js')).verifyHandshakeToken; + mockVerifyToken.mockResolvedValue(mockHandshakePayload); + + mockAuthenticateContext.handshakeToken = 'valid-token'; + const result = await handshakeService.resolveHandshake(); + + expect(result).toEqual( + signedOut(mockAuthenticateContext, AuthErrorReason.SessionTokenMissing, '', expect.any(Headers)), + ); + }); + + it('should handle development mode clock skew', async () => { + mockAuthenticateContext.instanceType = 'development'; + + const mockJwt = { + header: { + typ: 'JWT', + alg: 'RS256', + kid: 'test-kid', + }, + payload: { + sub: 'user_123', + __raw: 'raw-token', + iss: 'issuer', + sid: 'session-id', + nbf: 1234567890, + exp: 1234567890, + iat: 1234567890, + v: 2 as const, + fea: undefined, + pla: undefined, + o: undefined, + org_permissions: undefined, + org_id: undefined, + org_slug: undefined, + org_role: undefined, + }, + signature: new Uint8Array([1, 2, 3]), + raw: { + header: 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9', + payload: 'eyJzdWIiOiJ1c2VyXzEyMyJ9', + signature: 'signature', + text: 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ1c2VyXzEyMyJ9.signature', + }, + }; + const mockHandshakePayload = { + handshake: ['cookie1=value1', 'session=session-token'], + }; + + const mockVerifyToken = vi.mocked(await import('../handshake.js')).verifyHandshakeToken; + mockVerifyToken.mockResolvedValue(mockHandshakePayload); + + const mockVerifyTokenResult = vi.mocked(await import('../verify.js')).verifyToken; + mockVerifyTokenResult + .mockRejectedValueOnce( + new TokenVerificationError({ + reason: TokenVerificationErrorReason.TokenExpired, + message: 'Token expired', + }), + ) + .mockResolvedValueOnce({ + data: mockJwt.payload, + errors: undefined, + }); + + const mockDecodeJwt = vi.mocked(await import('../../jwt/verifyJwt.js')).decodeJwt; + mockDecodeJwt.mockReturnValue({ + data: mockJwt, + errors: undefined, + }); + + // Mock verifyHandshakeToken to return our mock data directly + vi.mocked(await import('../handshake.js')).verifyHandshakeToken.mockResolvedValue(mockHandshakePayload); + + mockAuthenticateContext.handshakeToken = 'any-token'; + const result = await handshakeService.resolveHandshake(); + + expect(result).toEqual( + signedIn( + mockAuthenticateContext, + { + sub: 'user_123', + __raw: 'raw-token', + iss: 'issuer', + sid: 'session-id', + nbf: 1234567890, + exp: 1234567890, + iat: 1234567890, + v: 2 as const, + fea: undefined, + pla: undefined, + o: undefined, + org_permissions: undefined, + org_id: undefined, + org_slug: undefined, + org_role: undefined, + }, + expect.any(Headers), + 'session-token', + ), + ); + }); + }); + + describe('handleHandshakeTokenVerificationErrorInDevelopment', () => { + it('should throw specific error for invalid signature', () => { + const error = new TokenVerificationError({ + reason: TokenVerificationErrorReason.TokenInvalidSignature, + message: 'Invalid signature', + }); + + expect(() => handshakeService.handleHandshakeTokenVerificationErrorInDevelopment(error)).toThrow( + 'Clerk: Handshake token verification failed due to an invalid signature', + ); + }); + + it('should throw generic error for other verification failures', () => { + const error = new TokenVerificationError({ + reason: TokenVerificationErrorReason.TokenExpired, + message: 'Token expired', + }); + + expect(() => handshakeService.handleHandshakeTokenVerificationErrorInDevelopment(error)).toThrow( + 'Clerk: Handshake token verification failed: Token expired', + ); + }); + }); + + describe('setHandshakeInfiniteRedirectionLoopHeaders', () => { + it('should return true after 3 redirects', () => { + const headers = new Headers(); + handshakeService['handshakeRedirectLoopCounter'] = 3; + + const result = handshakeService.setHandshakeInfiniteRedirectionLoopHeaders(headers); + + expect(result).toBe(true); + expect(headers.get('Set-Cookie')).toBeNull(); + }); + + it('should increment counter and set cookie for first redirect', () => { + const headers = new Headers(); + handshakeService['handshakeRedirectLoopCounter'] = 0; + + const result = handshakeService.setHandshakeInfiniteRedirectionLoopHeaders(headers); + + expect(result).toBe(false); + expect(headers.get('Set-Cookie')).toContain('__clerk_redirect_count=1'); + }); + }); +}); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 1ac4b95dc30..3ef29543719 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -1,9 +1,18 @@ +import type { Match, MatchFunction } from '@clerk/shared/pathToRegexp'; + +import { constants } from '../constants'; import { TokenVerificationError, TokenVerificationErrorAction, TokenVerificationErrorReason } from '../errors'; import type { VerifyJwtOptions } from '../jwt'; import { assertHeaderAlgorithm, assertHeaderType } from '../jwt/assertions'; import { decodeJwt, hasValidSignature } from '../jwt/verifyJwt'; +import type { AuthenticateContext } from './authenticateContext'; +import type { SignedInState, SignedOutState } from './authStatus'; +import { AuthErrorReason, signedIn, signedOut } from './authStatus'; +import { getCookieName, getCookieValue } from './cookie'; import { loadClerkJWKFromLocal, loadClerkJWKFromRemote } from './keys'; +import type { OrganizationSyncOptions } from './types'; import type { VerifyTokenOptions } from './verify'; +import { verifyToken } from './verify'; async function verifyHandshakeJwt(token: string, { key }: VerifyJwtOptions): Promise<{ handshake: string[] }> { const { data: decoded, errors } = decodeJwt(token); @@ -73,3 +82,277 @@ export async function verifyHandshakeToken( key, }); } + +export type OrganizationSyncTargetMatchers = { + OrganizationMatcher: MatchFunction>> | null; + PersonalAccountMatcher: MatchFunction>> | null; +}; + +export type OrganizationSyncTarget = + | { type: 'personalAccount' } + | { type: 'organization'; organizationId?: string; organizationSlug?: string }; + +export class HandshakeService { + private handshakeRedirectLoopCounter: number; + private readonly authenticateContext: AuthenticateContext; + private readonly organizationSyncTargetMatchers: OrganizationSyncTargetMatchers; + private readonly options: { organizationSyncOptions?: OrganizationSyncOptions }; + + constructor( + authenticateContext: AuthenticateContext, + organizationSyncTargetMatchers: OrganizationSyncTargetMatchers, + options: { organizationSyncOptions?: OrganizationSyncOptions }, + ) { + this.handshakeRedirectLoopCounter = 0; + this.authenticateContext = authenticateContext; + this.organizationSyncTargetMatchers = organizationSyncTargetMatchers; + this.options = options; + } + + /** + * Determines if a request is eligible for handshake based on its headers + * @returns boolean indicating if the request is eligible for handshake + */ + isRequestEligibleForHandshake(): boolean { + const { accept, secFetchDest } = this.authenticateContext; + + if (secFetchDest === 'document' || secFetchDest === 'iframe') { + return true; + } + + if (!secFetchDest && accept?.startsWith('text/html')) { + return true; + } + + return false; + } + + /** + * Builds the redirect headers for a handshake request + * @param reason - The reason for the handshake (e.g. 'session-token-expired') + * @returns Headers object containing the Location header for redirect + * @throws Error if clerkUrl is missing in authenticateContext + */ + buildRedirectToHandshake(reason: string): Headers { + if (!this.authenticateContext?.clerkUrl) { + throw new Error('Missing clerkUrl in authenticateContext'); + } + + const redirectUrl = this.removeDevBrowserFromURL(this.authenticateContext.clerkUrl); + const frontendApiNoProtocol = this.authenticateContext.frontendApi.replace(/http(s)?:\/\//, ''); + + const url = new URL(`https://${frontendApiNoProtocol}/v1/client/handshake`); + url.searchParams.append('redirect_url', redirectUrl?.href || ''); + url.searchParams.append( + constants.QueryParameters.SuffixedCookies, + this.authenticateContext.usesSuffixedCookies().toString(), + ); + url.searchParams.append(constants.QueryParameters.HandshakeReason, reason); + + if (this.authenticateContext.instanceType === 'development' && this.authenticateContext.devBrowserToken) { + url.searchParams.append(constants.QueryParameters.DevBrowser, this.authenticateContext.devBrowserToken); + } + + const toActivate = this.getOrganizationSyncTarget( + this.authenticateContext.clerkUrl, + this.options.organizationSyncOptions, + this.organizationSyncTargetMatchers, + ); + if (toActivate) { + const params = this.getOrganizationSyncQueryParams(toActivate); + params.forEach((value, key) => { + url.searchParams.append(key, value); + }); + } + + return new Headers({ [constants.Headers.Location]: url.href }); + } + + /** + * Resolves a handshake request by verifying the handshake token and setting appropriate cookies + * @returns Promise resolving to either a SignedInState or SignedOutState + * @throws Error if handshake verification fails or if there are issues with the session token + */ + async resolveHandshake(): Promise { + const headers = new Headers({ + 'Access-Control-Allow-Origin': 'null', + 'Access-Control-Allow-Credentials': 'true', + }); + + const cookiesToSet: string[] = []; + + if (this.authenticateContext.handshakeNonce) { + // TODO: implement handshake nonce handling, fetch handshake payload with nonce + } else if (this.authenticateContext.handshakeToken) { + const handshakePayload = await verifyHandshakeToken( + this.authenticateContext.handshakeToken, + this.authenticateContext, + ); + cookiesToSet.push(...handshakePayload.handshake); + } + + let sessionToken = ''; + cookiesToSet.forEach((x: string) => { + headers.append('Set-Cookie', x); + if (getCookieName(x).startsWith(constants.Cookies.Session)) { + sessionToken = getCookieValue(x); + } + }); + + if (this.authenticateContext.instanceType === 'development') { + const newUrl = new URL(this.authenticateContext.clerkUrl); + newUrl.searchParams.delete(constants.QueryParameters.Handshake); + newUrl.searchParams.delete(constants.QueryParameters.HandshakeHelp); + headers.append(constants.Headers.Location, newUrl.toString()); + headers.set(constants.Headers.CacheControl, 'no-store'); + } + + if (sessionToken === '') { + return signedOut(this.authenticateContext, AuthErrorReason.SessionTokenMissing, '', headers); + } + + const { data, errors: [error] = [] } = await verifyToken(sessionToken, this.authenticateContext); + if (data) { + return signedIn(this.authenticateContext, data, headers, sessionToken); + } + + if ( + this.authenticateContext.instanceType === 'development' && + (error?.reason === TokenVerificationErrorReason.TokenExpired || + error?.reason === TokenVerificationErrorReason.TokenNotActiveYet || + error?.reason === TokenVerificationErrorReason.TokenIatInTheFuture) + ) { + // Create a new error object with the same properties + const developmentError = new TokenVerificationError({ + action: error.action, + message: error.message, + reason: error.reason, + }); + // Set the tokenCarrier after construction + developmentError.tokenCarrier = 'cookie'; + + console.error( + `Clerk: Clock skew detected. This usually means that your system clock is inaccurate. Clerk will attempt to account for the clock skew in development. + +To resolve this issue, make sure your system's clock is set to the correct time (e.g. turn off and on automatic time synchronization). + +--- + +${developmentError.getFullMessage()}`, + ); + + const { data: retryResult, errors: [retryError] = [] } = await verifyToken(sessionToken, { + ...this.authenticateContext, + clockSkewInMs: 86_400_000, + }); + if (retryResult) { + return signedIn(this.authenticateContext, retryResult, headers, sessionToken); + } + + throw new Error(retryError?.message || 'Clerk: Handshake retry failed.'); + } + + throw new Error(error?.message || 'Clerk: Handshake failed.'); + } + + /** + * Handles handshake token verification errors in development mode + * @param error - The TokenVerificationError that occurred + * @throws Error with a descriptive message about the verification failure + */ + handleHandshakeTokenVerificationErrorInDevelopment(error: TokenVerificationError): void { + if (error.reason === TokenVerificationErrorReason.TokenInvalidSignature) { + const msg = `Clerk: Handshake token verification failed due to an invalid signature. If you have switched Clerk keys locally, clear your cookies and try again.`; + throw new Error(msg); + } + throw new Error(`Clerk: Handshake token verification failed: ${error.getFullMessage()}.`); + } + + /** + * Sets headers to prevent infinite handshake redirection loops + * @param headers - The Headers object to modify + * @returns boolean indicating if a redirect loop was detected (true) or if the request can proceed (false) + */ + setHandshakeInfiniteRedirectionLoopHeaders(headers: Headers): boolean { + if (this.handshakeRedirectLoopCounter === 3) { + return true; + } + + const newCounterValue = this.handshakeRedirectLoopCounter + 1; + const cookieName = constants.Cookies.RedirectCount; + headers.append('Set-Cookie', `${cookieName}=${newCounterValue}; SameSite=Lax; HttpOnly; Max-Age=3`); + return false; + } + + private removeDevBrowserFromURL(url: URL): URL { + const updatedURL = new URL(url); + updatedURL.searchParams.delete(constants.QueryParameters.DevBrowser); + updatedURL.searchParams.delete(constants.QueryParameters.LegacyDevBrowser); + return updatedURL; + } + + private getOrganizationSyncTarget( + url: URL, + options: OrganizationSyncOptions | undefined, + matchers: OrganizationSyncTargetMatchers, + ): OrganizationSyncTarget | null { + if (!options) { + return null; + } + + if (matchers.OrganizationMatcher) { + let orgResult: Match>>; + try { + orgResult = matchers.OrganizationMatcher(url.pathname); + } catch (e) { + console.error(`Clerk: Failed to apply organization pattern "${options.organizationPatterns}" to a path`, e); + return null; + } + + if (orgResult && 'params' in orgResult) { + const params = orgResult.params; + + if ('id' in params && typeof params.id === 'string') { + return { type: 'organization', organizationId: params.id }; + } + if ('slug' in params && typeof params.slug === 'string') { + return { type: 'organization', organizationSlug: params.slug }; + } + console.warn( + 'Clerk: Detected an organization pattern match, but no organization ID or slug was found in the URL. Does the pattern include `:id` or `:slug`?', + ); + } + } + + if (matchers.PersonalAccountMatcher) { + let personalResult: Match>>; + try { + personalResult = matchers.PersonalAccountMatcher(url.pathname); + } catch (e) { + console.error(`Failed to apply personal account pattern "${options.personalAccountPatterns}" to a path`, e); + return null; + } + + if (personalResult) { + return { type: 'personalAccount' }; + } + } + return null; + } + + private getOrganizationSyncQueryParams(toActivate: OrganizationSyncTarget): Map { + const ret = new Map(); + if (toActivate.type === 'personalAccount') { + ret.set('organization_id', ''); + } + if (toActivate.type === 'organization') { + if (toActivate.organizationId) { + ret.set('organization_id', toActivate.organizationId); + } + if (toActivate.organizationSlug) { + ret.set('organization_id', toActivate.organizationSlug); + } + } + return ret; + } +} diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 30bca5bbb1c..971ee331a78 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -15,7 +15,7 @@ import type { HandshakeState, RequestState, SignedInState, SignedOutState } from import { AuthErrorReason, handshake, signedIn, signedOut } from './authStatus'; import { createClerkRequest } from './clerkRequest'; import { getCookieName, getCookieValue } from './cookie'; -import { verifyHandshakeToken } from './handshake'; +import { HandshakeService } from './handshake'; import type { AuthenticateRequestOptions, OrganizationSyncOptions } from './types'; import { verifyToken } from './verify'; @@ -58,26 +58,6 @@ function assertSignInUrlFormatAndOrigin(_signInUrl: string, origin: string) { } } -/** - * Currently, a request is only eligible for a handshake if we can say it's *probably* a request for a document, not a fetch or some other exotic request. - * This heuristic should give us a reliable enough signal for browsers that support `Sec-Fetch-Dest` and for those that don't. - */ -function isRequestEligibleForHandshake(authenticateContext: { secFetchDest?: string; accept?: string }) { - const { accept, secFetchDest } = authenticateContext; - - // NOTE: we could also check sec-fetch-mode === navigate here, but according to the spec, sec-fetch-dest: document should indicate that the request is the data of a user navigation. - // Also, we check for 'iframe' because it's the value set when a doc request is made by an iframe. - if (secFetchDest === 'document' || secFetchDest === 'iframe') { - return true; - } - - if (!secFetchDest && accept?.startsWith('text/html')) { - return true; - } - - return false; -} - function isRequestEligibleForRefresh( err: TokenVerificationError, authenticateContext: { refreshTokenInCookie?: string }, @@ -105,124 +85,8 @@ export async function authenticateRequest( assertProxyUrlOrDomain(authenticateContext.proxyUrl || authenticateContext.domain); } - // NOTE(izaak): compute regex matchers early for efficiency - they can be used multiple times. const organizationSyncTargetMatchers = computeOrganizationSyncTargetMatchers(options.organizationSyncOptions); - - function removeDevBrowserFromURL(url: URL) { - const updatedURL = new URL(url); - - updatedURL.searchParams.delete(constants.QueryParameters.DevBrowser); - // Remove legacy dev browser query param key to support local app with v5 using AP with v4 - updatedURL.searchParams.delete(constants.QueryParameters.LegacyDevBrowser); - - return updatedURL; - } - - function buildRedirectToHandshake({ handshakeReason }: { handshakeReason: string }) { - const redirectUrl = removeDevBrowserFromURL(authenticateContext.clerkUrl); - const frontendApiNoProtocol = authenticateContext.frontendApi.replace(/http(s)?:\/\//, ''); - - const url = new URL(`https://${frontendApiNoProtocol}/v1/client/handshake`); - url.searchParams.append('redirect_url', redirectUrl?.href || ''); - url.searchParams.append( - constants.QueryParameters.SuffixedCookies, - authenticateContext.usesSuffixedCookies().toString(), - ); - url.searchParams.append(constants.QueryParameters.HandshakeReason, handshakeReason); - - if (authenticateContext.instanceType === 'development' && authenticateContext.devBrowserToken) { - url.searchParams.append(constants.QueryParameters.DevBrowser, authenticateContext.devBrowserToken); - } - - const toActivate = getOrganizationSyncTarget( - authenticateContext.clerkUrl, - options.organizationSyncOptions, - organizationSyncTargetMatchers, - ); - if (toActivate) { - const params = getOrganizationSyncQueryParams(toActivate); - - params.forEach((value, key) => { - url.searchParams.append(key, value); - }); - } - - return new Headers({ [constants.Headers.Location]: url.href }); - } - - async function resolveHandshake() { - const headers = new Headers({ - 'Access-Control-Allow-Origin': 'null', - 'Access-Control-Allow-Credentials': 'true', - }); - - const cookiesToSet: string[] = []; - - if (authenticateContext.handshakeNonce) { - // TODO: implement handshake nonce handling, fetch handshake payload with nonce - } else { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const handshakePayload = await verifyHandshakeToken(authenticateContext.handshakeToken!, authenticateContext); - cookiesToSet.push(...handshakePayload.handshake); - } - - let sessionToken = ''; - cookiesToSet.forEach((x: string) => { - headers.append('Set-Cookie', x); - if (getCookieName(x).startsWith(constants.Cookies.Session)) { - sessionToken = getCookieValue(x); - } - }); - - if (authenticateContext.instanceType === 'development') { - const newUrl = new URL(authenticateContext.clerkUrl); - newUrl.searchParams.delete(constants.QueryParameters.Handshake); - newUrl.searchParams.delete(constants.QueryParameters.HandshakeHelp); - headers.append(constants.Headers.Location, newUrl.toString()); - headers.set(constants.Headers.CacheControl, 'no-store'); - } - - if (sessionToken === '') { - return signedOut(authenticateContext, AuthErrorReason.SessionTokenMissing, '', headers); - } - - const { data, errors: [error] = [] } = await verifyToken(sessionToken, authenticateContext); - if (data) { - return signedIn(authenticateContext, data, headers, sessionToken); - } - - if ( - authenticateContext.instanceType === 'development' && - (error?.reason === TokenVerificationErrorReason.TokenExpired || - error?.reason === TokenVerificationErrorReason.TokenNotActiveYet || - error?.reason === TokenVerificationErrorReason.TokenIatInTheFuture) - ) { - error.tokenCarrier = 'cookie'; - // This probably means we're dealing with clock skew - console.error( - `Clerk: Clock skew detected. This usually means that your system clock is inaccurate. Clerk will attempt to account for the clock skew in development. - -To resolve this issue, make sure your system's clock is set to the correct time (e.g. turn off and on automatic time synchronization). - ---- - -${error.getFullMessage()}`, - ); - - // Retry with a generous clock skew allowance (1 day) - const { data: retryResult, errors: [retryError] = [] } = await verifyToken(sessionToken, { - ...authenticateContext, - clockSkewInMs: 86_400_000, - }); - if (retryResult) { - return signedIn(authenticateContext, retryResult, headers, sessionToken); - } - - throw new Error(retryError?.message || 'Clerk: Handshake retry failed.'); - } - - throw new Error(error?.message || 'Clerk: Handshake failed.'); - } + const handshakeService = new HandshakeService(authenticateContext, organizationSyncTargetMatchers, options); async function refreshToken( authenticateContext: AuthenticateContext, @@ -360,21 +224,14 @@ ${error.getFullMessage()}`, message: string, headers?: Headers, ): SignedInState | SignedOutState | HandshakeState { - if (isRequestEligibleForHandshake(authenticateContext)) { - // Right now the only usage of passing in different headers is for multi-domain sync, which redirects somewhere else. - // In the future if we want to decorate the handshake redirect with additional headers per call we need to tweak this logic. - const handshakeHeaders = headers ?? buildRedirectToHandshake({ handshakeReason: reason }); + if (handshakeService.isRequestEligibleForHandshake()) { + const handshakeHeaders = headers ?? handshakeService.buildRedirectToHandshake(reason); - // Chrome aggressively caches inactive tabs. If we don't set the header here, - // all 307 redirects will be cached and the handshake will end up in an infinite loop. if (handshakeHeaders.get(constants.Headers.Location)) { handshakeHeaders.set(constants.Headers.CacheControl, 'no-store'); } - // Introduce the mechanism to protect for infinite handshake redirect loops - // using a cookie and returning true if it's infinite redirect loop or false if we can - // proceed with triggering handshake. - const isRedirectLoop = setHandshakeInfiniteRedirectionLoopHeaders(handshakeHeaders); + const isRedirectLoop = handshakeService.setHandshakeInfiniteRedirectionLoopHeaders(handshakeHeaders); if (isRedirectLoop) { const msg = `Clerk: Refreshing the session token resulted in an infinite redirect loop. This usually means that your Clerk instance keys do not match - make sure to copy the correct publishable and secret keys from the Clerk dashboard.`; console.log(msg); @@ -465,34 +322,6 @@ ${error.getFullMessage()}`, } } - // We want to prevent infinite handshake redirection loops. - // We incrementally set a `__clerk_redirection_loop` cookie, and when it loops 3 times, we throw an error. - // We also utilize the `referer` header to skip the prefetch requests. - function setHandshakeInfiniteRedirectionLoopHeaders(headers: Headers): boolean { - if (authenticateContext.handshakeRedirectLoopCounter === 3) { - return true; - } - - const newCounterValue = authenticateContext.handshakeRedirectLoopCounter + 1; - const cookieName = constants.Cookies.RedirectCount; - headers.append('Set-Cookie', `${cookieName}=${newCounterValue}; SameSite=Lax; HttpOnly; Max-Age=3`); - return false; - } - - function handleHandshakeTokenVerificationErrorInDevelopment(error: TokenVerificationError) { - // In development, the handshake token is being transferred in the URL as a query parameter, so there is no - // possibility of collision with a handshake token of another app running on the same local domain - // (etc one app on localhost:3000 and one on localhost:3001). - // Therefore, if the handshake token is invalid, it is likely that the user has switched Clerk keys locally. - // We make sure to throw a descriptive error message and then stop the handshake flow in every case, - // to avoid the possibility of an infinite loop. - if (error.reason === TokenVerificationErrorReason.TokenInvalidSignature) { - const msg = `Clerk: Handshake token verification failed due to an invalid signature. If you have switched Clerk keys locally, clear your cookies and try again.`; - throw new Error(msg); - } - throw new Error(`Clerk: Handshake token verification failed: ${error.getFullMessage()}.`); - } - async function authenticateRequestWithTokenInCookie() { const hasActiveClient = authenticateContext.clientUat; const hasSessionToken = !!authenticateContext.sessionTokenInCookie; @@ -503,21 +332,10 @@ ${error.getFullMessage()}`, */ if (authenticateContext.handshakeToken) { try { - return await resolveHandshake(); + return await handshakeService.resolveHandshake(); } catch (error) { - // In production, the handshake token is being transferred as a cookie, so there is a possibility of collision - // with a handshake token of another app running on the same etld+1 domain. - // For example, if one app is running on sub1.clerk.com and another on sub2.clerk.com, the handshake token - // cookie for both apps will be set on etld+1 (clerk.com) so there's a possibility that one app will accidentally - // use the handshake token of a different app during the handshake flow. - // In this scenario, verification will fail with TokenInvalidSignature. In contrast to the development case, - // we need to allow the flow to continue so the app eventually retries another handshake with the correct token. - // We need to make sure, however, that we don't allow the flow to continue indefinitely, so we throw an error after X - // retries to avoid an infinite loop. An infinite loop can happen if the customer switched Clerk keys for their prod app. - - // Check the handleHandshakeTokenVerificationErrorInDevelopment function for the development case. if (error instanceof TokenVerificationError && authenticateContext.instanceType === 'development') { - handleHandshakeTokenVerificationErrorInDevelopment(error); + handshakeService.handleHandshakeTokenVerificationErrorInDevelopment(error); } else { console.error('Clerk: unable to resolve handshake:', error); } @@ -818,26 +636,6 @@ export type OrganizationSyncTarget = | { type: 'personalAccount' } | { type: 'organization'; organizationId?: string; organizationSlug?: string }; -/** - * Generates the query parameters to activate an organization or personal account - * via the FAPI handshake api. - */ -function getOrganizationSyncQueryParams(toActivate: OrganizationSyncTarget): Map { - const ret = new Map(); - if (toActivate.type === 'personalAccount') { - ret.set('organization_id', ''); - } - if (toActivate.type === 'organization') { - if (toActivate.organizationId) { - ret.set('organization_id', toActivate.organizationId); - } - if (toActivate.organizationSlug) { - ret.set('organization_id', toActivate.organizationSlug); - } - } - return ret; -} - const convertTokenVerificationErrorReasonToAuthErrorReason = ({ tokenError, refreshError, From ac2bc13a85ef09f4dab441475e8f809cdf157b7b Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 10:30:08 -0500 Subject: [PATCH 06/13] wip (cherry picked from commit 179622b824ae4d31e561c4258d3493e714f48900) --- packages/backend/src/tokens/handshake.ts | 12 ++++++++++++ packages/backend/src/tokens/request.ts | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 3ef29543719..6b61a653dc6 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -111,11 +111,17 @@ export class HandshakeService { /** * Determines if a request is eligible for handshake based on its headers + * + * Currently, a request is only eligible for a handshake if we can say it's *probably* a request for a document, not a fetch or some other exotic request. + * This heuristic should give us a reliable enough signal for browsers that support `Sec-Fetch-Dest` and for those that don't. + * * @returns boolean indicating if the request is eligible for handshake */ isRequestEligibleForHandshake(): boolean { const { accept, secFetchDest } = this.authenticateContext; + // NOTE: we could also check sec-fetch-mode === navigate here, but according to the spec, sec-fetch-dest: document should indicate that the request is the data of a user navigation. + // Also, we check for 'iframe' because it's the value set when a doc request is made by an iframe. if (secFetchDest === 'document' || secFetchDest === 'iframe') { return true; } @@ -261,6 +267,12 @@ ${developmentError.getFullMessage()}`, * @throws Error with a descriptive message about the verification failure */ handleHandshakeTokenVerificationErrorInDevelopment(error: TokenVerificationError): void { + // In development, the handshake token is being transferred in the URL as a query parameter, so there is no + // possibility of collision with a handshake token of another app running on the same local domain + // (etc one app on localhost:3000 and one on localhost:3001). + // Therefore, if the handshake token is invalid, it is likely that the user has switched Clerk keys locally. + // We make sure to throw a descriptive error message and then stop the handshake flow in every case, + // to avoid the possibility of an infinite loop. if (error.reason === TokenVerificationErrorReason.TokenInvalidSignature) { const msg = `Clerk: Handshake token verification failed due to an invalid signature. If you have switched Clerk keys locally, clear your cookies and try again.`; throw new Error(msg); diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 971ee331a78..adb13556075 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -225,12 +225,19 @@ export async function authenticateRequest( headers?: Headers, ): SignedInState | SignedOutState | HandshakeState { if (handshakeService.isRequestEligibleForHandshake()) { + // Right now the only usage of passing in different headers is for multi-domain sync, which redirects somewhere else. + // In the future if we want to decorate the handshake redirect with additional headers per call we need to tweak this logic. const handshakeHeaders = headers ?? handshakeService.buildRedirectToHandshake(reason); + // Chrome aggressively caches inactive tabs. If we don't set the header here, + // all 307 redirects will be cached and the handshake will end up in an infinite loop. if (handshakeHeaders.get(constants.Headers.Location)) { handshakeHeaders.set(constants.Headers.CacheControl, 'no-store'); } + // Introduce the mechanism to protect for infinite handshake redirect loops + // using a cookie and returning true if it's infinite redirect loop or false if we can + // proceed with triggering handshake. const isRedirectLoop = handshakeService.setHandshakeInfiniteRedirectionLoopHeaders(handshakeHeaders); if (isRedirectLoop) { const msg = `Clerk: Refreshing the session token resulted in an infinite redirect loop. This usually means that your Clerk instance keys do not match - make sure to copy the correct publishable and secret keys from the Clerk dashboard.`; @@ -334,6 +341,17 @@ export async function authenticateRequest( try { return await handshakeService.resolveHandshake(); } catch (error) { + // In production, the handshake token is being transferred as a cookie, so there is a possibility of collision + // with a handshake token of another app running on the same etld+1 domain. + // For example, if one app is running on sub1.clerk.com and another on sub2.clerk.com, the handshake token + // cookie for both apps will be set on etld+1 (clerk.com) so there's a possibility that one app will accidentally + // use the handshake token of a different app during the handshake flow. + // In this scenario, verification will fail with TokenInvalidSignature. In contrast to the development case, + // we need to allow the flow to continue so the app eventually retries another handshake with the correct token. + // We need to make sure, however, that we don't allow the flow to continue indefinitely, so we throw an error after X + // retries to avoid an infinite loop. An infinite loop can happen if the customer switched Clerk keys for their prod app. + + // Check the handleHandshakeTokenVerificationErrorInDevelopment function for the development case. if (error instanceof TokenVerificationError && authenticateContext.instanceType === 'development') { handshakeService.handleHandshakeTokenVerificationErrorInDevelopment(error); } else { From 653c85ca6393334f98c7526276bf10ebcfa5888e Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 11:08:53 -0500 Subject: [PATCH 07/13] changeset --- .changeset/better-vans-obey.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/better-vans-obey.md diff --git a/.changeset/better-vans-obey.md b/.changeset/better-vans-obey.md new file mode 100644 index 00000000000..82e2b151edd --- /dev/null +++ b/.changeset/better-vans-obey.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': minor +--- + +Support new handshake payload flow with nonce From b1b5bf6c7634f5ac233c35489169debf5196e9a9 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 11:30:01 -0500 Subject: [PATCH 08/13] wip --- packages/backend/src/tokens/handshake.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 6b61a653dc6..cf1374e9dda 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -189,7 +189,9 @@ export class HandshakeService { if (this.authenticateContext.handshakeNonce) { // TODO: implement handshake nonce handling, fetch handshake payload with nonce - } else if (this.authenticateContext.handshakeToken) { + console.warn('Clerk: Handshake nonce is not implemented yet.'); + } + if (this.authenticateContext.handshakeToken) { const handshakePayload = await verifyHandshakeToken( this.authenticateContext.handshakeToken, this.authenticateContext, From 9b1c003395606c8a276eb342bd7880ec12dc1528 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 11:37:44 -0500 Subject: [PATCH 09/13] dry up types --- .../src/tokens/__tests__/handshake.test.ts | 2 +- packages/backend/src/tokens/handshake.ts | 13 ++---------- packages/backend/src/tokens/request.ts | 20 ++++++------------- packages/backend/src/tokens/types.ts | 15 ++++++++++++++ 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts index 11bf493f34e..8fbdfcef2d2 100644 --- a/packages/backend/src/tokens/__tests__/handshake.test.ts +++ b/packages/backend/src/tokens/__tests__/handshake.test.ts @@ -4,8 +4,8 @@ import { constants } from '../../constants'; import { TokenVerificationError, TokenVerificationErrorReason } from '../../errors'; import type { AuthenticateContext } from '../authenticateContext'; import { AuthErrorReason, signedIn, signedOut } from '../authStatus'; -import type { OrganizationSyncTargetMatchers } from '../handshake'; import { HandshakeService } from '../handshake'; +import type { OrganizationSyncTargetMatchers } from '../types'; vi.mock('../handshake.js', async importOriginal => { const actual: any = await importOriginal(); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index cf1374e9dda..c7fd736649c 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -1,4 +1,4 @@ -import type { Match, MatchFunction } from '@clerk/shared/pathToRegexp'; +import type { Match } from '@clerk/shared/pathToRegexp'; import { constants } from '../constants'; import { TokenVerificationError, TokenVerificationErrorAction, TokenVerificationErrorReason } from '../errors'; @@ -10,7 +10,7 @@ import type { SignedInState, SignedOutState } from './authStatus'; import { AuthErrorReason, signedIn, signedOut } from './authStatus'; import { getCookieName, getCookieValue } from './cookie'; import { loadClerkJWKFromLocal, loadClerkJWKFromRemote } from './keys'; -import type { OrganizationSyncOptions } from './types'; +import type { OrganizationSyncOptions, OrganizationSyncTarget, OrganizationSyncTargetMatchers } from './types'; import type { VerifyTokenOptions } from './verify'; import { verifyToken } from './verify'; @@ -83,15 +83,6 @@ export async function verifyHandshakeToken( }); } -export type OrganizationSyncTargetMatchers = { - OrganizationMatcher: MatchFunction>> | null; - PersonalAccountMatcher: MatchFunction>> | null; -}; - -export type OrganizationSyncTarget = - | { type: 'personalAccount' } - | { type: 'organization'; organizationId?: string; organizationSlug?: string }; - export class HandshakeService { private handshakeRedirectLoopCounter: number; private readonly authenticateContext: AuthenticateContext; diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index adb13556075..c49faba9177 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -16,7 +16,12 @@ import { AuthErrorReason, handshake, signedIn, signedOut } from './authStatus'; import { createClerkRequest } from './clerkRequest'; import { getCookieName, getCookieValue } from './cookie'; import { HandshakeService } from './handshake'; -import type { AuthenticateRequestOptions, OrganizationSyncOptions } from './types'; +import type { + AuthenticateRequestOptions, + OrganizationSyncOptions, + OrganizationSyncTarget, + OrganizationSyncTargetMatchers, +} from './types'; import { verifyToken } from './verify'; export const RefreshTokenErrorReason = { @@ -548,11 +553,6 @@ export const debugRequestState = (params: RequestState) => { return { isSignedIn, proxyUrl, reason, message, publishableKey, isSatellite, domain }; }; -type OrganizationSyncTargetMatchers = { - OrganizationMatcher: MatchFunction>> | null; - PersonalAccountMatcher: MatchFunction>> | null; -}; - /** * Computes regex-based matchers from the given organization sync options. */ @@ -646,14 +646,6 @@ export function getOrganizationSyncTarget( return null; } -/** - * Represents an organization or a personal account - e.g. an - * entity that can be activated by the handshake API. - */ -export type OrganizationSyncTarget = - | { type: 'personalAccount' } - | { type: 'organization'; organizationId?: string; organizationSlug?: string }; - const convertTokenVerificationErrorReasonToAuthErrorReason = ({ tokenError, refreshError, diff --git a/packages/backend/src/tokens/types.ts b/packages/backend/src/tokens/types.ts index f96417c8182..ad282609dc6 100644 --- a/packages/backend/src/tokens/types.ts +++ b/packages/backend/src/tokens/types.ts @@ -1,3 +1,5 @@ +import type { MatchFunction } from '@clerk/shared/pathToRegexp'; + import type { ApiClient } from '../api'; import type { VerifyTokenOptions } from './verify'; @@ -116,3 +118,16 @@ export type OrganizationSyncOptions = { * ``` */ type Pattern = string; + +export type OrganizationSyncTargetMatchers = { + OrganizationMatcher: MatchFunction>> | null; + PersonalAccountMatcher: MatchFunction>> | null; +}; + +/** + * Represents an organization or a personal account - e.g. an + * entity that can be activated by the handshake API. + */ +export type OrganizationSyncTarget = + | { type: 'personalAccount' } + | { type: 'organization'; organizationId?: string; organizationSlug?: string }; From 7c2f549bd7f4aacd0472797c6c774143e805145d Mon Sep 17 00:00:00 2001 From: Jacek Radko Date: Tue, 29 Apr 2025 11:41:13 -0500 Subject: [PATCH 10/13] Update .changeset/better-vans-obey.md --- .changeset/better-vans-obey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/better-vans-obey.md b/.changeset/better-vans-obey.md index 82e2b151edd..86bf3d4e8c8 100644 --- a/.changeset/better-vans-obey.md +++ b/.changeset/better-vans-obey.md @@ -2,4 +2,4 @@ '@clerk/backend': minor --- -Support new handshake payload flow with nonce +Initial stub of the new handshake payload flow with nonce From 68efd1b91b63d2ae72601111ed1b85ea91f6277e Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 12:03:27 -0500 Subject: [PATCH 11/13] wip --- .../src/tokens/__tests__/handshake.test.ts | 18 +++---- packages/backend/src/tokens/handshake.ts | 18 +++---- packages/backend/src/tokens/request.ts | 52 ++++++++++--------- 3 files changed, 46 insertions(+), 42 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts index 8fbdfcef2d2..b30216accce 100644 --- a/packages/backend/src/tokens/__tests__/handshake.test.ts +++ b/packages/backend/src/tokens/__tests__/handshake.test.ts @@ -90,7 +90,7 @@ describe('HandshakeService', () => { }, }; - handshakeService = new HandshakeService(mockAuthenticateContext, mockOrganizationSyncTargetMatchers, mockOptions); + handshakeService = new HandshakeService(mockAuthenticateContext, mockOptions, mockOrganizationSyncTargetMatchers); }); describe('isRequestEligibleForHandshake', () => { @@ -343,14 +343,14 @@ describe('HandshakeService', () => { }); }); - describe('handleHandshakeTokenVerificationErrorInDevelopment', () => { + describe('handleTokenVerificationErrorInDevelopment', () => { it('should throw specific error for invalid signature', () => { const error = new TokenVerificationError({ reason: TokenVerificationErrorReason.TokenInvalidSignature, message: 'Invalid signature', }); - expect(() => handshakeService.handleHandshakeTokenVerificationErrorInDevelopment(error)).toThrow( + expect(() => handshakeService.handleTokenVerificationErrorInDevelopment(error)).toThrow( 'Clerk: Handshake token verification failed due to an invalid signature', ); }); @@ -361,18 +361,18 @@ describe('HandshakeService', () => { message: 'Token expired', }); - expect(() => handshakeService.handleHandshakeTokenVerificationErrorInDevelopment(error)).toThrow( + expect(() => handshakeService.handleTokenVerificationErrorInDevelopment(error)).toThrow( 'Clerk: Handshake token verification failed: Token expired', ); }); }); - describe('setHandshakeInfiniteRedirectionLoopHeaders', () => { + describe('checkAndTrackRedirectLoop', () => { it('should return true after 3 redirects', () => { const headers = new Headers(); - handshakeService['handshakeRedirectLoopCounter'] = 3; + handshakeService['redirectLoopCounter'] = 3; - const result = handshakeService.setHandshakeInfiniteRedirectionLoopHeaders(headers); + const result = handshakeService.checkAndTrackRedirectLoop(headers); expect(result).toBe(true); expect(headers.get('Set-Cookie')).toBeNull(); @@ -380,9 +380,9 @@ describe('HandshakeService', () => { it('should increment counter and set cookie for first redirect', () => { const headers = new Headers(); - handshakeService['handshakeRedirectLoopCounter'] = 0; + handshakeService['redirectLoopCounter'] = 0; - const result = handshakeService.setHandshakeInfiniteRedirectionLoopHeaders(headers); + const result = handshakeService.checkAndTrackRedirectLoop(headers); expect(result).toBe(false); expect(headers.get('Set-Cookie')).toContain('__clerk_redirect_count=1'); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index c7fd736649c..22e6ef60969 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -84,20 +84,20 @@ export async function verifyHandshakeToken( } export class HandshakeService { - private handshakeRedirectLoopCounter: number; + private redirectLoopCounter: number; private readonly authenticateContext: AuthenticateContext; private readonly organizationSyncTargetMatchers: OrganizationSyncTargetMatchers; private readonly options: { organizationSyncOptions?: OrganizationSyncOptions }; constructor( authenticateContext: AuthenticateContext, - organizationSyncTargetMatchers: OrganizationSyncTargetMatchers, options: { organizationSyncOptions?: OrganizationSyncOptions }, + organizationSyncTargetMatchers: OrganizationSyncTargetMatchers, ) { - this.handshakeRedirectLoopCounter = 0; this.authenticateContext = authenticateContext; - this.organizationSyncTargetMatchers = organizationSyncTargetMatchers; this.options = options; + this.organizationSyncTargetMatchers = organizationSyncTargetMatchers; + this.redirectLoopCounter = 0; } /** @@ -259,7 +259,7 @@ ${developmentError.getFullMessage()}`, * @param error - The TokenVerificationError that occurred * @throws Error with a descriptive message about the verification failure */ - handleHandshakeTokenVerificationErrorInDevelopment(error: TokenVerificationError): void { + handleTokenVerificationErrorInDevelopment(error: TokenVerificationError): void { // In development, the handshake token is being transferred in the URL as a query parameter, so there is no // possibility of collision with a handshake token of another app running on the same local domain // (etc one app on localhost:3000 and one on localhost:3001). @@ -274,16 +274,16 @@ ${developmentError.getFullMessage()}`, } /** - * Sets headers to prevent infinite handshake redirection loops + * Checks if a redirect loop is detected and sets headers to track redirect count * @param headers - The Headers object to modify * @returns boolean indicating if a redirect loop was detected (true) or if the request can proceed (false) */ - setHandshakeInfiniteRedirectionLoopHeaders(headers: Headers): boolean { - if (this.handshakeRedirectLoopCounter === 3) { + checkAndTrackRedirectLoop(headers: Headers): boolean { + if (this.redirectLoopCounter === 3) { return true; } - const newCounterValue = this.handshakeRedirectLoopCounter + 1; + const newCounterValue = this.redirectLoopCounter + 1; const cookieName = constants.Cookies.RedirectCount; headers.append('Set-Cookie', `${cookieName}=${newCounterValue}; SameSite=Lax; HttpOnly; Max-Age=3`); return false; diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index c49faba9177..a05f22ed680 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -91,7 +91,11 @@ export async function authenticateRequest( } const organizationSyncTargetMatchers = computeOrganizationSyncTargetMatchers(options.organizationSyncOptions); - const handshakeService = new HandshakeService(authenticateContext, organizationSyncTargetMatchers, options); + const handshakeService = new HandshakeService( + authenticateContext, + { organizationSyncOptions: options.organizationSyncOptions }, + organizationSyncTargetMatchers, + ); async function refreshToken( authenticateContext: AuthenticateContext, @@ -229,31 +233,31 @@ export async function authenticateRequest( message: string, headers?: Headers, ): SignedInState | SignedOutState | HandshakeState { - if (handshakeService.isRequestEligibleForHandshake()) { - // Right now the only usage of passing in different headers is for multi-domain sync, which redirects somewhere else. - // In the future if we want to decorate the handshake redirect with additional headers per call we need to tweak this logic. - const handshakeHeaders = headers ?? handshakeService.buildRedirectToHandshake(reason); - - // Chrome aggressively caches inactive tabs. If we don't set the header here, - // all 307 redirects will be cached and the handshake will end up in an infinite loop. - if (handshakeHeaders.get(constants.Headers.Location)) { - handshakeHeaders.set(constants.Headers.CacheControl, 'no-store'); - } + if (!handshakeService.isRequestEligibleForHandshake()) { + return signedOut(authenticateContext, reason, message); + } - // Introduce the mechanism to protect for infinite handshake redirect loops - // using a cookie and returning true if it's infinite redirect loop or false if we can - // proceed with triggering handshake. - const isRedirectLoop = handshakeService.setHandshakeInfiniteRedirectionLoopHeaders(handshakeHeaders); - if (isRedirectLoop) { - const msg = `Clerk: Refreshing the session token resulted in an infinite redirect loop. This usually means that your Clerk instance keys do not match - make sure to copy the correct publishable and secret keys from the Clerk dashboard.`; - console.log(msg); - return signedOut(authenticateContext, reason, message); - } + // Right now the only usage of passing in different headers is for multi-domain sync, which redirects somewhere else. + // In the future if we want to decorate the handshake redirect with additional headers per call we need to tweak this logic. + const handshakeHeaders = headers ?? handshakeService.buildRedirectToHandshake(reason); + + // Chrome aggressively caches inactive tabs. If we don't set the header here, + // all 307 redirects will be cached and the handshake will end up in an infinite loop. + if (handshakeHeaders.get(constants.Headers.Location)) { + handshakeHeaders.set(constants.Headers.CacheControl, 'no-store'); + } - return handshake(authenticateContext, reason, message, handshakeHeaders); + // Introduce the mechanism to protect for infinite handshake redirect loops + // using a cookie and returning true if it's infinite redirect loop or false if we can + // proceed with triggering handshake. + const isRedirectLoop = handshakeService.checkAndTrackRedirectLoop(handshakeHeaders); + if (isRedirectLoop) { + const msg = `Clerk: Refreshing the session token resulted in an infinite redirect loop. This usually means that your Clerk instance keys do not match - make sure to copy the correct publishable and secret keys from the Clerk dashboard.`; + console.log(msg); + return signedOut(authenticateContext, reason, message); } - return signedOut(authenticateContext, reason, message); + return handshake(authenticateContext, reason, message, handshakeHeaders); } /** @@ -356,9 +360,9 @@ export async function authenticateRequest( // We need to make sure, however, that we don't allow the flow to continue indefinitely, so we throw an error after X // retries to avoid an infinite loop. An infinite loop can happen if the customer switched Clerk keys for their prod app. - // Check the handleHandshakeTokenVerificationErrorInDevelopment function for the development case. + // Check the handleTokenVerificationErrorInDevelopment method for the development case. if (error instanceof TokenVerificationError && authenticateContext.instanceType === 'development') { - handshakeService.handleHandshakeTokenVerificationErrorInDevelopment(error); + handshakeService.handleTokenVerificationErrorInDevelopment(error); } else { console.error('Clerk: unable to resolve handshake:', error); } From 610f5b889cf0445e8fe40f6f1d03ad19b579bcd6 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 13:11:42 -0500 Subject: [PATCH 12/13] move organization matchers --- .../src/tokens/__tests__/handshake.test.ts | 14 +- .../__tests__/organizationMatcher.test.ts | 131 ++++++++++++++++++ .../src/tokens/__tests__/request.test.ts | 33 ++--- packages/backend/src/tokens/handshake.ts | 66 ++------- .../backend/src/tokens/organizationMatcher.ts | 60 ++++++++ packages/backend/src/tokens/request.ts | 113 +-------------- 6 files changed, 219 insertions(+), 198 deletions(-) create mode 100644 packages/backend/src/tokens/__tests__/organizationMatcher.test.ts create mode 100644 packages/backend/src/tokens/organizationMatcher.ts diff --git a/packages/backend/src/tokens/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts index b30216accce..ba2cf519b42 100644 --- a/packages/backend/src/tokens/__tests__/handshake.test.ts +++ b/packages/backend/src/tokens/__tests__/handshake.test.ts @@ -5,7 +5,7 @@ import { TokenVerificationError, TokenVerificationErrorReason } from '../../erro import type { AuthenticateContext } from '../authenticateContext'; import { AuthErrorReason, signedIn, signedOut } from '../authStatus'; import { HandshakeService } from '../handshake'; -import type { OrganizationSyncTargetMatchers } from '../types'; +import { OrganizationMatcher } from '../organizationMatcher'; vi.mock('../handshake.js', async importOriginal => { const actual: any = await importOriginal(); @@ -60,7 +60,7 @@ vi.mock('../../jwt/verifyJwt.js', () => ({ describe('HandshakeService', () => { let mockAuthenticateContext: AuthenticateContext; - let mockOrganizationSyncTargetMatchers: OrganizationSyncTargetMatchers; + let mockOrganizationMatcher: OrganizationMatcher; let mockOptions: { organizationSyncOptions?: { organizationPatterns?: string[]; personalAccountPatterns?: string[] }; }; @@ -78,10 +78,10 @@ describe('HandshakeService', () => { accept: 'text/html', } as AuthenticateContext; - mockOrganizationSyncTargetMatchers = { - OrganizationMatcher: null, - PersonalAccountMatcher: null, - }; + mockOrganizationMatcher = new OrganizationMatcher({ + organizationPatterns: ['/org/:id'], + personalAccountPatterns: ['/account'], + }); mockOptions = { organizationSyncOptions: { @@ -90,7 +90,7 @@ describe('HandshakeService', () => { }, }; - handshakeService = new HandshakeService(mockAuthenticateContext, mockOptions, mockOrganizationSyncTargetMatchers); + handshakeService = new HandshakeService(mockAuthenticateContext, mockOptions, mockOrganizationMatcher); }); describe('isRequestEligibleForHandshake', () => { diff --git a/packages/backend/src/tokens/__tests__/organizationMatcher.test.ts b/packages/backend/src/tokens/__tests__/organizationMatcher.test.ts new file mode 100644 index 00000000000..4e2874d7930 --- /dev/null +++ b/packages/backend/src/tokens/__tests__/organizationMatcher.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from 'vitest'; +import { OrganizationMatcher } from '../organizationMatcher'; + +describe('OrganizationMatcher', () => { + describe('constructor', () => { + it('should create matcher with no patterns when options are undefined', () => { + const matcher = new OrganizationMatcher(); + expect(matcher).toBeInstanceOf(OrganizationMatcher); + }); + + it('should create matcher with organization patterns', () => { + const matcher = new OrganizationMatcher({ + organizationPatterns: ['/orgs/:id'], + }); + expect(matcher).toBeInstanceOf(OrganizationMatcher); + }); + + it('should create matcher with personal account patterns', () => { + const matcher = new OrganizationMatcher({ + personalAccountPatterns: ['/account'], + }); + expect(matcher).toBeInstanceOf(OrganizationMatcher); + }); + + it('should throw error for invalid organization pattern', () => { + expect(() => { + new OrganizationMatcher({ + organizationPatterns: ['/orgs/:id/***'], // Definitely invalid pattern + }); + }).toThrow(/Invalid pattern/); + }); + + it('should throw error for invalid personal account pattern', () => { + expect(() => { + new OrganizationMatcher({ + personalAccountPatterns: ['/account/***'], // Definitely invalid pattern + }); + }).toThrow(/Invalid pattern/); + }); + }); + + describe('findTarget', () => { + it('should return null for no patterns', () => { + const matcher = new OrganizationMatcher(); + const url = new URL('http://localhost:3000/orgs/123'); + expect(matcher.findTarget(url)).toBeNull(); + }); + + it('should find organization by ID', () => { + const matcher = new OrganizationMatcher({ + organizationPatterns: ['/orgs/:id'], + }); + const url = new URL('http://localhost:3000/orgs/123'); + expect(matcher.findTarget(url)).toEqual({ + type: 'organization', + organizationId: '123', + }); + }); + + it('should find organization by slug', () => { + const matcher = new OrganizationMatcher({ + organizationPatterns: ['/orgs/:slug'], + }); + const url = new URL('http://localhost:3000/orgs/my-org'); + expect(matcher.findTarget(url)).toEqual({ + type: 'organization', + organizationSlug: 'my-org', + }); + }); + + it('should find personal account', () => { + const matcher = new OrganizationMatcher({ + personalAccountPatterns: ['/account'], + }); + const url = new URL('http://localhost:3000/account'); + expect(matcher.findTarget(url)).toEqual({ + type: 'personalAccount', + }); + }); + + it('should prioritize organization over personal account', () => { + const matcher = new OrganizationMatcher({ + organizationPatterns: ['/orgs/:id'], + personalAccountPatterns: ['/orgs/:id'], // Same pattern + }); + const url = new URL('http://localhost:3000/orgs/123'); + expect(matcher.findTarget(url)).toEqual({ + type: 'organization', + organizationId: '123', + }); + }); + + it('should handle nested paths', () => { + const matcher = new OrganizationMatcher({ + organizationPatterns: ['/orgs/:id/(.*)'], + }); + const url = new URL('http://localhost:3000/orgs/123/settings'); + expect(matcher.findTarget(url)).toEqual({ + type: 'organization', + organizationId: '123', + }); + }); + + it('should handle multiple patterns', () => { + const matcher = new OrganizationMatcher({ + organizationPatterns: ['/orgs/:id', '/teams/:id'], + }); + const url = new URL('http://localhost:3000/teams/123'); + expect(matcher.findTarget(url)).toEqual({ + type: 'organization', + organizationId: '123', + }); + }); + + it('should return null for non-matching paths', () => { + const matcher = new OrganizationMatcher({ + organizationPatterns: ['/orgs/:id'], + }); + const url = new URL('http://localhost:3000/other/123'); + expect(matcher.findTarget(url)).toBeNull(); + }); + }); + + describe('pathToRegexp behavior', () => { + it('should throw error for invalid pattern', () => { + expect(() => { + match(['/orgs/:id/***']); + }).toThrow(); + }); + }); +}); diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index c3409deceed..135f84ae384 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -1,5 +1,5 @@ import { http, HttpResponse } from 'msw'; -import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest'; import { TokenVerificationErrorReason } from '../../errors'; import { @@ -21,6 +21,7 @@ import { RefreshTokenErrorReason, } from '../request'; import type { AuthenticateRequestOptions, OrganizationSyncOptions } from '../types'; +import { OrganizationMatcher } from '../organizationMatcher'; const PK_TEST = 'pk_test_Y2xlcmsuaW5zcGlyZWQucHVtYS03NC5sY2wuZGV2JA'; const PK_LIVE = 'pk_live_Y2xlcmsuaW5zcGlyZWQucHVtYS03NC5sY2wuZGV2JA'; @@ -261,20 +262,8 @@ const mockRequestWithCookies = (headers?, cookies = {}, requestUrl?) => { return mockRequest({ cookie: cookieStr, ...headers }, requestUrl); }; -// Tests both getOrganizationSyncTarget and the organizationSyncOptions usage patterns -// that are recommended for typical use. -describe('tokens.getOrganizationSyncTarget(url,options)', _ => { - type testCase = { - name: string; - // When the customer app specifies these orgSyncOptions to middleware... - whenOrgSyncOptions: OrganizationSyncOptions | undefined; - // And the path arrives at this URL path... - whenAppRequestPath: string; - // A handshake should (or should not) occur: - thenExpectActivationEntity: OrganizationSyncTarget | null; - }; - - const testCases: testCase[] = [ +describe('getOrganizationSyncTarget', () => { + it.each([ { name: 'none activates nothing', whenOrgSyncOptions: undefined, @@ -426,16 +415,10 @@ describe('tokens.getOrganizationSyncTarget(url,options)', _ => { organizationSlug: 'org_bar', }, }, - ]; - - test.each(testCases)('$name', ({ name, whenOrgSyncOptions, whenAppRequestPath, thenExpectActivationEntity }) => { - if (!name) { - return; - } - - const path = new URL(`http://localhost:3000${whenAppRequestPath}`); - const matchers = computeOrganizationSyncTargetMatchers(whenOrgSyncOptions); - const toActivate = getOrganizationSyncTarget(path, whenOrgSyncOptions, matchers); + ])('$name', ({ whenOrgSyncOptions, whenAppRequestPath, thenExpectActivationEntity }) => { + const path = new URL(`http://localhost:3000${whenAppRequestPath || ''}`); + const matcher = new OrganizationMatcher(whenOrgSyncOptions); + const toActivate = matcher.findTarget(path); expect(toActivate).toEqual(thenExpectActivationEntity); }); }); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 22e6ef60969..a6a1fe43d8a 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -1,5 +1,3 @@ -import type { Match } from '@clerk/shared/pathToRegexp'; - import { constants } from '../constants'; import { TokenVerificationError, TokenVerificationErrorAction, TokenVerificationErrorReason } from '../errors'; import type { VerifyJwtOptions } from '../jwt'; @@ -10,7 +8,8 @@ import type { SignedInState, SignedOutState } from './authStatus'; import { AuthErrorReason, signedIn, signedOut } from './authStatus'; import { getCookieName, getCookieValue } from './cookie'; import { loadClerkJWKFromLocal, loadClerkJWKFromRemote } from './keys'; -import type { OrganizationSyncOptions, OrganizationSyncTarget, OrganizationSyncTargetMatchers } from './types'; +import type { OrganizationMatcher } from './organizationMatcher'; +import type { OrganizationSyncOptions, OrganizationSyncTarget } from './types'; import type { VerifyTokenOptions } from './verify'; import { verifyToken } from './verify'; @@ -86,17 +85,17 @@ export async function verifyHandshakeToken( export class HandshakeService { private redirectLoopCounter: number; private readonly authenticateContext: AuthenticateContext; - private readonly organizationSyncTargetMatchers: OrganizationSyncTargetMatchers; + private readonly organizationMatcher: OrganizationMatcher; private readonly options: { organizationSyncOptions?: OrganizationSyncOptions }; constructor( authenticateContext: AuthenticateContext, options: { organizationSyncOptions?: OrganizationSyncOptions }, - organizationSyncTargetMatchers: OrganizationSyncTargetMatchers, + organizationMatcher: OrganizationMatcher, ) { this.authenticateContext = authenticateContext; this.options = options; - this.organizationSyncTargetMatchers = organizationSyncTargetMatchers; + this.organizationMatcher = organizationMatcher; this.redirectLoopCounter = 0; } @@ -150,11 +149,7 @@ export class HandshakeService { url.searchParams.append(constants.QueryParameters.DevBrowser, this.authenticateContext.devBrowserToken); } - const toActivate = this.getOrganizationSyncTarget( - this.authenticateContext.clerkUrl, - this.options.organizationSyncOptions, - this.organizationSyncTargetMatchers, - ); + const toActivate = this.getOrganizationSyncTarget(this.authenticateContext.clerkUrl, this.organizationMatcher); if (toActivate) { const params = this.getOrganizationSyncQueryParams(toActivate); params.forEach((value, key) => { @@ -296,53 +291,8 @@ ${developmentError.getFullMessage()}`, return updatedURL; } - private getOrganizationSyncTarget( - url: URL, - options: OrganizationSyncOptions | undefined, - matchers: OrganizationSyncTargetMatchers, - ): OrganizationSyncTarget | null { - if (!options) { - return null; - } - - if (matchers.OrganizationMatcher) { - let orgResult: Match>>; - try { - orgResult = matchers.OrganizationMatcher(url.pathname); - } catch (e) { - console.error(`Clerk: Failed to apply organization pattern "${options.organizationPatterns}" to a path`, e); - return null; - } - - if (orgResult && 'params' in orgResult) { - const params = orgResult.params; - - if ('id' in params && typeof params.id === 'string') { - return { type: 'organization', organizationId: params.id }; - } - if ('slug' in params && typeof params.slug === 'string') { - return { type: 'organization', organizationSlug: params.slug }; - } - console.warn( - 'Clerk: Detected an organization pattern match, but no organization ID or slug was found in the URL. Does the pattern include `:id` or `:slug`?', - ); - } - } - - if (matchers.PersonalAccountMatcher) { - let personalResult: Match>>; - try { - personalResult = matchers.PersonalAccountMatcher(url.pathname); - } catch (e) { - console.error(`Failed to apply personal account pattern "${options.personalAccountPatterns}" to a path`, e); - return null; - } - - if (personalResult) { - return { type: 'personalAccount' }; - } - } - return null; + private getOrganizationSyncTarget(url: URL, matchers: OrganizationMatcher): OrganizationSyncTarget | null { + return matchers.findTarget(url); } private getOrganizationSyncQueryParams(toActivate: OrganizationSyncTarget): Map { diff --git a/packages/backend/src/tokens/organizationMatcher.ts b/packages/backend/src/tokens/organizationMatcher.ts new file mode 100644 index 00000000000..820c8dcc0bf --- /dev/null +++ b/packages/backend/src/tokens/organizationMatcher.ts @@ -0,0 +1,60 @@ +import type { MatchFunction } from '@clerk/shared/pathToRegexp'; +import { match } from '@clerk/shared/pathToRegexp'; + +import type { OrganizationSyncOptions, OrganizationSyncTarget } from './types'; + +export class OrganizationMatcher { + private readonly organizationPattern: MatchFunction | null; + private readonly personalAccountPattern: MatchFunction | null; + + constructor(options?: OrganizationSyncOptions) { + this.organizationPattern = this.createMatcher(options?.organizationPatterns); + this.personalAccountPattern = this.createMatcher(options?.personalAccountPatterns); + } + + private createMatcher(pattern?: string[]): MatchFunction | null { + if (!pattern) return null; + try { + return match(pattern); + } catch (e) { + throw new Error(`Invalid pattern "${pattern}": ${e}`); + } + } + + findTarget(url: URL): OrganizationSyncTarget | null { + const orgTarget = this.findOrganizationTarget(url); + if (orgTarget) return orgTarget; + + return this.findPersonalAccountTarget(url); + } + + private findOrganizationTarget(url: URL): OrganizationSyncTarget | null { + if (!this.organizationPattern) return null; + + try { + const result = this.organizationPattern(url.pathname); + if (!result || !('params' in result)) return null; + + const params = result.params as { id?: string; slug?: string }; + if (params.id) return { type: 'organization', organizationId: params.id }; + if (params.slug) return { type: 'organization', organizationSlug: params.slug }; + + return null; + } catch (e) { + console.error('Failed to match organization pattern:', e); + return null; + } + } + + private findPersonalAccountTarget(url: URL): OrganizationSyncTarget | null { + if (!this.personalAccountPattern) return null; + + try { + const result = this.personalAccountPattern(url.pathname); + return result ? { type: 'personalAccount' } : null; + } catch (e) { + console.error('Failed to match personal account pattern:', e); + return null; + } + } +} diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index a05f22ed680..882aae8158a 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -1,5 +1,3 @@ -import type { Match, MatchFunction } from '@clerk/shared/pathToRegexp'; -import { match } from '@clerk/shared/pathToRegexp'; import type { JwtPayload } from '@clerk/types'; import { constants } from '../constants'; @@ -16,12 +14,8 @@ import { AuthErrorReason, handshake, signedIn, signedOut } from './authStatus'; import { createClerkRequest } from './clerkRequest'; import { getCookieName, getCookieValue } from './cookie'; import { HandshakeService } from './handshake'; -import type { - AuthenticateRequestOptions, - OrganizationSyncOptions, - OrganizationSyncTarget, - OrganizationSyncTargetMatchers, -} from './types'; +import { OrganizationMatcher } from './organizationMatcher'; +import type { AuthenticateRequestOptions } from './types'; import { verifyToken } from './verify'; export const RefreshTokenErrorReason = { @@ -90,11 +84,11 @@ export async function authenticateRequest( assertProxyUrlOrDomain(authenticateContext.proxyUrl || authenticateContext.domain); } - const organizationSyncTargetMatchers = computeOrganizationSyncTargetMatchers(options.organizationSyncOptions); + const organizationMatcher = new OrganizationMatcher(options.organizationSyncOptions); const handshakeService = new HandshakeService( authenticateContext, { organizationSyncOptions: options.organizationSyncOptions }, - organizationSyncTargetMatchers, + organizationMatcher, ); async function refreshToken( @@ -273,11 +267,7 @@ export async function authenticateRequest( authenticateContext: AuthenticateContext, auth: SignedInAuthObject, ): HandshakeState | SignedOutState | null { - const organizationSyncTarget = getOrganizationSyncTarget( - authenticateContext.clerkUrl, - options.organizationSyncOptions, - organizationSyncTargetMatchers, - ); + const organizationSyncTarget = organizationMatcher.findTarget(authenticateContext.clerkUrl); if (!organizationSyncTarget) { return null; } @@ -557,99 +547,6 @@ export const debugRequestState = (params: RequestState) => { return { isSignedIn, proxyUrl, reason, message, publishableKey, isSatellite, domain }; }; -/** - * Computes regex-based matchers from the given organization sync options. - */ -export function computeOrganizationSyncTargetMatchers( - options: OrganizationSyncOptions | undefined, -): OrganizationSyncTargetMatchers { - let personalAccountMatcher: MatchFunction>> | null = null; - if (options?.personalAccountPatterns) { - try { - personalAccountMatcher = match(options.personalAccountPatterns); - } catch (e) { - // Likely to be encountered during development, so throwing the error is more prudent than logging - throw new Error(`Invalid personal account pattern "${options.personalAccountPatterns}": "${e}"`); - } - } - - let organizationMatcher: MatchFunction>> | null = null; - if (options?.organizationPatterns) { - try { - organizationMatcher = match(options.organizationPatterns); - } catch (e) { - // Likely to be encountered during development, so throwing the error is more prudent than logging - throw new Error(`Clerk: Invalid organization pattern "${options.organizationPatterns}": "${e}"`); - } - } - - return { - OrganizationMatcher: organizationMatcher, - PersonalAccountMatcher: personalAccountMatcher, - }; -} - -/** - * Determines if the given URL and settings indicate a desire to activate a specific - * organization or personal account. - * - * @param url - The URL of the original request. - * @param options - The organization sync options. - * @param matchers - The matchers for the organization and personal account patterns, as generated by `computeOrganizationSyncTargetMatchers`. - */ -export function getOrganizationSyncTarget( - url: URL, - options: OrganizationSyncOptions | undefined, - matchers: OrganizationSyncTargetMatchers, -): OrganizationSyncTarget | null { - if (!options) { - return null; - } - - // Check for organization activation - if (matchers.OrganizationMatcher) { - let orgResult: Match>>; - try { - orgResult = matchers.OrganizationMatcher(url.pathname); - } catch (e) { - // Intentionally not logging the path to avoid potentially leaking anything sensitive - console.error(`Clerk: Failed to apply organization pattern "${options.organizationPatterns}" to a path`, e); - return null; - } - - if (orgResult && 'params' in orgResult) { - const params = orgResult.params; - - if ('id' in params && typeof params.id === 'string') { - return { type: 'organization', organizationId: params.id }; - } - if ('slug' in params && typeof params.slug === 'string') { - return { type: 'organization', organizationSlug: params.slug }; - } - console.warn( - 'Clerk: Detected an organization pattern match, but no organization ID or slug was found in the URL. Does the pattern include `:id` or `:slug`?', - ); - } - } - - // Check for personal account activation - if (matchers.PersonalAccountMatcher) { - let personalResult: Match>>; - try { - personalResult = matchers.PersonalAccountMatcher(url.pathname); - } catch (e) { - // Intentionally not logging the path to avoid potentially leaking anything sensitive - console.error(`Failed to apply personal account pattern "${options.personalAccountPatterns}" to a path`, e); - return null; - } - - if (personalResult) { - return { type: 'personalAccount' }; - } - } - return null; -} - const convertTokenVerificationErrorReasonToAuthErrorReason = ({ tokenError, refreshError, From 0d92d376c8dfa393fa55738af628460af35199fb Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 29 Apr 2025 13:17:32 -0500 Subject: [PATCH 13/13] lint --- .../src/tokens/__tests__/organizationMatcher.test.ts | 1 + packages/backend/src/tokens/__tests__/request.test.ts | 10 ++-------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/organizationMatcher.test.ts b/packages/backend/src/tokens/__tests__/organizationMatcher.test.ts index 4e2874d7930..00c35acffea 100644 --- a/packages/backend/src/tokens/__tests__/organizationMatcher.test.ts +++ b/packages/backend/src/tokens/__tests__/organizationMatcher.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; + import { OrganizationMatcher } from '../organizationMatcher'; describe('OrganizationMatcher', () => { diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index 135f84ae384..16142791ab1 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -13,15 +13,9 @@ import { import { server } from '../../mock-server'; import type { AuthReason } from '../authStatus'; import { AuthErrorReason, AuthStatus } from '../authStatus'; -import { - authenticateRequest, - computeOrganizationSyncTargetMatchers, - getOrganizationSyncTarget, - type OrganizationSyncTarget, - RefreshTokenErrorReason, -} from '../request'; -import type { AuthenticateRequestOptions, OrganizationSyncOptions } from '../types'; import { OrganizationMatcher } from '../organizationMatcher'; +import { authenticateRequest, RefreshTokenErrorReason } from '../request'; +import type { AuthenticateRequestOptions } from '../types'; const PK_TEST = 'pk_test_Y2xlcmsuaW5zcGlyZWQucHVtYS03NC5sY2wuZGV2JA'; const PK_LIVE = 'pk_live_Y2xlcmsuaW5zcGlyZWQucHVtYS03NC5sY2wuZGV2JA';