app: use popup OAuth flow so Google/Apple sign-in works on Safari#970
Conversation
signInWithRedirect's result pickup loads an auth.simlin.com iframe that must read storage the sign-in helper wrote while it was the top-level page. WebKit partitions iframe storage per-origin (not per-site like Chromium), so on Safari and iOS that iframe sees an empty partition and the redirect sign-in silently never completes -- getRedirectResult reports null with no error anywhere (firebase-js-sdk issue 7824). The same-site auth.simlin.com authDomain only helps browsers that partition by registrable domain, which is why Chrome worked. signInWithPopup delivers the result to the opener via postMessage and needs no shared storage, so it works under per-origin partitioning; it should also complete on cross-site canary hosts, which the redirect flow never could. Redirect remains only as a fallback when a popup blocker rejects the popup (the buttons are direct user gestures, so blocking is rare); a user closing the popup is treated as a cancel, not an error.
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
Reviewed the PR: no blocking bugs found. The popup-first / redirect-fallback flow is cleanly implemented with a shared Overall correctness verdict: correct. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #970 +/- ##
=======================================
Coverage 91.52% 91.52%
=======================================
Files 234 234
Lines 156299 156299
=======================================
+ Hits 143052 143053 +1
+ Misses 13247 13246 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67c55450e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (code === 'auth/popup-closed-by-user' || code === 'auth/cancelled-popup-request') { | ||
| return; |
There was a problem hiding this comment.
If an earlier OAuth attempt has populated emailError, a subsequent attempt that the user closes returns here without clearing that state. The landing screen therefore continues to display the previous failure—potentially even a Google error after cancelling an Apple popup—despite treating the current action as a harmless cancellation. Clear the existing OAuth error when starting an attempt or before returning from these cancellation branches.
Useful? React with 👍 / 👎.
Codex review on PR #970 caught a stale-state path: a failed OAuth attempt sets emailError, and a later attempt the user cancels returns early through the popup-closed branch without touching it, so the old failure stays on screen misattributed to the action the user just dismissed (even across providers). Clear the error when an attempt starts rather than in each cancellation branch: a fresh attempt should always present a clean slate, and a real failure sets a new error anyway.
|
Code Review — reviewed the change end-to-end; no blocking issues found. Notes:
Overall correctness: CORRECT. The patch appears free of blocking issues. Real-device Safari verification on app.simlin.com after deploy — already noted in the PR description — remains the outstanding validation step. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Google sign-in on app.simlin.com worked in Chrome but silently failed on iOS Safari: after returning from Google, the user was dropped back on the login screen with no error.
Root cause:
signInWithRedirect's result pickup loads anauth.simlin.comiframe that must read storage the sign-in helper wrote while it was the top-level page. WebKit partitions iframe storage per-origin, while Chromium partitions per-site, so the same-site custom authDomain (auth.simlin.comunderapp.simlin.com) only rescues Chromium-family browsers. On Safari the iframe sees an empty partition andgetRedirectResultreports null with no error anywhere (firebase/firebase-js-sdk#7824; WebKit's per-origin partitioning is confirmed in https://webkit.org/blog/14403/updates-to-storage-policy/ and https://developer.apple.com/forums/thread/725074).Fix: use
signInWithPopup, which delivers the result to the opener via postMessage and needs no storage shared with the auth domain, so it works under per-origin partitioning.signInWithRedirectremains only as a fallback when a popup blocker rejects the popup (the provider buttons are direct user gestures, so blocking is rare); a user closing the popup is treated as a cancel, not an error. A side effect is that OAuth should now also complete on cross-site canary hosts, which the redirect flow structurally never could -- docs/dev/deploy.md is updated to reflect the new flow and the remaining canary caveat.Verified end-to-end against the dev stack (auth emulator): popup opens with
authType=signInViaPopup, sign-in completes, the/sessionexchange succeeds, and the new-user dialog renders. Real-device Safari verification still needs to happen on app.simlin.com after this deploys.https://claude.ai/code/session_01EQoRGRGfkerDZapRpyrgAJ