app: use popup OAuth flow so Google/Apple sign-in works on Safari

docs/dev/deploy.md

@@ -66,7 +66,7 @@ gcloud app services set-traffic default --splits=<version>=1
 
 ### Canary deploy + Firebase login: `pnpm deploy:canary`
 
-The `--no-promote` flow above lets you `curl` the canary, but you cannot actually **log in** to it: Firebase OAuth (`signInWithRedirect` via `auth.simlin.com`) rejects any origin not in Firebase's *Authorized domains*, and the canary is reachable only at a versioned `https://<version>-dot-<project>...appspot.com` URL that isn't on that list. So a full end-to-end test (including Google sign-in, the new-user flow, and saving a model) isn't possible on a bare `--no-promote` version.
+The `--no-promote` flow above lets you `curl` the canary, but you cannot actually **log in** to it: Firebase OAuth (`signInWithPopup` via `auth.simlin.com`) rejects any origin not in Firebase's *Authorized domains*, and the canary is reachable only at a versioned `https://<version>-dot-<project>...appspot.com` URL that isn't on that list. So a full end-to-end test (including Google sign-in, the new-user flow, and saving a model) isn't possible on a bare `--no-promote` version.
 
 [`scripts/deploy-canary.mjs`](/scripts/deploy-canary.mjs) (`pnpm deploy:canary`) closes that gap. It:
 
@@ -85,7 +85,7 @@ pnpm deploy:canary --cleanup <version>   # de-authorize the host + delete the ve
 
 Cleanup is the inverse: it removes the host from `authorizedDomains` and the API key's referrer allowlist (same read-modify-write; it refuses to empty the referrer list, so it can never un-restrict the key) and deletes the version (freeing its slot toward the GAE version cap). It deletes rather than `stop`s because production uses `automatic_scaling`, for which `gcloud app versions stop` is rejected. The delete is guarded by the version's current traffic share: if you have already promoted the canary it is now production, so cleanup refuses to delete it (de-authorizing the host only) and tells you to delete the previous, now-idle version instead -- cleanup can never cause an outage.
 
-**Known limitation: Google/Apple sign-in cannot complete on the canary host.** The redirect flow initiates and returns fine, but `signInWithRedirect`'s result pickup loads an `auth.simlin.com` iframe that must read storage written during the top-level redirect -- and browsers partition cross-**site** iframe storage, so under an `appspot.com` top-level site the iframe sees an empty bucket and the SDK silently reports no result (no error anywhere). No domain/key authorization can fix this; it is inherent to the host. `app.simlin.com` is same-site with the auth domain, so OAuth works there. On the canary, verify login with **email/password** (same identitytoolkit + `/session` path); verify Google/Apple on `app.simlin.com` immediately after promoting, with the rollback split ready.
+**OAuth flow: popup, with redirect only as a popup-blocked fallback.** Google/Apple sign-in uses `signInWithPopup`, which hands the result back to the opener via `postMessage` and needs no storage shared with `auth.simlin.com`. The previous `signInWithRedirect` flow silently failed on Safari/iOS even on `app.simlin.com`: its result pickup loads an `auth.simlin.com` iframe that must read storage written during the top-level redirect, and WebKit partitions iframe storage per-**origin** (Chromium partitions per-site, which is why Chrome worked). The same mechanism silently broke OAuth on the cross-site canary host in every browser. With the popup flow, OAuth should complete on the canary too (the host is added to *Authorized domains* by `deploy-canary.mjs`); the redirect fallback still cannot deliver a result there, so if a popup blocker forces the fallback on the canary, expect a silent bounce back to login. Verify Google/Apple on `app.simlin.com` (Safari included) after promoting, with the rollback split ready; email/password exercises the same identitytoolkit + `/session` path if popups are unavailable.
 
 This deliberately uses **your own** credentials (`gcloud auth print-access-token`), not the CI deploy service account: mutating Firebase auth config needs `roles/firebaseauth.admin`, an operator-level grant intentionally kept off the CI SA. The deploy/authorize and the traffic promote are separate, explicit steps so traffic is never switched implicitly. Keep the canary host authorized only while you are testing it -- leaving stale appspot hosts in `authorizedDomains` needlessly widens the OAuth surface.
 

src/app/Login.tsx

@@ -6,9 +6,11 @@ import * as React from 'react';
 import clsx from 'clsx';
 
 import {
+  signInWithPopup,
   signInWithRedirect,
   GoogleAuthProvider,
   OAuthProvider,
+  AuthProvider,
   Auth as FirebaseAuth,
   createUserWithEmailAndPassword,
   updateProfile,
@@ -130,28 +132,48 @@ export function Login(props: LoginProps): React.JSX.Element {
   );
   latest.current = { props, state };
 
-  // Surface OAuth redirect failures (provider misconfig, popup blocked,
-  // network errors, expired auth domain) into emailError so the user sees them.
-  const appleLoginClick = async () => {
-    const provider = appleProvider();
+  // OAuth uses the POPUP flow: the result comes back to the opener via
+  // postMessage, which needs no storage shared with auth.simlin.com. The
+  // redirect flow's result pickup reads storage the auth.simlin.com helper
+  // wrote while it was the top-level page, through an auth.simlin.com iframe
+  // embedded under app.simlin.com -- and WebKit partitions storage per-ORIGIN
+  // (Chromium partitions per-site), so on Safari/iOS that iframe sees an empty
+  // partition and the sign-in silently never completes (firebase-js-sdk #7824).
+  // Redirect remains only as the fallback when a popup blocker rejects the
+  // popup; a user cancelling the popup is not an error. Real failures
+  // (provider misconfig, network errors, unauthorized domain) surface into
+  // emailError so the user sees them.
+  const oauthLoginClick = async (provider: AuthProvider, genericError: string) => {
+    // A fresh attempt starts with a clean slate: without this, a cancelled
+    // popup's early return below would leave a PREVIOUS attempt's failure on
+    // screen, misattributed to the action the user just harmlessly dismissed.
+    setState({ emailError: undefined });
     try {
-      await signInWithRedirect(latest.current.props.auth, provider);
-    } catch (err) {
+      await signInWithPopup(latest.current.props.auth, provider);
+    } catch (popupErr) {
+      let err: unknown = popupErr;
+      const code = firebaseErrorCode(err);
+      if (code === 'auth/popup-closed-by-user' || code === 'auth/cancelled-popup-request') {
+        return;
+      }
+      if (code === 'auth/popup-blocked') {
+        try {
+          await signInWithRedirect(latest.current.props.auth, provider);
+          return;
+        } catch (redirectErr) {
+          err = redirectErr;
+        }
+      }
       setState({
-        emailError: err instanceof Error ? err.message : 'Sign in with Apple failed',
+        emailError: err instanceof Error ? err.message : genericError,
       });
     }
   };
-  const googleLoginClick = async () => {
+  const appleLoginClick = () => oauthLoginClick(appleProvider(), 'Sign in with Apple failed');
+  const googleLoginClick = () => {
     const provider = new GoogleAuthProvider();
     provider.addScope('profile');
-    try {
-      await signInWithRedirect(latest.current.props.auth, provider);
-    } catch (err) {
-      setState({
-        emailError: err instanceof Error ? err.message : 'Sign in with Google failed',
-      });
-    }
+    return oauthLoginClick(provider, 'Sign in with Google failed');
   };
   // Entering the email flow lands directly on the combined sign-in card. Clear
   // any stale OAuth error from the landing screen so it doesn't reappear under
@@ -510,9 +532,9 @@ export function Login(props: LoginProps): React.JSX.Element {
               Sign in with email
             </Button>
             {/* Visible error sink for OAuth click handlers. Without this, a
-             * rejected signInWithRedirect (popup blocked, provider misconfig,
-             * network failure) would set emailError but stay invisible because
-             * no email-flow card is mounted to render it. */}
+             * rejected signInWithPopup (provider misconfig, unauthorized
+             * domain, network failure) would set emailError but stay invisible
+             * because no email-flow card is mounted to render it. */}
             {state.emailError !== undefined && (
               <p role="alert" className={clsx(typography.body2, styles.formError)}>
                 {state.emailError}

src/app/tests/login.test.tsx

@@ -12,6 +12,7 @@
 // reaches for the deprecated, enumeration-leaking API again (issue #692).
 
 rs.mock('@firebase/auth', () => ({
+  signInWithPopup: rs.fn(),
   signInWithRedirect: rs.fn(),
   GoogleAuthProvider: class {
     addScope() {}
@@ -103,6 +104,7 @@ import { Login, GoogleIcon } from '../Login';
 // rs.mock replaced the module above, so importing it normally yields the mock.
 // (jest.requireMock had no async equivalent -- rs.importMock returns a promise.)
 const firebaseAuth = firebaseAuthModule as unknown as {
+  signInWithPopup: Mock;
   signInWithRedirect: Mock;
   fetchSignInMethodsForEmail: Mock;
   sendPasswordResetEmail: Mock;
@@ -125,6 +127,7 @@ function firebaseError(code: string, message = code): Error {
 }
 
 function resetAllMocks(): void {
+  firebaseAuth.signInWithPopup.mockReset();
   firebaseAuth.signInWithRedirect.mockReset();
   firebaseAuth.fetchSignInMethodsForEmail.mockReset();
   firebaseAuth.sendPasswordResetEmail.mockReset();
@@ -147,46 +150,122 @@ function openSignInCard(email?: string, password?: string): void {
 describe('Login OAuth click handlers (landing)', () => {
   beforeEach(resetAllMocks);
 
-  test('Google sign-in surfaces an error to UI when signInWithRedirect rejects', async () => {
-    firebaseAuth.signInWithRedirect.mockRejectedValueOnce(new Error('popup blocked'));
+  // signInWithPopup, not signInWithRedirect: the redirect flow's result pickup
+  // reads storage the auth.simlin.com helper wrote while it was the top-level
+  // page, through an iframe of that origin embedded under app.simlin.com.
+  // WebKit partitions storage per-ORIGIN (not per-site like Chromium), so on
+  // Safari/iOS that iframe sees an empty partition and the sign-in silently
+  // never completes. The popup flow hands the result back via postMessage and
+  // needs no shared storage, so it works everywhere.
+  test('Google sign-in uses the popup flow, not the redirect flow', async () => {
+    firebaseAuth.signInWithPopup.mockResolvedValueOnce({ user: {} });
 
     render(<Login disabled={false} auth={makeAuth()} />);
     fireEvent.click(screen.getByText('Sign in with Google'));
 
     await waitFor(() => {
-      expect(firebaseAuth.signInWithRedirect).toHaveBeenCalledTimes(1);
-      expect(screen.queryByText(/popup blocked/i)).not.toBeNull();
+      expect(firebaseAuth.signInWithPopup).toHaveBeenCalledTimes(1);
     });
+    expect(firebaseAuth.signInWithRedirect).not.toHaveBeenCalled();
   });
 
-  test('Apple sign-in surfaces an error to UI when signInWithRedirect rejects', async () => {
-    firebaseAuth.signInWithRedirect.mockRejectedValueOnce(new Error('apple unavailable'));
+  test('Apple sign-in uses the popup flow, not the redirect flow', async () => {
+    firebaseAuth.signInWithPopup.mockResolvedValueOnce({ user: {} });
 
     render(<Login disabled={false} auth={makeAuth()} />);
     fireEvent.click(screen.getByText('Sign in with Apple'));
 
     await waitFor(() => {
-      expect(firebaseAuth.signInWithRedirect).toHaveBeenCalledTimes(1);
+      expect(firebaseAuth.signInWithPopup).toHaveBeenCalledTimes(1);
+    });
+    expect(firebaseAuth.signInWithRedirect).not.toHaveBeenCalled();
+  });
+
+  test('Google sign-in surfaces an error to UI when signInWithPopup rejects', async () => {
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce(new Error('provider misconfigured'));
+
+    render(<Login disabled={false} auth={makeAuth()} />);
+    fireEvent.click(screen.getByText('Sign in with Google'));
+
+    await waitFor(() => {
+      expect(firebaseAuth.signInWithPopup).toHaveBeenCalledTimes(1);
+      expect(screen.queryByText(/provider misconfigured/i)).not.toBeNull();
+    });
+  });
+
+  test('Apple sign-in surfaces an error to UI when signInWithPopup rejects', async () => {
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce(new Error('apple unavailable'));
+
+    render(<Login disabled={false} auth={makeAuth()} />);
+    fireEvent.click(screen.getByText('Sign in with Apple'));
+
+    await waitFor(() => {
+      expect(firebaseAuth.signInWithPopup).toHaveBeenCalledTimes(1);
       expect(screen.queryByText(/apple unavailable/i)).not.toBeNull();
     });
   });
 
-  test('Google sign-in awaits the redirect promise (no fire-and-forget setTimeout)', async () => {
-    let resolveSignIn: () => void = () => {};
-    firebaseAuth.signInWithRedirect.mockReturnValueOnce(
-      new Promise<void>((resolve) => {
-        resolveSignIn = resolve;
-      }),
-    );
+  test('a blocked popup falls back to the redirect flow without showing an error', async () => {
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce(firebaseError('auth/popup-blocked'));
+    firebaseAuth.signInWithRedirect.mockResolvedValueOnce(undefined);
 
     render(<Login disabled={false} auth={makeAuth()} />);
     fireEvent.click(screen.getByText('Sign in with Google'));
 
     await waitFor(() => {
       expect(firebaseAuth.signInWithRedirect).toHaveBeenCalledTimes(1);
     });
+    expect(screen.queryByRole('alert')).toBeNull();
+  });
+
+  test('a rejected redirect fallback still surfaces its error', async () => {
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce(firebaseError('auth/popup-blocked'));
+    firebaseAuth.signInWithRedirect.mockRejectedValueOnce(new Error('redirect also failed'));
+
+    render(<Login disabled={false} auth={makeAuth()} />);
+    fireEvent.click(screen.getByText('Sign in with Google'));
+
+    await waitFor(() => {
+      expect(screen.queryByText(/redirect also failed/i)).not.toBeNull();
+    });
+  });
+
+  // A new attempt must clear the previous attempt's error up front: without
+  // that, a failed Google attempt followed by a CANCELLED attempt (even an
+  // Apple one) leaves the old failure on screen, misattributing it to the
+  // action the user just harmlessly dismissed.
+  test('starting a new OAuth attempt clears the previous attempt error, even when cancelled', async () => {
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce(new Error('provider misconfigured'));
+
+    render(<Login disabled={false} auth={makeAuth()} />);
+    fireEvent.click(screen.getByText('Sign in with Google'));
+    await waitFor(() => {
+      expect(screen.queryByText(/provider misconfigured/i)).not.toBeNull();
+    });
 
-    resolveSignIn();
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce(firebaseError('auth/popup-closed-by-user'));
+    fireEvent.click(screen.getByText('Sign in with Apple'));
+    await waitFor(() => {
+      expect(firebaseAuth.signInWithPopup).toHaveBeenCalledTimes(2);
+    });
+    expect(screen.queryByText(/provider misconfigured/i)).toBeNull();
+  });
+
+  test('closing the popup is treated as a cancel, not an error', async () => {
+    for (const code of ['auth/popup-closed-by-user', 'auth/cancelled-popup-request']) {
+      firebaseAuth.signInWithPopup.mockReset();
+      firebaseAuth.signInWithPopup.mockRejectedValueOnce(firebaseError(code));
+
+      const { unmount } = render(<Login disabled={false} auth={makeAuth()} />);
+      fireEvent.click(screen.getByText('Sign in with Google'));
+
+      await waitFor(() => {
+        expect(firebaseAuth.signInWithPopup).toHaveBeenCalledTimes(1);
+      });
+      expect(screen.queryByRole('alert')).toBeNull();
+      expect(firebaseAuth.signInWithRedirect).not.toHaveBeenCalled();
+      unmount();
+    }
   });
 });
 
@@ -597,7 +676,7 @@ describe('Login miscellaneous error paths', () => {
   });
 
   test('a non-Error OAuth rejection falls back to a generic provider message', async () => {
-    firebaseAuth.signInWithRedirect.mockRejectedValueOnce('nope');
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce('nope');
 
     render(<Login disabled={false} auth={makeAuth()} />);
     fireEvent.click(screen.getByText('Sign in with Google'));
@@ -608,7 +687,7 @@ describe('Login miscellaneous error paths', () => {
   });
 
   test('a non-Error Apple OAuth rejection falls back to a generic provider message', async () => {
-    firebaseAuth.signInWithRedirect.mockRejectedValueOnce('nope');
+    firebaseAuth.signInWithPopup.mockRejectedValueOnce('nope');
 
     render(<Login disabled={false} auth={makeAuth()} />);
     fireEvent.click(screen.getByText('Sign in with Apple'));