chore(repo): Retry handshake in case of handshake cookie collision

integration/tests/sessions/root-subdomain-prod-instances.test.ts

@@ -302,4 +302,77 @@ test.describe('root and subdomain production apps @sessions', () => {
       await u[1].po.expect.toBeSignedIn();
     });
   });
+
+  /**
+   * This smoke test verifies that the session is not shared between different apps running on different same-level subdomains, while
+   * using different Clerk instances. For extra details, look at the previous test.
+   *
+   * sub1.test.com <> clerk-instance-1
+   * sub2.test.com <> clerk-instance-2
+   *
+   */
+  test.describe('multiple apps same domain for different production instances', () => {
+    const hosts = ['sub-1.multiple-apps-e2e.clerk.app', 'sub-2.multiple-apps-e2e.clerk.app'];
+    let fakeUsers: FakeUser[];
+    let server: Server;
+    let apps: Array<{ app: Application; serverUrl: string }>;
+
+    test.beforeAll(async () => {
+      apps = await Promise.all([prepareApplication('sessions-prod-1'), prepareApplication('sessions-prod-2')]);
+
+      // TODO: Move this into createProxyServer
+      const ssl: Pick<ServerOptions, 'ca' | 'cert' | 'key'> = {
+        cert: fs.readFileSync(constants.CERTS_DIR + '/sessions.pem'),
+        key: fs.readFileSync(constants.CERTS_DIR + '/sessions-key.pem'),
+      };
+
+      server = createProxyServer({
+        ssl,
+        targets: {
+          [hosts[0]]: apps[0].serverUrl,
+          [hosts[1]]: apps[1].serverUrl,
+        },
+      });
+
+      const u = apps.map(a => createTestUtils({ app: a.app }));
+      fakeUsers = await Promise.all(u.map(u => u.services.users.createFakeUser()));
+      await Promise.all([
+        await u[0].services.users.createBapiUser(fakeUsers[0]),
+        await u[1].services.users.createBapiUser(fakeUsers[1]),
+      ]);
+    });
+
+    test.afterAll(async () => {
+      await Promise.all(fakeUsers.map(u => u.deleteIfExists()));
+      await Promise.all(apps.map(({ app }) => app.teardown()));
+      server.close();
+    });
+
+    test('the cookies are independent for the root and sub domains', async ({ context }) => {
+      const pages = await Promise.all([context.newPage(), context.newPage()]);
+      const u = [
+        createTestUtils({ app: apps[0].app, page: pages[0], context }),
+        createTestUtils({ app: apps[1].app, page: pages[1], context }),
+      ];
+
+      await u[0].page.goto(`https://${hosts[0]}`);
+      await u[0].po.signIn.goTo();
+      await u[0].po.signIn.signInWithEmailAndInstantPassword(fakeUsers[0]);
+      await u[0].po.expect.toBeSignedIn();
+      const tab0User = await u[0].po.clerk.getClientSideUser();
+      // make sure that the backend user now matches the user we signed in with on the client
+      expect((await u[0].page.evaluate(() => fetch('/api/me').then(r => r.json()))).userId).toBe(tab0User.id);
+
+      u[1].po.expect.toBeHandshake(await u[1].page.goto(`https://${hosts[1]}`));
+      await u[1].po.expect.toBeSignedOut();
+      expect((await u[1].page.evaluate(() => fetch('/api/me').then(r => r.json()))).userId).toBe(null);
+
+      await u[1].po.signIn.goTo();
+      await u[1].po.signIn.signInWithEmailAndInstantPassword(fakeUsers[1]);
+      await u[1].po.expect.toBeSignedIn();
+      const tab1User = await u[1].po.clerk.getClientSideUser();
+      // make sure that the backend user now matches the user we signed in with on the client
+      expect((await u[1].page.evaluate(() => fetch('/api/me').then(r => r.json()))).userId).toBe(tab1User.id);
+    });
+  });
 });

packages/backend/src/tokens/handshake.ts

@@ -38,7 +38,8 @@ async function verifyHandshakeJwt(token: string, { key }: VerifyJwtOptions): Pro
 }
 
 /**
- * Similar to our verifyToken flow for Clerk-issued JWTs, but this verification flow is for our signed handshake payload. The handshake payload requires fewer verification steps.
+ * Similar to our verifyToken flow for Clerk-issued JWTs, but this verification flow is for our signed handshake payload.
+ * The handshake payload requires fewer verification steps.
  */
 export async function verifyHandshakeToken(
   token: string,

packages/backend/src/tokens/request.ts

@@ -193,6 +193,45 @@ ${error.getFullMessage()}`,
     }
   }
 
+  function handleHandshakeTokenVerificationErrorInDevelopment(error: TokenVerificationError) {
+    // In development, the handshake token is being transferred in the URL as a query parameter, so there is no
+    // possibility of collision with a handshake token of another app running on the same local domain
+    // (etc one app on localhost:3000 and one on localhost:3001).
+    // Therefore, if the handshake token is invalid, it is likely that the user has switched Clerk keys locally.
+    // We make sure to throw a descriptive error message and then stop the handshake flow in every case,
+    // to avoid the possibility of an infinite loop.
+    if (error.reason === TokenVerificationErrorReason.TokenInvalidSignature) {
+      const msg = `Clerk: Handshake token verification failed due to an invalid signature. If you have switched Clerk keys locally, clear your cookies and try again.`;
+      throw new Error(msg);
+    }
+    throw new Error(`Clerk: Handshake token verification failed: ${error.getFullMessage()}.`);
+  }
+
+  function handleHandshakeTokenVerificationErrorInProduction(error: TokenVerificationError) {
+    // In production, the handshake token is being transferred as a cookie, so there is a possibility of collision
+    // with a handshake token of another app running on the same etld+1 domain.
+    // For example, if one app is running on sub1.clerk.com and another on sub2.clerk.com, the handshake token
+    // cookie for both apps will be set on etld+1 (clerk.com) so there's a possibility that one app will accidentally
+    // use the handshake token of a different app during the handshake flow.
+    // In this scenario, verification will fail with TokenInvalidSignature. In contrast to the development case,
+    // we need to allow the flow to continue so the app eventually retries another handshake with the correct token.
+    // We need to make sure, however, that we don't allow the flow to continue indefinitely, so we throw an error after X
+    // retries to avoid an infinite loop. An infinite loop can happen if the customer switched Clerk keys for their prod app.
+    if (
+      error.reason === TokenVerificationErrorReason.TokenInvalidSignature ||
+      error.reason === TokenVerificationErrorReason.InvalidSecretKey ||
+      error.reason === TokenVerificationErrorReason.JWKKidMismatch ||
+      error.reason === TokenVerificationErrorReason.JWKFailedToResolve
+    ) {
+      // Let the request go through and eventually retry another handshake
+      // TODO: set a cookie so break the infinite loop
+      return;
+    }
+    // TODO: if N retries reached, return signedOut
+    const msg = `Clerk: Handshake token verification failed with "${error.reason}"`;
+    return signedOut(authenticateContext, AuthErrorReason.UnexpectedError, msg);
+  }
+
   async function authenticateRequestWithTokenInCookie() {
     const hasActiveClient = authenticateContext.clientUat;
     const hasSessionToken = !!authenticateContext.sessionTokenInCookie;
@@ -210,30 +249,10 @@ ${error.getFullMessage()}`,
       try {
         return await resolveHandshake();
       } catch (error) {
-        // If for some reason the handshake token is invalid or stale, we ignore it and continue trying to authenticate the request.
-        // Worst case, the handshake will trigger again and return a refreshed token.
         if (error instanceof TokenVerificationError) {
-          if (authenticateContext.instanceType === 'development') {
-            if (error.reason === TokenVerificationErrorReason.TokenInvalidSignature) {
-              throw new Error(
-                `Clerk: Handshake token verification failed due to an invalid signature. If you have switched Clerk keys locally, clear your cookies and try again.`,
-              );
-            }
-
-            throw new Error(`Clerk: Handshake token verification failed: ${error.getFullMessage()}.`);
-          }
-
-          if (
-            error.reason === TokenVerificationErrorReason.TokenInvalidSignature ||
-            error.reason === TokenVerificationErrorReason.InvalidSecretKey
-          ) {
-            // Avoid infinite redirect loops due to incorrect secret-keys
-            return signedOut(
-              authenticateContext,
-              AuthErrorReason.UnexpectedError,
-              `Clerk: Handshake token verification failed with "${error.reason}"`,
-            );
-          }
+          authenticateContext.instanceType === 'development'
+            ? handleHandshakeTokenVerificationErrorInDevelopment(error)
+            : handleHandshakeTokenVerificationErrorInProduction(error);
         }
       }
     }