feat: Self-serve recovery for a lost passkey - #134
Merged
Merged
Conversation
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.
Contributor
🧪 Test Summary
|
…URL fragment Contract v2 (zitadel-provider #138) inverts who mints the passkey registration code. auth-ui used to mint it via passkeyRegisterLink and hand both halves to the webhook; now it asks for a mail and is told which code was minted. The request carries {userId, returnTo, requestedBy} — sending codeId/code is a 400 — and the 200 answers {codeId}. The raw code no longer passes through auth-ui at all. It reaches the user only through the Email body and the mail link's fragment (/recover/complete?userId&codeId#code=<code>). A fragment is never part of a request line, so the code cannot land in an access log, a proxy, an APM trace or a Referer — which a query parameter could never promise. /recover/complete's loader therefore no longer reads a code, and deliberately does not fall back to the query: doing so would quietly make the leaky link shape work again. The browser lifts the code into a hidden field and strips the fragment from the address bar with replaceState. The typed-code field is what SSR ships, so a reader with JS off — or one whose mail client dropped the fragment — still has somewhere to key in the code the mail prints. The hidden input replaces it only once a fragment is found. G7 is unchanged. The send moved ahead of the seal, so a refused delivery (the webhook's 429 cooldown included) has nothing to seal; it gets a filler ticket, which is the same fixed width as a real one. The response — status, body, Set-Cookie length — stays byte-identical, and the branch is on delivery, never on account state. A filler simply has no code to open, exactly as the typed-code path already answers a wrong code. postMailWebhook now resolves {status, body} so the caller can read the minted codeId. The status decides first and the body only after: a failing webhook that echoed a codeId-shaped diagnostic must not be mistaken for a mail that was sent. The body is length-capped, parsed, and never logged — a misconfigured endpoint could put anything in it, the code included.
/recover/complete's URL carries userId and codeId. The code itself is in the fragment, which a Referer never includes, but the rest should not travel either — not to an image host, not to a link the reader follows out of the page. It has to be a headers export. React Router's getDocumentHeaders carries only Set-Cookie off a loader's own headers onto a document response, so the same header returned from the loader's data() would have been dropped without a sound. That is what the spec pins: the export existing is the load-bearing fact, not the string it returns.
Contributor
🧪 Test Summary
|
Contributor
🧪 Test Summary
|
Recovery ended by sending the user to /login with a surviving passkey hint. The hint names an account the browser holds no session for, which breaks both passkey paths there: the shortcut bounces with no session to arm a challenge, and conditional-UI autofill arms a discoverable request whose arming mints a session, so the next discover answers 409 already_signed_in. The button spun and no prompt appeared. routes/signup/success.tsx already clears the hint for the same reason; both recovery exits now do too, and the redirect still carries loginName so nothing is lost. The check-email screen moves its explanatory text into AuthCard's description slot and wraps the start-over link the way every other screen does, which fixes the centring and the spacing that set it apart. The post-recovery notice on /login drops to one sentence and picks up the muted notice styling used on the accounts screen.
Contributor
🧪 Test Summary
|
yahyafakhroji
force-pushed
the
fix/otp-signin-guard-consistency
branch
from
September 25, 2026 07:52
81d0a8d to
60bd3c8
Compare
Only app/modules/i18n/locales/en.po conflicted. It is generated, so it was regenerated from source rather than hand-merged.
Removes the 12 Cypress specs this branch added and returns the five harness and support files it touched to main's version, to cut the PR roughly in half for review. This is deliberate, not an oversight. It also removes the only automated enforcement of four properties that came out of the security review: the identical-response-and-Set-Cookie-length shape of every /recover exit, that sendRecoveryMail never throws and never varies the response, that the code travels in the URL fragment and is stripped from the address bar, and that startup refuses a half-configured mTLS client. Nothing in CI will catch a regression of those now.
Contributor
🧪 Test Summary
|
yahyafakhroji
changed the title
yahyafakhroji
changed the title
yahyafakhroji
requested review from
gaghan430,
kevwilliams,
mattdjenkinson and
mdobush
September 25, 2026 09:12
mattdjenkinson
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.
Problem
A user whose only passkey is gone cannot sign in, and has no way back in on their own. There is no self-serve path to offer them.
Solution
A
/recoverflow that mails a passkey registration link. The user enters their email, opens the link on the device they want to sign in with, and enrolls a new passkey. Their old passkey stays listed and can be removed afterwards.The authn-webhook mints the code rather than auth-ui, and the code travels in the URL fragment, so it never reaches an access log or a Referer header. Every exit from the request step returns the same response, so the page cannot be used to learn which addresses have accounts.
Off by default behind
AUTH_ACCOUNT_RECOVERY_ENABLED. Staging sets it to true.The recovery specs were deliberately left out of this PR to keep the diff reviewable.
Refs #112