fix(gotrue)!: emit userUpdated instead of signedIn when an email change is confirmed - #1664
Conversation
…ge is confirmed Confirming an email or phone change emitted signedIn, which made it indistinguishable from an actual sign in. It now emits userUpdated, the same event that requesting the change through updateUser emits. This covers all three confirmation paths: verifyOTP with emailChange or phoneChange, an implicit type=email_change link through getSessionFromUrl, and a PKCE code through exchangeCodeForSession. updateUser now persists the userUpdated event name alongside the code verifier so the exchange can tell an email change apart from a sign in, using the same mechanism resetPasswordForEmail already used for passwordRecovery. Closes #1398
|
Warning Review limit reached
Next review available in: 8 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughEmail and phone confirmation flows now emit ChangesAuthentication event mapping
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant GoTrueClient
participant AuthStateStream
App->>GoTrueClient: Confirm email or phone change
GoTrueClient->>AuthStateStream: Emit userUpdated
GoTrueClient-->>App: Return session and redirectType userUpdated
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MIGRATION.md`:
- Around line 151-153: Update the migration text describing the authentication
event so “sign in” is hyphenated as the noun “sign-in,” while preserving the
surrounding wording and meaning.
In `@packages/gotrue/test/client_test.dart`:
- Around line 841-852: Update the auth-state observation around pkceClient so
replayed signedIn and userUpdated events cannot be mistaken for the exchange
event: begin collecting events before signInWithPassword(), then assert the
complete expected event order including the event emitted by
exchangeCodeForSession(). Preserve the existing exchange response assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bed7d9db-8d8f-4659-92c8-839ef2b118ca
📒 Files selected for processing (5)
MIGRATION.mdpackages/gotrue/lib/src/gotrue_client.dartpackages/gotrue/test/client_test.dartpackages/gotrue/test/otp_mock_test.dartpackages/supabase_flutter/test/deep_link_test.dart
# Conflicts: # MIGRATION.md
The PKCE email change test subscribed to onAuthStateChange after updateUser had already emitted userUpdated. The stream replays its latest event to new subscribers, so firstWhere resolved on that replayed event and the assertion passed regardless of what exchangeCodeForSession emitted. Collect events from before the sign in instead and assert the full order, which pins the exchange event to the last position. Verified to fail when exchangeCodeForSession falls back to signedIn.
avoid-wildcard-cases-with-enums fired on both new switch expressions. verifyOTP now enumerates every OtpType, so adding a value becomes a compile error rather than silently emitting signedIn. exchangeCodeForSession drops the switch entirely. The event name stored with the code verifier is the event to emit, so falling back to signedIn when none was stored says the same thing in one line.
2 participants
What
Confirming an email or phone change emitted
AuthChangeEvent.signedIn, which made it indistinguishable from an actual sign in. It now emitsAuthChangeEvent.userUpdated, the same event that requesting the change throughupdateUser()emits.Closes #1398.
Why this is breaking
The change is silent: nothing stops compiling, the session is still saved and
currentSessionstill updates, only the emitted event differs. AnyonAuthStateChangelistener that navigates or fetches onsignedInand relied on an email-change confirmation reaching it has to handleuserUpdatedas well. That is why it is landing on v3.The three confirmation paths
verifyOTP()withOtpType.emailChangeorOtpType.phoneChangesignedInuserUpdatedgetSessionFromUrl()with an implicittype=email_changelinksignedInuserUpdatedexchangeCodeForSession()for a PKCE code from an email changesignedInuserUpdatedThe PKCE path needed a little more than a branch.
exchangeCodeForSession()had no way to know what the code was issued for, so it always fell through tosignedIn.updateUser()now persistsuserUpdatedalongside the code verifier and the exchange reads it back, which is the same mechanismresetPasswordForEmail()already used forpasswordRecovery. As a side effectAuthSessionUrlResponse.redirectTypeis'userUpdated'for that flow instead ofnull, so the response can be branched on directly.phoneChangeis included next toemailChangebecause it is the same class of operation, and leaving it out would mean a confirmed phone change and a confirmed email change emit different events for no reason.Note that supabase-js still emits
SIGNED_INhere, so this is a deliberate divergence rather than a parity fix. The issue calls out the JS behaviour as wrong too.Tests
packages/gotrue/test/otp_mock_test.dart: five new tests covering the emitted event for each relevantOtpType, plus one assertingupdateUser()persists the event name with the code verifier.packages/gotrue/test/client_test.dart: the implicitemail_changetest now expectsuserUpdated, and the live PKCE email-change test asserts both the emitted event andredirectType.packages/supabase_flutter/test/deep_link_test.dart: thetype=email_changedeep-link test now waits foruserUpdated.Docs
New
MIGRATION.mdsection under v2 to v3 with the before/after table and a listener snippet, and dartdoc onverifyOTP(),getSessionFromUrl()andexchangeCodeForSession()now states which event each emits.Summary by CodeRabbit
Bug Fixes
userUpdatedauthentication event instead ofsignedIn.redirectTypeasuserUpdated.currentSessionupdates remain unchanged.Documentation