fix(login): refuse hidden email-OTP sign-in everywhere it was still reachable (C10) - #130
Merged
Merged
Conversation
Contributor
🧪 Test Summary
|
5 tasks
yahyafakhroji
force-pushed
the
fix/otp-signin-guard-consistency
branch
from
September 9, 2026 13:16
f13cd12 to
81d0a8d
Compare
Contributor
🧪 Test Summary
|
This was referenced Sep 9, 2026
method-chooser.cy.ts's "known user → 302 to /login/verify/email" case was asserting the hole: #128 hid the intent in the UI while the action still honoured a crafted POST. Updated to expect the 400 the branch now returns, with no redirect and no ceremony cookie. It flips back to the 302 when EMAIL_OTP_SIGNIN_ENABLED does.
yahyafakhroji
force-pushed
the
fix/otp-signin-guard-consistency
branch
from
September 25, 2026 07:52
81d0a8d to
60bd3c8
Compare
Contributor
🧪 Test Summary
|
gaghan430
approved these changes
Sep 25, 2026
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
2 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.
What
#128 hid email-OTP sign-in behind
EMAIL_OTP_SIGNIN_ENABLED = false, but only in the login view and the method chooser. Three readers of "isotp_emailusable" were never updated, so the flow stayed reachable server-side:shared/usable-methods.tscountedotp_emailas a usable method whenever delivery was on, so the last-method guard inremoveUserPasskeylet a passkey + otpEmail account remove its only passkey and strand itself.login/login-decision.tsstill listedotp_emailas an available primary method after the identifier step./login'sintent=email-linkaction still executed on a direct POST.This PR introduces one predicate,
isEmailOtpSignInUsable(emailDeliveryEnabled)inresources/login/email-otp-signin.ts, and applies it at all three sites.intent=email-linknow answers 400 while OTP sign-in is hidden. The chooser reads availability through the same predicate.Why
Phase C of the passkey program (#112, item C10) decided that email OTP is a recovery ceremony, never a login. Hidden was not disabled. This must land before
AUTH_EMAIL_DELIVERY_ENABLEDflips in production, or the passkey-removal hole opens the moment mail is wired.Tests
Six new Cypress cases across
email-otp-signin.cy.ts,login-decision.cy.ts,passkeys.service.cy.ts,email-otp-signin-hidden.cy.ts(the direct-POST refusal, incl. no session cookie and nocreateSession), and the chooser spec. Full suite 773/773; enumeration-parity unchanged. Theen.podiff is reference-line churn only.Known, out of scope
resources/reauth/reauth.service.ts:130still gatesotp_emailon delivery alone and dispatches a real OTP challenge for the sudo re-auth ceremony. Pre-existing. Re-auth is step-up for an already-signed-in user, not sign-in, so this is left as a documented exclusion rather than gated here; a follow-up can decide.login-view.ts:48-50inlines the same condition instead of calling the predicate; functionally identical today.CHANGELOG.mdentry in this PR. The visible effect is that an account whose only other method is hidden email OTP can no longer remove its last passkey; say the word and I will add an Unreleased line.Checklist
bun run typecheckpassesbun run lint:cipassesbun run test:unitpasses (773/773)bun run lint:boundariespasses (316 modules)Refs #112.