fix(auth)!: return VerifyOTPResponse from verifyOTP instead of AuthResponse - #1088
Merged
Merged
Conversation
grdsdev
added a commit
that referenced
this pull request
Jul 7, 2026
…ne doc Addresses code review feedback on #1088 (SDK-1022): the import ordering in AuthClientTests.swift had regressed to a non-swift-format-canonical layout, and the `.none` doc didn't make clear it's a catch-all for any unrecognized response shape, not just the email-change confirmation case.
Coverage Report for CI Build 31686113079Coverage increased (+0.2%) to 84.829%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
spydon
approved these changes
Jul 7, 2026
grdsdev
added a commit
that referenced
this pull request
Aug 12, 2026
…ne doc Addresses code review feedback on #1088 (SDK-1022): the import ordering in AuthClientTests.swift had regressed to a non-swift-format-canonical layout, and the `.none` doc didn't make clear it's a catch-all for any unrecognized response shape, not just the email-change confirmation case.
grdsdev
force-pushed
the
fix/sdk-1022-verifyotp-email-change-parity
branch
from
August 12, 2026 16:30
1a03e36 to
8df4718
Compare
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR changes all Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthClient
participant AuthAPI
Client->>AuthClient: Call verifyOTP
AuthClient->>AuthAPI: Send OTP verification request
AuthAPI-->>AuthClient: Return session or confirmation payload
AuthClient->>AuthClient: Decode VerifyOTPResponse
AuthClient-->>Client: Return verification result
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 |
grdsdev
added a commit
that referenced
this pull request
Aug 12, 2026
…ne doc Addresses code review feedback on #1088 (SDK-1022): the import ordering in AuthClientTests.swift had regressed to a non-swift-format-canonical layout, and the `.none` doc didn't make clear it's a catch-all for any unrecognized response shape, not just the email-change confirmation case.
grdsdev
force-pushed
the
fix/sdk-1022-verifyotp-email-change-parity
branch
from
August 12, 2026 17:03
9af3aa6 to
600d4bb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Tests/IntegrationTests/AuthClientIntegrationTests.swift`:
- Line 239: Update the comparison in the second-user sign-up test to use
`#require` on secondUserSession.user and obtain its non-optional id before
asserting it differs from user.id. Keep the existing distinct-ID assertion while
ensuring a missing second user fails the test rather than satisfying it.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 91798a58-0f1b-49c0-a525-c39dc5e797f9
📒 Files selected for processing (8)
.claude/skills/writing-migration-guides/SKILL.mdAGENTS.mdSources/Auth/Types.swiftTests/AuthTests/AuthClientTests.swiftTests/AuthTests/AuthResponseTests.swiftTests/AuthTests/Resources/email-change-single-confirmation.jsonTests/IntegrationTests/AuthClientIntegrationTests.swiftV3_MIGRATION.md
Contributor
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 `@V3_MIGRATION.md`:
- Around line 37-48: Update the verifyOTP migration example to switch over both
VerifyOTPResponse cases instead of accessing response.session unconditionally.
In the email-change confirmation-pending case, preserve and surface
EmailChangeConfirmation.message and code; retain the existing session user-email
handling for the successful verification case.
- Around line 40-48: Update the “Before” migration example to access
AuthResponse.user non-optionally as response.user.email, while preserving
optional chaining on VerifyOTPResponse.session in the “After” example.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27900e93-8631-440b-949d-c781e7416d0b
📒 Files selected for processing (7)
Sources/Auth/AuthClient.swiftSources/Auth/Types.swiftTests/AuthTests/AuthClientTests.swiftTests/AuthTests/AuthResponseTests.swiftTests/AuthTests/Resources/signup-response.jsonTests/IntegrationTests/AuthClientIntegrationTests.swiftV3_MIGRATION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Tests/AuthTests/AuthClientTests.swift
Contributor
|
The following capabilities are marked
These may have been renamed, removed, or never registered. Please update the capability matrix. |
…r user
verifyOTP(type: .emailChange) previously threw a DecodingError when
GoTrue's /verify endpoint returned the `{ msg, code }` body sent for the
first of the two confirmations required by a secure email change,
since AuthResponse could only decode a Session or a User.
AuthResponse gains a `.none` case for this shape, and `user` is now
`User?` instead of `User` so `(session: nil, user: nil)` is
representable.
BREAKING CHANGE: `AuthResponse.user` is now `User?` instead of `User`.
Update call sites that access `response.user` directly to handle the
optional (e.g. `response.user?.email`).
Linear: SDK-1022
…ne doc Addresses code review feedback on #1088 (SDK-1022): the import ordering in AuthClientTests.swift had regressed to a non-swift-format-canonical layout, and the `.none` doc didn't make clear it's a catch-all for any unrecognized response shape, not just the email-change confirmation case.
The tests re-created while resolving the rebase conflict onto main still used XCTest assertions and a duplicated import, left over from before main's migration to Swift Testing.
…reaking changes AuthResponse.user changed from User to User? in this release; document the migration and encode the requirement in AGENTS.md so future breaking changes ship with a guide.
Rename AuthResponse Migration Guide.md to Auth Migration Guide.md and restructure it as a per-module doc with a section per breaking change, so future Auth breaking changes get a new section instead of a new file. Update the AGENTS.md convention to match.
The previous commit renamed the file but left the old single-change title and heading levels; nest the AuthResponse change under its own section so future Auth breaking changes append sections instead of new files, matching the updated AGENTS.md convention.
… versioned naming Replace the AGENTS.md migration-guide paragraph with a writing-migration-guides skill (.claude/skills/), modeled on supabase-flutter's MIGRATION.md: name the exact symbol that changed, explain why, show before/after code, and call out whether a break is a compile error or silent. Rename Auth Migration Guide.md to Auth V3 Migration Guide.md to adopt the <Module> V<N> Migration Guide.md convention, tying the file to the major version it targets.
Replace the per-module docs/migrations/<Module> V<N> Migration Guide.md convention with a single root V<N>_MIGRATION.md covering every module, matching how release notes actually ship (one major-version bump at a time, not one per module). Update the writing-migration-guides skill to match.
The writing-migration-guides skill only covers how to write the entry once you know one is needed; nothing stated that breaking changes require one in the first place. State the requirement next to the existing breaking-change commit convention.
…sponse
GoTrue's /verify endpoint returns a bare `{ msg, code }` body for the
first of the two confirmations required by a secure email change, a
shape AuthResponse can't represent since it only holds a Session or a
User. AuthResponse is shared with signUp and the Passkey methods,
neither of which can ever produce that shape, so growing it there
would have leaked a case only verifyOTP can trigger onto every caller.
Revert AuthResponse to its original Session/User-only shape (user is
non-optional again) and give verifyOTP its own VerifyOTPResponse type
with the two shapes it can actually return: .session and
.emailChangeConfirmationPending(EmailChangeConfirmation), the latter
carrying the message/code GoTrue actually sends instead of discarding
it.
Verified against the supabase/auth source: only
mail.EmailChangeVerification's first confirmation ever emits the bare
body; signUp, the other verifyOTP types, and the Passkey completion
endpoint always return a session or a plain user shape.
BREAKING CHANGE: verifyOTP and its overloads now return
VerifyOTPResponse instead of AuthResponse. See V3_MIGRATION.md.
Linear: SDK-1022
The "Before" line still showed AuthResponse.user as optional, stale from an earlier iteration of this design; AuthResponse.user is non-optional. The "After" line only accessed .session, never showing the .emailChangeConfirmationPending case the whole change is about.
…e test Two CI failures on this PR: - "Check public API against capability matrix": VerifyOTPResponse and EmailChangeConfirmation were never registered. Add them as supporting_symbols under auth.sign_in.verify_otp. - "Integration Tests (Linux)": verifyOTPForSecureEmailChange failed because GOTRUE_MAILER_AUTOCONFIRM=true in the shared integration test project skips GoTrue's whole single-confirmation branch in emailChangeVerify, so the first confirmation completes the change immediately instead of returning the pending shape. Every other integration test relies on that autoconfirm setting to avoid confirming emails, so flipping it project-wide isn't an option. Add a second, minimal Supabase project (Tests/IntegrationTests/supabase-secure-email-change) with only that one setting flipped, and run this one test against it in its own CI step, skipping it in the main run. Documented in AGENTS.md.
…enHash Confirmed against the running local stack: GoTrue's admin/generate_link always returns hashedToken computed from the request's current email (crypto.GenerateTokenHash(params.Email, otp)), even for type=email_change_new, where the token actually stored server-side (EmailChangeTokenNew) is computed from the new email instead. The hashedToken field is simply wrong for that one case; emailOTP is not affected since GoTrue rehashes it against the email passed to /verify. Switch to the email+token overload for that confirmation.
v14.1.0's binary XCFramework releases are unreachable from GitHub
right now (downloadError("Could not connect to the server.")),
failing package resolution on every xcodebuild (legacy) job. Bump
past it to 18.1.0, which resolves and downloads cleanly.
…egacy)" This reverts commit d677e72.
grdsdev
force-pushed
the
fix/sdk-1022-verifyotp-email-change-parity
branch
from
August 13, 2026 09:20
2cc6b43 to
c4bcffb
Compare
grdsdev
changed the title
spydon
approved these changes
Aug 13, 2026
3 tasks
4 tasks
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
3 participants
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
verifyOTP(type: .emailChange)threw an uncaughtDecodingErrorwhen GoTrue's/verifyendpoint returned{ msg, code }— the body sent for the first of the two confirmations required by a secure email change.AuthResponsecould only decode aSessionor aUser, so any other shape crashed instead of returning a usable result.An earlier version of this fix gave
AuthResponseitself a.nonecase and madeuseroptional. ButAuthResponseis shared bysignUpand the Passkey methods, neither of which can ever produce that shape — every caller of those unrelated methods would have had to handle a case onlyverifyOTPcan trigger. Verified against thesupabase/authsource directly: onlyverifyOTP(type: .emailChange)'s first confirmation can ever produce this body.signUpcan return either a session or a plain user (confirmation pending); the otherverifyOTPtypes and the Passkey completion endpoint always return a session.Changes
Sources/Auth/Types.swift:verifyOTPand its overloads now return a newVerifyOTPResponsetype instead ofAuthResponse. It has exactly the two shapes GoTrue's/verifycan produce for it:.session(Session)and.emailChangeConfirmationPending(EmailChangeConfirmation), the latter carrying themessage/codeGoTrue actually sends (decoded losslessly, instead of being discarded).AuthResponseitself is unchanged from its currently-released shape.V3_MIGRATION.md: new root migration guide for this upcoming major version, covering every module. Documents this breaking change with before/after examples..claude/skills/writing-migration-guides/: new skill capturing the migration-guide format and convention (modeled onsupabase-flutter'sMIGRATION.md), since every breaking change now requires one (seeAGENTS.md).sdk-compliance.yaml: registeredVerifyOTPResponse/EmailChangeConfirmationas supporting symbols underauth.sign_in.verify_otp.VerifyOTPResponsedecoding against mocked fixtures, plus a new integration test (verifyOTPForSecureEmailChange) that drives the real two-step confirmation flow viaadmin.generateLink, since the shared integration-test project runs withauth.email.enable_confirmations = falseand can't reach this code path.Tests/IntegrationTests/supabase-secure-email-change/: a second, minimal local Supabase project with just that one setting flipped, so the new integration test runs against real GoTrue behavior without affecting the rest of the suite..github/workflows/ci.yml/AGENTS.md: wired the isolated project into CI as its own step, and documented running it locally.Breaking change
verifyOTPand its overloads (verifyOTP(email:token:type:...),verifyOTP(phone:token:type:...),verifyOTP(tokenHash:type:)) now returnVerifyOTPResponseinstead ofAuthResponse. SeeV3_MIGRATION.mdfor the full migration steps.Root cause
internal/api/verify.goinsupabase/auth: GoTrue returns a bare{msg, code}body (not aSession/Usershape) specifically for the first of the two confirmations required by a secure email change (mail.EmailChangeVerification, whenSecureEmailChangeEnabledandEmailChangeConfirmStatus == zeroConfirmation).AuthResponse.init(from:)had no case for that shape and threw.Test plan
Tests/AuthTests/AuthResponseTests.swift—VerifyOTPResponsedecodes the{msg,code}shape into.emailChangeConfirmationPending;AuthResponsecorrectly throws on it rather than silently swallowing itTests/AuthTests/AuthClientTests.swift—verifyOTPForEmailChangeSingleConfirmationTests/IntegrationTests/AuthClientIntegrationTests.swift—verifyOTPForSecureEmailChange, exercises the real two-step confirmation end-to-end against a live GoTrue instance (including a real bug found along the way inadmin/generate_link's response foremail_change_new)swift test --filter "AuthTests|SupabaseTests", 256 tests)Integration Tests (Linux)CI job greenvalidate / Check public API against capability matrixCI job greenLinear
Closes SDK-1022