Skip to content

Commit f2644c2

Browse files
authored
fix(backend): Remove handshake nonce query param to prevent infinite loops (#7054)
1 parent 25f54f6 commit f2644c2

3 files changed

Lines changed: 341 additions & 0 deletions

File tree

.changeset/spicy-queens-send.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/backend': patch
3+
---
4+
5+
Remove __clerk_handshake_nonce query parameter from redirect URLs in development mode to prevent infinite loops.

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

Lines changed: 335 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,4 +601,339 @@ describe('HandshakeService', () => {
601601
});
602602
});
603603
});
604+
605+
describe('Query Parameter Cleanup', () => {
606+
beforeEach(async () => {
607+
const { verifyToken } = vi.mocked(await import('../verify.js'));
608+
verifyToken.mockResolvedValue({
609+
data: {
610+
__raw: 'mock-token',
611+
sid: 'session-id',
612+
sub: 'user_123',
613+
iss: 'https://clerk.example.com',
614+
iat: Math.floor(Date.now() / 1000),
615+
exp: Math.floor(Date.now() / 1000) + 3600,
616+
nbf: Math.floor(Date.now() / 1000),
617+
azp: 'https://example.com',
618+
},
619+
errors: undefined,
620+
});
621+
});
622+
623+
describe('Development Mode', () => {
624+
beforeEach(() => {
625+
mockAuthenticateContext.instanceType = 'development';
626+
});
627+
628+
it('should remove __clerk_handshake_nonce from query params', async () => {
629+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar');
630+
mockAuthenticateContext.handshakeNonce = 'abc123';
631+
mockAuthenticateContext.apiClient = {
632+
clients: {
633+
getHandshakePayload: vi.fn().mockResolvedValue({
634+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
635+
}),
636+
},
637+
} as any;
638+
639+
const result = await handshakeService.resolveHandshake();
640+
641+
const location = result.headers.get(constants.Headers.Location);
642+
expect(location).toBeTruthy();
643+
644+
const url = new URL(location!);
645+
expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false);
646+
expect(url.searchParams.get('foo')).toBe('bar');
647+
});
648+
649+
it('should remove __clerk_handshake token from query params', async () => {
650+
const { verifyHandshakeToken } = vi.mocked(await import('../handshake.js'));
651+
verifyHandshakeToken.mockResolvedValue({
652+
handshake: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
653+
});
654+
655+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake=token123&foo=bar');
656+
mockAuthenticateContext.handshakeNonce = undefined;
657+
mockAuthenticateContext.handshakeToken = 'token123';
658+
659+
const result = await handshakeService.resolveHandshake();
660+
661+
const location = result.headers.get(constants.Headers.Location);
662+
expect(location).toBeTruthy();
663+
664+
const url = new URL(location!);
665+
expect(url.searchParams.has('__clerk_handshake')).toBe(false);
666+
expect(url.searchParams.get('foo')).toBe('bar');
667+
});
668+
669+
it('should remove __clerk_help from query params', async () => {
670+
mockAuthenticateContext.clerkUrl = new URL(
671+
'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_help=1',
672+
);
673+
mockAuthenticateContext.handshakeNonce = 'abc123';
674+
mockAuthenticateContext.apiClient = {
675+
clients: {
676+
getHandshakePayload: vi.fn().mockResolvedValue({
677+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
678+
}),
679+
},
680+
} as any;
681+
682+
const result = await handshakeService.resolveHandshake();
683+
684+
const location = result.headers.get(constants.Headers.Location);
685+
const url = new URL(location!);
686+
expect(url.searchParams.has('__clerk_help')).toBe(false);
687+
});
688+
689+
it('should remove __clerk_db_jwt (dev browser) from query params', async () => {
690+
mockAuthenticateContext.clerkUrl = new URL(
691+
'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_db_jwt=dev_token',
692+
);
693+
mockAuthenticateContext.handshakeNonce = 'abc123';
694+
mockAuthenticateContext.apiClient = {
695+
clients: {
696+
getHandshakePayload: vi.fn().mockResolvedValue({
697+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
698+
}),
699+
},
700+
} as any;
701+
702+
const result = await handshakeService.resolveHandshake();
703+
704+
const location = result.headers.get(constants.Headers.Location);
705+
const url = new URL(location!);
706+
expect(url.searchParams.has('__clerk_db_jwt')).toBe(false);
707+
});
708+
709+
it('should remove all handshake query params at once', async () => {
710+
mockAuthenticateContext.clerkUrl = new URL(
711+
'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_handshake=token&__clerk_help=1&__clerk_db_jwt=dev&foo=bar&baz=qux',
712+
);
713+
mockAuthenticateContext.handshakeNonce = 'abc123';
714+
mockAuthenticateContext.apiClient = {
715+
clients: {
716+
getHandshakePayload: vi.fn().mockResolvedValue({
717+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
718+
}),
719+
},
720+
} as any;
721+
722+
const result = await handshakeService.resolveHandshake();
723+
724+
const location = result.headers.get(constants.Headers.Location);
725+
const url = new URL(location!);
726+
727+
expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false);
728+
expect(url.searchParams.has('__clerk_handshake')).toBe(false);
729+
expect(url.searchParams.has('__clerk_help')).toBe(false);
730+
expect(url.searchParams.has('__clerk_db_jwt')).toBe(false);
731+
732+
expect(url.searchParams.get('foo')).toBe('bar');
733+
expect(url.searchParams.get('baz')).toBe('qux');
734+
});
735+
736+
it('should handle URL with only handshake params (clean URL result)', async () => {
737+
mockAuthenticateContext.clerkUrl = new URL(
738+
'https://example.com/page?__clerk_handshake_nonce=abc123&__clerk_help=1',
739+
);
740+
mockAuthenticateContext.handshakeNonce = 'abc123';
741+
mockAuthenticateContext.apiClient = {
742+
clients: {
743+
getHandshakePayload: vi.fn().mockResolvedValue({
744+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
745+
}),
746+
},
747+
} as any;
748+
749+
const result = await handshakeService.resolveHandshake();
750+
751+
const location = result.headers.get(constants.Headers.Location);
752+
const url = new URL(location!);
753+
754+
expect(url.search).toBe('');
755+
expect(url.href).toBe('https://example.com/page');
756+
});
757+
758+
it('should handle URL with no query params (nonce in cookie)', async () => {
759+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page');
760+
mockAuthenticateContext.handshakeNonce = 'abc123';
761+
mockAuthenticateContext.apiClient = {
762+
clients: {
763+
getHandshakePayload: vi.fn().mockResolvedValue({
764+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
765+
}),
766+
},
767+
} as any;
768+
769+
const result = await handshakeService.resolveHandshake();
770+
771+
const location = result.headers.get(constants.Headers.Location);
772+
const url = new URL(location!);
773+
774+
expect(url.href).toBe('https://example.com/page');
775+
expect(url.search).toBe('');
776+
});
777+
778+
it('should preserve URL-encoded query params', async () => {
779+
mockAuthenticateContext.clerkUrl = new URL(
780+
'https://example.com/page?__clerk_handshake_nonce=abc123&q=hello%20world&redirect=%2Fsome%2Fpath%3Ffoo%3Dbar',
781+
);
782+
mockAuthenticateContext.handshakeNonce = 'abc123';
783+
mockAuthenticateContext.apiClient = {
784+
clients: {
785+
getHandshakePayload: vi.fn().mockResolvedValue({
786+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
787+
}),
788+
},
789+
} as any;
790+
791+
const result = await handshakeService.resolveHandshake();
792+
793+
const location = result.headers.get(constants.Headers.Location);
794+
const url = new URL(location!);
795+
796+
expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false);
797+
expect(url.searchParams.get('q')).toBe('hello world');
798+
expect(url.searchParams.get('redirect')).toBe('/some/path?foo=bar');
799+
});
800+
801+
it('should preserve hash fragments when cleaning query params', async () => {
802+
mockAuthenticateContext.clerkUrl = new URL(
803+
'https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar#section-2',
804+
);
805+
mockAuthenticateContext.handshakeNonce = 'abc123';
806+
mockAuthenticateContext.apiClient = {
807+
clients: {
808+
getHandshakePayload: vi.fn().mockResolvedValue({
809+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
810+
}),
811+
},
812+
} as any;
813+
814+
const result = await handshakeService.resolveHandshake();
815+
816+
const location = result.headers.get(constants.Headers.Location);
817+
const url = new URL(location!);
818+
819+
expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false);
820+
expect(url.searchParams.get('foo')).toBe('bar');
821+
expect(url.hash).toBe('#section-2');
822+
});
823+
824+
it('should set Cache-Control header to no-store', async () => {
825+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123');
826+
mockAuthenticateContext.handshakeNonce = 'abc123';
827+
mockAuthenticateContext.apiClient = {
828+
clients: {
829+
getHandshakePayload: vi.fn().mockResolvedValue({
830+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
831+
}),
832+
},
833+
} as any;
834+
835+
const result = await handshakeService.resolveHandshake();
836+
837+
expect(result.headers.get(constants.Headers.CacheControl)).toBe('no-store');
838+
});
839+
});
840+
841+
describe('Production Mode', () => {
842+
beforeEach(() => {
843+
mockAuthenticateContext.instanceType = 'production';
844+
});
845+
846+
it('should NOT add Location header in production mode', async () => {
847+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar');
848+
mockAuthenticateContext.handshakeNonce = 'abc123';
849+
mockAuthenticateContext.apiClient = {
850+
clients: {
851+
getHandshakePayload: vi.fn().mockResolvedValue({
852+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
853+
}),
854+
},
855+
} as any;
856+
857+
const result = await handshakeService.resolveHandshake();
858+
859+
expect(result.headers.has(constants.Headers.Location)).toBe(false);
860+
expect(result.status).toBe('signed-in');
861+
});
862+
863+
it('should NOT set Cache-Control header in production mode', async () => {
864+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123');
865+
mockAuthenticateContext.handshakeNonce = 'abc123';
866+
mockAuthenticateContext.apiClient = {
867+
clients: {
868+
getHandshakePayload: vi.fn().mockResolvedValue({
869+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
870+
}),
871+
},
872+
} as any;
873+
874+
const result = await handshakeService.resolveHandshake();
875+
876+
expect(result.headers.has(constants.Headers.CacheControl)).toBe(false);
877+
});
878+
879+
it('should still set session cookies in production', async () => {
880+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123');
881+
mockAuthenticateContext.handshakeNonce = 'abc123';
882+
mockAuthenticateContext.apiClient = {
883+
clients: {
884+
getHandshakePayload: vi.fn().mockResolvedValue({
885+
directives: ['__session=eyJhbGc...; HttpOnly; Secure; SameSite=Lax'],
886+
}),
887+
},
888+
} as any;
889+
890+
const result = await handshakeService.resolveHandshake();
891+
892+
const setCookieHeaders = result.headers.getSetCookie?.() || [];
893+
expect(setCookieHeaders.length).toBeGreaterThan(0);
894+
expect(setCookieHeaders.some(h => h.startsWith('__session='))).toBe(true);
895+
});
896+
});
897+
898+
describe('Error Cases', () => {
899+
beforeEach(() => {
900+
mockAuthenticateContext.instanceType = 'development';
901+
});
902+
903+
it('should handle BAPI errors gracefully', async () => {
904+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123');
905+
mockAuthenticateContext.handshakeNonce = 'abc123';
906+
mockAuthenticateContext.apiClient = {
907+
clients: {
908+
getHandshakePayload: vi.fn().mockRejectedValue(new Error('BAPI error')),
909+
},
910+
} as any;
911+
912+
const result = await handshakeService.resolveHandshake();
913+
914+
expect(result.status).toBe('signed-out');
915+
});
916+
917+
it('should clean up query params even when handshake payload is empty', async () => {
918+
mockAuthenticateContext.clerkUrl = new URL('https://example.com/page?__clerk_handshake_nonce=abc123&foo=bar');
919+
mockAuthenticateContext.handshakeNonce = 'abc123';
920+
mockAuthenticateContext.apiClient = {
921+
clients: {
922+
getHandshakePayload: vi.fn().mockResolvedValue({
923+
directives: [],
924+
}),
925+
},
926+
} as any;
927+
928+
const result = await handshakeService.resolveHandshake();
929+
930+
const location = result.headers.get(constants.Headers.Location);
931+
expect(location).toBeTruthy();
932+
933+
const url = new URL(location!);
934+
expect(url.searchParams.has('__clerk_handshake_nonce')).toBe(false);
935+
expect(url.searchParams.get('foo')).toBe('bar');
936+
});
937+
});
938+
});
604939
});

packages/backend/src/tokens/handshake.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ export class HandshakeService {
221221
newUrl.searchParams.delete(constants.QueryParameters.Handshake);
222222
newUrl.searchParams.delete(constants.QueryParameters.HandshakeHelp);
223223
newUrl.searchParams.delete(constants.QueryParameters.DevBrowser);
224+
newUrl.searchParams.delete(constants.QueryParameters.HandshakeNonce);
224225
headers.append(constants.Headers.Location, newUrl.toString());
225226
headers.set(constants.Headers.CacheControl, 'no-store');
226227
}

0 commit comments

Comments
 (0)