refactor(auth)!: return Session from the auth methods that always issue one - #1845
Merged
spydon merged 2 commits intoSep 17, 2026
Merged
Conversation
Contributor
|
Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (24)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (24)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughAuthentication methods that always issue a session now return ChangesDirect session return contract
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthClient
participant SessionStorage
Client->>AuthClient: Call authentication method
AuthClient->>AuthClient: Parse Session
AuthClient->>SessionStorage: Save Session
AuthClient-->>Client: Return Session
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The breaking authentication API change includes migration guidance and updated consumers; no concrete current-head defect remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The missing-session branch lacks coverage, migration wording is inaccurate, and affected changelogs are absent.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Refactors authentication APIs that always establish a session to return Session directly instead of nullable AuthResponse.
Changes:
- Updates auth, passkey, restore-key, and refresh APIs.
- Migrates callers, examples, documentation, and tests.
- Centralizes missing-session validation.
File summaries
| File | Description |
|---|---|
MIGRATION.md |
Documents the breaking API migration. |
examples/authentication/lib/auth_repository.dart |
Updates repository return types. |
packages/supabase_auth/example/main.dart |
Uses the returned session directly. |
packages/supabase_auth/lib/src/auth_client.dart |
Refactors auth and refresh methods. |
packages/supabase_auth/lib/src/auth_passkey_api.dart |
Returns Session from passkey verification. |
packages/supabase_auth/lib/src/types/auth_response.dart |
Clarifies remaining response uses. |
packages/supabase_auth/test/client_test.dart |
Updates core auth tests. |
packages/supabase_auth/test/otp_mock_test.dart |
Updates password sign-in assertions. |
packages/supabase_auth/test/passkey_test.dart |
Updates passkey tests. |
packages/supabase_auth/test/refresh_token_race_test.dart |
Updates refresh race assertions. |
packages/supabase_auth/test/session_persistence_test.dart |
Updates persistence assertions. |
packages/supabase_auth/test/src/auth_mfa_api_test.dart |
Updates refresh-session usage. |
packages/supabase_auth/test/src/auth_oauth_api_test.dart |
Updates login helper typing. |
packages/supabase_auth/test/src/set_session_test.dart |
Updates session-setting tests. |
packages/supabase_auth/test/src/token_refresh_race_test.dart |
Updates refresh deduplication tests. |
packages/supabase_auth/test/web3_auth_integration_test.dart |
Updates Web3 integration assertions. |
packages/supabase_auth/test/web3_auth_test.dart |
Updates Web3 unit tests. |
packages/supabase_flutter/README.md |
Updates social sign-in examples. |
packages/supabase_flutter/lib/src/supabase_passkey.dart |
Changes passkey extension return type. |
packages/supabase_flutter/lib/src/supabase_restore_credential.dart |
Changes restore-key return type. |
packages/supabase_flutter/test/restore_credential_test.dart |
Updates restore-key tests. |
packages/supabase_test/lib/src/test_supabase_client.dart |
Simplifies the sign-in test helper. |
packages/supabase_test/test/mock_supabase_http_client_test.dart |
Updates mock sign-in assertions. |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
spydon
added this pull request to stack #1847
September 17, 2026 10:05
QuintinWillison
approved these changes
Sep 17, 2026
QuintinWillison
left a comment
QuintinWillison
left a comment
Member
There was a problem hiding this comment.
I don't really understand what I'm looking at, but is refactor the right nature of this change, because to my untrained (in Flutter/Dart) eyes the AuthResponse to Session rename looks breaking in nature... or is this explained by it being part of a wider refactor you're in the middle of (wider context)?
Contributor
Author
It is a breaking refactor, that is what the |
spydon
force-pushed
the
lukasklingsbo/sdk-1874-return-session-from-auth-methods
branch
from
September 17, 2026 15:10
14dd823 to
4bc9701
Compare
Collaborator
|
Let's say I sign up, but get no session because confirmation is required. What would be the result of signing in with password into that user? Would I get a user back from the server, which would throw an exception now because no session exists? |
Contributor
Author
|
…ue one signInAnonymously, signInWithPassword, signInWithIdToken, signInWithWeb3, linkIdentityWithIdToken, refreshSession, setSession, recoverSession and AuthPasskeyApi.verifyAuthentication (and with it signInWithPasskey and signInWithRestoreKey) return the Session directly instead of an AuthResponse with a nullable session. A token response without a session throws an AuthException instead of resolving with session set to null. signUp and verifyOTP keep returning AuthResponse, as both can legitimately complete without a session.
spydon
force-pushed
the
lukasklingsbo/sdk-1874-return-session-from-auth-methods
branch
from
September 17, 2026 15:46
4bc9701 to
5e96871
Compare
Vinzent03
approved these changes
Sep 17, 2026
spydon
deleted the
lukasklingsbo/sdk-1874-return-session-from-auth-methods
branch
September 17, 2026 16:37
spydon
added a commit
that referenced
this pull request
Sep 17, 2026
#1846) ## Summary Stacked on #1845. `AuthState` was one class for every `AuthChangeEvent`, so its payload rules lived in dartdoc: `session` is non-null for every event except `initialSession` and `signedOut`, and `signOutReason` is only set on `signedOut`. Listeners wrote `state.session!` and read a field that is `null` for six of the seven events. Resolves SDK-1876. ### The hierarchy `AuthState` is sealed with one subtype per event, each carrying exactly the data that event produces: | Subtype | `session` | Extra | | --- | --- | --- | | `AuthInitialSession` | `Session?` | | | `AuthSignedIn` | `Session` | | | `AuthSignedOut` | always `null` | `SignOutReason? reason` | | `AuthTokenRefreshed` | `Session` | | | `AuthUserUpdated` | `Session` | | | `AuthPasswordRecovery` | `Session` | | | `AuthMfaChallengeVerified` | `Session` | | ```dart switch (state) { case AuthSignedIn(:final session): showHome(session.user); case AuthSignedOut(reason: SignOutReason.sessionExpired): showSessionExpired(); ... } ``` ### What stays `AuthChangeEvent`, and the `event`, `session` and `fromBroadcast` getters on the base type. The enum still maps to the wire strings the cross-tab broadcast uses, and keeping `state.event == AuthChangeEvent.signedIn` compiling means most apps only have to move `signOutReason` to `AuthSignedOut.reason`. `SupabaseClient` and the existing tests keep using `event` unchanged, which is the evidence for that claim. ### What breaks - `AuthState.signOutReason` is `AuthSignedOut.reason`. - `AuthState` has no public constructor; construct the subtype. ### Client changes `notifyAllSubscribers` builds the state through a private `_authStateFor` switch over the event. Every local emit already saves the session first (checked each call site, `updateUser` throws `AuthSessionMissingException` before it gets there), so the only way to reach a session-carrying event without a session is a malformed cross-tab broadcast. That case is now dropped with a warning instead of emitted, where it used to surface as a `signedIn` with `session: null`. ## Tests and docs - New `auth_state_test.dart` against `MockSupabaseHttpClient`: initial, sign in, refresh, user update, sign out and late-subscriber cases assert the subtype, its payload and `event`, and an exhaustive `switch` expression over the hierarchy fails to compile if a subtype is added without handling. - The three `signOutReason` assertions use `isA<AuthSignedOut>().having(...)`. - `MIGRATION.md` section with before and after, the `AuthState` dartdoc carries the same example, and the `supabase_flutter` README listen snippet shows the switch. - The seven subtypes and their members are registered under `auth.session.subscribe_auth_events` in `sdk-compliance.yaml`, and `auth.session.sign_out_reason` points at `AuthSignedOut.reason`. Symbol, drift and schema checks pass locally against the pinned `capability-matrix-v1.9.0`. Full `supabase_auth` suite (578 with the new file), `supabase`, `supabase_flutter` and `supabase_test` pass. `dart analyze` and DCM are clean at warning level. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Auth state changes now use event-specific types, enabling exhaustive pattern matching. * Sign-out reasons are available through `AuthSignedOut.reason`. * Authenticated events provide session data, while signed-out events have no session. * **Bug Fixes** * User-update notifications are suppressed when no session is present. * Events requiring a session are no longer emitted without one. * **Documentation** * Updated migration guidance and listener examples for the new auth state API. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AuthResponsehas a nullablesessionand a nullableuser, and every signing-in method onAuthClientreturned it. For most of those methods a null session is not a reachable state, so callers wroteresponse.session!for an outcome that cannot happen, and three call sites in the client asserted the same invariant at runtime before using!themselves.Resolves SDK-1874.
Return
SessiondirectlysignInAnonymously,signInWithPassword,signInWithIdToken,signInWithWeb3,linkIdentityWithIdToken,refreshSession,setSessionandrecoverSessiononAuthClient.AuthPasskeyApi.verifyAuthentication, and with it thesupabase_flutterextensionssignInWithPasskeyandsignInWithRestoreKey._refreshAccessToken,_callRefreshToken,_doRefreshand the pending refresh completers) carries aSessionas well.A token response without a session now throws
AuthException('The server response did not contain a session.')from one private_sessionFromResponsehelper, replacing the threethrowthen!blocks and the two methods that silently resolved withsession: null.Kept on
AuthResponsesignUpandverifyOTP, since both can legitimately complete without a session: a sign-up that needs email confirmation first, and the first step of a secure email or phone change. TheAuthResponsedartdoc now says so.Server behaviour checked
Verified against the gotrue source that the tightened methods cannot get a 200 without a token:
SignupAnonymously, the password grant andPasskeyAuthenticationVerifyall end inissueRefreshTokenandsendJSON(w, http.StatusOK, token).Tests and docs
expect(x.session, isNotNull)assertions that the type now guarantees are dropped, and thesupabase_testsignInTestUserhelper loses its!.MIGRATION.mdgains a section listing the affected methods with a before and after snippet.supabase_flutterREADME social sign-in snippets and thesupabase_authexample returnSession.No public symbols are added or removed, so
sdk-compliance.yamlis unchanged.Out of scope
Making
AuthStatea sealed class and foldingAuthChangeEventinto it. Much larger blast radius ononAuthStateChange, discussed separately.Summary by CodeRabbit
API Updates
Sessiondirectly, including sign-in, refresh, recovery, passkey, Web3, restore credential, and session management flows.signUp()andverifyOTP()continue returningAuthResponsebecause they may complete without creating a session.Documentation
Sessionresponses.