fix(backend): Requests to satellite apps visited by non browsers shou…

…ld 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.
clerk · Jun 29, 2023 · 3fee736 · 3fee736
1 parent 566f7af
commit 3fee736
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 16 deletions.
diff --git a/.changeset/proud-cycles-type.md 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
@@ -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
@@ -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);
     });