Skip to content

Commit 8a4aeec

Browse files
committed
feat(backend): For production apps, let all errors end up in handshake
If a handshake loop occurs, it's going to be stopped by the infinite loop prevention mechanism and the request will terminate with a signed out state
1 parent 6b79914 commit 8a4aeec

File tree

4 files changed

+17
-36
lines changed

4 files changed

+17
-36
lines changed

packages/backend/src/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const Cookies = {
1919
ClientUat: '__client_uat',
2020
Handshake: '__clerk_handshake',
2121
DevBrowser: '__clerk_db_jwt',
22-
InfiniteRedirectionLoopCookie: '__clerk_redirection_loop',
22+
RedirectCount: '__clerk_redirect_count',
2323
} as const;
2424

2525
const QueryParameters = {

packages/backend/src/tokens/authenticateContext.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class AuthenticateContext {
107107
// Using getCookie since we don't suffix the handshake token cookie
108108
this.handshakeToken =
109109
this.getQueryParam(constants.QueryParameters.Handshake) || this.getCookie(constants.Cookies.Handshake);
110-
this.handshakeRedirectLoopCounter = Number(this.getCookie(constants.Cookies.InfiniteRedirectionLoopCookie)) || 0;
110+
this.handshakeRedirectLoopCounter = Number(this.getCookie(constants.Cookies.RedirectCount)) || 0;
111111
}
112112

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

packages/backend/src/tokens/request.ts

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ ${error.getFullMessage()}`,
179179
// proceed with triggering handshake.
180180
const isRedirectLoop = setHandshakeInfiniteRedirectionLoopHeaders(handshakeHeaders);
181181
if (isRedirectLoop) {
182+
const msg = `Clerk: Refreshing the session token resulted in an infinite redirect loop. This usually means that your Clerk instance keys do not match - make sure to copy the correct publishable and secret keys from the Clerk dashboard.`;
183+
console.log(msg);
182184
return signedOut(authenticateContext, reason, message);
183185
}
184186
return handshake(authenticateContext, reason, message, handshakeHeaders);
@@ -210,7 +212,7 @@ ${error.getFullMessage()}`,
210212
}
211213

212214
const newCounterValue = authenticateContext.handshakeRedirectLoopCounter + 1;
213-
const cookieName = constants.Cookies.InfiniteRedirectionLoopCookie;
215+
const cookieName = constants.Cookies.RedirectCount;
214216
headers.append('Set-Cookie', `${cookieName}=${newCounterValue}; SameSite=Lax; HttpOnly; Max-Age=3`);
215217
return false;
216218
}
@@ -229,30 +231,6 @@ ${error.getFullMessage()}`,
229231
throw new Error(`Clerk: Handshake token verification failed: ${error.getFullMessage()}.`);
230232
}
231233

232-
function handleHandshakeTokenVerificationErrorInProduction(error: TokenVerificationError) {
233-
// In production, the handshake token is being transferred as a cookie, so there is a possibility of collision
234-
// with a handshake token of another app running on the same etld+1 domain.
235-
// For example, if one app is running on sub1.clerk.com and another on sub2.clerk.com, the handshake token
236-
// cookie for both apps will be set on etld+1 (clerk.com) so there's a possibility that one app will accidentally
237-
// use the handshake token of a different app during the handshake flow.
238-
// In this scenario, verification will fail with TokenInvalidSignature. In contrast to the development case,
239-
// we need to allow the flow to continue so the app eventually retries another handshake with the correct token.
240-
// We need to make sure, however, that we don't allow the flow to continue indefinitely, so we throw an error after X
241-
// retries to avoid an infinite loop. An infinite loop can happen if the customer switched Clerk keys for their prod app.
242-
if (
243-
error.reason === TokenVerificationErrorReason.TokenInvalidSignature ||
244-
error.reason === TokenVerificationErrorReason.InvalidSecretKey ||
245-
error.reason === TokenVerificationErrorReason.JWKKidMismatch ||
246-
error.reason === TokenVerificationErrorReason.JWKFailedToResolve
247-
) {
248-
// Let the request go through and eventually retry another handshake,
249-
// only if needed - a handshake will be thrown if another rule matches
250-
return;
251-
}
252-
const msg = `Clerk: Handshake token verification failed with "${error.getFullMessage()}"`;
253-
return signedOut(authenticateContext, AuthErrorReason.UnexpectedError, msg);
254-
}
255-
256234
async function authenticateRequestWithTokenInCookie() {
257235
const hasActiveClient = authenticateContext.clientUat;
258236
const hasSessionToken = !!authenticateContext.sessionTokenInCookie;
@@ -270,19 +248,22 @@ ${error.getFullMessage()}`,
270248
try {
271249
return await resolveHandshake();
272250
} catch (error) {
251+
// In production, the handshake token is being transferred as a cookie, so there is a possibility of collision
252+
// with a handshake token of another app running on the same etld+1 domain.
253+
// For example, if one app is running on sub1.clerk.com and another on sub2.clerk.com, the handshake token
254+
// cookie for both apps will be set on etld+1 (clerk.com) so there's a possibility that one app will accidentally
255+
// use the handshake token of a different app during the handshake flow.
256+
// In this scenario, verification will fail with TokenInvalidSignature. In contrast to the development case,
257+
// we need to allow the flow to continue so the app eventually retries another handshake with the correct token.
258+
// We need to make sure, however, that we don't allow the flow to continue indefinitely, so we throw an error after X
259+
// retries to avoid an infinite loop. An infinite loop can happen if the customer switched Clerk keys for their prod app.
260+
261+
// Check the handleHandshakeTokenVerificationErrorInDevelopment function for the development case.
273262
if (error instanceof TokenVerificationError && authenticateContext.instanceType === 'development') {
274263
handleHandshakeTokenVerificationErrorInDevelopment(error);
275264
}
276-
277-
if (error instanceof TokenVerificationError && authenticateContext.instanceType === 'production') {
278-
const terminateEarly = handleHandshakeTokenVerificationErrorInProduction(error);
279-
if (terminateEarly) {
280-
return terminateEarly;
281-
}
282-
}
283265
}
284266
}
285-
286267
/**
287268
* Otherwise, check for "known unknown" auth states that we can resolve with a handshake.
288269
*/

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +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",
11+
"RedirectCount": "__clerk_redirect_count",
1212
"Session": "__session",
1313
},
1414
"Headers": {

0 commit comments

Comments
 (0)