diff --git a/.changeset/spicy-queens-send.md b/.changeset/spicy-queens-send.md new file mode 100644 index 00000000000..25f0f8bbcbd --- /dev/null +++ b/.changeset/spicy-queens-send.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +Remove __clerk_handshake_nonce query parameter from redirect URLs in development mode to prevent infinite loops. diff --git a/packages/backend/src/tokens/__tests__/handshake.test.ts b/packages/backend/src/tokens/__tests__/handshake.test.ts index 51c26f45ba1..29eed7d2838 100644 --- a/packages/backend/src/tokens/__tests__/handshake.test.ts +++ b/packages/backend/src/tokens/__tests__/handshake.test.ts @@ -601,4 +601,339 @@ describe('HandshakeService', () => { }); }); }); + + describe('Query Parameter Cleanup', () => { + beforeEach(async () => { + const { verifyToken } = vi.mocked(await import('../verify.js')); + verifyToken.mockResolvedValue({ + data: { + __raw: 'mock-token', + sid: 'session-id', + sub: 'user_123', + iss: 'https://clerk.example.com', + iat: Math.floor(Date.now() / 1000), + exp: Math.floor(Date.now() / 1000) + 3600, + nbf: Math.floor(Date.now() / 1000), + azp: 'https://example.com', + }, + errors: undefined, + }); + }); + + describe('Development Mode', () => { + beforeEach(() => { + mockAuthenticateContext.instanceType = 'development'; + }); + + it('should remove __clerk_handshake_nonce from query params', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + expect(location).toBeTruthy(); + + const url = new URL(location!); + expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false); + expect(url.searchParams.get('foo')).toBe('bar'); + }); + + it('should remove __clerk_handshake token from query params', async () => { + const { verifyHandshakeToken } = vi.mocked(await import('../handshake.js')); + verifyHandshakeToken.mockResolvedValue({ + handshake: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }); + + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake=token123&foo=bar'); + mockAuthenticateContext.handshakeNonce = undefined; + mockAuthenticateContext.handshakeToken = 'token123'; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + expect(location).toBeTruthy(); + + const url = new URL(location!); + expect(url.searchParams.has('__clerk_handshake')).toBe(false); + expect(url.searchParams.get('foo')).toBe('bar'); + }); + + it('should remove __clerk_help from query params', async () => { + mockAuthenticateContext.clerkUrl = new URL( + 'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_help=1', + ); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + const url = new URL(location!); + expect(url.searchParams.has('__clerk_help')).toBe(false); + }); + + it('should remove __clerk_db_jwt (dev browser) from query params', async () => { + mockAuthenticateContext.clerkUrl = new URL( + 'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_db_jwt=dev_token', + ); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + const url = new URL(location!); + expect(url.searchParams.has('__clerk_db_jwt')).toBe(false); + }); + + it('should remove all handshake query params at once', async () => { + mockAuthenticateContext.clerkUrl = new URL( + 'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_handshake=token&__clerk_help=1&__clerk_db_jwt=dev&foo=bar&baz=qux', + ); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + const url = new URL(location!); + + expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false); + expect(url.searchParams.has('__clerk_handshake')).toBe(false); + expect(url.searchParams.has('__clerk_help')).toBe(false); + expect(url.searchParams.has('__clerk_db_jwt')).toBe(false); + + expect(url.searchParams.get('foo')).toBe('bar'); + expect(url.searchParams.get('baz')).toBe('qux'); + }); + + it('should handle URL with only handshake params (clean URL result)', async () => { + mockAuthenticateContext.clerkUrl = new URL( + 'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_help=1', + ); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + const url = new URL(location!); + + expect(url.search).toBe(''); + expect(url.href).toBe('https://example.com/page'); + }); + + it('should handle URL with no query params (nonce in cookie)', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + const url = new URL(location!); + + expect(url.href).toBe('https://example.com/page'); + expect(url.search).toBe(''); + }); + + it('should preserve URL-encoded query params', async () => { + mockAuthenticateContext.clerkUrl = new URL( + 'https://example.com/page?__clerk_handshake_nonce=abc123&q=hello%20world&redirect=%2Fsome%2Fpath%3Ffoo%3Dbar', + ); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + const url = new URL(location!); + + expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false); + expect(url.searchParams.get('q')).toBe('hello world'); + expect(url.searchParams.get('redirect')).toBe('/some/path?foo=bar'); + }); + + it('should preserve hash fragments when cleaning query params', async () => { + mockAuthenticateContext.clerkUrl = new URL( + 'https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar#section-2', + ); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + const url = new URL(location!); + + expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false); + expect(url.searchParams.get('foo')).toBe('bar'); + expect(url.hash).toBe('#section-2'); + }); + + it('should set Cache-Control header to no-store', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + expect(result.headers.get(constants.Headers.CacheControl)).toBe('no-store'); + }); + }); + + describe('Production Mode', () => { + beforeEach(() => { + mockAuthenticateContext.instanceType = 'production'; + }); + + it('should NOT add Location header in production mode', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + expect(result.headers.has(constants.Headers.Location)).toBe(false); + expect(result.status).toBe('signed-in'); + }); + + it('should NOT set Cache-Control header in production mode', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + expect(result.headers.has(constants.Headers.CacheControl)).toBe(false); + }); + + it('should still set session cookies in production', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const setCookieHeaders = result.headers.getSetCookie?.() || []; + expect(setCookieHeaders.length).toBeGreaterThan(0); + expect(setCookieHeaders.some(h => h.startsWith('__session='))).toBe(true); + }); + }); + + describe('Error Cases', () => { + beforeEach(() => { + mockAuthenticateContext.instanceType = 'development'; + }); + + it('should handle BAPI errors gracefully', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockRejectedValue(new Error('BAPI error')), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + expect(result.status).toBe('signed-out'); + }); + + it('should clean up query params even when handshake payload is empty', async () => { + mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar'); + mockAuthenticateContext.handshakeNonce = 'abc123'; + mockAuthenticateContext.apiClient = { + clients: { + getHandshakePayload: vi.fn().mockResolvedValue({ + directives: [], + }), + }, + } as any; + + const result = await handshakeService.resolveHandshake(); + + const location = result.headers.get(constants.Headers.Location); + expect(location).toBeTruthy(); + + const url = new URL(location!); + expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false); + expect(url.searchParams.get('foo')).toBe('bar'); + }); + }); + }); }); diff --git a/packages/backend/src/tokens/handshake.ts b/packages/backend/src/tokens/handshake.ts index c6c9ccc3552..c19267e0506 100644 --- a/packages/backend/src/tokens/handshake.ts +++ b/packages/backend/src/tokens/handshake.ts @@ -221,6 +221,7 @@ export class HandshakeService { newUrl.searchParams.delete(constants.QueryParameters.Handshake); newUrl.searchParams.delete(constants.QueryParameters.HandshakeHelp); newUrl.searchParams.delete(constants.QueryParameters.DevBrowser); + newUrl.searchParams.delete(constants.QueryParameters.HandshakeNonce); headers.append(constants.Headers.Location, newUrl.toString()); headers.set(constants.Headers.CacheControl, 'no-store'); }