Recover customer account login from OAuth state mismatches

.changeset/quiet-webviews-recover.md

@@ -0,0 +1,5 @@
+---
+'@shopify/hydrogen': patch
+---
+
+Recover customer account login when switching browser contexts or opening multiple login tabs. OAuth state values now use cryptographically secure randomness.

packages/hydrogen/src/customer/auth.helpers.test.ts

@@ -1,7 +1,14 @@
 import {describe, it, expect, vi, beforeEach, afterEach} from 'vitest';
 import type {HydrogenSession} from '../types';
 import {CUSTOMER_ACCOUNT_SESSION_KEY} from './constants';
-import {checkExpires, clearSession, refreshToken} from './auth.helpers';
+import {
+  checkExpires,
+  clearSession,
+  generateState,
+  refreshToken,
+} from './auth.helpers';
+
+const OAUTH_STATE_BYTE_LENGTH = 32;
 
 vi.mock('./BadRequest', () => {
   return {
@@ -37,6 +44,32 @@ function createFetchResponse<T>(data: T, options: {ok: boolean}) {
 let session: HydrogenSession;
 
 describe('auth.helpers', () => {
+  describe('generateState', () => {
+    afterEach(() => {
+      vi.restoreAllMocks();
+    });
+
+    it('Generates a 256-bit base64url value', () => {
+      const randomBytes = Uint8Array.from(
+        {length: OAUTH_STATE_BYTE_LENGTH},
+        (_, index) => index,
+      );
+      const getRandomValuesSpy = vi
+        .spyOn(globalThis.crypto, 'getRandomValues')
+        .mockImplementation((array: any) => {
+          array.set(randomBytes);
+          return array;
+        });
+      const state = generateState();
+      const decodedState = Buffer.from(state, 'base64url');
+
+      expect(state).toBe(Buffer.from(randomBytes).toString('base64url'));
+      expect(state).toMatch(/^[A-Za-z0-9_-]+$/);
+      expect(decodedState).toHaveLength(OAUTH_STATE_BYTE_LENGTH);
+      expect(getRandomValuesSpy).toHaveBeenCalledOnce();
+    });
+  });
+
   describe('refreshToken', () => {
     beforeEach(() => {
       session = {

packages/hydrogen/src/customer/auth.helpers.ts

@@ -232,9 +232,7 @@ function convertBufferToString(hash: ArrayBuffer) {
 }
 
 export function generateState() {
-  const timestamp = Date.now().toString();
-  const randomString = Math.random().toString(36).substring(2);
-  return timestamp + randomString;
+  return base64UrlEncode(generateRandomCode());
 }
 
 export async function exchangeAccessToken(

packages/hydrogen/src/customer/customer.test.ts

@@ -79,6 +79,7 @@ describe('customer', () => {
 
   afterEach(() => {
     process.env.NODE_ENV = originalNodeEnv;
+    vi.restoreAllMocks();
     vi.clearAllMocks();
   });
 
@@ -1170,7 +1171,7 @@ describe('customer', () => {
 
   describe('authorize', () => {
     describe('using new auth url when shopId is present in env', () => {
-      it('Throws unauthorized if no code or state params are passed', async () => {
+      it('Rejects missing callback params without clearing the session', async () => {
         const customer = createCustomerAccountClient({
           session,
           customerAccountId: 'customerAccountId',
@@ -1182,19 +1183,177 @@ describe('customer', () => {
         await expect(customer.authorize()).rejects.toThrowError(
           'Unauthorized No code or state parameter found in the redirect URL.',
         );
+        expect(fetch).not.toHaveBeenCalled();
+        expect(session.unset).not.toHaveBeenCalled();
       });
 
-      it("Throws unauthorized if state doesn't match session value", async () => {
+      it('Restarts login if the browser does not have the original session state', async () => {
+        const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+        session.get = vi.fn(() => undefined) as HydrogenSession['get'];
         const customer = createCustomerAccountClient({
           session,
           customerAccountId: 'customerAccountId',
           shopId: '1',
-          request: new Request('https://localhost?state=nomatch&code=code'),
+          request: new Request(
+            'https://localhost/account/authorize?state=state&code=code',
+          ),
           waitUntil: vi.fn(),
         });
 
-        await expect(customer.authorize()).rejects.toThrowError(
-          'Unauthorized The session state does not match the state parameter. Make sure that the session is configured correctly and passed to `createCustomerAccountClient`.',
+        const response = await customer.authorize();
+
+        expect(response.status).toBe(302);
+        expect(response.headers.get('location')).toBe(
+          '/account/login?return_to=%2Faccount&oauth_recovery=true',
+        );
+        expect(fetch).not.toHaveBeenCalled();
+        expect(session.unset).not.toHaveBeenCalled();
+        expect(warnSpy).toHaveBeenCalledWith(
+          '[h2:warn:customerAccount] OAuth callback state mismatch. Restarting customer login once.',
+        );
+      });
+
+      it('Restarts login if another tab replaced the original session state', async () => {
+        session.get = vi.fn(() => ({
+          ...mockCustomerAccountSession,
+          state: 'newer-state',
+          redirectPath: '/account/orders',
+        })) as HydrogenSession['get'];
+        const customer = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request(
+            'https://localhost/account/authorize?state=older-state&code=code',
+          ),
+          waitUntil: vi.fn(),
+        });
+
+        const response = await customer.authorize();
+
+        expect(response.status).toBe(302);
+        expect(response.headers.get('location')).toBe(
+          '/account/login?return_to=%2Faccount%2Forders&oauth_recovery=true',
+        );
+        expect(fetch).not.toHaveBeenCalled();
+        expect(session.unset).not.toHaveBeenCalled();
+      });
+
+      it('Stops after one recovery attempt if the browser still has no session state', async () => {
+        session.get = vi.fn(() => undefined) as HydrogenSession['get'];
+        const firstAuthorize = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request(
+            'https://localhost/account/authorize?state=state&code=code',
+          ),
+          waitUntil: vi.fn(),
+        });
+        const firstAuthorizeResponse = await firstAuthorize.authorize();
+        const recoveryLoginLocation =
+          firstAuthorizeResponse.headers.get('location') ?? '';
+        const recoveryLogin = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request(
+            new URL(recoveryLoginLocation, 'https://localhost'),
+          ),
+          waitUntil: vi.fn(),
+        });
+        const recoveryLoginResponse = await recoveryLogin.login();
+        const recoveryState =
+          new URL(
+            recoveryLoginResponse.headers.get('location') ?? '',
+          ).searchParams.get('state') ?? '';
+        const secondAuthorize = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request(
+            `https://localhost/account/authorize?${new URLSearchParams({
+              state: recoveryState,
+              code: 'code',
+            })}`,
+          ),
+          waitUntil: vi.fn(),
+        });
+
+        expect(recoveryState).not.toBe('');
+        await expect(secondAuthorize.authorize()).rejects.toThrowError(
+          'Unable to complete sign in. Please try again. The session state does not match the state parameter. Make sure that the session is configured correctly and passed to `createCustomerAccountClient`.',
+        );
+        expect(fetch).not.toHaveBeenCalled();
+      });
+
+      it('Completes authorization after restarting login with a fresh session state', async () => {
+        session.get = vi.fn(() => undefined) as HydrogenSession['get'];
+        const firstAuthorize = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request(
+            'https://localhost/account/authorize?state=state&code=code',
+          ),
+          waitUntil: vi.fn(),
+        });
+        const firstAuthorizeResponse = await firstAuthorize.authorize();
+        const recoveryLogin = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request(
+            new URL(
+              firstAuthorizeResponse.headers.get('location') ?? '',
+              'https://localhost',
+            ),
+          ),
+          waitUntil: vi.fn(),
+        });
+        const recoveryLoginResponse = await recoveryLogin.login();
+        const recoverySession = (session.set as any).mock.calls.at(-1)?.[1];
+
+        if (!recoverySession) {
+          throw new Error('Expected recovery login to persist its session');
+        }
+
+        session.get = vi.fn(() => recoverySession) as HydrogenSession['get'];
+        fetch.mockResolvedValue(
+          createFetchResponse(
+            {
+              access_token: 'shcat_access_token',
+              expires_in: '',
+              id_token: `${btoa('{}')}.${btoa(
+                JSON.stringify({nonce: recoverySession.nonce}),
+              )}.signature`,
+              refresh_token: 'shcrt_refresh_token',
+            },
+            {ok: true},
+          ),
+        );
+        const recoveryState =
+          new URL(
+            recoveryLoginResponse.headers.get('location') ?? '',
+          ).searchParams.get('state') ?? '';
+        const secondAuthorize = createCustomerAccountClient({
+          session,
+          customerAccountId: 'customerAccountId',
+          shopId: '1',
+          request: new Request(
+            `https://localhost/account/authorize?${new URLSearchParams({
+              state: recoveryState,
+              code: 'code',
+            })}`,
+          ),
+          waitUntil: vi.fn(),
+        });
+
+        const secondAuthorizeResponse = await secondAuthorize.authorize();
+
+        expect(secondAuthorizeResponse.status).toBe(302);
+        expect(secondAuthorizeResponse.headers.get('location')).toBe(
+          '/account',
         );
       });
 

packages/hydrogen/src/customer/customer.ts

@@ -53,6 +53,9 @@ import {warnOnce} from '../utils/warning';
 import {LanguageCode} from '@shopify/hydrogen-react/customer-account-api-types';
 
 const HYDROGEN_TUNNEL_DOMAIN_SUFFIX = '.tryhydrogen.dev';
+const OAUTH_RECOVERY_PARAM = 'oauth_recovery';
+// State survives the OAuth round trip even when the browser drops the session cookie.
+const OAUTH_RECOVERY_STATE_PREFIX = 'oauth-recovery.';
 
 function checkTunnelDomain(
   hostname: string,
@@ -392,7 +395,11 @@ export function createCustomerAccountClient({
 
       const loginUrl = new URL(getCustomerAccountUrl(URL_TYPE.AUTH));
 
-      const state = generateState();
+      const isOAuthRecovery =
+        requestUrl.searchParams.get(OAUTH_RECOVERY_PARAM) === 'true';
+      const state = `${
+        isOAuthRecovery ? OAUTH_RECOVERY_STATE_PREFIX : ''
+      }${generateState()}`;
       const nonce = generateNonce();
 
       loginUrl.searchParams.set('client_id', customerAccountId);
@@ -510,20 +517,34 @@ export function createCustomerAccountClient({
       const state = requestUrl.searchParams.get('state');
 
       if (!code || !state) {
-        clearSession(session);
-
         throw new BadRequest(
           'Unauthorized',
           'No code or state parameter found in the redirect URL.',
         );
       }
 
-      if (session.get(CUSTOMER_ACCOUNT_SESSION_KEY)?.state !== state) {
-        clearSession(session);
+      const customerAccountSession = session.get(CUSTOMER_ACCOUNT_SESSION_KEY);
 
-        throw new BadRequest(
-          'Unauthorized',
-          'The session state does not match the state parameter. Make sure that the session is configured correctly and passed to `createCustomerAccountClient`.',
+      if (customerAccountSession?.state !== state) {
+        if (state.startsWith(OAUTH_RECOVERY_STATE_PREFIX)) {
+          throw new BadRequest(
+            'Unable to complete sign in. Please try again.',
+            'The session state does not match the state parameter. Make sure that the session is configured correctly and passed to `createCustomerAccountClient`.',
+          );
+        }
+
+        warnOnce(
+          '[h2:warn:customerAccount] OAuth callback state mismatch. Restarting customer login once.',
+        );
+        const recoveryLoginUrl = new URL(loginPath, request.url);
+        recoveryLoginUrl.searchParams.set(
+          'return_to',
+          customerAccountSession?.redirectPath ?? defaultRedirectPath,
+        );
+        recoveryLoginUrl.searchParams.set(OAUTH_RECOVERY_PARAM, 'true');
+
+        return redirect(
+          `${recoveryLoginUrl.pathname}${recoveryLoginUrl.search}`,
         );
       }