diff --git a/.changeset/itchy-papayas-dress.md b/.changeset/itchy-papayas-dress.md new file mode 100644 index 0000000000..8f2c037f51 --- /dev/null +++ b/.changeset/itchy-papayas-dress.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +Update the handshake flow to only trigger for document requests. diff --git a/integration/tests/handshake.test.ts b/integration/tests/handshake.test.ts index e4629ca010..23d1926dd8 100644 --- a/integration/tests/handshake.test.ts +++ b/integration/tests/handshake.test.ts @@ -83,6 +83,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -101,6 +102,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, Authorization: `Bearer ${token}`, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -118,6 +120,7 @@ test.describe('Client handshake @generic', () => { Cookie: `__client_uat=${clientUat}; __session=${token}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -153,6 +156,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -175,6 +179,7 @@ test.describe('Client handshake @generic', () => { Cookie: `__client_uat=${clientUat}; __session=${token}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -196,6 +201,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, Authorization: `Bearer ${token}`, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -216,6 +222,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -239,6 +246,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, Authorization: `Bearer ${token}`, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -262,6 +270,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, 'X-Proxy-Url': 'https://example.com/clerk', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -285,6 +294,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, 'X-Proxy-Url': 'https://example.com/clerk', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -306,6 +316,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, 'X-Domain': 'localhost:3000', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -329,6 +340,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, 'X-Domain': 'example.com', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -347,6 +359,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie} __client_uat=1`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -367,6 +380,7 @@ test.describe('Client handshake @generic', () => { Cookie: `__client_uat=1`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -385,6 +399,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie} __client_uat=0`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -400,6 +415,7 @@ test.describe('Client handshake @generic', () => { Cookie: `__client_uat=0`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -415,6 +431,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -429,6 +446,7 @@ test.describe('Client handshake @generic', () => { headers: new Headers({ 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -496,6 +514,7 @@ test.describe('Client handshake @generic', () => { headers: new Headers({ 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -513,6 +532,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -535,6 +555,7 @@ test.describe('Client handshake @generic', () => { Cookie: `__client_uat=${clientUat}; __session=${token}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -559,6 +580,7 @@ test.describe('Client handshake @generic', () => { 'X-Secret-Key': config.sk, 'X-Forwarded-Host': 'example.com', 'X-Forwarded-Proto': 'https', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -581,6 +603,7 @@ test.describe('Client handshake @generic', () => { 'X-Secret-Key': config.sk, 'X-Forwarded-Host': 'example.com', 'X-Forwarded-Proto': 'https', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -603,6 +626,7 @@ test.describe('Client handshake @generic', () => { 'X-Secret-Key': config.sk, 'X-Forwarded-Host': 'example.com:3213', 'X-Forwarded-Proto': 'https', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -625,6 +649,7 @@ test.describe('Client handshake @generic', () => { 'X-Secret-Key': config.sk, 'X-Forwarded-Host': 'example.com:3213', 'X-Forwarded-Proto': 'https', + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -646,6 +671,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -669,6 +695,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -688,6 +715,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -707,6 +735,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -726,6 +755,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -746,6 +776,7 @@ test.describe('Client handshake @generic', () => { Cookie: `${devBrowserCookie}`, 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -769,6 +800,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, Cookie: `__clerk_handshake=${handshake}`, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -791,6 +823,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, Cookie: `__clerk_handshake=${handshake}`, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -809,6 +842,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, Cookie: `__clerk_handshake=${handshake}`, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); @@ -828,6 +862,7 @@ test.describe('Client handshake @generic', () => { 'X-Publishable-Key': config.pk, 'X-Secret-Key': config.sk, Cookie: `__clerk_handshake=${handshake}`, + 'Sec-Fetch-Dest': 'document', }), redirect: 'manual', }); diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index 9361864da5..47b9d7506a 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -35,6 +35,7 @@ const Headers = { ForwardedPort: 'x-forwarded-port', ForwardedProto: 'x-forwarded-proto', ForwardedHost: 'x-forwarded-host', + Accept: 'accept', Referrer: 'referer', UserAgent: 'user-agent', Origin: 'origin', diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index a2a03e9ce8..68e82ec75d 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -123,6 +123,7 @@ export default (QUnit: QUnit) => { const defaultHeaders: Record = { host: 'example.com', 'user-agent': 'Mozilla/TestAgent', + 'sec-fetch-dest': 'document', }; const mockRequest = (headers = {}, requestUrl = 'http://clerk.com/path') => { @@ -268,9 +269,7 @@ export default (QUnit: QUnit) => { test('cookieToken: returns handshake when clientUat is missing or equals to 0 and is satellite and not is synced [11y]', async assert => { const requestState = await authenticateRequest( mockRequestWithCookies( - { - 'Sec-Fetch-Dest': 'document', - }, + {}, { __client_uat: '0', }, @@ -299,6 +298,7 @@ export default (QUnit: QUnit) => { mockRequestWithCookies( { ...defaultHeaders, + 'sec-fetch-dest': 'empty', 'user-agent': '[some-agent]', }, { __client_uat: '0' }, @@ -388,12 +388,13 @@ export default (QUnit: QUnit) => { assert.strictEqual(requestState.toAuth(), null); }); - test('cookieToken: returns undefined when crossOriginReferrer in development and is satellite [6n]', async assert => { + test('cookieToken: returns signedIn when satellite but valid token and clientUat', async assert => { // Scenario: after auth action on Clerk-hosted UIs const requestState = await authenticateRequest( mockRequestWithCookies( { ...defaultHeaders, + 'sec-fetch-dest': 'empty', // this is not a typo, it's intentional to be `referer` to match HTTP header key referer: 'https://clerk.com', }, diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index a659698613..cdea3bd71b 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -53,6 +53,25 @@ function assertSignInUrlFormatAndOrigin(_signInUrl: string, origin: string) { } } +/** + * Currently, a request is only eligible for a handshake if we can say it's *probably* a request for a document, not a fetch or some other exotic request. + * This heuristic should give us a reliable enough signal for browsers that support `Sec-Fetch-Dest` and for those that don't. + */ +function isRequestEligibleForHandshake(authenticateContext: { secFetchDest?: string; accept?: string }) { + const { accept, secFetchDest } = authenticateContext; + + // NOTE: we could also check sec-fetch-mode === navigate here, but according to the spec, sec-fetch-dest: document should indicate that the request is the result of a user navigation. + if (secFetchDest === 'document') { + return true; + } + + if (!secFetchDest && accept?.startsWith('text/html')) { + return true; + } + + return false; +} + export async function authenticateRequest( request: Request, options: AuthenticateRequestOptions, @@ -159,6 +178,20 @@ ${err.getFullMessage()}`, return signedIn(authenticateContext, verifyResult!, headers); } + function handleMaybeHandshakeStatus( + context: typeof authenticateContext, + reason: AuthErrorReason, + message: string, + headers?: Headers, + ) { + if (isRequestEligibleForHandshake(context)) { + // Right now the only usage of passing in different headers is for multi-domain sync, which redirects somewhere else. + // In the future if we want to decorate the handshake redirect with additional headers per call we need to tweak this logic. + return handshake(context, reason, message, headers ?? buildRedirectToHandshake()); + } + return signedOut(context, reason, message, new Headers()); + } + const pk = parsePublishableKey(options.publishableKey, { fatal: true, proxyUrl: options.proxyUrl, @@ -208,16 +241,14 @@ ${err.getFullMessage()}`, * Otherwise, check for "known unknown" auth states that we can resolve with a handshake. */ if (instanceType === 'development' && derivedRequestUrl.searchParams.has(constants.Cookies.DevBrowser)) { - const headers = buildRedirectToHandshake(); - return handshake(authenticateContext, AuthErrorReason.DevBrowserSync, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.DevBrowserSync, ''); } /** * Begin multi-domain sync flows */ if (instanceType === 'production' && isRequestEligibleForMultiDomainSync) { - const headers = buildRedirectToHandshake(); - return handshake(authenticateContext, AuthErrorReason.SatelliteCookieNeedsSyncing, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.SatelliteCookieNeedsSyncing, ''); } // Multi-domain development sync flow @@ -229,7 +260,7 @@ ${err.getFullMessage()}`, redirectURL.searchParams.append(constants.QueryParameters.ClerkRedirectUrl, derivedRequestUrl.toString()); const headers = new Headers({ location: redirectURL.toString() }); - return handshake(authenticateContext, AuthErrorReason.SatelliteCookieNeedsSyncing, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.SatelliteCookieNeedsSyncing, '', headers); } // Multi-domain development sync flow @@ -244,32 +275,29 @@ ${err.getFullMessage()}`, redirectBackToSatelliteUrl.searchParams.append(constants.QueryParameters.ClerkSynced, 'true'); const headers = new Headers({ location: redirectBackToSatelliteUrl.toString() }); - return handshake(authenticateContext, AuthErrorReason.PrimaryRespondsToSyncing, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.PrimaryRespondsToSyncing, '', headers); } /** * End multi-domain sync flows */ if (!hasActiveClient && !hasSessionToken) { - return signedOut(authenticateContext, AuthErrorReason.SessionTokenAndUATMissing); + return signedOut(authenticateContext, AuthErrorReason.SessionTokenAndUATMissing, ''); } // This can eagerly run handshake since client_uat is SameSite=Strict in dev if (!hasActiveClient && hasSessionToken) { - const headers = buildRedirectToHandshake(); - return handshake(authenticateContext, AuthErrorReason.SessionTokenWithoutClientUAT, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.SessionTokenWithoutClientUAT, ''); } if (hasActiveClient && !hasSessionToken) { - const headers = buildRedirectToHandshake(); - return handshake(authenticateContext, AuthErrorReason.ClientUATWithoutSessionToken, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.ClientUATWithoutSessionToken, ''); } const decodeResult = decodeJwt(sessionToken!); if (decodeResult.payload.iat < clientUat) { - const headers = buildRedirectToHandshake(); - return handshake(authenticateContext, AuthErrorReason.SessionTokenOutdated, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.SessionTokenOutdated, ''); } try { @@ -294,8 +322,11 @@ ${err.getFullMessage()}`, ].includes(err.reason); if (reasonToHandshake) { - const headers = buildRedirectToHandshake(); - return handshake(authenticateContext, AuthErrorReason.SessionTokenOutdated, err.getFullMessage(), headers); + return handleMaybeHandshakeStatus( + authenticateContext, + AuthErrorReason.SessionTokenOutdated, + err.getFullMessage(), + ); } return signedOut(authenticateContext, err.reason, err.getFullMessage()); } @@ -336,6 +367,7 @@ export const loadOptionsFromHeaders = (headers: ReturnType[ referrer: headers(constants.Headers.Referrer), userAgent: headers(constants.Headers.UserAgent), secFetchDest: headers(constants.Headers.SecFetchDest), + accept: headers(constants.Headers.Accept), }; }; diff --git a/packages/fastify/src/__snapshots__/constants.test.ts.snap b/packages/fastify/src/__snapshots__/constants.test.ts.snap index 7e3776bd19..99998a7f10 100644 --- a/packages/fastify/src/__snapshots__/constants.test.ts.snap +++ b/packages/fastify/src/__snapshots__/constants.test.ts.snap @@ -11,6 +11,7 @@ exports[`constants from environment variables 1`] = ` "Session": "__session", }, "Headers": { + "Accept": "accept", "AuthMessage": "x-clerk-auth-message", "AuthReason": "x-clerk-auth-reason", "AuthStatus": "x-clerk-auth-status",