Skip to content

Commit acc9802

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

File tree

4 files changed

+44
-10
lines changed

4 files changed

+44
-10
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

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
}

0 commit comments

Comments
 (0)