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 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..1a721c1a5b7 --- /dev/null +++ b/packages/backend/src/api/__tests__/ClientApi.test.ts @@ -0,0 +1,98 @@ +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 () => { + const mockHandshakePayload = { + directives: ['directive1', 'directive2'], + }; + + server.use( + http.get( + 'https://api.clerk.test/v1/clients/handshake_payload', + validateHeaders(({ request }) => { + const url = new URL(request.url); + expect(url.searchParams.get('nonce')).toBe('test-nonce-123'); + return HttpResponse.json(mockHandshakePayload); + }), + ), + ); + + const response = await apiClient.clients.getHandshakePayload({ + nonce: 'test-nonce-123', + }); + + expect(response.directives).toEqual(['directive1', 'directive2']); + expect(response.directives.length).toBe(2); + }); + + it('handles error responses correctly', async () => { + 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'; + + 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 } }, + ); + }), + ), + ); + + const errResponse = await apiClient.clients.getHandshakePayload({ nonce: 'invalid-nonce' }).catch(err => err); + + 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 () => { + 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); + + expect(errResponse.status).toBe(400); + expect(errResponse.errors[0].code).toBe('missing_parameter'); + }); + }); +}); diff --git a/packages/backend/src/api/endpoints/ClientApi.ts b/packages/backend/src/api/endpoints/ClientApi.ts index b888e8c7cb7..f04202dff38 100644 --- a/packages/backend/src/api/endpoints/ClientApi.ts +++ b/packages/backend/src/api/endpoints/ClientApi.ts @@ -3,10 +3,15 @@ import type { ClerkPaginationRequest } from '@clerk/types'; 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'; +type GetHandshakePayloadParams = { + nonce: string; +}; + export class ClientAPI extends AbstractAPI { public async getClientList(params: ClerkPaginationRequest = {}) { return this.request>({ @@ -31,4 +36,12 @@ export class ClientAPI extends AbstractAPI { bodyParams: { token }, }); } + + public async getHandshakePayload(queryParams: GetHandshakePayloadParams) { + return this.request({ + method: 'GET', + path: joinPaths(basePath, '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/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/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts index ba2cf519b42..48044a17c8a 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.getCookiesFromHandshake(); + + 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.getCookiesFromHandshake(); + + 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.getCookiesFromHandshake(); + + 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.getCookiesFromHandshake(); + + expect(result).toEqual([]); + + spy.mockRestore(); + }); + }); }); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index 8f416bd72a9..417bab8c999 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -161,6 +161,37 @@ export class HandshakeService { return new Headers({ [constants.Headers.Location]: url.href }); } + /** + * Gets cookies from either a handshake nonce or a handshake token + * @returns Promise resolving to string array of cookie directives + */ + public async getCookiesFromHandshake(): Promise { + const cookiesToSet: string[] = []; + + if (this.authenticateContext.handshakeNonce) { + try { + const handshakePayload = await this.authenticateContext.apiClient?.clients.getHandshakePayload({ + nonce: this.authenticateContext.handshakeNonce, + }); + if (handshakePayload) { + cookiesToSet.push(...handshakePayload.directives); + } + } catch (error) { + console.error('Clerk: HandshakeService: error getting handshake payload:', error); + } + } else if (this.authenticateContext.handshakeToken) { + const handshakePayload = await verifyHandshakeToken( + this.authenticateContext.handshakeToken, + this.authenticateContext, + ); + if (handshakePayload && Array.isArray(handshakePayload.handshake)) { + cookiesToSet.push(...handshakePayload.handshake); + } + } + + return cookiesToSet; + } + /** * Resolves a handshake request by verifying the handshake token and setting appropriate cookies * @returns Promise resolving to either a SignedInState or SignedOutState @@ -172,19 +203,7 @@ export class HandshakeService { 'Access-Control-Allow-Credentials': 'true', }); - 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.authenticateContext.handshakeToken) { - const handshakePayload = await verifyHandshakeToken( - this.authenticateContext.handshakeToken, - this.authenticateContext, - ); - cookiesToSet.push(...handshakePayload.handshake); - } + const cookiesToSet = await this.getCookiesFromHandshake(); let sessionToken = ''; cookiesToSet.forEach((x: string) => { diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 882aae8158a..2330a6fa32e 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -336,7 +336,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,