From 8181f6b52824be028b240c5f0ef21f414c382fb1 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 6 May 2025 13:33:21 -0500 Subject: [PATCH 01/11] feat(backend): handle handshake nonce payload --- packages/backend/src/api/endpoints/index.ts | 1 + packages/backend/src/api/factory.ts | 2 ++ packages/backend/src/tokens/handshake.ts | 16 ++++++++++++++-- packages/backend/src/tokens/request.ts | 1 + 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/api/endpoints/index.ts b/packages/backend/src/api/endpoints/index.ts index 2cf61f0af75..7172e215a04 100644 --- a/packages/backend/src/api/endpoints/index.ts +++ b/packages/backend/src/api/endpoints/index.ts @@ -7,6 +7,7 @@ export * from './BlocklistIdentifierApi'; export * from './ClientApi'; export * from './DomainApi'; export * from './EmailAddressApi'; +export * from './HandshakePayloadApi'; export * from './InstanceApi'; export * from './InvitationApi'; export * from './JwksApi'; diff --git a/packages/backend/src/api/factory.ts b/packages/backend/src/api/factory.ts index 164cfb87d04..e300a3da074 100644 --- a/packages/backend/src/api/factory.ts +++ b/packages/backend/src/api/factory.ts @@ -7,6 +7,7 @@ import { ClientAPI, DomainAPI, EmailAddressAPI, + HandshakePayloadAPI, InstanceAPI, InvitationAPI, JwksAPI, @@ -45,6 +46,7 @@ export function createBackendApiClient(options: CreateBackendApiOptions) { clients: new ClientAPI(request), domains: new DomainAPI(request), emailAddresses: new EmailAddressAPI(request), + handshakePayloads: new HandshakePayloadAPI(request), instance: new InstanceAPI(request), invitations: new InvitationAPI(request), jwks: new JwksAPI(request), diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 8f416bd72a9..d499b40def5 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -1,3 +1,4 @@ +import type { HandshakePayloadAPI } from '../api/endpoints'; import { constants, SUPPORTED_BAPI_VERSION } from '../constants'; import { TokenVerificationError, TokenVerificationErrorAction, TokenVerificationErrorReason } from '../errors'; import type { VerifyJwtOptions } from '../jwt'; @@ -87,15 +88,18 @@ export class HandshakeService { private readonly authenticateContext: AuthenticateContext; private readonly organizationMatcher: OrganizationMatcher; private readonly options: { organizationSyncOptions?: OrganizationSyncOptions }; + private readonly handshakePayloadApi?: HandshakePayloadAPI; constructor( authenticateContext: AuthenticateContext, options: { organizationSyncOptions?: OrganizationSyncOptions }, organizationMatcher: OrganizationMatcher, + handshakePayloadApi?: HandshakePayloadAPI, ) { this.authenticateContext = authenticateContext; this.options = options; this.organizationMatcher = organizationMatcher; + this.handshakePayloadApi = handshakePayloadApi; this.redirectLoopCounter = 0; } @@ -175,8 +179,16 @@ export class HandshakeService { const cookiesToSet: string[] = []; if (this.authenticateContext.handshakeNonce) { - // TODO: implement handshake nonce handling, fetch handshake payload with nonce - console.warn('Clerk: Handshake nonce is not implemented yet.'); + if (!this.handshakePayloadApi) { + console.error('Clerk: HandshakeService: handshakePayloadApi is not available, cannot process handshake nonce.'); + } else { + const handshakePayload = await this.handshakePayloadApi.getHandshakePayload({ + nonce: this.authenticateContext.handshakeNonce, + }); + if (handshakePayload) { + console.log('Clerk: Handshake payload by nonce:', handshakePayload.payload); + } + } } if (this.authenticateContext.handshakeToken) { const handshakePayload = await verifyHandshakeToken( diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 882aae8158a..5f5bd8f0d31 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -89,6 +89,7 @@ export async function authenticateRequest( authenticateContext, { organizationSyncOptions: options.organizationSyncOptions }, organizationMatcher, + options.apiClient?.handshakePayloads, ); async function refreshToken( From f8fba5718b6c7a32ecafb15a0f6ce22dbc6c5fcb Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 7 May 2025 08:59:29 -0500 Subject: [PATCH 02/11] move HandshakePayload under clients/ --- .../backend/src/api/endpoints/ClientApi.ts | 16 ++++++++++++++++ .../src/api/endpoints/HandshakePayloadApi.ts | 18 ------------------ packages/backend/src/api/endpoints/index.ts | 1 - packages/backend/src/api/factory.ts | 2 -- .../src/api/resources/HandshakePayload.ts | 10 +++------- packages/backend/src/tokens/handshake.ts | 18 ++++++++++-------- packages/backend/src/tokens/request.ts | 2 +- 7 files changed, 30 insertions(+), 37 deletions(-) delete mode 100644 packages/backend/src/api/endpoints/HandshakePayloadApi.ts diff --git a/packages/backend/src/api/endpoints/ClientApi.ts b/packages/backend/src/api/endpoints/ClientApi.ts index b888e8c7cb7..a7c9cc06a03 100644 --- a/packages/backend/src/api/endpoints/ClientApi.ts +++ b/packages/backend/src/api/endpoints/ClientApi.ts @@ -1,4 +1,5 @@ import type { ClerkPaginationRequest } from '@clerk/types'; +import type { HandshakePayload } from '../resources/HandshakePayload'; import { joinPaths } from '../../util/path'; import type { Client } from '../resources/Client'; @@ -7,6 +8,10 @@ import { AbstractAPI } from './AbstractApi'; const basePath = '/clients'; +type GetHandshakePayloadParams = { + nonce: string; +}; + export class ClientAPI extends AbstractAPI { public async getClientList(params: ClerkPaginationRequest = {}) { return this.request>({ @@ -31,4 +36,15 @@ export class ClientAPI extends AbstractAPI { bodyParams: { token }, }); } + + public async getHandshakePayload(queryParams: GetHandshakePayloadParams) { + const { nonce } = queryParams; + console.log('Clerk: ClientAPI: getHandshakePayload:', nonce); + + return this.request({ + method: 'GET', + path: '/clients/handshake_payload', + queryParams, + }); + } } diff --git a/packages/backend/src/api/endpoints/HandshakePayloadApi.ts b/packages/backend/src/api/endpoints/HandshakePayloadApi.ts deleted file mode 100644 index 5bdf9f24404..00000000000 --- a/packages/backend/src/api/endpoints/HandshakePayloadApi.ts +++ /dev/null @@ -1,18 +0,0 @@ -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/endpoints/index.ts b/packages/backend/src/api/endpoints/index.ts index 7172e215a04..2cf61f0af75 100644 --- a/packages/backend/src/api/endpoints/index.ts +++ b/packages/backend/src/api/endpoints/index.ts @@ -7,7 +7,6 @@ export * from './BlocklistIdentifierApi'; export * from './ClientApi'; export * from './DomainApi'; export * from './EmailAddressApi'; -export * from './HandshakePayloadApi'; export * from './InstanceApi'; export * from './InvitationApi'; export * from './JwksApi'; diff --git a/packages/backend/src/api/factory.ts b/packages/backend/src/api/factory.ts index e300a3da074..164cfb87d04 100644 --- a/packages/backend/src/api/factory.ts +++ b/packages/backend/src/api/factory.ts @@ -7,7 +7,6 @@ import { ClientAPI, DomainAPI, EmailAddressAPI, - HandshakePayloadAPI, InstanceAPI, InvitationAPI, JwksAPI, @@ -46,7 +45,6 @@ export function createBackendApiClient(options: CreateBackendApiOptions) { clients: new ClientAPI(request), domains: new DomainAPI(request), emailAddresses: new EmailAddressAPI(request), - handshakePayloads: new HandshakePayloadAPI(request), instance: new InstanceAPI(request), invitations: new InvitationAPI(request), jwks: new JwksAPI(request), diff --git a/packages/backend/src/api/resources/HandshakePayload.ts b/packages/backend/src/api/resources/HandshakePayload.ts index 229acb92951..b47930b57ff 100644 --- a/packages/backend/src/api/resources/HandshakePayload.ts +++ b/packages/backend/src/api/resources/HandshakePayload.ts @@ -1,15 +1,11 @@ export type HandshakePayloadJSON = { - nonce: string; - payload: string; + directives: string[]; }; export class HandshakePayload { - constructor( - readonly nonce: string, - readonly payload: string, - ) {} + constructor(readonly directives: string[]) {} static fromJSON(data: HandshakePayloadJSON): HandshakePayload { - return new HandshakePayload(data.nonce, data.payload); + return new HandshakePayload(data.directives); } } diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index d499b40def5..cbb0a10edd1 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -1,4 +1,4 @@ -import type { HandshakePayloadAPI } from '../api/endpoints'; +import type { ClientAPI } from '../api/endpoints/ClientApi'; import { constants, SUPPORTED_BAPI_VERSION } from '../constants'; import { TokenVerificationError, TokenVerificationErrorAction, TokenVerificationErrorReason } from '../errors'; import type { VerifyJwtOptions } from '../jwt'; @@ -88,18 +88,18 @@ export class HandshakeService { private readonly authenticateContext: AuthenticateContext; private readonly organizationMatcher: OrganizationMatcher; private readonly options: { organizationSyncOptions?: OrganizationSyncOptions }; - private readonly handshakePayloadApi?: HandshakePayloadAPI; + private readonly clientApi?: ClientAPI; constructor( authenticateContext: AuthenticateContext, options: { organizationSyncOptions?: OrganizationSyncOptions }, organizationMatcher: OrganizationMatcher, - handshakePayloadApi?: HandshakePayloadAPI, + clientApi?: ClientAPI, ) { this.authenticateContext = authenticateContext; this.options = options; this.organizationMatcher = organizationMatcher; - this.handshakePayloadApi = handshakePayloadApi; + this.clientApi = clientApi; this.redirectLoopCounter = 0; } @@ -179,16 +179,18 @@ export class HandshakeService { const cookiesToSet: string[] = []; if (this.authenticateContext.handshakeNonce) { - if (!this.handshakePayloadApi) { - console.error('Clerk: HandshakeService: handshakePayloadApi is not available, cannot process handshake nonce.'); + if (!this.clientApi) { + console.error('Clerk: HandshakeService: clientApi is not available, cannot process handshake nonce.'); } else { - const handshakePayload = await this.handshakePayloadApi.getHandshakePayload({ + const handshakePayload = await this.clientApi.getHandshakePayload({ nonce: this.authenticateContext.handshakeNonce, }); if (handshakePayload) { - console.log('Clerk: Handshake payload by nonce:', handshakePayload.payload); + console.log('Clerk: Handshake payload by nonce:', handshakePayload); + cookiesToSet.push(...handshakePayload.directives); } } + cookiesToSet.push(`${constants.Cookies.HandshakeNonce}=; Max-Age=0; Path=/; HttpOnly; SameSite=Lax`); } if (this.authenticateContext.handshakeToken) { const handshakePayload = await verifyHandshakeToken( diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 5f5bd8f0d31..07c9cc3fccc 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -89,7 +89,7 @@ export async function authenticateRequest( authenticateContext, { organizationSyncOptions: options.organizationSyncOptions }, organizationMatcher, - options.apiClient?.handshakePayloads, + options.apiClient?.clients, ); async function refreshToken( From 08aae86882a866a0767c428770a33095505dd9b8 Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 7 May 2025 12:20:40 -0500 Subject: [PATCH 03/11] wip --- packages/backend/src/api/endpoints/ClientApi.ts | 7 ++----- packages/backend/src/tokens/handshake.ts | 9 +++++++-- packages/backend/src/tokens/request.ts | 2 +- packages/clerk-js/src/core/clerk.ts | 1 + packages/clerk-js/src/utils/getClerkQueryParam.ts | 1 + 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/backend/src/api/endpoints/ClientApi.ts b/packages/backend/src/api/endpoints/ClientApi.ts index a7c9cc06a03..f04202dff38 100644 --- a/packages/backend/src/api/endpoints/ClientApi.ts +++ b/packages/backend/src/api/endpoints/ClientApi.ts @@ -1,9 +1,9 @@ import type { ClerkPaginationRequest } from '@clerk/types'; -import type { HandshakePayload } from '../resources/HandshakePayload'; import { joinPaths } from '../../util/path'; import type { Client } from '../resources/Client'; import type { PaginatedResourceResponse } from '../resources/Deserializer'; +import type { HandshakePayload } from '../resources/HandshakePayload'; import { AbstractAPI } from './AbstractApi'; const basePath = '/clients'; @@ -38,12 +38,9 @@ export class ClientAPI extends AbstractAPI { } public async getHandshakePayload(queryParams: GetHandshakePayloadParams) { - const { nonce } = queryParams; - console.log('Clerk: ClientAPI: getHandshakePayload:', nonce); - return this.request({ method: 'GET', - path: '/clients/handshake_payload', + path: joinPaths(basePath, 'handshake_payload'), queryParams, }); } diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index cbb0a10edd1..882eda18cf8 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -191,8 +191,7 @@ export class HandshakeService { } } cookiesToSet.push(`${constants.Cookies.HandshakeNonce}=; Max-Age=0; Path=/; HttpOnly; SameSite=Lax`); - } - if (this.authenticateContext.handshakeToken) { + } else if (this.authenticateContext.handshakeToken) { const handshakePayload = await verifyHandshakeToken( this.authenticateContext.handshakeToken, this.authenticateContext, @@ -209,6 +208,10 @@ export class HandshakeService { }); if (this.authenticateContext.instanceType === 'development') { + console.log( + 'HandshakeService: this.authenticateContext.instanceType === "development"', + this.authenticateContext.clerkUrl, + ); const newUrl = new URL(this.authenticateContext.clerkUrl); newUrl.searchParams.delete(constants.QueryParameters.Handshake); newUrl.searchParams.delete(constants.QueryParameters.HandshakeHelp); @@ -217,11 +220,13 @@ export class HandshakeService { } if (sessionToken === '') { + console.log('HandshakeService: missing session token', this.authenticateContext); return signedOut(this.authenticateContext, AuthErrorReason.SessionTokenMissing, '', headers); } const { data, errors: [error] = [] } = await verifyToken(sessionToken, this.authenticateContext); if (data) { + console.log('HandshakeService: signed in VERIFY TOKEN', data); return signedIn(this.authenticateContext, data, headers, sessionToken); } diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 07c9cc3fccc..7aa025724ea 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -337,7 +337,7 @@ export async function authenticateRequest( /** * If we have a handshakeToken, resolve the handshake and attempt to return a definitive signed in or signed out state. */ - if (authenticateContext.handshakeToken) { + if (authenticateContext.handshakeNonce || authenticateContext.handshakeToken) { try { return await handshakeService.resolveHandshake(); } catch (error) { diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 05aab92a514..cd0a44ba59b 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -2637,6 +2637,7 @@ export class Clerk implements ClerkInterface { // in the meantime, we're removing it here to keep the URL clean removeClerkQueryParam(CLERK_SUFFIXED_COOKIES); removeClerkQueryParam('__clerk_handshake'); + removeClerkQueryParam('__clerk_handshake_nonce'); removeClerkQueryParam('__clerk_help'); } catch { // ignore diff --git a/packages/clerk-js/src/utils/getClerkQueryParam.ts b/packages/clerk-js/src/utils/getClerkQueryParam.ts index 307b35b6147..cebe54ba20a 100644 --- a/packages/clerk-js/src/utils/getClerkQueryParam.ts +++ b/packages/clerk-js/src/utils/getClerkQueryParam.ts @@ -10,6 +10,7 @@ const _ClerkQueryParams = [ '__clerk_ticket', '__clerk_modal_state', '__clerk_handshake', + '__clerk_handshake_nonce', '__clerk_help', CLERK_NETLIFY_CACHE_BUST_PARAM, CLERK_SYNCED, From 85e630d2f52fae59597c6c6ba5dae77d13cb3727 Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 7 May 2025 20:11:27 -0500 Subject: [PATCH 04/11] cleanup --- packages/backend/src/tokens/handshake.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 882eda18cf8..9849e87447c 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -179,18 +179,16 @@ export class HandshakeService { const cookiesToSet: string[] = []; if (this.authenticateContext.handshakeNonce) { - if (!this.clientApi) { - console.error('Clerk: HandshakeService: clientApi is not available, cannot process handshake nonce.'); - } else { - const handshakePayload = await this.clientApi.getHandshakePayload({ + try { + const handshakePayload = await this.clientApi?.getHandshakePayload({ nonce: this.authenticateContext.handshakeNonce, }); if (handshakePayload) { - console.log('Clerk: Handshake payload by nonce:', handshakePayload); cookiesToSet.push(...handshakePayload.directives); } + } catch (error) { + console.error('Clerk: HandshakeService: error getting handshake payload:', error); } - cookiesToSet.push(`${constants.Cookies.HandshakeNonce}=; Max-Age=0; Path=/; HttpOnly; SameSite=Lax`); } else if (this.authenticateContext.handshakeToken) { const handshakePayload = await verifyHandshakeToken( this.authenticateContext.handshakeToken, @@ -208,10 +206,6 @@ export class HandshakeService { }); if (this.authenticateContext.instanceType === 'development') { - console.log( - 'HandshakeService: this.authenticateContext.instanceType === "development"', - this.authenticateContext.clerkUrl, - ); const newUrl = new URL(this.authenticateContext.clerkUrl); newUrl.searchParams.delete(constants.QueryParameters.Handshake); newUrl.searchParams.delete(constants.QueryParameters.HandshakeHelp); @@ -220,13 +214,11 @@ export class HandshakeService { } if (sessionToken === '') { - console.log('HandshakeService: missing session token', this.authenticateContext); return signedOut(this.authenticateContext, AuthErrorReason.SessionTokenMissing, '', headers); } const { data, errors: [error] = [] } = await verifyToken(sessionToken, this.authenticateContext); if (data) { - console.log('HandshakeService: signed in VERIFY TOKEN', data); return signedIn(this.authenticateContext, data, headers, sessionToken); } From b720296bfd7fd9892d9aaab4eed5eb5050fcaa36 Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 7 May 2025 20:51:57 -0500 Subject: [PATCH 05/11] use existing api client --- packages/backend/src/tokens/handshake.ts | 6 +----- packages/backend/src/tokens/request.ts | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 9849e87447c..677ff3e8c00 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -1,4 +1,3 @@ -import type { ClientAPI } from '../api/endpoints/ClientApi'; import { constants, SUPPORTED_BAPI_VERSION } from '../constants'; import { TokenVerificationError, TokenVerificationErrorAction, TokenVerificationErrorReason } from '../errors'; import type { VerifyJwtOptions } from '../jwt'; @@ -88,18 +87,15 @@ export class HandshakeService { private readonly authenticateContext: AuthenticateContext; private readonly organizationMatcher: OrganizationMatcher; private readonly options: { organizationSyncOptions?: OrganizationSyncOptions }; - private readonly clientApi?: ClientAPI; constructor( authenticateContext: AuthenticateContext, options: { organizationSyncOptions?: OrganizationSyncOptions }, organizationMatcher: OrganizationMatcher, - clientApi?: ClientAPI, ) { this.authenticateContext = authenticateContext; this.options = options; this.organizationMatcher = organizationMatcher; - this.clientApi = clientApi; this.redirectLoopCounter = 0; } @@ -180,7 +176,7 @@ export class HandshakeService { if (this.authenticateContext.handshakeNonce) { try { - const handshakePayload = await this.clientApi?.getHandshakePayload({ + const handshakePayload = await this.authenticateContext.apiClient?.clients.getHandshakePayload({ nonce: this.authenticateContext.handshakeNonce, }); if (handshakePayload) { diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 7aa025724ea..2330a6fa32e 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -89,7 +89,6 @@ export async function authenticateRequest( authenticateContext, { organizationSyncOptions: options.organizationSyncOptions }, organizationMatcher, - options.apiClient?.clients, ); async function refreshToken( From ee3936bc6eacf1969b9540bfc525be0ed35933d5 Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 8 May 2025 15:02:24 -0500 Subject: [PATCH 06/11] changeset --- .changeset/eight-heads-act.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/eight-heads-act.md diff --git a/.changeset/eight-heads-act.md b/.changeset/eight-heads-act.md new file mode 100644 index 00000000000..be3a542f187 --- /dev/null +++ b/.changeset/eight-heads-act.md @@ -0,0 +1,6 @@ +--- +'@clerk/clerk-js': minor +'@clerk/backend': minor +--- + +Add handling of new Handshake nonce flow when authenticating requests From 4c9e76a9d27594725cfc96bfc6044bde77528c8d Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 8 May 2025 15:40:32 -0500 Subject: [PATCH 07/11] add client api test --- .../src/api/__tests__/ClientApi.test.ts | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 packages/backend/src/api/__tests__/ClientApi.test.ts diff --git a/packages/backend/src/api/__tests__/ClientApi.test.ts b/packages/backend/src/api/__tests__/ClientApi.test.ts new file mode 100644 index 00000000000..5bb73e0121c --- /dev/null +++ b/packages/backend/src/api/__tests__/ClientApi.test.ts @@ -0,0 +1,109 @@ +import { http, HttpResponse } from 'msw'; +import { describe, expect, it } from 'vitest'; + +import { server, validateHeaders } from '../../mock-server'; +import { createBackendApiClient } from '../factory'; + +describe('ClientAPI', () => { + const apiClient = createBackendApiClient({ + apiUrl: 'https://api.clerk.test', + secretKey: 'deadbeef', + }); + + describe('getHandshakePayload', () => { + it('successfully fetches the handshake payload with a valid nonce', async () => { + // Mock response data + const mockHandshakePayload = { + directives: ['directive1', 'directive2'], + }; + + // Set up mock server response + server.use( + http.get( + 'https://api.clerk.test/v1/clients/handshake_payload', + validateHeaders(({ request }) => { + const url = new URL(request.url); + // Verify that the nonce was properly passed as a query parameter + expect(url.searchParams.get('nonce')).toBe('test-nonce-123'); + return HttpResponse.json(mockHandshakePayload); + }), + ), + ); + + // Call the method being tested + const response = await apiClient.clients.getHandshakePayload({ + nonce: 'test-nonce-123', + }); + + // Verify the response + expect(response.directives).toEqual(['directive1', 'directive2']); + expect(response.directives.length).toBe(2); + }); + + it('handles error responses correctly', async () => { + // Mock error response + const mockErrorPayload = { + code: 'invalid_nonce', + message: 'Invalid nonce provided', + long_message: 'The nonce provided is invalid or has expired', + meta: { param_name: 'nonce' }, + }; + const traceId = 'trace_id_handshake'; + + // Set up mock server error response + server.use( + http.get( + 'https://api.clerk.test/v1/clients/handshake_payload', + validateHeaders(() => { + return HttpResponse.json( + { errors: [mockErrorPayload], clerk_trace_id: traceId }, + { status: 400, headers: { 'cf-ray': traceId } }, + ); + }), + ), + ); + + // Call the method and catch the error + const errResponse = await apiClient.clients.getHandshakePayload({ nonce: 'invalid-nonce' }).catch(err => err); + + // Verify error handling + expect(errResponse.clerkTraceId).toBe(traceId); + expect(errResponse.status).toBe(400); + expect(errResponse.errors[0].code).toBe('invalid_nonce'); + expect(errResponse.errors[0].message).toBe('Invalid nonce provided'); + expect(errResponse.errors[0].longMessage).toBe('The nonce provided is invalid or has expired'); + expect(errResponse.errors[0].meta.paramName).toBe('nonce'); + }); + + it('requires a nonce parameter', async () => { + // Set up mock server response for missing nonce parameter + server.use( + http.get( + 'https://api.clerk.test/v1/clients/handshake_payload', + validateHeaders(() => { + return HttpResponse.json( + { + errors: [ + { + code: 'missing_parameter', + message: 'Missing required parameter', + long_message: 'The nonce parameter is required', + meta: { param_name: 'nonce' }, + }, + ], + }, + { status: 400 }, + ); + }), + ), + ); + + // @ts-expect-error Testing invalid input + const errResponse = await apiClient.clients.getHandshakePayload({}).catch(err => err); + + // Verify error handling + expect(errResponse.status).toBe(400); + expect(errResponse.errors[0].code).toBe('missing_parameter'); + }); + }); +}); From 39d237c9cd8938fcf803663a381840009ffe3219 Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 8 May 2025 15:41:22 -0500 Subject: [PATCH 08/11] wip --- packages/backend/src/api/__tests__/ClientApi.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/backend/src/api/__tests__/ClientApi.test.ts b/packages/backend/src/api/__tests__/ClientApi.test.ts index 5bb73e0121c..1a721c1a5b7 100644 --- a/packages/backend/src/api/__tests__/ClientApi.test.ts +++ b/packages/backend/src/api/__tests__/ClientApi.test.ts @@ -12,36 +12,30 @@ describe('ClientAPI', () => { describe('getHandshakePayload', () => { it('successfully fetches the handshake payload with a valid nonce', async () => { - // Mock response data const mockHandshakePayload = { directives: ['directive1', 'directive2'], }; - // Set up mock server response server.use( http.get( 'https://api.clerk.test/v1/clients/handshake_payload', validateHeaders(({ request }) => { const url = new URL(request.url); - // Verify that the nonce was properly passed as a query parameter expect(url.searchParams.get('nonce')).toBe('test-nonce-123'); return HttpResponse.json(mockHandshakePayload); }), ), ); - // Call the method being tested const response = await apiClient.clients.getHandshakePayload({ nonce: 'test-nonce-123', }); - // Verify the response expect(response.directives).toEqual(['directive1', 'directive2']); expect(response.directives.length).toBe(2); }); it('handles error responses correctly', async () => { - // Mock error response const mockErrorPayload = { code: 'invalid_nonce', message: 'Invalid nonce provided', @@ -50,7 +44,6 @@ describe('ClientAPI', () => { }; const traceId = 'trace_id_handshake'; - // Set up mock server error response server.use( http.get( 'https://api.clerk.test/v1/clients/handshake_payload', @@ -63,10 +56,8 @@ describe('ClientAPI', () => { ), ); - // Call the method and catch the error const errResponse = await apiClient.clients.getHandshakePayload({ nonce: 'invalid-nonce' }).catch(err => err); - // Verify error handling expect(errResponse.clerkTraceId).toBe(traceId); expect(errResponse.status).toBe(400); expect(errResponse.errors[0].code).toBe('invalid_nonce'); @@ -76,7 +67,6 @@ describe('ClientAPI', () => { }); it('requires a nonce parameter', async () => { - // Set up mock server response for missing nonce parameter server.use( http.get( 'https://api.clerk.test/v1/clients/handshake_payload', @@ -101,7 +91,6 @@ describe('ClientAPI', () => { // @ts-expect-error Testing invalid input const errResponse = await apiClient.clients.getHandshakePayload({}).catch(err => err); - // Verify error handling expect(errResponse.status).toBe(400); expect(errResponse.errors[0].code).toBe('missing_parameter'); }); From 94f234efe1c7e874d0477dcbb3584d3df598b9c4 Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 8 May 2025 21:49:19 -0500 Subject: [PATCH 09/11] small refactor for testability --- .../src/tokens/__tests__/handshake.test.ts | 288 ++++++------------ packages/backend/src/tokens/handshake.ts | 44 ++- 2 files changed, 125 insertions(+), 207 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts index ba2cf519b42..ff78de82686 100644 --- a/packages/backend/src/tokens/__tests__/handshake.test.ts +++ b/packages/backend/src/tokens/__tests__/handshake.test.ts @@ -3,7 +3,6 @@ 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 { HandshakeService } from '../handshake'; import { OrganizationMatcher } from '../organizationMatcher'; @@ -11,9 +10,7 @@ vi.mock('../handshake.js', async importOriginal => { const actual: any = await importOriginal(); return { ...actual, - verifyHandshakeToken: vi.fn().mockResolvedValue({ - handshake: ['cookie1=value1', 'session=session-token'], - }), + verifyHandshakeToken: vi.fn(), }; }); @@ -56,6 +53,30 @@ vi.mock('../../jwt/verifyJwt.js', () => ({ }, errors: undefined, }), + hasValidSignature: vi.fn().mockResolvedValue({ + data: true, + errors: undefined, + }), +})); + +vi.mock('../keys.js', async importOriginal => { + const actual: any = await importOriginal(); + return { + ...actual, + loadClerkJWKFromRemote: vi.fn().mockResolvedValue({ + kty: 'RSA', + kid: 'test-kid', + use: 'sig', + alg: 'RS256', + n: 'test-n', + e: 'AQAB', + }), + }; +}); + +vi.mock('../../jwt/assertions.js', () => ({ + assertHeaderAlgorithm: vi.fn(), + assertHeaderType: vi.fn(), })); describe('HandshakeService', () => { @@ -76,6 +97,8 @@ describe('HandshakeService', () => { usesSuffixedCookies: () => true, secFetchDest: 'document', accept: 'text/html', + apiUrl: 'https://api.clerk.dev', + secretKey: 'test-secret-key', } as AuthenticateContext; mockOrganizationMatcher = new OrganizationMatcher({ @@ -154,195 +177,6 @@ describe('HandshakeService', () => { }); }); - 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('handleTokenVerificationErrorInDevelopment', () => { it('should throw specific error for invalid signature', () => { const error = new TokenVerificationError({ @@ -388,4 +222,72 @@ describe('HandshakeService', () => { expect(headers.get('Set-Cookie')).toContain('__clerk_redirect_count=1'); }); }); + + describe('getHandshakePayload', () => { + it('should return cookies from handshakeNonce when available', async () => { + const mockDirectives = ['cookie1=value1', 'cookie2=value2']; + const getHandshakePayloadMock = vi.fn().mockResolvedValue({ + directives: mockDirectives, + }); + + mockAuthenticateContext.handshakeNonce = 'test-nonce'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: getHandshakePayloadMock, + }, + } as any; + + const result = await handshakeService.getHandshakePayload(); + + expect(result).toEqual(mockDirectives); + expect(getHandshakePayloadMock).toHaveBeenCalledWith({ + nonce: 'test-nonce', + }); + }); + + it('should handle API errors when getting handshake payload with nonce', async () => { + const mockError = new Error('API error'); + const getHandshakePayloadMock = vi.fn().mockRejectedValue(mockError); + + mockAuthenticateContext.handshakeNonce = 'test-nonce'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: getHandshakePayloadMock, + }, + } as any; + + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const result = await handshakeService.getHandshakePayload(); + + expect(result).toEqual([]); + expect(consoleSpy).toHaveBeenCalledWith('Clerk: HandshakeService: error getting handshake payload:', mockError); + + consoleSpy.mockRestore(); + }); + + it.todo('should return cookies from handshakeToken when nonce is not available'); + + it('should return empty array when neither handshakeNonce nor handshakeToken is available', async () => { + mockAuthenticateContext.handshakeNonce = undefined; + mockAuthenticateContext.handshakeToken = undefined; + + const result = await handshakeService.getHandshakePayload(); + + expect(result).toEqual([]); + }); + + it('should handle token verification errors gracefully', async () => { + mockAuthenticateContext.handshakeNonce = undefined; + mockAuthenticateContext.handshakeToken = 'test-token'; + + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const result = await handshakeService.getHandshakePayload(); + + expect(result).toEqual([]); + + spy.mockRestore(); + }); + }); }); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 677ff3e8c00..8091cb64660 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -162,16 +162,10 @@ export class HandshakeService { } /** - * 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 + * Gets handshake payload from either a nonce or a token + * @returns Promise resolving to string array of cookie directives */ - async resolveHandshake(): Promise { - const headers = new Headers({ - 'Access-Control-Allow-Origin': 'null', - 'Access-Control-Allow-Credentials': 'true', - }); - + public async getHandshakePayload(): Promise { const cookiesToSet: string[] = []; if (this.authenticateContext.handshakeNonce) { @@ -186,13 +180,35 @@ export class HandshakeService { console.error('Clerk: HandshakeService: error getting handshake payload:', error); } } else if (this.authenticateContext.handshakeToken) { - const handshakePayload = await verifyHandshakeToken( - this.authenticateContext.handshakeToken, - this.authenticateContext, - ); - cookiesToSet.push(...handshakePayload.handshake); + try { + const handshakePayload = await verifyHandshakeToken( + this.authenticateContext.handshakeToken, + this.authenticateContext, + ); + if (handshakePayload && Array.isArray(handshakePayload.handshake)) { + cookiesToSet.push(...handshakePayload.handshake); + } + } catch (error) { + console.error('Clerk: HandshakeService: error verifying handshake token:', error); + } } + return cookiesToSet; + } + + /** + * 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 = await this.getHandshakePayload(); + let sessionToken = ''; cookiesToSet.forEach((x: string) => { headers.append('Set-Cookie', x); From 4d75597abd49436d46fec670f82f54c9d0384190 Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 9 May 2025 09:42:28 -0500 Subject: [PATCH 10/11] fix: do not catch token verification errors --- packages/backend/src/tokens/handshake.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 8091cb64660..8b9c88c290a 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -180,16 +180,12 @@ export class HandshakeService { console.error('Clerk: HandshakeService: error getting handshake payload:', error); } } else if (this.authenticateContext.handshakeToken) { - try { - const handshakePayload = await verifyHandshakeToken( - this.authenticateContext.handshakeToken, - this.authenticateContext, - ); - if (handshakePayload && Array.isArray(handshakePayload.handshake)) { - cookiesToSet.push(...handshakePayload.handshake); - } - } catch (error) { - console.error('Clerk: HandshakeService: error verifying handshake token:', error); + const handshakePayload = await verifyHandshakeToken( + this.authenticateContext.handshakeToken, + this.authenticateContext, + ); + if (handshakePayload && Array.isArray(handshakePayload.handshake)) { + cookiesToSet.push(...handshakePayload.handshake); } } From 5a4f25eba8c0abfabfcafb44d6fb7ea4395709ed Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 9 May 2025 10:16:30 -0500 Subject: [PATCH 11/11] rename getHandshakePayload to getCookiesFromHandshake --- packages/backend/src/tokens/__tests__/handshake.test.ts | 8 ++++---- packages/backend/src/tokens/handshake.ts | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts index ff78de82686..48044a17c8a 100644 --- a/packages/backend/src/tokens/__tests__/handshake.test.ts +++ b/packages/backend/src/tokens/__tests__/handshake.test.ts @@ -237,7 +237,7 @@ describe('HandshakeService', () => { }, } as any; - const result = await handshakeService.getHandshakePayload(); + const result = await handshakeService.getCookiesFromHandshake(); expect(result).toEqual(mockDirectives); expect(getHandshakePayloadMock).toHaveBeenCalledWith({ @@ -258,7 +258,7 @@ describe('HandshakeService', () => { const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - const result = await handshakeService.getHandshakePayload(); + const result = await handshakeService.getCookiesFromHandshake(); expect(result).toEqual([]); expect(consoleSpy).toHaveBeenCalledWith('Clerk: HandshakeService: error getting handshake payload:', mockError); @@ -272,7 +272,7 @@ describe('HandshakeService', () => { mockAuthenticateContext.handshakeNonce = undefined; mockAuthenticateContext.handshakeToken = undefined; - const result = await handshakeService.getHandshakePayload(); + const result = await handshakeService.getCookiesFromHandshake(); expect(result).toEqual([]); }); @@ -283,7 +283,7 @@ describe('HandshakeService', () => { const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); - const result = await handshakeService.getHandshakePayload(); + const result = await handshakeService.getCookiesFromHandshake(); expect(result).toEqual([]); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 8b9c88c290a..417bab8c999 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -162,10 +162,10 @@ export class HandshakeService { } /** - * Gets handshake payload from either a nonce or a token + * Gets cookies from either a handshake nonce or a handshake token * @returns Promise resolving to string array of cookie directives */ - public async getHandshakePayload(): Promise { + public async getCookiesFromHandshake(): Promise { const cookiesToSet: string[] = []; if (this.authenticateContext.handshakeNonce) { @@ -203,7 +203,7 @@ export class HandshakeService { 'Access-Control-Allow-Credentials': 'true', }); - const cookiesToSet = await this.getHandshakePayload(); + const cookiesToSet = await this.getCookiesFromHandshake(); let sessionToken = ''; cookiesToSet.forEach((x: string) => {