Skip to content

Commit 6b79914

Browse files
dimklnikosdouvlis
authored andcommitted
feat(backend): Introduce a handshake redirect loop detection mechanism
1 parent b623d00 commit 6b79914

File tree

6 files changed

+46
-11
lines changed

6 files changed

+46
-11
lines changed

.changeset/sixty-mugs-clap.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+
Retry handshake in case of handshake cookie collision in order to support multiple apps on same-level subdomains

integration/tests/sessions/root-subdomain-prod-instances.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ test.describe('root and subdomain production apps @sessions', () => {
311311
* sub2.test.com <> clerk-instance-2
312312
*
313313
*/
314-
test.describe('multiple apps same domain for different production instances', () => {
314+
test.describe('multiple apps different same-level subdomains for different production instances', () => {
315315
const hosts = ['sub-1.multiple-apps-e2e.clerk.app', 'sub-2.multiple-apps-e2e.clerk.app'];
316316
let fakeUsers: FakeUser[];
317317
let server: Server;

packages/backend/src/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const Cookies = {
1919
ClientUat: '__client_uat',
2020
Handshake: '__clerk_handshake',
2121
DevBrowser: '__clerk_db_jwt',
22+
InfiniteRedirectionLoopCookie: '__clerk_redirection_loop',
2223
} as const;
2324

2425
const QueryParameters = {

packages/backend/src/tokens/authenticateContext.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ interface AuthenticateContextInterface extends AuthenticateRequestOptions {
2626
// handshake-related values
2727
devBrowserToken: string | undefined;
2828
handshakeToken: string | undefined;
29+
handshakeRedirectLoopCounter: number;
2930
// url derived from headers
3031
clerkUrl: URL;
3132
// cookie or header session token
@@ -106,6 +107,7 @@ class AuthenticateContext {
106107
// Using getCookie since we don't suffix the handshake token cookie
107108
this.handshakeToken =
108109
this.getQueryParam(constants.QueryParameters.Handshake) || this.getCookie(constants.Cookies.Handshake);
110+
this.handshakeRedirectLoopCounter = Number(this.getCookie(constants.Cookies.InfiniteRedirectionLoopCookie)) || 0;
109111
}
110112

111113
private stripAuthorizationHeader(authValue: string | undefined | null): string | undefined {

packages/backend/src/tokens/request.ts

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,17 @@ ${error.getFullMessage()}`,
173173
if (isRequestEligibleForHandshake(authenticateContext)) {
174174
// Right now the only usage of passing in different headers is for multi-domain sync, which redirects somewhere else.
175175
// In the future if we want to decorate the handshake redirect with additional headers per call we need to tweak this logic.
176-
return handshake(authenticateContext, reason, message, headers ?? buildRedirectToHandshake());
176+
const handshakeHeaders = headers ?? buildRedirectToHandshake();
177+
// Introduce the mechanism to protect for infinite handshake redirect loops
178+
// using a cookie and returning true if it's infinite redirect loop or false if we can
179+
// proceed with triggering handshake.
180+
const isRedirectLoop = setHandshakeInfiniteRedirectionLoopHeaders(handshakeHeaders);
181+
if (isRedirectLoop) {
182+
return signedOut(authenticateContext, reason, message);
183+
}
184+
return handshake(authenticateContext, reason, message, handshakeHeaders);
177185
}
178-
return signedOut(authenticateContext, reason, message, new Headers());
186+
return signedOut(authenticateContext, reason, message);
179187
}
180188

181189
async function authenticateRequestWithTokenInHeader() {
@@ -193,6 +201,20 @@ ${error.getFullMessage()}`,
193201
}
194202
}
195203

204+
// We want to prevent infinite handshake redirection loops.
205+
// We incrementally set a `__clerk_redirection_loop` cookie, and when it loops 3 times, we throw an error.
206+
// We also utilize the `referer` header to skip the prefetch requests.
207+
function setHandshakeInfiniteRedirectionLoopHeaders(headers: Headers): boolean {
208+
if (authenticateContext.handshakeRedirectLoopCounter === 3) {
209+
return true;
210+
}
211+
212+
const newCounterValue = authenticateContext.handshakeRedirectLoopCounter + 1;
213+
const cookieName = constants.Cookies.InfiniteRedirectionLoopCookie;
214+
headers.append('Set-Cookie', `${cookieName}=${newCounterValue}; SameSite=Lax; HttpOnly; Max-Age=3`);
215+
return false;
216+
}
217+
196218
function handleHandshakeTokenVerificationErrorInDevelopment(error: TokenVerificationError) {
197219
// In development, the handshake token is being transferred in the URL as a query parameter, so there is no
198220
// possibility of collision with a handshake token of another app running on the same local domain
@@ -223,12 +245,11 @@ ${error.getFullMessage()}`,
223245
error.reason === TokenVerificationErrorReason.JWKKidMismatch ||
224246
error.reason === TokenVerificationErrorReason.JWKFailedToResolve
225247
) {
226-
// Let the request go through and eventually retry another handshake
227-
// TODO: set a cookie so break the infinite loop
248+
// Let the request go through and eventually retry another handshake,
249+
// only if needed - a handshake will be thrown if another rule matches
228250
return;
229251
}
230-
// TODO: if N retries reached, return signedOut
231-
const msg = `Clerk: Handshake token verification failed with "${error.reason}"`;
252+
const msg = `Clerk: Handshake token verification failed with "${error.getFullMessage()}"`;
232253
return signedOut(authenticateContext, AuthErrorReason.UnexpectedError, msg);
233254
}
234255

@@ -249,10 +270,15 @@ ${error.getFullMessage()}`,
249270
try {
250271
return await resolveHandshake();
251272
} catch (error) {
252-
if (error instanceof TokenVerificationError) {
253-
authenticateContext.instanceType === 'development'
254-
? handleHandshakeTokenVerificationErrorInDevelopment(error)
255-
: handleHandshakeTokenVerificationErrorInProduction(error);
273+
if (error instanceof TokenVerificationError && authenticateContext.instanceType === 'development') {
274+
handleHandshakeTokenVerificationErrorInDevelopment(error);
275+
}
276+
277+
if (error instanceof TokenVerificationError && authenticateContext.instanceType === 'production') {
278+
const terminateEarly = handleHandshakeTokenVerificationErrorInProduction(error);
279+
if (terminateEarly) {
280+
return terminateEarly;
281+
}
256282
}
257283
}
258284
}

packages/fastify/src/__tests__/__snapshots__/constants.test.ts.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ exports[`constants from environment variables 1`] = `
88
"ClientUat": "__client_uat",
99
"DevBrowser": "__clerk_db_jwt",
1010
"Handshake": "__clerk_handshake",
11+
"InfiniteRedirectionLoopCookie": "__clerk_redirection_loop",
1112
"Session": "__session",
1213
},
1314
"Headers": {

0 commit comments

Comments
 (0)