From 3fee736c993b0a8fd157d716890810d04e632962 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 29 Jun 2023 11:41:12 +0300 Subject: [PATCH] fix(backend): Requests to satellite apps visited by non browsers should be treated as signed out This is necessary, because tools like `opengraph.xyz` will not follow redirects, and they will always end up with parsing our interstitial template. --- .changeset/proud-cycles-type.md | 5 ++ .../backend/src/tokens/interstitialRule.ts | 15 +++- packages/backend/src/tokens/request.test.ts | 75 +++++++++++++++---- 3 files changed, 79 insertions(+), 16 deletions(-) create mode 100644 .changeset/proud-cycles-type.md diff --git a/.changeset/proud-cycles-type.md b/.changeset/proud-cycles-type.md new file mode 100644 index 00000000000..6e737524546 --- /dev/null +++ b/.changeset/proud-cycles-type.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +Treat expired JWT as signed-out state for requests originated from non-browser clients on satellite apps diff --git a/packages/backend/src/tokens/interstitialRule.ts b/packages/backend/src/tokens/interstitialRule.ts index 2ab5f6a8b99..b1177005a1a 100644 --- a/packages/backend/src/tokens/interstitialRule.ts +++ b/packages/backend/src/tokens/interstitialRule.ts @@ -15,6 +15,9 @@ const hasJustSynced = (qp?: URLSearchParams) => qp?.get('__clerk_synced') === 't const isReturningFromPrimary = (qp?: URLSearchParams) => qp?.get('__clerk_referrer_primary') === 'true'; const VALID_USER_AGENTS = /^Mozilla\/|(Amazon CloudFront)/; + +const isBrowser = (userAgent: string | undefined) => VALID_USER_AGENTS.test(userAgent || ''); + // In development or staging environments only, based on the request's // User Agent, detect non-browser requests (e.g. scripts). Since there // is no Authorization header, consider the user as signed out and @@ -24,7 +27,7 @@ const VALID_USER_AGENTS = /^Mozilla\/|(Amazon CloudFront)/; export const nonBrowserRequestInDevRule: InterstitialRule = options => { const { apiKey, secretKey, userAgent } = options; const key = secretKey || apiKey; - if (isDevelopmentFromApiKey(key) && !VALID_USER_AGENTS.test(userAgent || '')) { + if (isDevelopmentFromApiKey(key) && !isBrowser(userAgent)) { return signedOut(options, AuthErrorReason.HeaderMissingNonBrowser); } return undefined; @@ -184,12 +187,18 @@ async function verifyRequestState(options: AuthenticateRequestOptions, token: st * Let the next rule for UatMissing to fire if needed */ export const isSatelliteAndNeedsSyncing: InterstitialRule = options => { - const { clientUat, isSatellite, searchParams, secretKey, apiKey } = options; + const { clientUat, isSatellite, searchParams, secretKey, apiKey, userAgent } = options; const key = secretKey || apiKey; const isDev = isDevelopmentFromApiKey(key); - if (isSatellite && (!clientUat || clientUat === '0') && !hasJustSynced(searchParams) && !isDev) { + const isSignedOut = !clientUat || clientUat === '0'; + + if (isSatellite && isSignedOut && !isBrowser(userAgent)) { + return signedOut(options, AuthErrorReason.SatelliteCookieNeedsSyncing); + } + + if (isSatellite && isSignedOut && !hasJustSynced(searchParams) && !isDev) { return interstitial(options, AuthErrorReason.SatelliteCookieNeedsSyncing); } diff --git a/packages/backend/src/tokens/request.test.ts b/packages/backend/src/tokens/request.test.ts index 66cfe4fe5b6..78fb1e3c7b0 100644 --- a/packages/backend/src/tokens/request.test.ts +++ b/packages/backend/src/tokens/request.test.ts @@ -3,13 +3,23 @@ import sinon from 'sinon'; import runtime from '../runtime'; import { jsonOk } from '../util/mockFetch'; -import { type AuthReason, type RequestState, AuthErrorReason, AuthStatus } from './authStatus'; +import { AuthErrorReason, type AuthReason, AuthStatus, type RequestState } from './authStatus'; import { TokenVerificationErrorReason } from './errors'; import { mockInvalidSignatureJwt, mockJwks, mockJwt, mockJwtPayload, mockMalformedJwt } from './fixtures'; import type { AuthenticateRequestOptions } from './request'; import { authenticateRequest } from './request'; -function assertSignedOut(assert, requestState: RequestState, reason: AuthReason, message = '') { +function assertSignedOut( + assert, + requestState: RequestState, + expectedState: { + reason: AuthReason; + isSatellite?: boolean; + domain?: string; + signInUrl?: string; + message?: string; + }, +) { assert.propEqual(requestState, { frontendApi: 'cafe.babe.clerk.ts', publishableKey: '', @@ -21,9 +31,9 @@ function assertSignedOut(assert, requestState: RequestState, reason: AuthReason, isSatellite: false, signInUrl: '', domain: '', - message, - reason, + message: '', toAuth: {}, + ...expectedState, }); } @@ -174,7 +184,10 @@ export default (QUnit: QUnit) => { const errMessage = 'The JWKS endpoint did not contain any signing keys. Contact support@clerk.com. Contact support@clerk.com (reason=jwk-remote-failed-to-load, token-carrier=header)'; - assertSignedOut(assert, requestState, TokenVerificationErrorReason.RemoteJWKFailedToLoad, errMessage); + assertSignedOut(assert, requestState, { + reason: TokenVerificationErrorReason.RemoteJWKFailedToLoad, + message: errMessage, + }); assertSignedOutToAuth(assert, requestState); }); @@ -204,7 +217,10 @@ export default (QUnit: QUnit) => { const errMessage = 'Invalid JWT Authorized party claim (azp) "https://accounts.inspired.puma-74.lcl.dev". Expected "whatever". (reason=token-invalid-authorized-parties, token-carrier=header)'; - assertSignedOut(assert, requestState, TokenVerificationErrorReason.TokenInvalidAuthorizedParties, errMessage); + assertSignedOut(assert, requestState, { + reason: TokenVerificationErrorReason.TokenInvalidAuthorizedParties, + message: errMessage, + }); assertSignedOutToAuth(assert, requestState); }); @@ -228,7 +244,10 @@ export default (QUnit: QUnit) => { }); const errMessage = 'JWT signature is invalid. (reason=token-invalid-signature, token-carrier=header)'; - assertSignedOut(assert, requestState, TokenVerificationErrorReason.TokenInvalidSignature, errMessage); + assertSignedOut(assert, requestState, { + reason: TokenVerificationErrorReason.TokenInvalidSignature, + message: errMessage, + }); assertSignedOutToAuth(assert, requestState); }); @@ -240,7 +259,10 @@ export default (QUnit: QUnit) => { const errMessage = 'Invalid JWT form. A JWT consists of three parts separated by dots. (reason=token-invalid, token-carrier=header)'; - assertSignedOut(assert, requestState, TokenVerificationErrorReason.TokenInvalid, errMessage); + assertSignedOut(assert, requestState, { + reason: TokenVerificationErrorReason.TokenInvalid, + message: errMessage, + }); assertSignedOutToAuth(assert, requestState); }); @@ -256,7 +278,9 @@ export default (QUnit: QUnit) => { cookieToken: mockJwt, }); - assertSignedOut(assert, requestState, AuthErrorReason.HeaderMissingCORS); + assertSignedOut(assert, requestState, { + reason: AuthErrorReason.HeaderMissingCORS, + }); assertSignedOutToAuth(assert, requestState); }); @@ -270,7 +294,7 @@ export default (QUnit: QUnit) => { cookieToken: mockJwt, }); - assertSignedOut(assert, requestState, AuthErrorReason.HeaderMissingNonBrowser); + assertSignedOut(assert, requestState, { reason: AuthErrorReason.HeaderMissingNonBrowser }); assertSignedOutToAuth(assert, requestState); }); @@ -292,6 +316,24 @@ export default (QUnit: QUnit) => { assert.strictEqual(requestState.toAuth(), null); }); + test('cookieToken: returns signed out is satellite but a non-browser request [11y]', async assert => { + const requestState = await authenticateRequest({ + ...defaultMockAuthenticateRequestOptions, + apiKey: 'deadbeef', + clientUat: '0', + isSatellite: true, + domain: 'satellite.dev', + userAgent: '[some-agent]', + }); + + assertSignedOut(assert, requestState, { + reason: AuthErrorReason.SatelliteCookieNeedsSyncing, + isSatellite: true, + domain: 'satellite.dev', + }); + assertSignedOutToAuth(assert, requestState); + }); + test('returns interstitial when app is satellite, returns from primary and is dev instance [13y]', async assert => { const sp = new URLSearchParams(); sp.set('__clerk_referrer_primary', 'true'); @@ -339,7 +381,9 @@ export default (QUnit: QUnit) => { apiKey: 'live_deadbeef', }); - assertSignedOut(assert, requestState, AuthErrorReason.CookieAndUATMissing); + assertSignedOut(assert, requestState, { + reason: AuthErrorReason.CookieAndUATMissing, + }); assertSignedOutToAuth(assert, requestState); }); @@ -430,7 +474,9 @@ export default (QUnit: QUnit) => { clientUat: '0', }); - assertSignedOut(assert, requestState, AuthErrorReason.StandardSignedOut); + assertSignedOut(assert, requestState, { + reason: AuthErrorReason.StandardSignedOut, + }); assertSignedOutToAuth(assert, requestState); }); @@ -455,7 +501,10 @@ export default (QUnit: QUnit) => { const errMessage = 'Subject claim (sub) is required and must be a string. Received undefined. Make sure that this is a valid Clerk generate JWT. (reason=token-verification-failed, token-carrier=cookie)'; - assertSignedOut(assert, requestState, TokenVerificationErrorReason.TokenVerificationFailed, errMessage); + assertSignedOut(assert, requestState, { + reason: TokenVerificationErrorReason.TokenVerificationFailed, + message: errMessage, + }); assertSignedOutToAuth(assert, requestState); });