Skip to content

Commit 7701f84

Browse files
jacekradkojacekradko
committed
fix(backend): Consider proxyUrl in determining frontendApi URL
1 parent 4319257 commit 7701f84

File tree

3 files changed

+301
-16
lines changed

3 files changed

+301
-16
lines changed

‎packages/backend/src/tokens/__tests__/handshake.test.ts‎

Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ describe('HandshakeService', () => {
179179

180180
it('should use proxy URL when available', () => {
181181
mockAuthenticateContext.proxyUrl = 'https://my-proxy.example.com';
182+
// Simulate what parsePublishableKey does when proxy URL is provided
183+
mockAuthenticateContext.frontendApi = 'https://my-proxy.example.com';
182184
const headers = handshakeService.buildRedirectToHandshake('test-reason');
183185
const location = headers.get(constants.Headers.Location);
184186
if (!location) {
@@ -195,6 +197,7 @@ describe('HandshakeService', () => {
195197

196198
it('should handle proxy URL with trailing slash', () => {
197199
mockAuthenticateContext.proxyUrl = 'https://my-proxy.example.com/';
200+
mockAuthenticateContext.frontendApi = 'https://my-proxy.example.com/';
198201
const headers = handshakeService.buildRedirectToHandshake('test-reason');
199202
const location = headers.get(constants.Headers.Location);
200203
if (!location) {
@@ -205,6 +208,224 @@ describe('HandshakeService', () => {
205208
expect(url.hostname).toBe('my-proxy.example.com');
206209
expect(url.pathname).toBe('/v1/client/handshake');
207210
});
211+
212+
it('should handle proxy URL with multiple trailing slashes', () => {
213+
mockAuthenticateContext.proxyUrl = 'https://my-proxy.example.com//';
214+
mockAuthenticateContext.frontendApi = 'https://my-proxy.example.com//';
215+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
216+
const location = headers.get(constants.Headers.Location);
217+
if (!location) {
218+
throw new Error('Location header is missing');
219+
}
220+
const url = new URL(location);
221+
222+
expect(url.hostname).toBe('my-proxy.example.com');
223+
expect(url.pathname).toBe('/v1/client/handshake');
224+
expect(location).not.toContain('//v1/client/handshake');
225+
});
226+
227+
it('should handle proxy URL with many trailing slashes', () => {
228+
mockAuthenticateContext.proxyUrl = 'https://my-proxy.example.com///';
229+
mockAuthenticateContext.frontendApi = 'https://my-proxy.example.com///';
230+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
231+
const location = headers.get(constants.Headers.Location);
232+
if (!location) {
233+
throw new Error('Location header is missing');
234+
}
235+
const url = new URL(location);
236+
237+
expect(url.hostname).toBe('my-proxy.example.com');
238+
expect(url.pathname).toBe('/v1/client/handshake');
239+
expect(location).not.toContain('//v1/client/handshake');
240+
});
241+
242+
it('should handle proxy URL without trailing slash', () => {
243+
mockAuthenticateContext.proxyUrl = 'https://my-proxy.example.com';
244+
mockAuthenticateContext.frontendApi = 'https://my-proxy.example.com';
245+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
246+
const location = headers.get(constants.Headers.Location);
247+
if (!location) {
248+
throw new Error('Location header is missing');
249+
}
250+
const url = new URL(location);
251+
252+
expect(url.hostname).toBe('my-proxy.example.com');
253+
expect(url.pathname).toBe('/v1/client/handshake');
254+
});
255+
256+
it('should handle proxy URL with path and trailing slashes', () => {
257+
mockAuthenticateContext.proxyUrl = 'https://my-proxy.example.com/clerk-proxy//';
258+
mockAuthenticateContext.frontendApi = 'https://my-proxy.example.com/clerk-proxy//';
259+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
260+
const location = headers.get(constants.Headers.Location);
261+
if (!location) {
262+
throw new Error('Location header is missing');
263+
}
264+
const url = new URL(location);
265+
266+
expect(url.hostname).toBe('my-proxy.example.com');
267+
expect(url.pathname).toBe('/clerk-proxy/v1/client/handshake');
268+
expect(location).not.toContain('clerk-proxy//v1/client/handshake');
269+
});
270+
271+
it('should handle non-HTTP frontendApi (domain only)', () => {
272+
mockAuthenticateContext.frontendApi = 'api.clerk.com';
273+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
274+
const location = headers.get(constants.Headers.Location);
275+
if (!location) {
276+
throw new Error('Location header is missing');
277+
}
278+
const url = new URL(location);
279+
280+
expect(url.protocol).toBe('https:');
281+
expect(url.hostname).toBe('api.clerk.com');
282+
expect(url.pathname).toBe('/v1/client/handshake');
283+
});
284+
285+
it('should not include dev browser token in production mode', () => {
286+
mockAuthenticateContext.instanceType = 'production';
287+
mockAuthenticateContext.devBrowserToken = 'dev-token';
288+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
289+
const location = headers.get(constants.Headers.Location);
290+
if (!location) {
291+
throw new Error('Location header is missing');
292+
}
293+
const url = new URL(location);
294+
295+
expect(url.searchParams.get(constants.QueryParameters.DevBrowser)).toBeNull();
296+
});
297+
298+
it('should not include dev browser token when not available in development', () => {
299+
mockAuthenticateContext.instanceType = 'development';
300+
mockAuthenticateContext.devBrowserToken = undefined;
301+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
302+
const location = headers.get(constants.Headers.Location);
303+
if (!location) {
304+
throw new Error('Location header is missing');
305+
}
306+
const url = new URL(location);
307+
308+
expect(url.searchParams.get(constants.QueryParameters.DevBrowser)).toBeNull();
309+
});
310+
311+
it('should handle usesSuffixedCookies returning false', () => {
312+
mockAuthenticateContext.usesSuffixedCookies = vi.fn().mockReturnValue(false);
313+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
314+
const location = headers.get(constants.Headers.Location);
315+
if (!location) {
316+
throw new Error('Location header is missing');
317+
}
318+
const url = new URL(location);
319+
320+
expect(url.searchParams.get(constants.QueryParameters.SuffixedCookies)).toBe('false');
321+
});
322+
323+
it('should include organization sync parameters when organization target is found', () => {
324+
// Mock the organization sync methods
325+
const mockTarget = { type: 'organization', id: 'org_123' };
326+
const mockParams = new Map([
327+
['org_id', 'org_123'],
328+
['org_slug', 'test-org'],
329+
]);
330+
331+
vi.spyOn(handshakeService as any, 'getOrganizationSyncTarget').mockReturnValue(mockTarget);
332+
vi.spyOn(handshakeService as any, 'getOrganizationSyncQueryParams').mockReturnValue(mockParams);
333+
334+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
335+
const location = headers.get(constants.Headers.Location);
336+
if (!location) {
337+
throw new Error('Location header is missing');
338+
}
339+
const url = new URL(location);
340+
341+
expect(url.searchParams.get('org_id')).toBe('org_123');
342+
expect(url.searchParams.get('org_slug')).toBe('test-org');
343+
});
344+
345+
it('should not include organization sync parameters when no target is found', () => {
346+
vi.spyOn(handshakeService as any, 'getOrganizationSyncTarget').mockReturnValue(null);
347+
348+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
349+
const location = headers.get(constants.Headers.Location);
350+
if (!location) {
351+
throw new Error('Location header is missing');
352+
}
353+
const url = new URL(location);
354+
355+
expect(url.searchParams.get('org_id')).toBeNull();
356+
expect(url.searchParams.get('org_slug')).toBeNull();
357+
});
358+
359+
it('should handle different handshake reasons', () => {
360+
const reasons = ['session-token-expired', 'dev-browser-sync', 'satellite-cookie-needs-syncing'];
361+
362+
reasons.forEach(reason => {
363+
const headers = handshakeService.buildRedirectToHandshake(reason);
364+
const location = headers.get(constants.Headers.Location);
365+
if (!location) {
366+
throw new Error('Location header is missing');
367+
}
368+
const url = new URL(location);
369+
370+
expect(url.searchParams.get(constants.QueryParameters.HandshakeReason)).toBe(reason);
371+
});
372+
});
373+
374+
it('should handle complex clerkUrl with query parameters and fragments', () => {
375+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/path?existing=param#fragment');
376+
377+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
378+
const location = headers.get(constants.Headers.Location);
379+
if (!location) {
380+
throw new Error('Location header is missing');
381+
}
382+
const url = new URL(location);
383+
384+
const redirectUrl = url.searchParams.get('redirect_url');
385+
expect(redirectUrl).toBe('https://example.com/path?existing=param#fragment');
386+
});
387+
388+
it('should create valid URLs with different frontend API formats', () => {
389+
const frontendApiFormats = [
390+
'api.clerk.com',
391+
'https://api.clerk.com',
392+
'https://api.clerk.com/',
393+
'foo-bar-13.clerk.accounts.dev',
394+
'https://foo-bar-13.clerk.accounts.dev',
395+
'clerk.example.com',
396+
'https://clerk.example.com/proxy-path',
397+
];
398+
399+
frontendApiFormats.forEach(frontendApi => {
400+
mockAuthenticateContext.frontendApi = frontendApi;
401+
402+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
403+
const location = headers.get(constants.Headers.Location);
404+
405+
expect(location).toBeDefined();
406+
expect(() => new URL(location!)).not.toThrow();
407+
408+
const url = new URL(location!);
409+
// Path should end with '/v1/client/handshake' (may have proxy path prefix)
410+
expect(url.pathname).toMatch(/\/v1\/client\/handshake$/);
411+
expect(url.searchParams.get(constants.QueryParameters.HandshakeReason)).toBe('test-reason');
412+
});
413+
});
414+
415+
it('should always include required query parameters', () => {
416+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
417+
const location = headers.get(constants.Headers.Location);
418+
if (!location) {
419+
throw new Error('Location header is missing');
420+
}
421+
const url = new URL(location);
422+
423+
// Verify all required parameters are present
424+
expect(url.searchParams.get('redirect_url')).toBeDefined();
425+
expect(url.searchParams.get('__clerk_api_version')).toBe('2025-04-10');
426+
expect(url.searchParams.get(constants.QueryParameters.SuffixedCookies)).toMatch(/^(true|false)$/);
427+
expect(url.searchParams.get(constants.QueryParameters.HandshakeReason)).toBe('test-reason');
428+
});
208429
});
209430

210431
describe('handleTokenVerificationErrorInDevelopment', () => {
@@ -320,4 +541,51 @@ describe('HandshakeService', () => {
320541
spy.mockRestore();
321542
});
322543
});
544+
545+
describe('URL construction edge cases', () => {
546+
const trailingSlashTestCases = [
547+
{ input: 'https://example.com', expected: 'https://example.com' },
548+
{ input: 'https://example.com/', expected: 'https://example.com' },
549+
{ input: 'https://example.com//', expected: 'https://example.com' },
550+
{ input: 'https://example.com///', expected: 'https://example.com' },
551+
{ input: 'https://example.com/path', expected: 'https://example.com/path' },
552+
{ input: 'https://example.com/path/', expected: 'https://example.com/path' },
553+
{ input: 'https://example.com/path//', expected: 'https://example.com/path' },
554+
{ input: 'https://example.com/proxy-path///', expected: 'https://example.com/proxy-path' },
555+
];
556+
557+
trailingSlashTestCases.forEach(({ input, expected }) => {
558+
it(`should correctly handle trailing slashes: "${input}" -> "${expected}"`, () => {
559+
const result = input.replace(/\/+$/, '');
560+
expect(result).toBe(expected);
561+
});
562+
});
563+
564+
it('should construct valid handshake URLs with various proxy configurations', () => {
565+
const proxyConfigs = [
566+
'https://proxy.example.com',
567+
'https://proxy.example.com/',
568+
'https://proxy.example.com//',
569+
'https://proxy.example.com/clerk',
570+
'https://proxy.example.com/clerk/',
571+
'https://proxy.example.com/clerk//',
572+
'https://api.example.com/v1/clerk///',
573+
];
574+
575+
proxyConfigs.forEach(proxyUrl => {
576+
mockAuthenticateContext.proxyUrl = proxyUrl;
577+
mockAuthenticateContext.frontendApi = proxyUrl; // Simulate parsePublishableKey behavior
578+
579+
const headers = handshakeService.buildRedirectToHandshake('test-reason');
580+
const location = headers.get(constants.Headers.Location);
581+
582+
expect(location).toBeDefined();
583+
expect(location).toContain('/v1/client/handshake');
584+
expect(location).not.toContain('//v1/client/handshake'); // No double slashes
585+
586+
// Ensure URL is valid
587+
expect(() => new URL(location!)).not.toThrow();
588+
});
589+
});
590+
});
323591
});

‎packages/backend/src/tokens/authenticateContext.ts‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,33 @@ import type { AuthenticateRequestOptions } from './types';
1010

1111
interface AuthenticateContext extends AuthenticateRequestOptions {
1212
// header-based values
13-
tokenInHeader: string | undefined;
14-
origin: string | undefined;
15-
host: string | undefined;
13+
accept: string | undefined;
1614
forwardedHost: string | undefined;
1715
forwardedProto: string | undefined;
16+
host: string | undefined;
17+
origin: string | undefined;
1818
referrer: string | undefined;
19-
userAgent: string | undefined;
2019
secFetchDest: string | undefined;
21-
accept: string | undefined;
20+
tokenInHeader: string | undefined;
21+
userAgent: string | undefined;
22+
2223
// cookie-based values
23-
sessionTokenInCookie: string | undefined;
24-
refreshTokenInCookie: string | undefined;
2524
clientUat: number;
25+
refreshTokenInCookie: string | undefined;
26+
sessionTokenInCookie: string | undefined;
27+
2628
// handshake-related values
2729
devBrowserToken: string | undefined;
2830
handshakeNonce: string | undefined;
29-
handshakeToken: string | undefined;
3031
handshakeRedirectLoopCounter: number;
32+
handshakeToken: string | undefined;
3133

3234
// url derived from headers
3335
clerkUrl: URL;
3436
// enforce existence of the following props
35-
publishableKey: string;
36-
instanceType: string;
3737
frontendApi: string;
38+
instanceType: string;
39+
publishableKey: string;
3840
}
3941

4042
/**
@@ -44,6 +46,12 @@ interface AuthenticateContext extends AuthenticateRequestOptions {
4446
* to perform a handshake.
4547
*/
4648
class AuthenticateContext implements AuthenticateContext {
49+
/**
50+
* The original Clerk frontend API URL, extracted from publishable key before proxy URL override.
51+
* Used for backend operations like token validation and issuer checking.
52+
*/
53+
private originalFrontendApi: string = '';
54+
4755
/**
4856
* Retrieves the session token from either the cookie or the header.
4957
*
@@ -163,6 +171,13 @@ class AuthenticateContext implements AuthenticateContext {
163171
assertValidPublishableKey(options.publishableKey);
164172
this.publishableKey = options.publishableKey;
165173

174+
const originalPk = parsePublishableKey(this.publishableKey, {
175+
fatal: true,
176+
domain: options.domain,
177+
isSatellite: options.isSatellite,
178+
});
179+
this.originalFrontendApi = originalPk.frontendApi;
180+
166181
const pk = parsePublishableKey(this.publishableKey, {
167182
fatal: true,
168183
proxyUrl: options.proxyUrl,
@@ -266,7 +281,8 @@ class AuthenticateContext implements AuthenticateContext {
266281
return false;
267282
}
268283
const tokenIssuer = data.payload.iss.replace(/https?:\/\//gi, '');
269-
return this.frontendApi === tokenIssuer;
284+
// Use original frontend API for token validation since tokens are issued by the actual Clerk API, not proxy
285+
return this.originalFrontendApi === tokenIssuer;
270286
}
271287

272288
private sessionExpired(jwt: Jwt | undefined): boolean {

‎packages/backend/src/tokens/handshake.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,14 @@ export class HandshakeService {
134134
}
135135

136136
const redirectUrl = this.removeDevBrowserFromURL(this.authenticateContext.clerkUrl);
137-
const frontendApiNoProtocol = this.authenticateContext.frontendApi.replace(/http(s)?:\/\//, '');
138137

139-
const baseUrl = this.authenticateContext.proxyUrl
140-
? this.authenticateContext.proxyUrl.replace(/\/$/, '')
141-
: `https://${frontendApiNoProtocol}`;
138+
let baseUrl = this.authenticateContext.frontendApi.startsWith('http')
139+
? this.authenticateContext.frontendApi
140+
: `https://${this.authenticateContext.frontendApi}`;
142141

143-
const url = new URL(`${baseUrl}/v1/client/handshake`);
142+
baseUrl = baseUrl.replace(/\/+$/, '') + '/';
143+
144+
const url = new URL('v1/client/handshake', baseUrl);
144145
url.searchParams.append('redirect_url', redirectUrl?.href || '');
145146
url.searchParams.append('__clerk_api_version', SUPPORTED_BAPI_VERSION);
146147
url.searchParams.append(

0 commit comments

Comments
 (0)