Passkey management page, and a registration bypass fix - #644
Merged
Conversation
The list sat in the account security page's label/value/action grid, so every entry's kebab menu floated in the middle column aligned with nothing, and a row that had to hold a whole list left no room for the detail you need before removing one: which device a passkey belongs to, when it was added, whether it is still in use. It now lives at /admin/account/passkeys, the way the password already has a page of its own, as a table with the kebab in a trailing actions column like every other list in the admin UI. Each row carries a device icon and label derived from the transports the authenticator reported at registration, when it was added, and when it was last used. The security page keeps a summary row, because a page that answers "what can sign in as me" is wrong without passkeys in it, and that row links to the page rather than repeating its contents. Sign-ins are stamped now so "last used" is answerable: updateCounter became recordAuthentication and writes the timestamp alongside the counter it already stored. A credential registered before that carries no stamp, so the column shows "-" rather than claiming it has never been used - a passkey in daily use labelled unused would invite removing the wrong one. The 2FA row says what passkeys do to the guarantee its badge implies. Passkey sign-in sets requireTotp false, so "Authenticator app" promised a code on every sign-in and "Not configured" suggested the password was the only way in; both are qualified when passkeys exist. Sign-in methods is a status board, so its actions stopped competing with it. Every row action is neutral, error is kept for the one that weakens the account (disabling 2FA) and primary for creating the first password, which is the only thing the page tells an unprotected instance to do. With the color gone the leading icon is what tells them apart, and they share one width so the column reads as a column. The password row's value became a badge like the others instead of a row of asterisks. The SSO row's two buttons opened documentation rather than configuring anything, so they are one "Setup instructions" button opening one dialog with a tab per provider, OpenID Connect first because it covers every compliant provider with Okta as a special case of it. The settings rows were a bare grid-cols-3 at every width. At 390px each column is about 110px, narrow enough that multi-word button labels wrapped out of their own fixed-height boxes; they stack below the sm breakpoint now. Supporting changes: ui/btn takes a disabled param, which also retires the hand-rolled disabled-button-inside-a-tooltip on the account page; a new ui/empty-state partial for a card that brings its own header and action; updateRelativeTimes skips an unparsable stamp, which used to throw and take the .local-time and drop-target wiring below it down as well; and a dialog containing tabs closes on Escape again, which it stopped doing because FlyonUI resolves the handler from the event target and a tab strip reports itself opened while defining no Escape handler of its own. Covered by test/e2e/pages-admin.spec.js: the summary row and its count, the 2FA note following a removal, the passkeys page empty state and register dialog, and the list itself with credentials seeded through a new bootstrap helper. Registering one for real needs an authenticator the browser does not have, and the e2e serviceUrl names a different host than the one under test, so the table assertions could never have run without seeding.
A challenge is a bare nonce, and nothing in the signed response an authenticator returns says which ceremony the server issued it for; that binding is the server's job and it was not being done. storeChallenge wrote every challenge to one keyspace and consumeChallenge accepted any id, so the two ceremonies were interchangeable. They are not equally trusted. Registering a passkey is gated behind a password confirmation, while POST /admin/passkey/auth/options is the sign-in endpoint and mints challenges unauthenticated by design. And the gate sat on register/options, the request before the one that stores the credential: register/verify checked only that an account password existed, never that the caller had proved it. So an actor holding an admin session but not the password could take a nonce from the sign-in endpoint, run a genuine navigator.credentials .create() ceremony against it, and register a permanent credential. verifyRegistrationResponse passes, because everything it checks is true - the type is webauthn.create, the challenge echoes, origin and RP ID match. Reproduced end to end against a live instance with a virtual authenticator before the fix, and blocked by it after. That upgrades revocable access into unrevocable access. A session expires and "sign out everywhere" kills it; a passkey is permanent, passwordless, survives both, and only a password change clears credentials. A stolen cookie, an XSS on any admin page or an unattended browser is enough, and so is an SSO session, which by design never holds the local password. Challenges are now keyed by purpose, so a registration can only spend a nonce that register/options minted and that route verifies the password before minting. An unknown or missing purpose throws rather than falling back to a shared key, because a store and a consume that both got it wrong would agree on it and quietly restore the bypass. register/verify also confirms the password itself now, before it consumes the challenge, so the check that authorizes a permanent credential sits on the request that writes one instead of one hop upstream. A mistyped password no longer burns the nonce. The password check in register/options moved below the Service URL and passkey-limit checks, so a request that was going to be refused anyway does not pay for a PBKDF2 verification first. verifyAdminPassword replaces the three copies of the verify-and-treat-a -throw-as-failure block in this file (sign-in, password change, and both registration steps), so how a failed password check is interpreted is decided in one place. Covered by test/passkeys-test.js: neither ceremony can consume the other's challenge, a refused consume does not burn the nonce, and a missing purpose throws. test/e2e/pages-admin.spec.js reproduces the attack itself, taking a real challenge from the unauthenticated sign-in endpoint and being refused by the registration endpoint, and separately asserts that a valid registration challenge without the password is refused too.
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
1 participant
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.
Reworks the account security page's passkey list, and fixes a passkey registration bypass found while doing it.
fix(ui): move the passkey list onto a page of its own
The list sat in the security page's label/value/action grid, so every entry's kebab floated in the middle column aligned with nothing, and a row holding a whole list had no room for the detail you need before removing a credential.
/admin/account/passkeys, the way the password already has its own page, as a table with the kebab in a trailing actions column. Each row shows a device icon and label (derived from the transports the authenticator reported), when it was added, and when it was last used.updateCounterbecamerecordAuthentication). A credential from before that shows-rather than claiming it has never been used.requireTotpfalse).sm; they were a baregrid-cols-3at every width, and at 390px multi-word button labels wrapped out of their own fixed-height boxes.ui/btntakes adisabledparam (retiring a hand-rolled twin on the account page), a newui/empty-statepartial,updateRelativeTimesskips an unparsable stamp, and a dialog containing tabs closes on Escape again.fix: bind WebAuthn challenges to the ceremony that minted them
storeChallengewrote every challenge to one keyspace andconsumeChallengeaccepted any id, so the two ceremonies were interchangeable. They are not equally trusted: registration is gated behind a password confirmation,POST /admin/passkey/auth/optionsis the sign-in endpoint and mints challenges unauthenticated by design, and the gate sat onregister/optionsrather than on the request that stores the credential.An actor holding an admin session but not the password could therefore take a nonce from the sign-in endpoint, run a genuine
navigator.credentials.create()ceremony against it, and register a permanent passwordless credential that outlives "sign out everywhere". Reproduced end to end against a live instance with a virtual authenticator before the fix, and blocked by it after.Challenges are now keyed by purpose, an unknown or missing purpose throws rather than falling back to a shared key, and
register/verifyconfirms the password itself before consuming the challenge.Verification
400 Challenge expired or invalid, list unchanged.test/passkeys-test.js; the attack itself plus the unconfirmed-password refusal intest/e2e/pages-admin.spec.js; the passkeys page, its list, the summary row and the SSO dialog tabs in the admin e2e spec.🤖 Generated with Claude Code
https://claude.ai/code/session_01NspqTRiN8AAL84S54NJERZ