Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Stale add-passkey ceremonies can survive key replacement, and noncanonical origins can pass startup validation.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Adds passkey registration and login to relay accounts, retaining eight-word keys for recovery.
Changes:
- Adds WebAuthn server routes, credential storage, and challenge verification.
- Adds passkey account UI and browser/authenticator tests.
- Updates deployment configuration, packaging, and documentation.
| File | Description |
|---|---|
wasm/web/passkeys.mjs |
Implements passkey ceremonies and routes. |
wasm/web/accounts.mjs |
Adds credential persistence and key-reset handling. |
wasm/web/accountui.js |
Adds passkey account workflows. |
wasm/web/accountstest.mjs |
Tests server-side passkey behavior. |
wasm/web/jamtest.mjs |
Tests browser passkey workflows. |
wasm/web/relay.mjs |
Integrates passkeys into the relay. |
wasm/web/relaytest.mjs |
Checks disabled passkey health state. |
wasm/web/style.css |
Styles the passkey list. |
wasm/web/package.json |
Adds WebAuthn dependencies. |
wasm/web/package-lock.json |
Locks new dependencies. |
docs/RELAY.md |
Documents passkey deployment and behavior. |
docs/JAM.md |
Updates account capabilities. |
docs/JAM_BACKLOG.md |
Records passkey account support. |
docker/relay.Dockerfile |
Packages passkey server code. |
docker/relay.Dockerfile.dockerignore |
Includes the passkey module. |
docker/compose.yaml |
Exposes passkey configuration. |
.github/workflows/ci.yml |
Updates account-test documentation. |
Files not reviewed (1)
- wasm/web/package-lock.json: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
WebAuthn through @simplewebauthn/server, under /api/account/passkey/. A passkey logs in to the same s_ session a key does. - register-options/register-verify make the account, its first passkey and its key at once; the key is the recovery code. - add-options takes the session and the key: a passkey added from a borrowed browser would outlast every session the owner ends. add-verify must come from the same session. - login-options is discoverable (no allowCredentials, UV preferred); login-verify is on the key-request limit. - list and remove take a session; removing ends no sessions. - Challenges are in memory, single use, 5 minutes, bound to their purpose and, for add, to the account and session. - A count that did not go up is refused, checked again as it is written. Banned accounts are refused; a deleted account's passkeys go with it. - A credentials table (migration 2). WebAuthn responses may be up to 16 KiB; every other body stays capped at 1 KiB. - PASSKEY_RP_ID names the site's domain and CORS_ORIGIN, which must be on it, is the one origin a response is taken from. Unset, there are no passkeys, and the health line's `passkeys' is null. accountstest drives it with a software ES256 authenticator: register, login, another origin or RP ID, replayed, expired and cross-purpose challenges, a count that went back, adding, removing, bans and deletes.
Where the relay has passkeys and the page is on their RP ID, Create account makes a passkey and then shows the key as the recovery key, in the same password-manager form. "Create with a key only" and the key login stay. "Log in with a passkey", and the login form's handle field offers passkeys in autofill where the browser can. Logged in, the passkeys are listed with when each was added and last used, with Add (which takes the key) and Remove. A ceremony the person cancels says nothing. jamtest: a Chromium page on localhost makes an account with a virtual authenticator's passkey, logs in through autofill, then with the button on a reload. Firefox has no virtual authenticator Playwright drives.
Whoever had the old key could have added any of them, so replacing the key removes them all, in the same transaction, and the response says how many (`passkeysRemoved'). The dialog says so, and once the new key is saved the passkey form has it filled in, so Add is one click. accountstest: after a new key the list is empty and the passkey no longer logs in. jamtest: the list is empty, and a passkey added with the filled-in key is listed.
A challenge is now an HMAC, by a key that lives as long as the process, over what it was issued for: its purpose, when it lapses, a nonce, the handle and user handle for a registration, and for an add the account and session it must come back with. Nothing is kept for one until it is answered, and then only until it lapses, so it is good for one try. No number of challenges asked for can push out anyone else's, where the map of outstanding ones evicted the oldest at 10000. - register-options is on the key limits, so asking whether a handle is taken is no cheaper than registering. - A registration whose id is not the credential id the authenticator signed is refused. - Transports that are not an array no longer fail the request; only known ones are kept, once each. accountstest: an add from another session of the same account is refused; of two logins with one count at once, one gets in; a login challenge survives twelve thousand more; the id mismatch; transports "usb", null and duplicates; register-options past the key limit.
The logged-out screen asks for a fresh autofill offer before its challenge lapses, shortly after one fails, and once a passkey ceremony of its own (Create account, Log in with a passkey) is over, which aborts the offer while it runs. A key filled in for adding a passkey is cleared once the passkey is added or the screen changes, and the Passkeys section says removing one logs nobody out, and that a new key does.
…n the page's host does
An add's challenge is signed over the account's key hash too, beside the account and session, so one asked for under the old key cannot add a passkey once the key is replaced. A challenge is taken only in its one base64url spelling: Node's decoder skips padding and stray characters, so `c', `c=' and `c.' were three tries at one challenge. RELAY.md: what not requiring user verification lets through, credProtect level 1 included, and that a passkey's session outlasts the passkey. accountstest: an add held across a key replacement is refused; the three spellings give one login.
…sskeys The first-time steps put PASSKEY_RP_ID in the .env beside CORS_ORIGIN, check that the health line names it, and say where to look when the container restarts over a pair the relay refuses. A restore brings back the passkeys as they were on the day of the backup. CI's image job runs its accounts container with PASSKEY_RP_ID too, and checks each health line's `passkeys'.
passkeyConfig checked only CORS_ORIGIN's host, and kept the string as the origin every response must name: `https://page.example.org/' would have passed, and refused every passkey, since a browser writes the origin bare. It now has to be exactly its URL's origin. accountstest: a trailing slash and a path are refused.
The virtual authenticator answers the autofill offer as soon as the logged-out screen makes it: the screen lived about 10 ms, and waiting for its key field, as the check did, missed it on CI and threw. The check now waits for a session other than the one logged out, with the logged-in screen back. A failure in the passkey case now says what the dialog's status line and the page's log end with.
This branch has not been deployed
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.
Stacked on #326.
Passkeys as the way to log in, with the 8-word key as the recovery code. A passkey login issues the same session as a key login, so nothing downstream changes.
Relay (
wasm/web/passkeys.mjs,@simplewebauthn/server)/api/account/passkey/:register-options/register-verifymake the account, its first passkey, its key and a session in one transaction;add-options(session and key) /add-verify(the same session);login-options(discoverable, user verification preferred) /login-verify;list;remove. All behind the account API's JSON, Origin, CORS and rate-limit rules, and absent withoutCORS_ORIGIN.PASSKEY_RP_ID(a domain the page's origin is on) andCORS_ORIGINas the one expected origin, which must be a bare origin (no path or trailing slash), both checked exactly. A mismatch stops the relay at start. WithoutPASSKEY_RP_IDthere are no passkeys.credentialstable (migration 2). A counter that went back is refused, again in the UPDATE so two logins on one count can't both pass; synced passkeys that report 0 work. A credential id already registered, or not the one the authenticator signed, is refused. Banned and deleted accounts are refused. Transports are kept as known values only.{ key, session, passkeysRemoved }): whoever had the old key could have added one. An add asked for under the old key is void.Page
Tests
PASSKEY_RP_IDand checks the health line'spasskeys, passes.Not covered
Deploying
PASSKEY_RP_ID=<site-domain>(optionallyPASSKEY_RP_NAME) to the relay's.envbesideCORS_ORIGIN, thendocker compose pull && docker compose up -d; migration 2 runs on start.curl 127.0.0.1:8787/should name the RP ID inpasskeys; a pair the relay refuses leaves the container restarting, anddocker logs thinksynth-relaysays why. RELAY.md's runbook has the steps.