feat: add passkey-first authentication and account passkey management#154
feat: add passkey-first authentication and account passkey management#154
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
nodewarden | 607c8c3 | Mar 30 2026, 05:21 PM |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 964b12224f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| id: generateUUID(), | ||
| userId, | ||
| credentialId, | ||
| publicKey: 'client-asserted', |
There was a problem hiding this comment.
The registration flow stores a hardcoded publicKey value ('client-asserted') instead of extracting and validating the credential public key from the attestation response. Because of this, newly created passkeys are not cryptographically bound to a real authenticator, and later login verification cannot be done correctly. This undermines the security guarantees of passkey enrollment and makes spoofed credential registration possible for any authenticated session.
Useful? React with 👍 / 👎.
| const parsedClientData = parseClientDataJSON(clientData); | ||
| const origin = new URL(request.url).origin; | ||
| if (!parsedClientData || parsedClientData.type !== 'webauthn.get' || parsedClientData.challenge !== challengeRecord.challenge || parsedClientData.origin !== origin) { | ||
| return identityErrorResponse('Passkey assertion invalid', 'invalid_grant', 400); |
There was a problem hiding this comment.
The login flow accepts a passkey assertion when clientDataJSON challenge/type/origin match, but it never verifies authenticatorData + signature against the stored credential key before minting access and refresh tokens. In this state, possession of a valid credentialId is enough to attempt forged assertions without proving control of the authenticator private key, which is a direct authentication bypass risk for passkey-based sign-in.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds passkey-first (WebAuthn) authentication and account-level passkey management so the web client can sign in/unlock and optionally receive vault unlock keys after a passkey login.
Changes:
- Adds passkey credential/challenge persistence (new tables + storage helpers) and new passkey registration/login/management handlers + routes.
- Extends token response/types to optionally return
VaultKeysafter passkey authentication. - Adds webapp WebAuthn helpers and UI wiring for passkey login/unlock and passkey CRUD in Settings.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| webapp/src/lib/types.ts | Extends token success typing to include VaultKeys. |
| webapp/src/lib/passkey.ts | Adds browser-side base64url + WebAuthn create/get helpers. |
| webapp/src/lib/app-auth.ts | Adds performPasskeyLogin flow and maps VaultKeys into session. |
| webapp/src/lib/api/auth.ts | Adds passkey API calls for login and account passkey CRUD. |
| webapp/src/components/SettingsPage.tsx | Adds account passkey management UI (create/rename/delete). |
| webapp/src/components/AuthViews.tsx | Adds passkey login/unlock buttons when supported. |
| webapp/src/components/AppMainRoutes.tsx | Plumbs passkey props/handlers into Settings route. |
| webapp/src/App.tsx | Wires passkey login, TOTP fallback handling, and passkey management actions/queries. |
| src/utils/passkey.ts | Adds server-side base64url + challenge + clientDataJSON parsing helpers. |
| src/types/index.ts | Extends TokenResponse and introduces PasskeyCredential type. |
| src/services/storage.ts | Implements passkey storage/challenge helpers; bumps schema version. |
| src/services/storage-schema.ts | Adds D1 schema statements for passkey tables + indexes. |
| src/router-public.ts | Exposes public passkey login endpoints. |
| src/router-authenticated.ts | Exposes authenticated passkey management endpoints. |
| src/handlers/passkeys.ts | Implements passkey registration/login flows and CRUD endpoints. |
| migrations/0001_init.sql | Adds passkey tables/indexes to initial migration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function handlePasskeyLogin() { | ||
| if (pendingAuthAction) return; | ||
| if (!passkeySupported()) { | ||
| pushToast('error', '当前浏览器不支持 Passkey'); | ||
| return; | ||
| } | ||
| setPendingAuthAction('login'); | ||
| try { | ||
| const result = await performPasskeyLogin(loginValues.email); |
There was a problem hiding this comment.
handlePasskeyLogin() always uses loginValues.email when calling performPasskeyLogin(). In the locked/unlock flow the canonical email is profile?.email || session?.email (emailForLock), and loginValues.email may be empty/stale; this can break passkey unlock or trigger the empty-allowCredentials path.
| const login = pendingTotp | ||
| ? await performTotpLogin(pendingTotp, totpCode, rememberDevice) | ||
| : (await (async () => { | ||
| const passkeyResult = await performPasskeyLogin(loginValues.email, totpCode); | ||
| if (passkeyResult.kind !== 'success') throw new Error(t('txt_totp_verify_failed')); | ||
| return passkeyResult.login; | ||
| })()); |
There was a problem hiding this comment.
When passkey login requires TOTP, handleTotpVerify() re-runs performPasskeyLogin() which will prompt for a passkey assertion again. This is inefficient UX and forces a second WebAuthn ceremony; consider preserving the first assertion (or keeping the login challenge/pending state server-side) so the TOTP step can complete without repeating passkey auth.
| {props.passkeySupported && ( | ||
| <button type="button" className="btn btn-secondary full" onClick={props.onSubmitPasskey} disabled={loginBusy}> | ||
| <Fingerprint size={16} className="btn-icon" /> | ||
| Passkey 登录 |
There was a problem hiding this comment.
Passkey button labels are hard-coded ("Passkey 登录") instead of using the existing i18n mechanism (t('...')). Since the surrounding UI strings are localized via t(), these should be moved into the translation messages for consistency.
| Passkey 登录 | |
| {t('txt_passkey_login')} |
| export async function listAccountPasskeys(authedFetch: AuthedFetch): Promise<AccountPasskey[]> { | ||
| const resp = await authedFetch('/api/accounts/passkeys'); | ||
| if (!resp.ok) throw new Error('Failed to load passkeys'); | ||
| const body = (await parseJson<{ data?: AccountPasskey[] }>(resp)) || {}; | ||
| return Array.isArray(body.data) ? body.data : []; | ||
| } | ||
|
|
||
| export async function registerAccountPasskey(authedFetch: AuthedFetch, name: string, session: SessionState): Promise<void> { | ||
| const beginResp = await authedFetch('/api/accounts/passkeys/begin-registration', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: '{}', | ||
| }); | ||
| if (!beginResp.ok) throw new Error('Failed to start passkey registration'); | ||
| const begin = (await parseJson<{ challengeId: string; publicKey: Record<string, any> }>(beginResp)) || {}; | ||
| if (!begin.challengeId || !begin.publicKey) throw new Error('Invalid registration challenge'); |
There was a problem hiding this comment.
Passkey-related API helpers throw hard-coded English error strings (e.g., "Failed to load passkeys" / "Failed to start passkey registration") while this module otherwise uses t('txt_...') for user-facing errors. For consistent localization, these messages should use t() keys and be added to the i18n catalog.
| return jsonResponse({ | ||
| challengeId, | ||
| publicKey: { | ||
| challenge, | ||
| rpId: rpIdFromUrl(request.url), | ||
| timeout: 60000, | ||
| userVerification: 'preferred', | ||
| allowCredentials: passkeys.map((pk) => ({ type: 'public-key', id: pk.credentialId })), | ||
| }, |
There was a problem hiding this comment.
begin-login always includes allowCredentials, and when the user/email is unknown this becomes an empty array. Passing an empty allowCredentials to navigator.credentials.get() typically prevents discoverable credentials from being used; omit allowCredentials when you don't have a credential list (and consider rate-limiting this endpoint).
| return jsonResponse({ | |
| challengeId, | |
| publicKey: { | |
| challenge, | |
| rpId: rpIdFromUrl(request.url), | |
| timeout: 60000, | |
| userVerification: 'preferred', | |
| allowCredentials: passkeys.map((pk) => ({ type: 'public-key', id: pk.credentialId })), | |
| }, | |
| const allowCredentials = | |
| passkeys.length > 0 ? passkeys.map((pk) => ({ type: 'public-key', id: pk.credentialId })) : undefined; | |
| const publicKey: { | |
| challenge: string; | |
| rpId: string; | |
| timeout: number; | |
| userVerification: 'preferred' | 'required' | 'discouraged'; | |
| allowCredentials?: Array<{ type: 'public-key'; id: string }>; | |
| } = { | |
| challenge, | |
| rpId: rpIdFromUrl(request.url), | |
| timeout: 60000, | |
| userVerification: 'preferred', | |
| }; | |
| if (allowCredentials) { | |
| publicKey.allowCredentials = allowCredentials; | |
| } | |
| return jsonResponse({ | |
| challengeId, | |
| publicKey, |
| challengeId: begin.challengeId, | ||
| name, | ||
| wrappedVaultKeys: JSON.stringify({ | ||
| symEncKey: session.symEncKey || '', | ||
| symMacKey: session.symMacKey || '', | ||
| }), | ||
| credential, |
There was a problem hiding this comment.
registerAccountPasskey uploads wrappedVaultKeys as JSON containing the session's decrypted symEncKey/symMacKey. Despite the name, these are not wrapped/encrypted here, which means the server will end up storing recoverable vault unlock keys; these should be encrypted/wrapped client-side (or via a proper WebAuthn extension) before being sent.
| const parsedClientData = parseClientDataJSON(clientData); | ||
| const origin = new URL(request.url).origin; | ||
| if (!parsedClientData || parsedClientData.type !== 'webauthn.get' || parsedClientData.challenge !== challengeRecord.challenge || parsedClientData.origin !== origin) { | ||
| return identityErrorResponse('Passkey assertion invalid', 'invalid_grant', 400); | ||
| } | ||
|
|
||
| const credential = await storage.getPasskeyByCredentialId(credentialId); | ||
| if (!credential) return identityErrorResponse('Passkey not recognized', 'invalid_grant', 400); |
There was a problem hiding this comment.
Passkey login currently trusts clientDataJSON + credentialId without verifying authenticatorData/signature against the stored credential public key (and without checking the sign counter / rpIdHash / flags). As-is, anyone who can obtain a credentialId can forge a login by crafting clientDataJSON for the issued challenge.
| counter: 0, | ||
| transports: null, | ||
| name: name.slice(0, 100), | ||
| wrappedVaultKeys, | ||
| createdAt: now, |
There was a problem hiding this comment.
wrappedVaultKeys is persisted verbatim and then returned to the client as VaultKeys. In the current flow the webapp uploads decrypted symEncKey/symMacKey JSON, so this stores vault unlock keys server-side in recoverable form; these should be encrypted/wrapped (or avoided entirely) before storage to prevent DB compromise from yielding vault keys.
| if (path === '/identity/passkeys/begin-login' && method === 'POST') { | ||
| return handleBeginPasskeyLogin(request, env); | ||
| } | ||
|
|
||
| if (path === '/identity/passkeys/finish-login' && method === 'POST') { |
There was a problem hiding this comment.
The new passkey endpoints are exposed publicly without the rate limiting applied to other sensitive public routes (e.g., prelogin, password-hint, revocation). Consider adding enforcePublicRateLimit here to reduce credential enumeration/abuse risk, especially for begin-login.
| if (path === '/identity/passkeys/begin-login' && method === 'POST') { | |
| return handleBeginPasskeyLogin(request, env); | |
| } | |
| if (path === '/identity/passkeys/finish-login' && method === 'POST') { | |
| if (path === '/identity/passkeys/begin-login' && method === 'POST') { | |
| const blocked = await enforcePublicRateLimit( | |
| 'public-sensitive', | |
| LIMITS.rateLimit.sensitivePublicRequestsPerMinute, | |
| ); | |
| if (blocked) return blocked; | |
| return handleBeginPasskeyLogin(request, env); | |
| } | |
| if (path === '/identity/passkeys/finish-login' && method === 'POST') { | |
| const blocked = await enforcePublicRateLimit( | |
| 'public-sensitive', | |
| LIMITS.rateLimit.sensitivePublicRequestsPerMinute, | |
| ); | |
| if (blocked) return blocked; |
Motivation
Description
passkey_credentialsandpasskey_challengestables tomigrations/0001_init.sqlandsrc/services/storage-schema.ts, and bump storage schema version insrc/services/storage.ts.StorageService(createPasskeyChallenge,consumePasskeyChallenge,listPasskeysByUserId,getPasskeyByCredentialId,createPasskey,updatePasskeyName,deletePasskey,touchPasskeyUsage).src/handlers/passkeys.tswith endpoints for passkey registration/login flows and management, expose public endpoints (/identity/passkeys/*) insrc/router-public.tsand authenticated management endpoints (/api/accounts/passkeys*) insrc/router-authenticated.ts.TokenResponsewithVaultKeysand addPasskeyCredentialtype insrc/types/index.ts, and addsrc/utils/passkey.tsto handle base64url/challenge parsing on server-side.webapp/src/lib/passkey.tsclient helpers, extendwebapp/src/lib/api/auth.tswithloginWithPasskey,registerAccountPasskey,listAccountPasskeys,renameAccountPasskey,deleteAccountPasskey, and wireperformPasskeyLogininwebapp/src/lib/app-auth.ts.webapp/src/components/AuthViews.tsxandwebapp/src/App.tsx, add account Passkey management UI (create/rename/delete) inwebapp/src/components/SettingsPage.tsx, and surface passkey data inAppMainRoutesandAppprops.VaultKeysin the token response so the web app can unlock without the master password.Testing
npm run build(Vite build completed).npx tsc --noEmitand it passed without errors.Codex Task