diff --git a/package-lock.json b/package-lock.json index 64a8476..4e820b9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ ], "dependencies": { "@electric-sql/pglite": "^0.5.4", + "@simplewebauthn/server": "^13.3.2", "@supabase/supabase-js": "^2.110.6", "jose": "^6.2.3", "mimetext": "^3.0.28", @@ -859,6 +860,12 @@ "resolved": "web", "link": true }, + "node_modules/@hexagon/base64": { + "version": "1.1.28", + "resolved": "https://registry.npmjs.org/@hexagon/base64/-/base64-1.1.28.tgz", + "integrity": "sha512-lhqDEAvWixy3bZ+UOYbPwUbBkwBq5C1LAJ/xPC8Oi+lL54oyakv/npbA0aU2hgCsx/1NUd4IBvV03+aUBWxerw==", + "license": "MIT" + }, "node_modules/@img/colour": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@img/colour/-/colour-1.1.0.tgz", @@ -1353,6 +1360,12 @@ "@jridgewell/sourcemap-codec": "^1.4.14" } }, + "node_modules/@levischuck/tiny-cbor": { + "version": "0.2.11", + "resolved": "https://registry.npmjs.org/@levischuck/tiny-cbor/-/tiny-cbor-0.2.11.tgz", + "integrity": "sha512-llBRm4dT4Z89aRsm6u2oEZ8tfwL/2l6BwpZ7JcyieouniDECM5AqNgr/y08zalEIvW3RSK4upYyybDcmjXqAow==", + "license": "MIT" + }, "node_modules/@napi-rs/wasm-runtime": { "version": "1.1.6", "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.1.6.tgz", @@ -1516,6 +1529,174 @@ "url": "https://github.com/sponsors/Boshen" } }, + "node_modules/@peculiar/asn1-android": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-android/-/asn1-android-2.8.0.tgz", + "integrity": "sha512-skLbS+IOGv1lUgDqtChr8xvtvEr3HMse/JGBaL2r1J1o/n7a8wqOrovMtlRq/UXLhxvmLaONP67hwtshgzwfzA==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-cms": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-cms/-/asn1-cms-2.8.0.tgz", + "integrity": "sha512-NgekZOrSJFSBFLFoLfwePguAWAx7z1+f2TEsWFUMyiqqfntZ4+S/S5hzqME3q4pCA0iOsFKdwiQ35dwY24eVqA==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/asn1-x509": "^2.8.0", + "@peculiar/asn1-x509-attr": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-csr": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-csr/-/asn1-csr-2.8.0.tgz", + "integrity": "sha512-akbF8+uvleHs8sejNPQxwmVFuInAg6FMNHOwMILXfP518YfFJwdR3jr6oNUPOaEJfuEhn/vkNOCIT6ASUd4mbg==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/asn1-x509": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-ecc": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-ecc/-/asn1-ecc-2.8.0.tgz", + "integrity": "sha512-ohwlk+u9Rv2NOAY1c6MfHj45ATVF8R1DUN/WCgABiRtLi2ZftlZWZX7KvpAbU8v9xPcmoILfELeEABj/rn18AQ==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/asn1-x509": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-pfx": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-pfx/-/asn1-pfx-2.8.0.tgz", + "integrity": "sha512-5yof1ytoB++RQtaFbqSUJ8pxDJtZT6vbVqZ8XoJ61ph7UjNVvfFwAilnCodqkNsAodpy13gDhoxZXw00pghnyg==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-cms": "^2.8.0", + "@peculiar/asn1-pkcs8": "^2.8.0", + "@peculiar/asn1-rsa": "^2.8.0", + "@peculiar/asn1-schema": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-pkcs8": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-pkcs8/-/asn1-pkcs8-2.8.0.tgz", + "integrity": "sha512-qAKXtLpBEw9LqhKpjw3ajZSXlBur+ipW+y2ivVBQAG6F6qRx94yO+1ZR4mvw+YaCfKSaOzLeYEzsPaBp4SJELA==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/asn1-x509": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-pkcs9": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-pkcs9/-/asn1-pkcs9-2.8.0.tgz", + "integrity": "sha512-b5nDWCnkV60+cQ141D6sVVwK9nz64R5n3zSVnklGd+ECdkW2Ol3U1a6yYFlalpSOaD557yuJB64A+q42jG7lUQ==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-cms": "^2.8.0", + "@peculiar/asn1-pfx": "^2.8.0", + "@peculiar/asn1-pkcs8": "^2.8.0", + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/asn1-x509": "^2.8.0", + "@peculiar/asn1-x509-attr": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-rsa": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-rsa/-/asn1-rsa-2.8.0.tgz", + "integrity": "sha512-zHEUlCqB2mk7x2lxDwHHJy7hWZOPdGHVlsmITWKB5/PbQo61atbu9PJ/0r9dQNMwFzbKPXZ8uK8/91eUhRznSg==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/asn1-x509": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-schema": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-schema/-/asn1-schema-2.8.0.tgz", + "integrity": "sha512-7YT0U/ze0tF2QOBbE15gKZwy5tvgGyLRiRHLzhlbOpf7BT032oBSd0haZqXn5W6l26WLlu3dyxzjM+2638/z2Q==", + "license": "MIT", + "dependencies": { + "@peculiar/utils": "^2.0.2", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-x509": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-x509/-/asn1-x509-2.8.0.tgz", + "integrity": "sha512-N0CMuhWUzsWEVq6F1q9X6+VKUnWzSW+cSVg+aPaGGwDdbFoFWTYgin5MHwXgpWd6y9COMBxnfy/Qc+Xc7F0Zwg==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/utils": "^2.0.2", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/asn1-x509-attr": { + "version": "2.8.0", + "resolved": "https://registry.npmjs.org/@peculiar/asn1-x509-attr/-/asn1-x509-attr-2.8.0.tgz", + "integrity": "sha512-tHjkfS/qhMnmrlB2J9NhflQlQ7In3khO3CfmVrriOlpTeErY9ZIKOso1hQ5JQiyrJ7ShvqVPk7E5fQmbclkSKA==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-schema": "^2.8.0", + "@peculiar/asn1-x509": "^2.8.0", + "asn1js": "^3.0.10", + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/utils": { + "version": "2.0.3", + "resolved": "https://registry.npmjs.org/@peculiar/utils/-/utils-2.0.3.tgz", + "integrity": "sha512-+oL3HPFRIZ1St2K50lWCXiioIgSoxzz7R1J3uF6neO2yl1sgmpgY6XXJH4BdpoDkMWznQTeYF6oWNDZLCdQ4eQ==", + "license": "MIT", + "dependencies": { + "tslib": "^2.8.1" + } + }, + "node_modules/@peculiar/x509": { + "version": "1.14.3", + "resolved": "https://registry.npmjs.org/@peculiar/x509/-/x509-1.14.3.tgz", + "integrity": "sha512-C2Xj8FZ0uHWeCXXqX5B4/gVFQmtSkiuOolzAgutjTfseNOHT3pUjljDZsTSxXFGgio54bCzVFqmEOUrIVk8RDA==", + "license": "MIT", + "dependencies": { + "@peculiar/asn1-cms": "^2.6.0", + "@peculiar/asn1-csr": "^2.6.0", + "@peculiar/asn1-ecc": "^2.6.0", + "@peculiar/asn1-pkcs9": "^2.6.0", + "@peculiar/asn1-rsa": "^2.6.0", + "@peculiar/asn1-schema": "^2.6.0", + "@peculiar/asn1-x509": "^2.6.0", + "pvtsutils": "^1.3.6", + "reflect-metadata": "^0.2.2", + "tslib": "^2.8.1", + "tsyringe": "^4.10.0" + }, + "engines": { + "node": ">=20.0.0" + } + }, "node_modules/@rolldown/binding-android-arm64": { "version": "1.1.5", "resolved": "https://registry.npmjs.org/@rolldown/binding-android-arm64/-/binding-android-arm64-1.1.5.tgz", @@ -1780,6 +1961,25 @@ "dev": true, "license": "MIT" }, + "node_modules/@simplewebauthn/server": { + "version": "13.3.2", + "resolved": "https://registry.npmjs.org/@simplewebauthn/server/-/server-13.3.2.tgz", + "integrity": "sha512-KEDhfcGP1PAKRVSDjA3npTQFqS2b/srm+ipoNBNHdkzrHAlaRQUTE+a5f4ywsx6thxAw1NU2rYcLEY1949RGbQ==", + "license": "MIT", + "dependencies": { + "@hexagon/base64": "^1.1.27", + "@levischuck/tiny-cbor": "^0.2.2", + "@peculiar/asn1-android": "^2.6.0", + "@peculiar/asn1-ecc": "^2.6.1", + "@peculiar/asn1-rsa": "^2.6.1", + "@peculiar/asn1-schema": "^2.6.0", + "@peculiar/asn1-x509": "^2.6.1", + "@peculiar/x509": "^1.14.3" + }, + "engines": { + "node": ">=20.0.0" + } + }, "node_modules/@standard-schema/spec": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@standard-schema/spec/-/spec-1.1.0.tgz", @@ -2109,6 +2309,20 @@ "url": "https://opencollective.com/vitest" } }, + "node_modules/asn1js": { + "version": "3.0.10", + "resolved": "https://registry.npmjs.org/asn1js/-/asn1js-3.0.10.tgz", + "integrity": "sha512-S2s3aOytiKdFRdulw2qPE51MzjzVOisppcVv7jVFR+Kw0kxwvFrDcYA0h7Ndqbmj0HkMIXYWaoj7fli8kgx1eg==", + "license": "BSD-3-Clause", + "dependencies": { + "pvtsutils": "^1.3.6", + "pvutils": "^1.1.5", + "tslib": "^2.8.1" + }, + "engines": { + "node": ">=12.0.0" + } + }, "node_modules/assertion-error": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/assertion-error/-/assertion-error-2.0.1.tgz", @@ -3008,6 +3222,24 @@ "node": ">=0.10.0" } }, + "node_modules/pvtsutils": { + "version": "1.3.6", + "resolved": "https://registry.npmjs.org/pvtsutils/-/pvtsutils-1.3.6.tgz", + "integrity": "sha512-PLgQXQ6H2FWCaeRak8vvk1GW462lMxB5s3Jm673N82zI4vqtVUPuZdffdZbPDFRoU8kAhItWFtPCWiPpp4/EDg==", + "license": "MIT", + "dependencies": { + "tslib": "^2.8.1" + } + }, + "node_modules/pvutils": { + "version": "1.1.5", + "resolved": "https://registry.npmjs.org/pvutils/-/pvutils-1.1.5.tgz", + "integrity": "sha512-KTqnxsgGiQ6ZAzZCVlJH5eOjSnvlyEgx1m8bkRJfOhmGRqfo5KLvmAlACQkrjEtOQ4B7wF9TdSLIs9O90MX9xA==", + "license": "MIT", + "engines": { + "node": ">=16.0.0" + } + }, "node_modules/react": { "version": "19.2.7", "resolved": "https://registry.npmjs.org/react/-/react-19.2.7.tgz", @@ -3029,6 +3261,12 @@ "react": "^19.2.7" } }, + "node_modules/reflect-metadata": { + "version": "0.2.2", + "resolved": "https://registry.npmjs.org/reflect-metadata/-/reflect-metadata-0.2.2.tgz", + "integrity": "sha512-urBwgfrvVP/eAyXx4hluJivBKzuEbSQs9rKWCrCkbSxNv8mxPcUZKeuoF3Uy4mJl3Lwprp6yy5/39VWigZ4K6Q==", + "license": "Apache-2.0" + }, "node_modules/rolldown": { "version": "1.1.5", "resolved": "https://registry.npmjs.org/rolldown/-/rolldown-1.1.5.tgz", @@ -3277,6 +3515,24 @@ "fsevents": "~2.3.3" } }, + "node_modules/tsyringe": { + "version": "4.10.0", + "resolved": "https://registry.npmjs.org/tsyringe/-/tsyringe-4.10.0.tgz", + "integrity": "sha512-axr3IdNuVIxnaK5XGEUFTu3YmAQ6lllgrvqfEoR16g/HGnYY/6We4oWENtAnzK6/LpJ2ur9PAb80RBt7/U4ugw==", + "license": "MIT", + "dependencies": { + "tslib": "^1.9.3" + }, + "engines": { + "node": ">= 6.0.0" + } + }, + "node_modules/tsyringe/node_modules/tslib": { + "version": "1.14.1", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz", + "integrity": "sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==", + "license": "0BSD" + }, "node_modules/typescript": { "version": "5.9.3", "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", diff --git a/package.json b/package.json index eacc8c1..e8a6230 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ }, "dependencies": { "@electric-sql/pglite": "^0.5.4", + "@simplewebauthn/server": "^13.3.2", "@supabase/supabase-js": "^2.110.6", "jose": "^6.2.3", "mimetext": "^3.0.28", diff --git a/specs/auth/passkeys.md b/specs/auth/passkeys.md index 4d29597..a140e6c 100644 --- a/specs/auth/passkeys.md +++ b/specs/auth/passkeys.md @@ -1,16 +1,17 @@ # Passkeys (WebAuthn) login for Agents -Status: **draft.3** (2026-07-19). HT-75. Extends the auth-provider seam +Status: **draft.4** (2026-07-19). HT-75. Extends the auth-provider seam `specs/auth/agents-and-auth.md` (HT-54) built with exactly one provider, `password` — this is the second provider, and the first thing to actually -exercise the seam's "marketplace boundary" claim (agents-and-auth.md §1, §4) +exercise the seam's multi-provider extensibility (agents-and-auth.md §1, §4) with real code. Spec only: no migrations, no implementation. Every schema block below is a design artifact, not a runnable migration — same convention agents-and-auth.md's own `CREATE TABLE` blocks use. **draft.2** is a review-driven revision (lead-tier + Codex, 2026-07-19) — see the Changelog for the full list of what changed and why; nothing in draft.1 survives unexamined, several conclusions reverse outright (§8 most -notably). +notably). **draft.4** corrects a core-vs-marketplace classification error +that survived drafts 1–3 unnoticed — see §1 and the Changelog. Read first: `specs/auth/agents-and-auth.md` §3.2 (`agent_auth_identities` — **amended in this same review round**, §2.1 below), §4 (the seam), §8 @@ -24,14 +25,36 @@ password-provider.ts`, `src/auth/invite-token.ts`, `src/auth/invite-email.ts` ## 1. Purpose & scope -Per agents-and-auth.md §1, `password` is the free-core provider; passkeys -are a **licensed marketplace module** — same boundary, same waiting-on-HT-5 -posture (the AGPL §7 exception text must be counsel-final before this or any -premium module merges). This spec is written now so the boundary has a -second real provider to test itself against, exactly as agents-and-auth.md -§4 anticipated ("adding Google SSO later is still a core code edit... a -module targets [the interface]"). Building this spec does not authorize -merging the module ahead of HT-5. +**Passkey login (WebAuthn) is core — not a marketplace module.** +agents-and-auth.md §1 states this as the one deliberate exception to the +core/marketplace boundary: "Passkey login (WebAuthn) is the one +exception — it is core, not a marketplace module... it ships as a second +**core** auth provider on this same seam (catalog §2.2), never through the +marketplace path." This matches the module catalog's own free-core line +(`specs/modules/catalog.md` §1, §2.2 — accepted 2026-07-18, HT-66): +"Security hygiene is always free: passkey login (WebAuthn) is core, +deliberately, where the reference ecosystem sells 2FA." + +**Corrected here, draft.4 (2026-07-19).** Drafts 1–3 of this spec carried a +stale "licensed marketplace module... waiting-on-HT-5" framing — written +*after* the catalog decision above but *before* `agents-and-auth.md` itself +caught and fixed the identical staleness in its own text (HT-76, PR #85, +merged 2026-07-19, same day). This spec inherited the error rather than the +fix; nothing downstream of §1 depended on the wrong framing (no code, +schema, or endpoint in this spec is gated behind an entitlement check +anywhere), so this is a documentation-only correction. HT-5 (the AGPL §7 +module exception) gates **in-process third-party modules and external +contributions** — Google SSO, magic-link, and SAML/enterprise SSO remain +marketplace and wait on it (agents-and-auth.md §11) — it does **not** gate +first-party core code, and never gated this spec. + +This is still the second real provider to exercise the auth-provider seam +(agents-and-auth.md §4) with real code beyond `password` — exactly as §4 +anticipated ("adding Google SSO later is still a core code edit... a module +targets [the interface]") — it simply is not the marketplace-module case +that sentence was illustrating; it's core proving the same seam +accommodates a second **core** provider just as readily as a future +marketplace one. **Additive only.** No existing Agent, provisioning path, or endpoint from HT-54 changes. Every Agent still gets a `password` identity through the @@ -1024,9 +1047,12 @@ footgun someone has to remember to re-derive later. platform feature this spec doesn't need to do anything special to support — `transports` simply records `hybrid` when reported — but no bespoke UI is designed around it here). -- **No entitlement/licensing enforcement** for this being a paid module — - same carve-out agents-and-auth.md §11 already states for the seam - generally; separate marketplace infrastructure. +- **No entitlement/licensing enforcement — none is needed.** Passkey login + is core (§1, corrected draft.4), not a paid module, so there is no + license check for anything in this spec to ever gate. Contrast a genuine + marketplace provider (Google SSO, magic-link, SAML/enterprise SSO), which + DOES wait on the HT-5 §7 exception text and on entitlement infrastructure + that neither exists yet nor is built here (agents-and-auth.md §11). - **No rate limiting** (§10) — HT-53, unresolved, called out not solved. - **No session revocation / active-session management for Agents.** §10 states plainly that a session cannot be individually invalidated today; @@ -1129,6 +1155,19 @@ recollection of the package's reputation. ## Changelog +- **draft.4 (2026-07-19, HT-75 engine-implementation review):** Corrects a + core-vs-marketplace classification error that survived drafts 1–3 + unnoticed (§1, §12). Passkey login is core, not a licensed marketplace + module — `specs/modules/catalog.md` §1/§2.2 decided this on 2026-07-18 + (HT-66, the day before draft.1), and `agents-and-auth.md` §1 already + carries the corrected framing as of HT-76/PR #85 (merged 2026-07-19, + the same day as draft.1–3 were written). This spec's own "licensed + marketplace module... waiting-on-HT-5" language was stale from the + moment it was written; caught during HT-75's engine-implementation + review, not by an independent re-read of the catalog. No prior draft's + technical content (schema, ceremonies, tokens, counter policy, endpoint + surface) depended on the marketplace framing — this is a documentation + correction only, not a design change. - **draft.3 (2026-07-19, CodeRabbit review, PR #88):** Three fixes. (1) §6.2's counter check and its persistence are now specified as one atomic unit — a `SELECT ... FOR UPDATE` re-read inside a transaction, not a diff --git a/src/api/agents.ts b/src/api/agents.ts index 8e8f0a6..b7de6b6 100644 --- a/src/api/agents.ts +++ b/src/api/agents.ts @@ -54,6 +54,7 @@ import { buildInviteEmail } from '../auth/invite-email.js' import { mintInviteToken, verifyInviteToken } from '../auth/invite-token.js' import { hashPassword, MAX_PASSWORD_LENGTH } from '../auth/password-hash.js' import type { AuthAttempt, AuthProvider } from '../auth/provider.js' +import { WebAuthnChallengeExpiredError } from '../auth/webauthn-provider.js' import type { Keyring } from '../mail/reply-token.js' import type { OutboundEmail } from '../providers/email-sender.js' import type { EmailSender } from '../providers/index.js' @@ -285,8 +286,14 @@ export async function handleSetup( * `POST /api/v1/auth/verify` (spec §6, §9) — dispatch to the named * provider. EVERY failure mode is the SAME generic `401` (unknown email, * wrong password, an unknown `providerKey`, a malformed body, an - * `invited`/`disabled` Agent) — spec §9: "no oracle." No acting-Agent - * header (pre-session). + * `invited`/`disabled` Agent) — spec §9: "no oracle." — with ONE deliberate + * exception (HT-75; specs/auth/passkeys.md §6.2): a webauthn attempt whose + * challenge token expired, was already used, or was minted for a different + * ceremony throws {@link WebAuthnChallengeExpiredError}, caught HERE and + * mapped to a distinguishable `challenge_expired` code — safe to + * distinguish per that section (it signals ceremony freshness, never + * account existence). No other provider ever throws through this path. No + * acting-Agent header (pre-session). */ export async function handleAuthVerify( request: Request, @@ -304,7 +311,15 @@ export async function handleAuthVerify( if (provider === undefined) return INVALID_CREDENTIALS() const attempt: AuthAttempt = { ...body, providerKey } - const verified = await provider.authenticate(attempt) + let verified: Awaited> + try { + verified = await provider.authenticate(attempt) + } catch (err) { + if (err instanceof WebAuthnChallengeExpiredError) { + return apiError(401, 'challenge_expired', 'This passkey challenge expired. Please try again.') + } + throw err + } if (verified === null) return INVALID_CREDENTIALS() const agent = await deps.store.getAgent(verified.agentId) diff --git a/src/api/index.ts b/src/api/index.ts index 04a8836..15d7aea 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -105,6 +105,18 @@ import { handlePatchSavedReply, type SavedRepliesApiDeps, } from './saved-replies.js' +import { + handleAuthenticationOptions, + handleDeleteCredential, + handleListCredentials, + handlePatchCredential, + handleRegistrationOptions, + handleRegistrationVerify, + handleStepUpPassword, + handleStepUpWebAuthnOptions, + handleStepUpWebAuthnVerify, + type WebAuthnApiDeps, +} from './webauthn.js' import { handleCreateWebhook, handleDeleteWebhook, @@ -273,6 +285,16 @@ export interface InboxApiDeps { * is required. */ savedReplies: SavedRepliesApiDeps + /** + * Passkey (WebAuthn) login (HT-75; specs/auth/passkeys.md) — ABSENT BY + * DEFAULT, like `openTracking`/`gmailPush`: a deployment with no known UI + * origin (`config.uiBaseUrl` unset) has no safe origin to bind WebAuthn + * ceremonies to (spec §3), so the composition root simply never + * configures this and every route below 404s / `GET /auth/providers` + * omits the `webauthn` descriptor — the exact degrade-by-omission shape + * `agents.uiBaseUrl` already uses for invites. + */ + webauthn?: WebAuthnApiDeps } /** @@ -632,6 +654,91 @@ export function createInboxApi(deps: InboxApiDeps): (request: Request) => Promis deps.agents, ) + // --- Passkeys (WebAuthn) (HT-75; specs/auth/passkeys.md) ------------ + // + // Every route here 404s when deps.webauthn is absent (config.uiBaseUrl + // unset) — the same absent-by-default degrade `gmailConnect`/ + // `gmailDisconnect` above use. + + case 'webauthn-authentication-options': + return deps.webauthn !== undefined + ? await handleAuthenticationOptions(deps.webauthn) + : apiError(404, 'not_found', 'No such route.') + + case 'step-up-password': + return deps.webauthn !== undefined + ? await handleStepUpPassword( + await resolveActingAgent(request, deps.agents.store), + request, + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + + case 'step-up-webauthn-options': + return deps.webauthn !== undefined + ? await handleStepUpWebAuthnOptions( + await resolveActingAgent(request, deps.agents.store), + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + + case 'step-up-webauthn-verify': + return deps.webauthn !== undefined + ? await handleStepUpWebAuthnVerify( + await resolveActingAgent(request, deps.agents.store), + request, + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + + case 'webauthn-registration-options': + return deps.webauthn !== undefined + ? await handleRegistrationOptions( + await resolveActingAgent(request, deps.agents.store), + request, + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + + case 'webauthn-registration-verify': + return deps.webauthn !== undefined + ? await handleRegistrationVerify( + await resolveActingAgent(request, deps.agents.store), + request, + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + + case 'agent-webauthn-credentials-list': + return deps.webauthn !== undefined + ? await handleListCredentials( + route.id, + await resolveActingAgent(request, deps.agents.store), + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + + case 'agent-webauthn-credential-patch': + return deps.webauthn !== undefined + ? await handlePatchCredential( + route.id, + route.credentialId, + await resolveActingAgent(request, deps.agents.store), + request, + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + + case 'agent-webauthn-credential-delete': + return deps.webauthn !== undefined + ? await handleDeleteCredential( + route.id, + route.credentialId, + await resolveActingAgent(request, deps.agents.store), + deps.webauthn, + ) + : apiError(404, 'not_found', 'No such route.') + // --- Saved replies & macros (HT-76) --------------------------------- case 'saved-replies-list': diff --git a/src/api/router.test.ts b/src/api/router.test.ts index 45da854..8dbb814 100644 --- a/src/api/router.test.ts +++ b/src/api/router.test.ts @@ -328,6 +328,66 @@ describe('matchRoute', () => { allow: ['POST'], }) }) + + // --- Passkeys (WebAuthn) (HT-75) -------------------------------------------- + + it('matches the two pre-session webauthn/step-up minting routes', () => { + expect(matchRoute('POST', '/api/v1/auth/webauthn/authentication/options')).toEqual({ + kind: 'webauthn-authentication-options', + }) + expect(matchRoute('POST', '/api/v1/auth/step-up/password')).toEqual({ + kind: 'step-up-password', + }) + expect(matchRoute('POST', '/api/v1/auth/step-up/webauthn/options')).toEqual({ + kind: 'step-up-webauthn-options', + }) + expect(matchRoute('POST', '/api/v1/auth/step-up/webauthn/verify')).toEqual({ + kind: 'step-up-webauthn-verify', + }) + expect(matchRoute('POST', '/api/v1/auth/webauthn/registration/options')).toEqual({ + kind: 'webauthn-registration-options', + }) + expect(matchRoute('POST', '/api/v1/auth/webauthn/registration/verify')).toEqual({ + kind: 'webauthn-registration-verify', + }) + }) + + it('matches GET/PATCH/DELETE .../agents/{id}/webauthn-credentials(/{credentialId})', () => { + expect(matchRoute('GET', '/api/v1/agents/abc-123/webauthn-credentials')).toEqual({ + kind: 'agent-webauthn-credentials-list', + id: 'abc-123', + }) + expect(matchRoute('PATCH', '/api/v1/agents/abc-123/webauthn-credentials/cred-1')).toEqual({ + kind: 'agent-webauthn-credential-patch', + id: 'abc-123', + credentialId: 'cred-1', + }) + expect(matchRoute('DELETE', '/api/v1/agents/abc-123/webauthn-credentials/cred-1')).toEqual({ + kind: 'agent-webauthn-credential-delete', + id: 'abc-123', + credentialId: 'cred-1', + }) + }) + + it('webauthn-credentials routes never collide with AGENT_ITEM’s bare {id} pattern', () => { + // A bare /agents/{id} still resolves as agent-item, not swallowed by the + // webauthn-credentials patterns. + expect(matchRoute('GET', '/api/v1/agents/abc-123')).toEqual({ + kind: 'agent-item', + id: 'abc-123', + }) + }) + + it('wrong methods on the webauthn routes are method-not-allowed, not not-found', () => { + expect(matchRoute('GET', '/api/v1/auth/webauthn/authentication/options')).toEqual({ + kind: 'method-not-allowed', + allow: ['POST'], + }) + expect(matchRoute('POST', '/api/v1/agents/abc-123/webauthn-credentials')).toEqual({ + kind: 'method-not-allowed', + allow: ['GET'], + }) + }) }) describe('matchGmailPushWebhook', () => { diff --git a/src/api/router.ts b/src/api/router.ts index 6d5ce21..29bf5bc 100644 --- a/src/api/router.ts +++ b/src/api/router.ts @@ -150,6 +150,62 @@ const AGENT_INVITE: RouteDef = { methods: ['POST'], } +// --- Passkeys (WebAuthn) (HT-75; specs/auth/passkeys.md §9) ----------------- +// +// The two pre-session rows (`authentication/options`, and `/auth/verify`'s +// existing generic `providerKey: 'webauthn'` dispatch — no new route needed +// for that one) join agents-and-auth.md §8's bootstrap set. Every other row +// below is session-required, joining the "header required" set alongside +// `/agents/*`. + +/** `/api/v1/auth/webauthn/authentication/options` — mint a login challenge (spec §6.2, §9), POST only, no acting-Agent header (pre-session). */ +const WEBAUTHN_AUTHENTICATION_OPTIONS: RouteDef = { + pattern: /^\/api\/v1\/auth\/webauthn\/authentication\/options$/, + methods: ['POST'], +} + +/** `/api/v1/auth/step-up/password` — step-up via the acting Agent's own password (spec §5.1, §9), POST only, acting-Agent header REQUIRED. */ +const STEP_UP_PASSWORD: RouteDef = { + pattern: /^\/api\/v1\/auth\/step-up\/password$/, + methods: ['POST'], +} + +/** `/api/v1/auth/step-up/webauthn/options` — mint a step-up challenge against the acting Agent's own credentials (spec §5.1, §9), POST only, acting-Agent header REQUIRED. */ +const STEP_UP_WEBAUTHN_OPTIONS: RouteDef = { + pattern: /^\/api\/v1\/auth\/step-up\/webauthn\/options$/, + methods: ['POST'], +} + +/** `/api/v1/auth/step-up/webauthn/verify` — verify the step-up assertion (spec §5.1, §9), POST only, acting-Agent header REQUIRED. Anchored `verify$` so it never collides with `STEP_UP_WEBAUTHN_OPTIONS`'s `options$`. */ +const STEP_UP_WEBAUTHN_VERIFY: RouteDef = { + pattern: /^\/api\/v1\/auth\/step-up\/webauthn\/verify$/, + methods: ['POST'], +} + +/** `/api/v1/auth/webauthn/registration/options` — mint a registration challenge, step-up-gated (spec §5, §6.1, §9), POST only, acting-Agent header REQUIRED. */ +const WEBAUTHN_REGISTRATION_OPTIONS: RouteDef = { + pattern: /^\/api\/v1\/auth\/webauthn\/registration\/options$/, + methods: ['POST'], +} + +/** `/api/v1/auth/webauthn/registration/verify` — verify + insert the new credential, step-up-gated (spec §5, §6.1, §9), POST only, acting-Agent header REQUIRED. */ +const WEBAUTHN_REGISTRATION_VERIFY: RouteDef = { + pattern: /^\/api\/v1\/auth\/webauthn\/registration\/verify$/, + methods: ['POST'], +} + +/** `/api/v1/agents/{id}/webauthn-credentials` — list (self, or admin) — spec §9, GET only. Anchored (`webauthn-credentials$`) so it never collides with `AGENT_ITEM`'s bare `{id}` pattern, mirroring `AGENT_PASSWORD`/`AGENT_INVITE`/`AGENT_MAILBOXES`. */ +const AGENT_WEBAUTHN_CREDENTIALS: RouteDef = { + pattern: /^\/api\/v1\/agents\/(?[^/]+)\/webauthn-credentials$/, + methods: ['GET'], +} + +/** `/api/v1/agents/{id}/webauthn-credentials/{credentialId}` — rename (PATCH) or revoke (DELETE), self or admin, NOT step-up-gated (spec §5.4, §9). */ +const AGENT_WEBAUTHN_CREDENTIAL_ITEM: RouteDef = { + pattern: /^\/api\/v1\/agents\/(?[^/]+)\/webauthn-credentials\/(?[^/]+)$/, + methods: ['PATCH', 'DELETE'], +} + // --- Mailbox access (HT-54 follow-up; spec §3.4/§6) ------------------------- /** `/api/v1/mailboxes` — the full mailbox roster (admin only) — spec §3.4/§6, GET only. */ @@ -274,6 +330,14 @@ const ROUTES: readonly RouteDef[] = [ AGENT_INVITE, MAILBOXES_LIST, AGENT_MAILBOXES, + WEBAUTHN_AUTHENTICATION_OPTIONS, + STEP_UP_PASSWORD, + STEP_UP_WEBAUTHN_OPTIONS, + STEP_UP_WEBAUTHN_VERIFY, + WEBAUTHN_REGISTRATION_OPTIONS, + WEBAUTHN_REGISTRATION_VERIFY, + AGENT_WEBAUTHN_CREDENTIAL_ITEM, + AGENT_WEBAUTHN_CREDENTIALS, SAVED_REPLY_ITEM, SAVED_REPLIES_LIST, AGENT_ITEM, @@ -315,6 +379,15 @@ export type RouteMatch = | { kind: 'mailboxes-list' } | { kind: 'agent-mailboxes-get'; id: string } | { kind: 'agent-mailboxes-put'; id: string } + | { kind: 'webauthn-authentication-options' } + | { kind: 'step-up-password' } + | { kind: 'step-up-webauthn-options' } + | { kind: 'step-up-webauthn-verify' } + | { kind: 'webauthn-registration-options' } + | { kind: 'webauthn-registration-verify' } + | { kind: 'agent-webauthn-credentials-list'; id: string } + | { kind: 'agent-webauthn-credential-patch'; id: string; credentialId: string } + | { kind: 'agent-webauthn-credential-delete'; id: string; credentialId: string } | { kind: 'saved-replies-list'; mailboxId: string } | { kind: 'saved-replies-create'; mailboxId: string } | { kind: 'saved-reply-patch'; mailboxId: string; replyId: string } @@ -457,6 +530,35 @@ export function matchRoute(method: string, pathname: string): RouteMatch { if (route === MAILBOXES_LIST) { return { kind: 'mailboxes-list' } } + if (route === WEBAUTHN_AUTHENTICATION_OPTIONS) { + return { kind: 'webauthn-authentication-options' } + } + if (route === STEP_UP_PASSWORD) { + return { kind: 'step-up-password' } + } + if (route === STEP_UP_WEBAUTHN_OPTIONS) { + return { kind: 'step-up-webauthn-options' } + } + if (route === STEP_UP_WEBAUTHN_VERIFY) { + return { kind: 'step-up-webauthn-verify' } + } + if (route === WEBAUTHN_REGISTRATION_OPTIONS) { + return { kind: 'webauthn-registration-options' } + } + if (route === WEBAUTHN_REGISTRATION_VERIFY) { + return { kind: 'webauthn-registration-verify' } + } + if (route === AGENT_WEBAUTHN_CREDENTIALS) { + const id = match.groups?.id as string + return { kind: 'agent-webauthn-credentials-list', id } + } + if (route === AGENT_WEBAUTHN_CREDENTIAL_ITEM) { + const id = match.groups?.id as string + const credentialId = match.groups?.credentialId as string + return method === 'DELETE' + ? { kind: 'agent-webauthn-credential-delete', id, credentialId } + : { kind: 'agent-webauthn-credential-patch', id, credentialId } + } if (route === WEBHOOKS_LIST) { return method === 'GET' ? { kind: 'webhooks-list' } : { kind: 'webhooks-create' } } diff --git a/src/api/webauthn.test.ts b/src/api/webauthn.test.ts new file mode 100644 index 0000000..74e50ae --- /dev/null +++ b/src/api/webauthn.test.ts @@ -0,0 +1,630 @@ +/** + * End-to-end tests for the passkey (WebAuthn) management API (HT-75; + * specs/auth/passkeys.md), driven through the real `createInboxApi` + * pipeline — same convention as `src/api/agents.test.ts`. Only + * `@simplewebauthn/server`'s four ceremony functions are mocked (the + * library's own CBOR/COSE/signature correctness is out of scope here; what + * this suite proves is OUR wiring, step-up gating, ceremony discrimination, + * and the counter/last-credential policies around it). + */ + +import { randomBytes } from 'node:crypto' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { hashPassword } from '../auth/password-hash.js' +import { createPasswordAuthProvider } from '../auth/password-provider.js' +import { createWebAuthnAuthProvider } from '../auth/webauthn-provider.js' +import type { WebAuthnRpConfig } from '../auth/webauthn-rp.js' +import { createPgliteDb, type Db } from '../db/client.js' +import { migrate } from '../db/migrate.js' +import type { Keyring } from '../mail/reply-token.js' +import type { EmailSender, OutboundEmail } from '../providers/index.js' +import { type AgentRecord, type AgentStore, createAgentStore } from '../store/agents.js' +import { createAssistantStore } from '../store/assistants.js' +import { createConversationStore } from '../store/conversations.js' +import { createMailboxStore } from '../store/mailboxes.js' +import { createSavedReplyStore } from '../store/saved-replies.js' +import { ENCRYPTION_KEY_BYTES } from '../store/token-crypto.js' +import { createWebAuthnStore, type WebAuthnStore } from '../store/webauthn.js' +import { createWebhookEndpointStore } from '../store/webhook-endpoints.js' +import { createInboxApi } from './index.js' + +const { + generateAuthenticationOptions, + generateRegistrationOptions, + verifyAuthenticationResponse, + verifyRegistrationResponse, +} = vi.hoisted(() => ({ + generateAuthenticationOptions: vi.fn(), + generateRegistrationOptions: vi.fn(), + verifyAuthenticationResponse: vi.fn(), + verifyRegistrationResponse: vi.fn(), +})) + +vi.mock('@simplewebauthn/server', () => ({ + generateAuthenticationOptions, + generateRegistrationOptions, + verifyAuthenticationResponse, + verifyRegistrationResponse, +})) + +const WEBHOOKS_ENC_KEY = randomBytes(ENCRYPTION_KEY_BYTES) +const TOKEN = 'test-token-for-the-webauthn-suite' +const MAIL_DOMAIN = 'mail.example.test' +const SUPPORT_ADDRESS = 'support@example.test' +const UI_BASE_URL = 'https://inbox.example.test' +const KEYRING: Keyring = { current: { keyId: 'k1', secret: 'a'.repeat(32) } } +const AGENT_HEADER = 'X-Helpthread-Agent-Id' +const RP: WebAuthnRpConfig = { rpId: 'inbox.example.test', expectedOrigin: UI_BASE_URL } + +function createFakeSender(): { sender: EmailSender; sent: OutboundEmail[] } { + const sent: OutboundEmail[] = [] + return { + sender: { + maxSendMs: 30_000, + async send(email) { + sent.push(email) + return {} + }, + }, + sent, + } +} + +function registrationVerified( + credentialId: string, + overrides: Partial> = {}, +) { + return { + verified: true, + registrationInfo: { + credential: { + id: credentialId, + publicKey: new Uint8Array([1, 2, 3]), + counter: 0, + transports: ['internal'], + }, + credentialDeviceType: 'multiDevice', + credentialBackedUp: false, + ...overrides, + }, + } +} + +describe('Passkey (WebAuthn) API', () => { + let db: Db | undefined + + afterEach(async () => { + vi.clearAllMocks() + await db?.close() + db = undefined + }) + + async function freshApi(): Promise<{ + db: Db + agentStore: AgentStore + webAuthnStore: WebAuthnStore + api: (request: Request) => Promise + sent: OutboundEmail[] + }> { + db = await createPgliteDb() + await migrate(db) + const agentStore = createAgentStore(db) + const mailboxStore = createMailboxStore(db) + const webAuthnStore = createWebAuthnStore(db) + const { sender, sent } = createFakeSender() + const providers = [ + createPasswordAuthProvider({ agentStore }), + createWebAuthnAuthProvider({ db, store: webAuthnStore, keyring: KEYRING, rp: RP }), + ] + const api = createInboxApi({ + store: createConversationStore(db), + apiToken: TOKEN, + sender, + keyring: KEYRING, + mailDomain: MAIL_DOMAIN, + supportAddress: SUPPORT_ADDRESS, + agents: { store: agentStore, providers, mailboxStore, uiBaseUrl: UI_BASE_URL }, + webhooks: { + store: createWebhookEndpointStore(db, WEBHOOKS_ENC_KEY), + queue: { async enqueue() {} }, + }, + assistants: { store: createAssistantStore(db) }, + savedReplies: { store: createSavedReplyStore(db), mailboxStore }, + webauthn: { + db, + store: webAuthnStore, + agentStore, + providers, + keyring: KEYRING, + rp: RP, + rpName: 'Helpthread', + sender, + mailDomain: MAIL_DOMAIN, + supportAddress: SUPPORT_ADDRESS, + }, + }) + return { db, agentStore, webAuthnStore, api, sent } + } + + function req( + method: string, + path: string, + opts: { agentId?: string; body?: unknown } = {}, + ): Request { + const headers: Record = { Authorization: `Bearer ${TOKEN}` } + if (opts.agentId !== undefined) headers[AGENT_HEADER] = opts.agentId + const init: RequestInit = { method, headers } + if (opts.body !== undefined) { + headers['Content-Type'] = 'application/json' + init.body = JSON.stringify(opts.body) + } + return new Request(`https://x.example.test${path}`, init) + } + + const PASSWORD = 'correct-horse-battery-staple' + + async function createActiveAgent( + agentStore: AgentStore, + email = 'agent@example.test', + ): Promise { + const result = await agentStore.createAgent({ + name: 'Test Agent', + email, + role: 'agent', + status: 'active', + passwordHash: hashPassword(PASSWORD), + }) + if (!result.ok) throw new Error('expected ok') + return result.agent + } + + async function stepUpToken( + api: (r: Request) => Promise, + agentId: string, + ): Promise { + const res = await api( + req('POST', '/api/v1/auth/step-up/password', { agentId, body: { password: PASSWORD } }), + ) + expect(res.status).toBe(200) + return ((await res.json()) as { stepUpToken: string }).stepUpToken + } + + // --- GET /auth/providers — the webauthn descriptor appears ----------------- + + it('GET /auth/providers reports BOTH password and webauthn descriptors', async () => { + const { api } = await freshApi() + const res = await api(req('GET', '/api/v1/auth/providers')) + const body = (await res.json()) as { providers: { key: string; kind: string }[] } + expect(body.providers).toEqual( + expect.arrayContaining([ + { key: 'password', label: expect.any(String), kind: 'credentials' }, + { key: 'webauthn', label: expect.any(String), kind: 'webauthn' }, + ]), + ) + }) + + // --- POST /auth/webauthn/authentication/options ----------------------------- + + it('authentication/options is pre-session (no acting-Agent header needed) and omits allowCredentials', async () => { + const { api } = await freshApi() + generateAuthenticationOptions.mockResolvedValue({ challenge: 'x', rpId: RP.rpId }) + + const res = await api( + new Request('https://x.example.test/api/v1/auth/webauthn/authentication/options', { + method: 'POST', + headers: { Authorization: `Bearer ${TOKEN}` }, + }), + ) + expect(res.status).toBe(200) + const body = (await res.json()) as { options: unknown; challengeToken: string } + expect(body.challengeToken).toMatch(/^htw\./) + expect(generateAuthenticationOptions).toHaveBeenCalledWith( + expect.not.objectContaining({ allowCredentials: expect.anything() }), + ) + }) + + // --- POST /auth/step-up/password -------------------------------------------- + + describe('POST /auth/step-up/password', () => { + it('mints a step-up token on the correct password', async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + const res = await api( + req('POST', '/api/v1/auth/step-up/password', { + agentId: agent.id, + body: { password: PASSWORD }, + }), + ) + expect(res.status).toBe(200) + const body = (await res.json()) as { stepUpToken: string } + expect(body.stepUpToken).toMatch(/^htsu\./) + }) + + it('401s on a wrong password', async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + const res = await api( + req('POST', '/api/v1/auth/step-up/password', { + agentId: agent.id, + body: { password: 'wrong' }, + }), + ) + expect(res.status).toBe(401) + }) + + it('401s with no acting-Agent header', async () => { + const { api } = await freshApi() + const res = await api( + req('POST', '/api/v1/auth/step-up/password', { body: { password: PASSWORD } }), + ) + expect(res.status).toBe(401) + }) + }) + + // --- POST /auth/webauthn/registration/options + /verify --------------------- + + describe('registration/options + registration/verify', () => { + it('registration/options 401s without a valid stepUpToken', async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + const res = await api( + req('POST', '/api/v1/auth/webauthn/registration/options', { agentId: agent.id, body: {} }), + ) + expect(res.status).toBe(401) + }) + + it("registration/options 401s with another Agent's stepUpToken", async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore, 'a@example.test') + const other = await createActiveAgent(agentStore, 'b@example.test') + const su = await stepUpToken(api, other.id) + const res = await api( + req('POST', '/api/v1/auth/webauthn/registration/options', { + agentId: agent.id, + body: { stepUpToken: su }, + }), + ) + expect(res.status).toBe(401) + }) + + it('the step-up token is single-use — spent by registration/options, refused on retry', async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + generateRegistrationOptions.mockResolvedValue({ challenge: 'x' }) + const su = await stepUpToken(api, agent.id) + + const first = await api( + req('POST', '/api/v1/auth/webauthn/registration/options', { + agentId: agent.id, + body: { stepUpToken: su }, + }), + ) + expect(first.status).toBe(200) + + const second = await api( + req('POST', '/api/v1/auth/webauthn/registration/options', { + agentId: agent.id, + body: { stepUpToken: su }, + }), + ) + expect(second.status).toBe(401) + }) + + it('a full registration round trip: options → verify → 201, credential never exposes the public key/credentialId, and a notification email is sent', async () => { + const { api, agentStore, sent } = await freshApi() + const agent = await createActiveAgent(agentStore) + generateRegistrationOptions.mockResolvedValue({ challenge: 'x' }) + verifyRegistrationResponse.mockResolvedValue(registrationVerified('cred-abc')) + + const su = await stepUpToken(api, agent.id) + const optionsRes = await api( + req('POST', '/api/v1/auth/webauthn/registration/options', { + agentId: agent.id, + body: { stepUpToken: su }, + }), + ) + const { challengeToken } = (await optionsRes.json()) as { challengeToken: string } + + const verifyRes = await api( + req('POST', '/api/v1/auth/webauthn/registration/verify', { + agentId: agent.id, + body: { response: {}, challengeToken, stepUpToken: su, name: 'My MacBook' }, + }), + ) + expect(verifyRes.status).toBe(201) + const body = (await verifyRes.json()) as { credential: Record } + expect(body.credential.name).toBe('My MacBook') + expect(body.credential).not.toHaveProperty('publicKey') + expect(body.credential).not.toHaveProperty('credentialId') + expect(sent).toHaveLength(1) + expect(sent[0].to).toEqual([agent.email]) + }) + + it('defaults a blank name to "Passkey — {date}"', async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + generateRegistrationOptions.mockResolvedValue({ challenge: 'x' }) + verifyRegistrationResponse.mockResolvedValue(registrationVerified('cred-blank-name')) + + const su = await stepUpToken(api, agent.id) + const optionsRes = await api( + req('POST', '/api/v1/auth/webauthn/registration/options', { + agentId: agent.id, + body: { stepUpToken: su }, + }), + ) + const { challengeToken } = (await optionsRes.json()) as { challengeToken: string } + const verifyRes = await api( + req('POST', '/api/v1/auth/webauthn/registration/verify', { + agentId: agent.id, + body: { response: {}, challengeToken, stepUpToken: su, name: ' ' }, + }), + ) + const body = (await verifyRes.json()) as { credential: { name: string } } + expect(body.credential.name).toMatch(/^Passkey — \d{4}-\d{2}-\d{2}$/) + }) + + it('409s when the credential_id is already registered', async () => { + const { api, agentStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + generateRegistrationOptions.mockResolvedValue({ challenge: 'x' }) + verifyRegistrationResponse.mockResolvedValue(registrationVerified('dup-cred')) + + async function attempt() { + const su = await stepUpToken(api, agent.id) + const optionsRes = await api( + req('POST', '/api/v1/auth/webauthn/registration/options', { + agentId: agent.id, + body: { stepUpToken: su }, + }), + ) + const { challengeToken } = (await optionsRes.json()) as { challengeToken: string } + return api( + req('POST', '/api/v1/auth/webauthn/registration/verify', { + agentId: agent.id, + body: { response: {}, challengeToken, stepUpToken: su }, + }), + ) + } + + expect((await attempt()).status).toBe(201) + const second = await attempt() + expect(second.status).toBe(409) + }) + }) + + // --- step-up/webauthn/options + verify --------------------------------------- + + it('step-up/webauthn/options populates allowCredentials with the acting Agent’s own credentials', async () => { + const { api, agentStore, webAuthnStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + await webAuthnStore.insertCredential({ + agentId: agent.id, + credentialId: 'my-cred', + publicKey: new Uint8Array([1]), + signCount: 0, + transports: ['internal'], + backupEligible: true, + backupState: false, + name: 'Key', + }) + generateAuthenticationOptions.mockResolvedValue({ challenge: 'x' }) + + const res = await api( + req('POST', '/api/v1/auth/step-up/webauthn/options', { agentId: agent.id, body: {} }), + ) + expect(res.status).toBe(200) + expect(generateAuthenticationOptions).toHaveBeenCalledWith( + expect.objectContaining({ allowCredentials: [{ id: 'my-cred', transports: ['internal'] }] }), + ) + }) + + it('step-up/webauthn/verify rejects a credential belonging to a DIFFERENT Agent than the session', async () => { + const { api, agentStore, webAuthnStore } = await freshApi() + const owner = await createActiveAgent(agentStore, 'owner@example.test') + const impersonator = await createActiveAgent(agentStore, 'impersonator@example.test') + await webAuthnStore.insertCredential({ + agentId: owner.id, + credentialId: 'owner-cred', + publicKey: new Uint8Array([1]), + signCount: 0, + transports: [], + backupEligible: false, + backupState: false, + name: 'Key', + }) + generateAuthenticationOptions.mockResolvedValue({ challenge: 'x' }) + verifyAuthenticationResponse.mockResolvedValue({ + verified: true, + authenticationInfo: { newCounter: 1, credentialBackedUp: false }, + }) + + const optionsRes = await api( + req('POST', '/api/v1/auth/step-up/webauthn/options', { agentId: impersonator.id, body: {} }), + ) + const { challengeToken } = (await optionsRes.json()) as { challengeToken: string } + + const verifyRes = await api( + req('POST', '/api/v1/auth/step-up/webauthn/verify', { + agentId: impersonator.id, + body: { response: { id: 'owner-cred' }, challengeToken }, + }), + ) + expect(verifyRes.status).toBe(401) + }) + + // --- /auth/verify's webauthn case + challenge_expired ------------------------- + + describe("POST /auth/verify { providerKey: 'webauthn' }", () => { + it('logs in on a fully valid assertion', async () => { + const { api, agentStore, webAuthnStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + await webAuthnStore.insertCredential({ + agentId: agent.id, + credentialId: 'login-cred', + publicKey: new Uint8Array([1]), + signCount: 0, + transports: [], + backupEligible: false, + backupState: false, + name: 'Key', + }) + generateAuthenticationOptions.mockResolvedValue({ challenge: 'x' }) + verifyAuthenticationResponse.mockResolvedValue({ + verified: true, + authenticationInfo: { newCounter: 1, credentialBackedUp: false }, + }) + + const optionsRes = await api( + new Request('https://x.example.test/api/v1/auth/webauthn/authentication/options', { + method: 'POST', + headers: { Authorization: `Bearer ${TOKEN}` }, + }), + ) + const { challengeToken } = (await optionsRes.json()) as { challengeToken: string } + + const verifyRes = await api( + req('POST', '/api/v1/auth/verify', { + body: { providerKey: 'webauthn', response: { id: 'login-cred' }, challengeToken }, + }), + ) + expect(verifyRes.status).toBe(200) + const body = (await verifyRes.json()) as { agent: { id: string } } + expect(body.agent.id).toBe(agent.id) + }) + + it('returns the distinguishable challenge_expired code, not a generic 401, for an expired/reused challenge', async () => { + const { api } = await freshApi() + generateAuthenticationOptions.mockResolvedValue({ challenge: 'x' }) + const optionsRes = await api( + new Request('https://x.example.test/api/v1/auth/webauthn/authentication/options', { + method: 'POST', + headers: { Authorization: `Bearer ${TOKEN}` }, + }), + ) + const { challengeToken } = (await optionsRes.json()) as { challengeToken: string } + + // Consume it once via a normal (failing, unknown-credential) attempt so the + // DB row is gone — the second attempt then finds no row at all. + await api( + req('POST', '/api/v1/auth/verify', { + body: { providerKey: 'webauthn', response: { id: 'no-such-credential' }, challengeToken }, + }), + ) + const res = await api( + req('POST', '/api/v1/auth/verify', { + body: { providerKey: 'webauthn', response: { id: 'no-such-credential' }, challengeToken }, + }), + ) + expect(res.status).toBe(401) + expect(await res.json()).toEqual({ + error: { code: 'challenge_expired', message: expect.any(String) }, + }) + }) + }) + + // --- credential list/rename/revoke ------------------------------------------- + + describe('GET/PATCH/DELETE /agents/{id}/webauthn-credentials', () => { + it('self may list, rename, and revoke their own credential', async () => { + const { api, agentStore, webAuthnStore } = await freshApi() + const agent = await createActiveAgent(agentStore) + const inserted = await webAuthnStore.insertCredential({ + agentId: agent.id, + credentialId: 'self-cred', + publicKey: new Uint8Array([1]), + signCount: 0, + transports: [], + backupEligible: false, + backupState: false, + name: 'Original name', + }) + if (!inserted.ok) throw new Error('expected ok') + + const listRes = await api( + req('GET', `/api/v1/agents/${agent.id}/webauthn-credentials`, { agentId: agent.id }), + ) + expect(listRes.status).toBe(200) + const listBody = (await listRes.json()) as { credentials: { id: string; name: string }[] } + expect(listBody.credentials).toHaveLength(1) + expect(listBody.credentials[0]).not.toHaveProperty('publicKey') + + const patchRes = await api( + req('PATCH', `/api/v1/agents/${agent.id}/webauthn-credentials/${inserted.credential.id}`, { + agentId: agent.id, + body: { name: 'Renamed' }, + }), + ) + expect(patchRes.status).toBe(200) + + const deleteRes = await api( + req('DELETE', `/api/v1/agents/${agent.id}/webauthn-credentials/${inserted.credential.id}`, { + agentId: agent.id, + }), + ) + expect(deleteRes.status).toBe(204) + }) + + it('a non-admin, non-self Agent is forbidden', async () => { + const { api, agentStore } = await freshApi() + const owner = await createActiveAgent(agentStore, 'owner@example.test') + const other = await createActiveAgent(agentStore, 'other@example.test') + const res = await api( + req('GET', `/api/v1/agents/${owner.id}/webauthn-credentials`, { agentId: other.id }), + ) + expect(res.status).toBe(403) + }) + + it('an admin may list/rename/revoke another Agent’s credentials', async () => { + const { api, agentStore, webAuthnStore } = await freshApi() + const target = await createActiveAgent(agentStore, 'target@example.test') + const admin = await agentStore.createAgent({ + name: 'Admin', + email: 'admin@example.test', + role: 'admin', + status: 'active', + passwordHash: hashPassword(PASSWORD), + }) + if (!admin.ok) throw new Error('expected ok') + await webAuthnStore.insertCredential({ + agentId: target.id, + credentialId: 'admin-managed', + publicKey: new Uint8Array([1]), + signCount: 0, + transports: [], + backupEligible: false, + backupState: false, + name: 'Key', + }) + + const res = await api( + req('GET', `/api/v1/agents/${target.id}/webauthn-credentials`, { agentId: admin.agent.id }), + ) + expect(res.status).toBe(200) + }) + + it("409s ('conflict') revoking the Agent's only credential once they have no password identity — the spec §9.1 defensive guard", async () => { + const { api, agentStore, webAuthnStore, db: testDb } = await freshApi() + const agent = await createActiveAgent(agentStore) + await testDb.query('DELETE FROM agent_auth_identities WHERE agent_id = $1', [agent.id]) + const inserted = await webAuthnStore.insertCredential({ + agentId: agent.id, + credentialId: 'only-cred', + publicKey: new Uint8Array([1]), + signCount: 0, + transports: [], + backupEligible: false, + backupState: false, + name: 'Key', + }) + if (!inserted.ok) throw new Error('expected ok') + + const res = await api( + req('DELETE', `/api/v1/agents/${agent.id}/webauthn-credentials/${inserted.credential.id}`, { + agentId: agent.id, + }), + ) + expect(res.status).toBe(409) + }) + }) +}) diff --git a/src/api/webauthn.ts b/src/api/webauthn.ts new file mode 100644 index 0000000..3a0df10 --- /dev/null +++ b/src/api/webauthn.ts @@ -0,0 +1,518 @@ +/** + * Passkey (WebAuthn) management API handlers (HT-75; specs/auth/passkeys.md + * §5, §6, §9) — everything EXCEPT the login verify step, which dispatches + * through the existing generic `POST /auth/verify` (`handleAuthVerify`, + * `src/api/agents.ts`) via `WebAuthnAuthProvider` (`src/auth/ + * webauthn-provider.ts`). Per spec §4.2, options-minting is deliberately + * OUTSIDE the `AuthProvider` seam — every handler here is provider-specific + * HTTP surface, not something the seam needs to know about. + * + * Same shape as `src/api/agents.ts`: each handler is a pure function of an + * already-authenticated, already-routed `Request` plus its dependencies. + * + * ## Step-up (spec §5) + * + * `registration/options` and `registration/verify` both require a valid, + * unexpired `stepUpToken` naming the SAME acting Agent — minted by either + * `step-up/password` or the `step-up/webauthn/*` pair. `options` CONSUMES + * the token (single-use, DB-backed); `verify` re-validates signature+TTL+ + * agent-match but does NOT re-consume (spec §5.2 — a second consume would + * always fail, which is not the property wanted there). + * + * ## Uniform failure shape + * + * Every ceremony/step-up/registration failure in this module is the SAME + * generic `401 unauthorized` — no finer-grained code is ever returned here. + * The one distinguishable exception the spec defines (`challenge_expired`, + * §6.2) applies ONLY to the login path and is handled entirely in + * `handleAuthVerify`/`webauthn-provider.ts`, not in this module. + */ + +import type { AuthenticatorTransportFuture, RegistrationResponseJSON } from '@simplewebauthn/server' +import { + generateAuthenticationOptions, + generateRegistrationOptions, + verifyRegistrationResponse, +} from '@simplewebauthn/server' +import type { AuthProvider } from '../auth/provider.js' +import { + uuidToBytes, + verifyAuthenticationCeremony, + type WebAuthnCeremonyDeps, +} from '../auth/webauthn-ceremony.js' +import { buildPasskeyAddedEmail } from '../auth/webauthn-notify-email.js' +import type { WebAuthnRpConfig } from '../auth/webauthn-rp.js' +import { + DEFAULT_CHALLENGE_TOKEN_TTL_MS, + DEFAULT_STEPUP_TOKEN_TTL_MS, + mintChallengeToken, + mintStepUpToken, + verifyChallengeToken, + verifyStepUpToken, + type WebAuthnCeremony, +} from '../auth/webauthn-token.js' +import type { Db } from '../db/client.js' +import type { Keyring } from '../mail/reply-token.js' +import type { EmailSender } from '../providers/index.js' +import type { AgentRecord, AgentStore } from '../store/agents.js' +import type { WebAuthnCredentialRecord, WebAuthnStore } from '../store/webauthn.js' +import { apiError, json, noContent } from './responses.js' +import { isUuid } from './uuid.js' + +/** Dependencies every handler in this module needs. Built once per request by `src/api/index.ts`, from the `InboxApiDeps.webauthn` bag the composition root wires only when `config.uiBaseUrl` is set (spec §3 — "root.ts refuses to wire up WebAuthnAuthProvider when uiBaseUrl is unset", the identical rule for this whole feature). */ +export interface WebAuthnApiDeps { + db: Db + store: WebAuthnStore + agentStore: AgentStore + /** The full provider registry — `step-up/password` re-dispatches to the registered `password` provider (spec §5.1) rather than re-implementing verification. */ + providers: AuthProvider[] + keyring: Keyring + rp: WebAuthnRpConfig + /** Deployment display name shown in the OS passkey UI (spec §6.1's `rpName`). */ + rpName: string + sender: EmailSender + mailDomain: string + supportAddress: string +} + +function ceremonyDeps(deps: WebAuthnApiDeps): WebAuthnCeremonyDeps { + return { db: deps.db, store: deps.store, keyring: deps.keyring, rp: deps.rp } +} + +// --- shared helpers ---------------------------------------------------- + +async function parseJsonBody( + request: Request, +): Promise<{ ok: true; value: unknown } | { ok: false }> { + try { + return { ok: true, value: await request.json() } + } catch { + return { ok: false } + } +} + +function asRecord(value: unknown): Record | null { + return typeof value === 'object' && value !== null ? (value as Record) : null +} + +const UNAUTHORIZED = () => apiError(401, 'unauthorized', 'Missing or invalid Agent identity.') +const FORBIDDEN = () => apiError(403, 'forbidden', 'You may only manage your own passkeys.') +const NOT_FOUND = () => apiError(404, 'not_found', 'No such Agent or credential.') +const STEP_UP_REQUIRED = () => + apiError(401, 'unauthorized', 'Step-up verification is required and was not satisfied.') +const CEREMONY_FAILED = () => apiError(401, 'unauthorized', 'Passkey ceremony verification failed.') + +/** Mint an `htw.` challenge token AND its `webauthn_challenges` DB row, in one call — every options-minting endpoint below does exactly this. Returns the raw challenge bytes too, ready for `generateRegistrationOptions`/`generateAuthenticationOptions`'s `challenge` param (module doc on webauthn-token.ts: passing a `Uint8Array`, not the base64url string, avoids a UTF8 double-encoding mismatch). */ +async function mintAndStoreChallenge( + deps: Pick, + ceremony: WebAuthnCeremony, + agentId: string | null, +): Promise<{ challengeToken: string; challengeBytes: Uint8Array }> { + const minted = mintChallengeToken(ceremony, agentId, deps.keyring) + await deps.store.mintChallenge({ + nonce: minted.nonce, + ceremony, + agentId, + expiresAt: new Date(Date.now() + DEFAULT_CHALLENGE_TOKEN_TTL_MS), + }) + // A fresh `Uint8Array` (not a `Buffer`) — see webauthn-ceremony.ts's + // identical note on why `@simplewebauthn/server`'s types need this. + return { + challengeToken: minted.token, + challengeBytes: new Uint8Array(Buffer.from(minted.challengeB64, 'base64url')), + } +} + +/** Mint an `htsu.` step-up token AND its `webauthn_stepup_tokens` DB row (spec §5.1) — the shared success path for both step-up proof mechanisms. */ +async function mintAndStoreStepUpToken( + deps: Pick, + agentId: string, +): Promise { + const minted = mintStepUpToken(agentId, deps.keyring) + await deps.store.mintStepUpToken({ + nonce: minted.nonce, + agentId, + expiresAt: new Date(Date.now() + DEFAULT_STEPUP_TOKEN_TTL_MS), + }) + return minted.token +} + +/** Re-validate (never re-consume — spec §5.2) a `stepUpToken` string against `agentId`. `null` on any failure (missing, malformed, expired, wrong-signature, or minted for a different Agent). */ +function checkStepUpToken(raw: unknown, agentId: string, keyring: Keyring): boolean { + if (typeof raw !== 'string' || raw.length === 0) return false + const verified = verifyStepUpToken(raw, keyring) + return verified !== null && verified.agentId === agentId +} + +const MAX_CREDENTIAL_NAME_LENGTH = 200 + +/** `PATCH .../webauthn-credentials/{id}`'s `name` — required, non-blank, ≤200 chars. `null` on any violation. */ +function validateCredentialName(raw: unknown): string | null { + if (typeof raw !== 'string') return null + const trimmed = raw.trim() + return trimmed.length > 0 && trimmed.length <= MAX_CREDENTIAL_NAME_LENGTH ? trimmed : null +} + +/** `registration/verify`'s `name` — optional/lenient on the wire (spec §6.1, §9): an omitted, blank, or over-length value is replaced with a server-computed default, `"Passkey — {date}"`, before the INSERT ever runs — the write path (`webauthn_credentials.name NOT NULL`) is never at risk from a lenient caller. */ +function normalizeRegistrationCredentialName(raw: unknown): string { + const trimmed = typeof raw === 'string' ? raw.trim() : '' + if (trimmed.length === 0 || trimmed.length > MAX_CREDENTIAL_NAME_LENGTH) { + return `Passkey — ${new Date().toISOString().slice(0, 10)}` + } + return trimmed +} + +// --- wire shape ---------------------------------------------------------- + +interface CredentialJson { + id: string + name: string + transports: string[] + backupEligible: boolean + backupState: boolean + createdAt: string + lastUsedAt: string | null +} + +function toCredentialJson(credential: WebAuthnCredentialRecord): CredentialJson { + return { + id: credential.id, + name: credential.name, + transports: credential.transports, + backupEligible: credential.backupEligible, + backupState: credential.backupState, + createdAt: credential.createdAt.toISOString(), + lastUsedAt: credential.lastUsedAt === null ? null : credential.lastUsedAt.toISOString(), + } +} + +function excludeOrAllowList( + credentials: WebAuthnCredentialRecord[], +): { id: string; transports?: AuthenticatorTransportFuture[] }[] { + return credentials.map((credential) => ({ + id: credential.credentialId, + transports: credential.transports as AuthenticatorTransportFuture[], + })) +} + +// --- POST /api/v1/auth/webauthn/authentication/options ------------------- + +/** `POST /api/v1/auth/webauthn/authentication/options` (spec §6.2, §9) — pre-session, no input. `allowCredentials` is deliberately OMITTED (required for conditional-UI discoverable-credential autofill). */ +export async function handleAuthenticationOptions( + deps: Pick, +): Promise { + const { challengeToken, challengeBytes } = await mintAndStoreChallenge( + deps, + 'authentication', + null, + ) + const options = await generateAuthenticationOptions({ + rpID: deps.rp.rpId, + challenge: challengeBytes, + userVerification: 'required', + }) + return json(200, { options, challengeToken }) +} + +// --- POST /api/v1/auth/step-up/password ----------------------------------- + +/** `POST /api/v1/auth/step-up/password` (spec §5.1) — session-required. Re-runs the registered `password` provider against the ACTING Agent's own (session-resolved) email — never client input. */ +export async function handleStepUpPassword( + actingAgent: AgentRecord | null, + request: Request, + deps: Pick, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + + const parsed = await parseJsonBody(request) + const body = parsed.ok ? asRecord(parsed.value) : null + const password = body?.password + if (typeof password !== 'string') { + return apiError(400, 'validation_failed', 'password is required.') + } + + const passwordProvider = deps.providers.find((candidate) => candidate.key === 'password') + if (passwordProvider === undefined) return STEP_UP_REQUIRED() + + const verified = await passwordProvider.authenticate({ + providerKey: 'password', + email: actingAgent.email, + password, + }) + if (verified === null || verified.agentId !== actingAgent.id) return STEP_UP_REQUIRED() + + const stepUpToken = await mintAndStoreStepUpToken(deps, actingAgent.id) + return json(200, { stepUpToken }) +} + +// --- POST /api/v1/auth/step-up/webauthn/options ---------------------------- + +/** `POST /api/v1/auth/step-up/webauthn/options` (spec §5.1) — session-required. Unlike login's `authentication/options`, `allowCredentials` IS populated (the ACTING Agent's own existing credentials) — the caller already knows who's asking. */ +export async function handleStepUpWebAuthnOptions( + actingAgent: AgentRecord | null, + deps: Pick, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + + const existing = await deps.store.listCredentialsForAgent(actingAgent.id) + const { challengeToken, challengeBytes } = await mintAndStoreChallenge( + deps, + 'step-up', + actingAgent.id, + ) + const options = await generateAuthenticationOptions({ + rpID: deps.rp.rpId, + allowCredentials: excludeOrAllowList(existing), + challenge: challengeBytes, + userVerification: 'required', + }) + return json(200, { options, challengeToken }) +} + +// --- POST /api/v1/auth/step-up/webauthn/verify ----------------------------- + +/** `POST /api/v1/auth/step-up/webauthn/verify` (spec §5.1) — session-required. `{ response, challengeToken }`. Shares `verifyAuthenticationCeremony` with the login path (`webauthn-provider.ts`), `ceremony: 'step-up'`, and additionally requires the resolved credential's Agent to equal the acting Agent (spec: "proving a factor for a different, even genuinely valid, Agent does not step up this session"). */ +export async function handleStepUpWebAuthnVerify( + actingAgent: AgentRecord | null, + request: Request, + deps: WebAuthnApiDeps, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + + const parsed = await parseJsonBody(request) + const body = parsed.ok ? asRecord(parsed.value) : null + const challengeToken = body?.challengeToken + const response = body?.response + if (typeof challengeToken !== 'string' || typeof response !== 'object' || response === null) { + return CEREMONY_FAILED() + } + + const result = await verifyAuthenticationCeremony(ceremonyDeps(deps), { + ceremony: 'step-up', + responseJson: response, + challengeToken, + requireAgentId: actingAgent.id, + }) + if (!result.ok) return CEREMONY_FAILED() + + const stepUpToken = await mintAndStoreStepUpToken(deps, actingAgent.id) + return json(200, { stepUpToken }) +} + +// --- POST /api/v1/auth/webauthn/registration/options ------------------------ + +/** `POST /api/v1/auth/webauthn/registration/options` (spec §5.2, §6.1) — session-required, step-up-required. `{ stepUpToken }`: re-validated AND consumed here (the DB-backed single-use layer, spec §5.2). */ +export async function handleRegistrationOptions( + actingAgent: AgentRecord | null, + request: Request, + deps: WebAuthnApiDeps, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + + const parsed = await parseJsonBody(request) + const body = parsed.ok ? asRecord(parsed.value) : null + const stepUpTokenRaw = body?.stepUpToken + + if (typeof stepUpTokenRaw !== 'string' || stepUpTokenRaw.length === 0) return STEP_UP_REQUIRED() + const verifiedStepUp = verifyStepUpToken(stepUpTokenRaw, deps.keyring) + if (verifiedStepUp === null || verifiedStepUp.agentId !== actingAgent.id) + return STEP_UP_REQUIRED() + const consumed = await deps.store.consumeStepUpToken(verifiedStepUp.nonce) + if (!consumed) return STEP_UP_REQUIRED() + + const existing = await deps.store.listCredentialsForAgent(actingAgent.id) + const { challengeToken, challengeBytes } = await mintAndStoreChallenge( + deps, + 'registration', + actingAgent.id, + ) + const options = await generateRegistrationOptions({ + rpName: deps.rpName, + rpID: deps.rp.rpId, + userName: actingAgent.email, + userID: uuidToBytes(actingAgent.id), + userDisplayName: actingAgent.name, + challenge: challengeBytes, + attestationType: 'none', + excludeCredentials: excludeOrAllowList(existing), + authenticatorSelection: { residentKey: 'required', userVerification: 'required' }, + }) + return json(200, { options, challengeToken }) +} + +// --- POST /api/v1/auth/webauthn/registration/verify ------------------------- + +/** `POST /api/v1/auth/webauthn/registration/verify` (spec §5.2, §6.1) — session-required, step-up-required. `{ response, challengeToken, stepUpToken, name? }`; re-validates (does NOT re-consume) `stepUpToken`. On success: sends the "new passkey added" notification (best-effort, spec §5.3); `409` if `credential_id` already claimed. */ +export async function handleRegistrationVerify( + actingAgent: AgentRecord | null, + request: Request, + deps: WebAuthnApiDeps, +): Promise { + if (actingAgent === null) return UNAUTHORIZED() + + const parsed = await parseJsonBody(request) + const body = parsed.ok ? asRecord(parsed.value) : null + if (body === null) + return apiError(400, 'validation_failed', 'Request body must be a JSON object.') + + if (!checkStepUpToken(body.stepUpToken, actingAgent.id, deps.keyring)) return STEP_UP_REQUIRED() + + const challengeTokenRaw = body.challengeToken + const responseRaw = body.response + if ( + typeof challengeTokenRaw !== 'string' || + typeof responseRaw !== 'object' || + responseRaw === null + ) { + return CEREMONY_FAILED() + } + + // Challenge token: application-level ceremony + agent-binding check + // (spec §7's "registration's extra check"), THEN the DB-level single-use + // consume. + const verifiedChallenge = verifyChallengeToken(challengeTokenRaw, deps.keyring) + if ( + verifiedChallenge === null || + verifiedChallenge.ceremony !== 'registration' || + verifiedChallenge.agentId !== actingAgent.id + ) { + return CEREMONY_FAILED() + } + const consumed = await deps.store.consumeChallenge(verifiedChallenge.nonce, 'registration') + if (!consumed) return CEREMONY_FAILED() + + let verification: Awaited> + try { + verification = await verifyRegistrationResponse({ + response: responseRaw as RegistrationResponseJSON, + expectedChallenge: verifiedChallenge.challengeB64, + expectedOrigin: deps.rp.expectedOrigin, + expectedRPID: deps.rp.rpId, + requireUserVerification: true, + }) + } catch { + return CEREMONY_FAILED() + } + if (!verification.verified || verification.registrationInfo === undefined) + return CEREMONY_FAILED() + + const { registrationInfo } = verification + const name = normalizeRegistrationCredentialName(body.name) + + const inserted = await deps.store.insertCredential({ + agentId: actingAgent.id, + credentialId: registrationInfo.credential.id, + publicKey: registrationInfo.credential.publicKey, + signCount: registrationInfo.credential.counter, + transports: registrationInfo.credential.transports ?? [], + backupEligible: registrationInfo.credentialDeviceType === 'multiDevice', + backupState: registrationInfo.credentialBackedUp, + name, + }) + if (!inserted.ok) { + return apiError(409, 'conflict', 'This passkey is already registered.') + } + + try { + await deps.sender.send( + buildPasskeyAddedEmail({ + to: actingAgent.email, + credentialName: inserted.credential.name, + supportAddress: deps.supportAddress, + mailDomain: deps.mailDomain, + }), + ) + } catch (err) { + // Best-effort, non-blocking (spec §5.3) — the credential is already + // durably created; a notification-send failure must not fail the + // registration response. + console.error('[webauthn] passkey-added notification send failed', err) + } + + return json(201, { credential: toCredentialJson(inserted.credential) }) +} + +// --- GET /api/v1/agents/{id}/webauthn-credentials --------------------------- + +async function resolveTargetAgent( + id: string, + actingAgent: AgentRecord | null, + agentStore: AgentStore, +): Promise<{ ok: true } | { ok: false; response: Response }> { + if (actingAgent === null) return { ok: false, response: UNAUTHORIZED() } + if (actingAgent.role !== 'admin' && actingAgent.id !== id) { + return { ok: false, response: FORBIDDEN() } + } + if (!isUuid(id)) return { ok: false, response: NOT_FOUND() } + const target = await agentStore.getAgent(id) + if (target === null) return { ok: false, response: NOT_FOUND() } + return { ok: true } +} + +/** `GET /api/v1/agents/{id}/webauthn-credentials` (spec §9) — self, or admin. Never returns the public key or raw `credential_id` (spec §9, §10). */ +export async function handleListCredentials( + id: string, + actingAgent: AgentRecord | null, + deps: Pick, +): Promise { + const resolved = await resolveTargetAgent(id, actingAgent, deps.agentStore) + if (!resolved.ok) return resolved.response + + const credentials = await deps.store.listCredentialsForAgent(id) + return json(200, { credentials: credentials.map(toCredentialJson) }) +} + +// --- PATCH /api/v1/agents/{id}/webauthn-credentials/{credentialId} ---------- + +/** `PATCH .../webauthn-credentials/{credentialId}` (spec §9, §5.4) — self, or admin. Rename only; NOT step-up-gated. */ +export async function handlePatchCredential( + id: string, + credentialId: string, + actingAgent: AgentRecord | null, + request: Request, + deps: Pick, +): Promise { + const resolved = await resolveTargetAgent(id, actingAgent, deps.agentStore) + if (!resolved.ok) return resolved.response + if (!isUuid(credentialId)) return NOT_FOUND() + + const parsed = await parseJsonBody(request) + const body = parsed.ok ? asRecord(parsed.value) : null + const name = body === null ? null : validateCredentialName(body.name) + if (name === null) { + return apiError( + 400, + 'validation_failed', + `name is required and must be 1-${MAX_CREDENTIAL_NAME_LENGTH} characters.`, + ) + } + + const updated = await deps.store.renameCredential(credentialId, id, name) + if (updated === null) return NOT_FOUND() + return json(200, { credential: toCredentialJson(updated) }) +} + +// --- DELETE /api/v1/agents/{id}/webauthn-credentials/{credentialId} -------- + +/** `DELETE .../webauthn-credentials/{credentialId}` (spec §9, §5.4, §9.1) — self, or admin. NOT step-up-gated (revoking shrinks an attacker's foothold, it doesn't create one). `409` on the defensive last-credential guard. */ +export async function handleDeleteCredential( + id: string, + credentialId: string, + actingAgent: AgentRecord | null, + deps: Pick, +): Promise { + const resolved = await resolveTargetAgent(id, actingAgent, deps.agentStore) + if (!resolved.ok) return resolved.response + if (!isUuid(credentialId)) return NOT_FOUND() + + const result = await deps.store.deleteCredential(credentialId, id) + if (result === 'not_found') return NOT_FOUND() + if (result === 'last_credential') { + return apiError( + 409, + 'conflict', + 'Cannot revoke this Agent’s only remaining credential without a password identity.', + ) + } + return noContent() +} diff --git a/src/auth/provider.ts b/src/auth/provider.ts index 54e627a..726acd0 100644 --- a/src/auth/provider.ts +++ b/src/auth/provider.ts @@ -18,15 +18,22 @@ /** * What the login UI needs to render one login method, serialized verbatim - * by `GET /api/v1/auth/providers` (spec §6). `kind: 'credentials'` is the - * only kind the core seam defines today (a password form) — deliberately - * not widened to anticipate an OAuth `kind` before a module that needs one - * actually ships (module doc's "do not speculate" note). + * by `GET /api/v1/auth/providers` (spec §6). `kind: 'credentials'` was the + * only kind the core seam defined at HT-54 (a password form) — deliberately + * not widened to anticipate a kind before a module that needs one actually + * shipped (module doc's "do not speculate" note). HT-75 (specs/auth/ + * passkeys.md §4.1) is that module: `kind` widens to `'credentials' | + * 'webauthn'`, the exact type-level change that spec section names as the + * seam's only required edit — no other field is added, since a webauthn + * login needs nothing beyond `{ key: 'webauthn', label, kind: 'webauthn' }` + * to know to render a passkey control; every ceremony detail is fetched + * fresh per-attempt from the options endpoints (passkeys.md §9), never + * baked into this static descriptor. */ export interface AuthProviderDescriptor { key: string label: string - kind: 'credentials' + kind: 'credentials' | 'webauthn' } /** diff --git a/src/auth/webauthn-ceremony.test.ts b/src/auth/webauthn-ceremony.test.ts new file mode 100644 index 0000000..1ab2e31 --- /dev/null +++ b/src/auth/webauthn-ceremony.test.ts @@ -0,0 +1,389 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createPgliteDb, type Db } from '../db/client.js' +import { migrate } from '../db/migrate.js' +import type { Keyring } from '../mail/reply-token.js' +import { type AgentStore, createAgentStore } from '../store/agents.js' +import { createWebAuthnStore, type WebAuthnStore } from '../store/webauthn.js' +import type { WebAuthnRpConfig } from './webauthn-rp.js' +import { mintChallengeToken } from './webauthn-token.js' + +const { verifyAuthenticationResponse } = vi.hoisted(() => ({ + verifyAuthenticationResponse: vi.fn(), +})) + +vi.mock('@simplewebauthn/server', () => ({ verifyAuthenticationResponse })) + +// Imported AFTER the mock is registered, per vitest's hoisting contract. +const { uuidToBytes, verifyAuthenticationCeremony } = await import('./webauthn-ceremony.js') + +const KEYRING: Keyring = { current: { keyId: 'k1', secret: 'a'.repeat(32) } } +const RP: WebAuthnRpConfig = { + rpId: 'inbox.example.test', + expectedOrigin: 'https://inbox.example.test', +} + +function verifiedResult(newCounter: number, credentialBackedUp = false) { + return { + verified: true, + authenticationInfo: { newCounter, credentialBackedUp, credentialDeviceType: 'multiDevice' }, + } +} + +describe('verifyAuthenticationCeremony', () => { + let db: Db | undefined + let store: WebAuthnStore | undefined + let agentStore: AgentStore | undefined + + afterEach(async () => { + vi.clearAllMocks() + await db?.close() + db = undefined + store = undefined + agentStore = undefined + }) + + async function setup(): Promise<{ db: Db; store: WebAuthnStore; agentStore: AgentStore }> { + db = await createPgliteDb() + await migrate(db) + store = createWebAuthnStore(db) + agentStore = createAgentStore(db) + return { db, store, agentStore } + } + + async function makeAgent(a: AgentStore, email: string): Promise { + const result = await a.createAgent({ + name: 'Agent', + email, + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!result.ok) throw new Error('expected ok') + return result.agent.id + } + + async function makeCredential(s: WebAuthnStore, agentId: string, signCount = 0): Promise { + const inserted = await s.insertCredential({ + agentId, + credentialId: 'cred-1', + publicKey: new Uint8Array([1, 2, 3]), + signCount, + transports: ['internal'], + backupEligible: true, + backupState: false, + name: 'Test Passkey', + }) + if (!inserted.ok) throw new Error('expected ok') + return inserted.credential.id + } + + async function mintAuthChallenge( + s: WebAuthnStore, + ceremony: 'authentication' | 'registration' | 'step-up' = 'authentication', + ) { + const minted = mintChallengeToken(ceremony, null, KEYRING) + await s.mintChallenge({ + nonce: minted.nonce, + ceremony, + agentId: null, + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + }) + return minted + } + + function responseFor(credentialId: string, agentId?: string) { + return { + id: credentialId, + rawId: credentialId, + type: 'public-key', + response: { + clientDataJSON: 'x', + authenticatorData: 'x', + signature: 'x', + ...(agentId !== undefined + ? { userHandle: Buffer.from(uuidToBytes(agentId)).toString('base64url') } + : {}), + }, + clientExtensionResults: {}, + } + } + + it('happy path: verified, non-regressing counter — resolves the agentId and persists the new counter', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 0) + const minted = await mintAuthChallenge(s) + verifyAuthenticationResponse.mockResolvedValue(verifiedResult(1)) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', agentId), + challengeToken: minted.token, + }, + ) + + expect(result).toEqual({ ok: true, agentId }) + const after = await s.getCredentialByCredentialId('cred-1') + expect(after?.signCount).toBe(1) + expect(after?.lastUsedAt).not.toBeNull() + }) + + it('challenge_expired: the challenge token verifies but the DB row was never minted (or already consumed)', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 0) + // A signature+TTL-valid token whose nonce has no corresponding DB row. + const minted = mintChallengeToken('authentication', null, KEYRING) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1'), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: false, reason: 'challenge_expired' }) + expect(verifyAuthenticationResponse).not.toHaveBeenCalled() + }) + + it('ceremony mismatch is rejected at the application level, BEFORE the DB consume — the row stays available for its real ceremony', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 0) + const minted = await mintAuthChallenge(s, 'registration') + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1'), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: false, reason: 'invalid' }) + expect(verifyAuthenticationResponse).not.toHaveBeenCalled() + // The row minted for 'registration' is untouched — still consumable under its real ceremony. + expect(await s.consumeChallenge(minted.nonce, 'registration')).toBe(true) + }) + + it('unknown credential id is rejected', async () => { + const { store: s, agentStore: as } = await setup() + await makeAgent(as, 'a@example.test') + const minted = await mintAuthChallenge(s) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('no-such-credential'), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: false, reason: 'invalid' }) + }) + + it('step-up requireAgentId mismatch is rejected before running cryptographic verification', async () => { + const { store: s, agentStore: as } = await setup() + const owner = await makeAgent(as, 'owner@example.test') + const impersonator = await makeAgent(as, 'other@example.test') + await makeCredential(s, owner, 0) + const minted = await mintAuthChallenge(s, 'step-up') + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'step-up', + responseJson: responseFor('cred-1'), + challengeToken: minted.token, + requireAgentId: impersonator, + }, + ) + expect(result).toEqual({ ok: false, reason: 'invalid' }) + expect(verifyAuthenticationResponse).not.toHaveBeenCalled() + }) + + it('a userHandle that resolves to a different Agent than the credential is rejected (defense in depth)', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + const otherAgentId = await makeAgent(as, 'b@example.test') + await makeCredential(s, agentId, 0) + const minted = await mintAuthChallenge(s) + verifyAuthenticationResponse.mockResolvedValue(verifiedResult(1)) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', otherAgentId), // userHandle names the WRONG agent + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: false, reason: 'invalid' }) + }) + + it('Tier 1 (never reported nonzero) is exempt from regression checks — a repeated 0 counter is accepted', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 0) + const minted = await mintAuthChallenge(s) + verifyAuthenticationResponse.mockResolvedValue(verifiedResult(0)) // still 0 — the sentinel + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', agentId), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: true, agentId }) + }) + + it("Tier 2 (has ever reported nonzero): a counter <= the stored maximum is REJECTED and the regression is marked — using a mock that FAITHFULLY reproduces the real library's own throw-on-regression behavior", async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 10) // already graduated to Tier 2 + const minted = await mintAuthChallenge(s) + + // The REAL `verifyAuthenticationResponse` runs its own regression guard + // BEFORE the signature check: `if ((counter > 0 || credential.counter > + // 0) && counter <= credential.counter) throw`. A mock that just + // RESOLVES with a regressed counter (the old version of this test) is a + // false-green over a dead path — the real library never resolves in + // that shape, it throws. This mock reproduces the library's exact + // guard against whatever `credential.counter` our code actually passes, + // so it throws in EXACTLY the case the real library would. + const RESPONSE_COUNTER = 5 // <= the stored maximum of 10: a genuine regression + verifyAuthenticationResponse.mockImplementation(async (opts) => { + const credentialCounter = (opts as { credential: { counter: number } }).credential.counter + if ( + (RESPONSE_COUNTER > 0 || credentialCounter > 0) && + RESPONSE_COUNTER <= credentialCounter + ) { + throw new Error( + `Response counter value ${RESPONSE_COUNTER} was lower than expected ${credentialCounter}`, + ) + } + return verifiedResult(RESPONSE_COUNTER) + }) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', agentId), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: false, reason: 'invalid' }) + + // Proves the fix directly: we deliberately pass `counter: 0` (never the + // real stored `signCount`) so the library's own guard above can never + // fire — our locked Tier-1/Tier-2 logic is the sole authority. If a + // future change regressed to passing the real counter, THIS mock would + // throw before our own logic ever ran, `markCounterRegression` would + // never fire, and the assertions below would fail. + expect(verifyAuthenticationResponse).toHaveBeenCalledWith( + expect.objectContaining({ credential: expect.objectContaining({ counter: 0 }) }), + ) + + const after = await s.getCredentialByCredentialId('cred-1') + expect(after?.signCountRegressionAt).not.toBeNull() // the HT-44 health signal persisted + expect(after?.signCount).toBe(10) // NOT overwritten by the lower, rejected value + }) + + it('a loud, structured log line is emitted at the point a regression is detected (spec §8: "the log line is what makes it investigable")', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + const credentialId = await makeCredential(s, agentId, 10) + const minted = await mintAuthChallenge(s) + verifyAuthenticationResponse.mockResolvedValue(verifiedResult(3)) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + try { + await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', agentId), + challengeToken: minted.token, + }, + ) + const call = warnSpy.mock.calls.find((c) => + String(c[0]).includes('webauthn_counter_regression'), + ) + expect(call).toBeDefined() + const logged = JSON.parse(call?.[0] as string) + expect(logged).toMatchObject({ + event: 'webauthn_counter_regression', + credentialId, + agentId, + storedCounter: 10, + responseCounter: 3, + }) + } finally { + warnSpy.mockRestore() + } + }) + + it('Tier 2: a counter strictly greater than the stored maximum is accepted and persisted', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 10) + const minted = await mintAuthChallenge(s) + verifyAuthenticationResponse.mockResolvedValue(verifiedResult(11)) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', agentId), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: true, agentId }) + const after = await s.getCredentialByCredentialId('cred-1') + expect(after?.signCount).toBe(11) + expect(after?.signCountRegressionAt).toBeNull() + }) + + it('a disabled Agent is rejected even with a fully valid ceremony', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 0) + await as.updateAgent(agentId, { status: 'disabled' }) + const minted = await mintAuthChallenge(s) + verifyAuthenticationResponse.mockResolvedValue(verifiedResult(1)) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', agentId), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: false, reason: 'invalid' }) + }) + + it('a thrown verifyAuthenticationResponse (a malformed response) is treated as invalid, not a crash', async () => { + const { store: s, agentStore: as } = await setup() + const agentId = await makeAgent(as, 'a@example.test') + await makeCredential(s, agentId, 0) + const minted = await mintAuthChallenge(s) + verifyAuthenticationResponse.mockRejectedValue(new Error('malformed clientDataJSON')) + + const result = await verifyAuthenticationCeremony( + { db: db as Db, store: s, keyring: KEYRING, rp: RP }, + { + ceremony: 'authentication', + responseJson: responseFor('cred-1', agentId), + challengeToken: minted.token, + }, + ) + expect(result).toEqual({ ok: false, reason: 'invalid' }) + }) +}) diff --git a/src/auth/webauthn-ceremony.ts b/src/auth/webauthn-ceremony.ts new file mode 100644 index 0000000..4214610 --- /dev/null +++ b/src/auth/webauthn-ceremony.ts @@ -0,0 +1,254 @@ +/** + * The shared "authentication-shaped" WebAuthn response verifier (HT-75; + * specs/auth/passkeys.md §6.2, §8) — the one code path BOTH the login + * ceremony (`webauthn-provider.ts`'s `'authentication'` case, pre-session) + * and the step-up webauthn ceremony (`src/api/webauthn.ts`'s + * `'step-up'` case, session-required) run through. `generateAuthenticationOptions`/ + * `verifyAuthenticationResponse` are the SAME library calls for both — the + * two ceremonies differ only in what `allowCredentials` the OPTIONS step + * offers (spec §5.1: step-up's options call already knows who's asking) and + * in step-up's extra post-verify `requireAgentId` check — not in how a + * signed assertion is actually checked. One reviewed path for a + * security-critical check, per spec §7's own "one mechanism, not several" + * reasoning applied here to the sibling ceremony-verify problem. + * + * ## The TOCTOU fix (spec §6.2's draft.3 CodeRabbit fix) + * + * The counter/clone comparison and the write that updates it happen inside + * ONE transaction, against a `SELECT ... FOR UPDATE` re-read of the SAME + * row `verifyAuthenticationResponse` was handed — NOT the earlier, unlocked + * read used only to supply the library with a public key. See + * `src/store/webauthn.ts`'s `getCredentialForUpdateInTx` doc for why an + * unlocked compare-then-write here would let two concurrent valid + * authentications silently understate the true stored maximum. + * + * ## The library's OWN counter check is deliberately disabled + * + * `verifyAuthenticationResponse` is called with `credential.counter: 0` + * ALWAYS, never the credential's real stored `signCount` — see the inline + * comment at the call site ("Why counter: 0") for the full reasoning. In + * short: the library throws its own regression error using the UNLOCKED + * pre-verification counter, which (if not suppressed) swallows the spec §8 + * signal for every ordinary, sequential replay before this module's own + * locked Tier-1/Tier-2 logic ever runs — leaving `markCounterRegression` + * reachable only in a narrow concurrent-race window. This module is the + * sole authority on counter policy; the library verifies the signature + * only. + * + * ## Counter regression is a COMMIT, not a rollback + * + * When a Tier-2 regression is detected, `markCounterRegression`'s write + * MUST survive — it is the HT-44 health-check signal (spec §8). The + * transaction callback below therefore RETURNS a rejection outcome rather + * than throwing one: throwing would roll back the very write this code + * path exists to persist. Only a genuine DB error (an actual thrown + * exception) rolls back — every business rejection is a returned value, + * mirroring `src/store/agents.ts`'s last-admin-guard shape. + */ + +import type { AuthenticatorTransportFuture } from '@simplewebauthn/server' +import { + type AuthenticationResponseJSON, + verifyAuthenticationResponse, +} from '@simplewebauthn/server' +import type { Db, Queryable } from '../db/client.js' +import type { Keyring } from '../mail/reply-token.js' +import type { WebAuthnStore } from '../store/webauthn.js' +import type { WebAuthnRpConfig } from './webauthn-rp.js' +import { verifyChallengeToken } from './webauthn-token.js' + +/** Convert an Agent's raw uuid bytes (WebAuthn `userHandle`/registration `userID`) to its canonical hyphenated string form. Total over any 16-byte input; a non-16-byte input simply produces a string that will not match any real Agent id, which is the correct (safe) outcome for a malformed/forged `userHandle`. */ +export function bytesToUuid(bytes: Uint8Array): string { + const hex = Buffer.from(bytes).toString('hex') + return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20, 32)}` +} + +/** Convert an Agent's canonical uuid string to the raw 16 bytes minted as WebAuthn `userID` at registration (spec §6.1). */ +export function uuidToBytes(uuid: string): Uint8Array { + return new Uint8Array(Buffer.from(uuid.replace(/-/g, ''), 'hex')) +} + +export interface WebAuthnCeremonyDeps { + db: Db + store: WebAuthnStore + keyring: Keyring + rp: WebAuthnRpConfig +} + +export interface VerifyAuthenticationCeremonyParams { + /** Which ceremony this verify call expects — checked against the token's OWN `ceremony` field before anything else runs (spec §7's application-level discriminator check). */ + ceremony: 'authentication' | 'step-up' + /** The raw, untrusted request-body `response` field. */ + responseJson: unknown + challengeToken: string + /** Step-up only (spec §5.1): the resolved credential's `agent_id` must equal this, or the ceremony is rejected — proving a factor for a DIFFERENT Agent does not step up THIS session. */ + requireAgentId?: string +} + +/** Every rejection this function can return collapses to one of two client-visible outcomes (spec §4.3, §6.2): `'challenge_expired'` (the one deliberate, safe exception to uniform 401 — see webauthn-provider.ts) or `'invalid'` (everything else, including a ceremony mismatch, unknown credential, bad signature, counter regression, inactive Agent, or userHandle mismatch — no finer distinction is ever surfaced). */ +export type CeremonyVerifyResult = + | { ok: true; agentId: string } + | { ok: false; reason: 'challenge_expired' | 'invalid' } + +/** + * Verify an authentication-shaped WebAuthn response end to end: challenge + * token (signature+TTL, then DB single-use consume), credential lookup, + * cryptographic verification, `userHandle` cross-check, and the atomic + * counter/clone policy (spec §8) inside one locked transaction. See the + * module doc for the two ceremonies that share this path. + */ +export async function verifyAuthenticationCeremony( + deps: WebAuthnCeremonyDeps, + params: VerifyAuthenticationCeremonyParams, +): Promise { + // --- Challenge token: application-level ceremony check BEFORE the DB (spec §7). --- + const verifiedToken = verifyChallengeToken(params.challengeToken, deps.keyring) + if (verifiedToken === null || verifiedToken.ceremony !== params.ceremony) { + return { ok: false, reason: 'invalid' } + } + + // --- DB-level single-use consume — the actual enforcement (spec §7). A + // zero-row consume (missing, expired, already-used, or wrong ceremony) + // is the one case the caller may surface as `challenge_expired`. --- + const consumed = await deps.store.consumeChallenge(verifiedToken.nonce, params.ceremony) + if (!consumed) return { ok: false, reason: 'challenge_expired' } + + // --- Resolve the credential by the assertion's OWN id (spec §4.3 — + // discovered, never asserted by the caller). --- + if (typeof params.responseJson !== 'object' || params.responseJson === null) { + return { ok: false, reason: 'invalid' } + } + const responseJson = params.responseJson as AuthenticationResponseJSON + if (typeof responseJson.id !== 'string' || responseJson.id.length === 0) { + return { ok: false, reason: 'invalid' } + } + + const credential = await deps.store.getCredentialByCredentialId(responseJson.id) + if (credential === null) return { ok: false, reason: 'invalid' } + + if (params.requireAgentId !== undefined && credential.agentId !== params.requireAgentId) { + return { ok: false, reason: 'invalid' } + } + + let verification: Awaited> + try { + verification = await verifyAuthenticationResponse({ + response: responseJson, + expectedChallenge: verifiedToken.challengeB64, + expectedOrigin: deps.rp.expectedOrigin, + expectedRPID: deps.rp.rpId, + credential: { + id: credential.credentialId, + // A fresh `Uint8Array` (not the `Buffer` the pg/PGlite driver hands + // back for a `bytea` column) — `@simplewebauthn/server`'s types + // require `Uint8Array` specifically, which a `Buffer` + // (typed `Uint8Array`) does not structurally + // satisfy even though it works correctly at runtime. + publicKey: new Uint8Array(credential.publicKey), + // DELIBERATELY 0, never `credential.signCount` — see "Why counter: + // 0" below. This is load-bearing, not a placeholder. + counter: 0, + transports: credential.transports as AuthenticatorTransportFuture[], + }, + requireUserVerification: true, + }) + } catch { + return { ok: false, reason: 'invalid' } + } + if (!verification.verified) return { ok: false, reason: 'invalid' } + + // --- Why counter: 0 above --------------------------------------------- + // + // `verifyAuthenticationResponse` runs its OWN counter-regression guard + // BEFORE the signature check, against whatever `credential.counter` it's + // handed: `if ((counter > 0 || credential.counter > 0) && counter <= + // credential.counter) throw`. Had this been given the real + // `credential.signCount` (from the UNLOCKED pre-verification read above), + // that guard would THROW on any ordinary, non-concurrent counter + // regression — caught by the try/catch above and folded into a generic + // `{ ok: false, reason: 'invalid' }` BEFORE `authenticationInfo.newCounter` + // is ever obtained and BEFORE the transaction below runs at all. That + // makes `markCounterRegression` (and the HT-44 alert it feeds) reachable + // ONLY in the narrow race where the unlocked read was stale enough to slip + // past the library's check but the locked re-read below still catches + // it — the library's own guard would silently eat the spec §8 signal for + // the common, sequential-replay case, which is exactly the case the + // signal exists to catch. + // + // Passing `counter: 0` makes `(counter > 0 || false) && counter <= 0` + // structurally unsatisfiable (a uint32 response counter cannot be both + // `> 0` and `<= 0`), so the library NEVER throws for this reason — it + // verifies ONLY the cryptographic signature and hands back the response's + // raw counter in `authenticationInfo.newCounter`. This is also the + // architecture spec §6.2 itself describes: "On a successful SIGNATURE + // verification, the handler re-reads the same row with SELECT ... FOR + // UPDATE... applies §8's Tier 1/Tier 2 comparison" — signature + // verification and counter policy are two separate steps, and OUR + // Tier-1/Tier-2 logic under the lock below is the sole, sequential- and + // concurrent-case-alike authority on the latter. + + // --- userHandle cross-check (spec §6.2) — defense in depth, not the + // identity-resolution path (that was credential_id, above). Optional + // chaining: `response.response` being absent/malformed would already + // have been rejected by the try/catch above against the REAL library + // (its own verification touches `response.response.*` first), but this + // function must not assume that of a mocked/future verify implementation + // — a malformed shape here is just another "invalid", never a crash. --- + const userHandleB64 = responseJson.response?.userHandle + if (userHandleB64 !== undefined) { + const userHandleAgentId = bytesToUuid(Buffer.from(userHandleB64, 'base64url')) + if (userHandleAgentId !== credential.agentId) return { ok: false, reason: 'invalid' } + } + + const newCounter = verification.authenticationInfo.newCounter + const backedUp = verification.authenticationInfo.credentialBackedUp + + type TxOutcome = { kind: 'ok'; agentId: string } | { kind: 'rejected' } + const outcome = await deps.db.transaction(async (tx: Queryable) => { + // The locked re-read the TOCTOU fix requires (module doc) — every + // subsequent decision uses THIS row, not the unlocked one above. + const locked = await deps.store.getCredentialForUpdateInTx(credential.credentialId, tx) + if (locked === null) return { kind: 'rejected' } + + // Spec §8: Tier 1 (never reported nonzero) is exempt; Tier 2 (has ever + // reported nonzero) rejects any counter <= the locked stored maximum. + const isTierTwo = locked.signCount > 0 + const isRegression = isTierTwo && newCounter <= locked.signCount + if (isRegression) { + // The log line is what makes this investigable; the DB column + // (below) is what makes it alertable (spec §8, mirroring + // `src/mail/ingest.ts`'s `forged_token_detected` event — the + // identical "structured warn + a health-check-visible column" + // shape for a sibling clone/forgery signal). + console.warn( + JSON.stringify({ + event: 'webauthn_counter_regression', + credentialId: locked.id, + agentId: locked.agentId, + storedCounter: locked.signCount, + responseCounter: newCounter, + }), + ) + // COMMIT this write — see the module doc on why this is a return, + // not a throw. + await deps.store.markCounterRegression(locked.id, tx) + return { kind: 'rejected' } + } + + const statusRows = await tx.query<{ status: string }>( + 'SELECT status FROM agents WHERE id = $1', + [locked.agentId], + ) + if (statusRows[0]?.status !== 'active') return { kind: 'rejected' } + + await deps.store.updateAfterSuccessfulAuth( + locked.id, + { signCount: newCounter, backupState: backedUp }, + tx, + ) + return { kind: 'ok', agentId: locked.agentId } + }) + + if (outcome.kind !== 'ok') return { ok: false, reason: 'invalid' } + return { ok: true, agentId: outcome.agentId } +} diff --git a/src/auth/webauthn-notify-email.ts b/src/auth/webauthn-notify-email.ts new file mode 100644 index 0000000..b7a3acc --- /dev/null +++ b/src/auth/webauthn-notify-email.ts @@ -0,0 +1,50 @@ +/** + * Build the "new passkey added" notification email (HT-75; + * specs/auth/passkeys.md §5.3) — sent on every successful + * `registration/verify`, out-of-band evidence that a credential was + * created, since §10 notes a stolen session's registration attempt would + * otherwise be invisible to the legitimate Agent. + * + * Same precedent as `src/auth/invite-email.ts`'s `buildInviteEmail`: build + * a fresh {@link OutboundEmail} and hand it directly to the configured + * `EmailSender`, NOT through `sendReply`/`src/mail/send.ts` — there is no + * conversation this belongs to. `messageId` is likewise a bare id, not a + * reply token — nothing ever routes an inbound reply back to it. + */ + +import { randomUUID } from 'node:crypto' +import type { OutboundEmail } from '../providers/email-sender.js' + +/** Input to {@link buildPasskeyAddedEmail}. */ +export interface PasskeyAddedEmailInput { + /** The Agent's own email address. */ + to: string + /** The credential's user-assigned (or server-defaulted) name — spec §9's `name` field. */ + credentialName: string + /** The deployment's configured support address — the `from` on this email. */ + supportAddress: string + /** Domain minted into the bare `Message-ID` — matches every other outbound message's `@domain` part. */ + mailDomain: string +} + +/** + * Build the notification email. Text-only, minimal — matches + * `buildInviteEmail`'s "no fabricated tracking or styling" posture. + * Content per spec §5.3: which credential, roughly when, and a one-line + * "if this wasn't you" remediation pointer. + */ +export function buildPasskeyAddedEmail(input: PasskeyAddedEmailInput): OutboundEmail { + return { + messageId: ``, + from: input.supportAddress, + to: [input.to], + subject: 'A new passkey was added to your Helpthread account', + text: [ + `A new passkey named "${input.credentialName}" was just added to your account.`, + '', + `Time: ${new Date().toISOString()}`, + '', + "If this wasn't you, revoke it from your profile and change your password immediately.", + ].join('\n'), + } +} diff --git a/src/auth/webauthn-provider.test.ts b/src/auth/webauthn-provider.test.ts new file mode 100644 index 0000000..b1ee021 --- /dev/null +++ b/src/auth/webauthn-provider.test.ts @@ -0,0 +1,137 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createPgliteDb, type Db } from '../db/client.js' +import { migrate } from '../db/migrate.js' +import type { Keyring } from '../mail/reply-token.js' +import { type AgentStore, createAgentStore } from '../store/agents.js' +import { createWebAuthnStore, type WebAuthnStore } from '../store/webauthn.js' +import type { AuthProvider } from './provider.js' +import type { WebAuthnRpConfig } from './webauthn-rp.js' +import { mintChallengeToken } from './webauthn-token.js' + +const { verifyAuthenticationResponse } = vi.hoisted(() => ({ + verifyAuthenticationResponse: vi.fn(), +})) + +vi.mock('@simplewebauthn/server', () => ({ verifyAuthenticationResponse })) + +const { createWebAuthnAuthProvider, WebAuthnChallengeExpiredError } = await import( + './webauthn-provider.js' +) + +const KEYRING: Keyring = { current: { keyId: 'k1', secret: 'a'.repeat(32) } } +const RP: WebAuthnRpConfig = { + rpId: 'inbox.example.test', + expectedOrigin: 'https://inbox.example.test', +} + +describe('createWebAuthnAuthProvider', () => { + let db: Db | undefined + let store: WebAuthnStore | undefined + let agentStore: AgentStore | undefined + let provider: AuthProvider | undefined + + afterEach(async () => { + vi.clearAllMocks() + await db?.close() + db = undefined + store = undefined + agentStore = undefined + provider = undefined + }) + + async function freshProvider(): Promise<{ + store: WebAuthnStore + agentStore: AgentStore + provider: AuthProvider + }> { + db = await createPgliteDb() + await migrate(db) + store = createWebAuthnStore(db) + agentStore = createAgentStore(db) + provider = createWebAuthnAuthProvider({ db, store, keyring: KEYRING, rp: RP }) + return { store, agentStore, provider } + } + + it('descriptor() reports key: webauthn, kind: webauthn', async () => { + const { provider: p } = await freshProvider() + expect(p.descriptor()).toEqual({ key: 'webauthn', label: expect.any(String), kind: 'webauthn' }) + }) + + it('resolves the correct Agent on a fully valid ceremony', async () => { + const { store: s, agentStore: as, provider: p } = await freshProvider() + const created = await as.createAgent({ + name: 'Agent', + email: 'a@example.test', + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!created.ok) throw new Error('expected ok') + await s.insertCredential({ + agentId: created.agent.id, + credentialId: 'cred-1', + publicKey: new Uint8Array([1]), + signCount: 0, + transports: [], + backupEligible: false, + backupState: false, + name: 'Key', + }) + const minted = mintChallengeToken('authentication', null, KEYRING) + await s.mintChallenge({ + nonce: minted.nonce, + ceremony: 'authentication', + agentId: null, + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + }) + verifyAuthenticationResponse.mockResolvedValue({ + verified: true, + authenticationInfo: { newCounter: 1, credentialBackedUp: false }, + }) + + const result = await p.authenticate({ + providerKey: 'webauthn', + response: { id: 'cred-1', response: {} }, + challengeToken: minted.token, + }) + expect(result).toEqual({ agentId: created.agent.id }) + }) + + it('returns null (not a throw) for an ordinary invalid ceremony — e.g. an unknown credential', async () => { + const { provider: p } = await freshProvider() + const minted = mintChallengeToken('authentication', null, KEYRING) + await (store as WebAuthnStore).mintChallenge({ + nonce: minted.nonce, + ceremony: 'authentication', + agentId: null, + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + }) + + const result = await p.authenticate({ + providerKey: 'webauthn', + response: { id: 'no-such-credential', response: {} }, + challengeToken: minted.token, + }) + expect(result).toBeNull() + }) + + it('THROWS WebAuthnChallengeExpiredError specifically when the challenge token is expired/missing/already-used', async () => { + const { provider: p } = await freshProvider() + // A signature+TTL-valid token whose DB row was never minted. + const minted = mintChallengeToken('authentication', null, KEYRING) + + await expect( + p.authenticate({ + providerKey: 'webauthn', + response: { id: 'cred-1', response: {} }, + challengeToken: minted.token, + }), + ).rejects.toBeInstanceOf(WebAuthnChallengeExpiredError) + }) + + it('returns null for a malformed attempt (missing challengeToken/response) without touching the store', async () => { + const { provider: p } = await freshProvider() + expect(await p.authenticate({ providerKey: 'webauthn' })).toBeNull() + expect(await p.authenticate({ providerKey: 'webauthn', challengeToken: 'x' })).toBeNull() + }) +}) diff --git a/src/auth/webauthn-provider.ts b/src/auth/webauthn-provider.ts new file mode 100644 index 0000000..eb8233b --- /dev/null +++ b/src/auth/webauthn-provider.ts @@ -0,0 +1,73 @@ +/** + * `WebAuthnAuthProvider` — the passkey login `AuthProvider` (HT-75; + * specs/auth/passkeys.md §4). This is ONLY the final verify step dispatched + * via the existing generic `POST /auth/verify { providerKey: 'webauthn', + * response, challengeToken }` (spec §4.2) — the options-minting pre-step + * (`authentication/options`) lives outside the `AuthProvider` interface + * entirely, in `src/api/webauthn.ts`, per spec §4.2's own reasoning + * (`authenticate()` is a single-shot contract; a two-step ceremony's + * options-minting doesn't fit it and shouldn't grow a hook for one provider). + * + * How this differs from `PasswordAuthProvider` (spec §4.3): + * + * - No identifier is asserted by the caller — the identity is DISCOVERED + * from the assertion's own credential id (`verifyAuthenticationCeremony`). + * - Verification is cryptographic, not a KDF comparison — no `DUMMY_HASH`- + * style timing equalization is needed (a credential id carries no + * enumeration oracle the way an email address does). + * - One narrow, deliberate exception to "uniform 401": a `challenge_expired` + * ceremony-freshness signal (spec §6.2) is distinguishable — this + * provider signals it by throwing {@link WebAuthnChallengeExpiredError}, + * which `handleAuthVerify` (`src/api/agents.ts`) catches specifically and + * nothing else does; every OTHER rejection returns `null` exactly like + * `password`'s provider. + */ + +import type { + AuthAttempt, + AuthProvider, + AuthProviderDescriptor, + VerifiedIdentity, +} from './provider.js' +import { verifyAuthenticationCeremony, type WebAuthnCeremonyDeps } from './webauthn-ceremony.js' + +/** Thrown by {@link createWebAuthnAuthProvider}'s `authenticate()` on the one distinguishable failure mode (spec §6.2). Caught ONLY by `handleAuthVerify`'s webauthn dispatch — never surfaces past `src/api/agents.ts`. */ +export class WebAuthnChallengeExpiredError extends Error { + constructor() { + super('webauthn: challenge expired, missing, or already used') + this.name = 'WebAuthnChallengeExpiredError' + } +} + +const DESCRIPTOR: AuthProviderDescriptor = { + key: 'webauthn', + label: 'Passkey', + kind: 'webauthn', +} + +/** Build the passkey login `AuthProvider`. `deps` is the same {@link WebAuthnCeremonyDeps} the step-up webauthn ceremony (`src/api/webauthn.ts`) shares — one RP config, one store, one keyring, wired once at the composition root. */ +export function createWebAuthnAuthProvider(deps: WebAuthnCeremonyDeps): AuthProvider { + return { + key: 'webauthn', + + descriptor(): AuthProviderDescriptor { + return DESCRIPTOR + }, + + async authenticate(attempt: AuthAttempt): Promise { + const { response, challengeToken } = attempt + if (typeof challengeToken !== 'string' || challengeToken.length === 0) return null + if (typeof response !== 'object' || response === null) return null + + const result = await verifyAuthenticationCeremony(deps, { + ceremony: 'authentication', + responseJson: response, + challengeToken, + }) + + if (result.ok) return { agentId: result.agentId } + if (result.reason === 'challenge_expired') throw new WebAuthnChallengeExpiredError() + return null + }, + } +} diff --git a/src/auth/webauthn-rp.test.ts b/src/auth/webauthn-rp.test.ts new file mode 100644 index 0000000..2225c00 --- /dev/null +++ b/src/auth/webauthn-rp.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest' +import { resolveWebAuthnRp } from './webauthn-rp.js' + +describe('resolveWebAuthnRp', () => { + it('resolves a normal https origin to its hostname', () => { + expect(resolveWebAuthnRp('https://inbox.resonantiq.app')).toEqual({ + rpId: 'inbox.resonantiq.app', + expectedOrigin: 'https://inbox.resonantiq.app', + }) + }) + + it('accepts localhost (a valid domain-form hostname) for dev', () => { + expect(resolveWebAuthnRp('http://localhost:3000')).toEqual({ + rpId: 'localhost', + expectedOrigin: 'http://localhost:3000', + }) + }) + + it('rejects an IPv4 loopback literal even though config.ts accepts it as a UI base URL generally', () => { + expect(() => resolveWebAuthnRp('http://127.0.0.1:3000')).toThrow(/domain name/) + }) + + it('rejects a bracketed IPv6 loopback literal', () => { + expect(() => resolveWebAuthnRp('http://[::1]:3000')).toThrow(/domain name/) + }) + + it('rejects a non-loopback IPv4 literal too (not just loopback)', () => { + expect(() => resolveWebAuthnRp('https://93.184.216.34')).toThrow(/domain name/) + }) + + it('rejects a malformed URL', () => { + expect(() => resolveWebAuthnRp('not a url')).toThrow() + }) + + it('preserves the exact origin verbatim, including a non-default port', () => { + const resolved = resolveWebAuthnRp('https://inbox.example.test:8443') + expect(resolved.expectedOrigin).toBe('https://inbox.example.test:8443') + expect(resolved.rpId).toBe('inbox.example.test') + }) +}) diff --git a/src/auth/webauthn-rp.ts b/src/auth/webauthn-rp.ts new file mode 100644 index 0000000..aac890b --- /dev/null +++ b/src/auth/webauthn-rp.ts @@ -0,0 +1,58 @@ +/** + * Relying Party (RP) id/origin resolution (HT-75; specs/auth/passkeys.md + * §3) — the one place `rpId`/`expectedOrigin` are derived, from + * `config.uiBaseUrl` and NOTHING else. This is the load-bearing + * phishing-resistance property (spec §3): deriving the expected origin from + * anything request-supplied would let an attacker assert the origin they + * want checked against, collapsing WebAuthn's whole protection to nothing. + * + * `HELPTHREAD_UI_BASE_URL`'s general validator (`src/composition/config.ts`) + * already enforces "https, or http on a loopback host" — this module adds + * the ONE constraint that validator doesn't know about and can't, because + * it's specific to WebAuthn: an RP ID must be a domain-form hostname (MDN: + * "must be a domain name"), which `127.0.0.1`/`[::1]` are not, even though + * `config.ts` accepts them as a valid UI base URL for every OTHER purpose + * (spec §3's documented gap — local passkey dev must use + * `http://localhost:`). + */ + +/** IPv4 dotted-quad, or a bracketed/unbracketed IPv6 literal — none of these are a valid WebAuthn RP ID (module doc). */ +function isIpLiteralHostname(hostname: string): boolean { + if (hostname.startsWith('[') || hostname.includes(':')) return true // IPv6 literal + return /^\d{1,3}(\.\d{1,3}){3}$/.test(hostname) // IPv4 dotted-quad +} + +/** The resolved RP id + expected origin every WebAuthn ceremony (registration/authentication/step-up) is bound to. */ +export interface WebAuthnRpConfig { + /** The bare hostname (no scheme, no port) of `uiBaseUrl` — e.g. `inbox.resonantiq.app`. */ + rpId: string + /** `uiBaseUrl` verbatim — the exact origin `clientDataJSON.origin` must match. */ + expectedOrigin: string +} + +/** + * Resolve `{ rpId, expectedOrigin }` from `uiBaseUrl` (spec §3). Throws if + * `uiBaseUrl` is not a well-formed absolute URL, or if its hostname is an IP + * literal — WebAuthn's RP ID requirement is narrower than `config.ts`'s + * general UI-base-URL validator (module doc). Called ONCE at composition + * (the caller decides whether to wire the passkey provider at all when this + * throws — `uiBaseUrl` unset means the caller never calls this). + */ +export function resolveWebAuthnRp(uiBaseUrl: string): WebAuthnRpConfig { + let parsed: URL + try { + parsed = new URL(uiBaseUrl) + } catch { + throw new Error( + `resolveWebAuthnRp: uiBaseUrl is not a valid absolute URL (got ${JSON.stringify(uiBaseUrl)})`, + ) + } + if (isIpLiteralHostname(parsed.hostname)) { + throw new Error( + `resolveWebAuthnRp: uiBaseUrl's host must be a domain name, not an IP literal (got ${JSON.stringify( + parsed.hostname, + )}) — passkeys require a domain-form RP ID; use http://localhost: for local dev (spec §3).`, + ) + } + return { rpId: parsed.hostname, expectedOrigin: uiBaseUrl } +} diff --git a/src/auth/webauthn-token.test.ts b/src/auth/webauthn-token.test.ts new file mode 100644 index 0000000..df3d390 --- /dev/null +++ b/src/auth/webauthn-token.test.ts @@ -0,0 +1,134 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Keyring } from '../mail/reply-token.js' +import { + mintChallengeToken, + mintStepUpToken, + verifyChallengeToken, + verifyStepUpToken, +} from './webauthn-token.js' + +const KEYRING: Keyring = { current: { keyId: 'k1', secret: 'a'.repeat(32) } } +const AGENT_ID = '00000000-0000-4000-8000-000000000000' +const OTHER_AGENT_ID = '11111111-1111-4111-8111-111111111111' + +describe('mintChallengeToken / verifyChallengeToken', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('a freshly minted token verifies and recovers the full payload', () => { + const minted = mintChallengeToken('registration', AGENT_ID, KEYRING) + expect(verifyChallengeToken(minted.token, KEYRING)).toEqual({ + ceremony: 'registration', + challengeB64: minted.challengeB64, + agentId: AGENT_ID, + nonce: minted.nonce, + }) + }) + + it('authentication tokens carry a null agentId (pre-identification)', () => { + const minted = mintChallengeToken('authentication', null, KEYRING) + const verified = verifyChallengeToken(minted.token, KEYRING) + expect(verified?.agentId).toBeNull() + }) + + it('is shaped htw.{keyId}.{payload}.{sig} — four dot-separated segments, htw prefix', () => { + const minted = mintChallengeToken('step-up', AGENT_ID, KEYRING) + const segments = minted.token.split('.') + expect(segments).toHaveLength(4) + expect(segments[0]).toBe('htw') + expect(segments[1]).toBe('k1') + }) + + it('two mints produce different nonces and different challenges', () => { + const a = mintChallengeToken('authentication', null, KEYRING) + const b = mintChallengeToken('authentication', null, KEYRING) + expect(a.nonce).not.toBe(b.nonce) + expect(a.challengeB64).not.toBe(b.challengeB64) + }) + + it('rejects an expired token (past the TTL)', () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')) + const minted = mintChallengeToken('authentication', null, KEYRING) + vi.setSystemTime(new Date('2026-01-01T00:05:01Z')) // TTL is 5 minutes + expect(verifyChallengeToken(minted.token, KEYRING)).toBeNull() + }) + + it('a forged signature does not verify', () => { + const minted = mintChallengeToken('authentication', null, KEYRING) + const segments = minted.token.split('.') + segments[3] = 'A'.repeat(segments[3].length) + expect(verifyChallengeToken(segments.join('.'), KEYRING)).toBeNull() + }) + + it('a tampered ceremony field does not verify (the payload is part of the signed canonical string)', () => { + const registration = mintChallengeToken('registration', AGENT_ID, KEYRING) + const authentication = mintChallengeToken('authentication', null, KEYRING) + const segments = registration.token.split('.') + const otherSegments = authentication.token.split('.') + segments[2] = otherSegments[2] // splice a different payload onto this token's signature + expect(verifyChallengeToken(segments.join('.'), KEYRING)).toBeNull() + }) + + it('mintChallengeToken throws for a non-uuid agentId', () => { + expect(() => mintChallengeToken('registration', 'not-a-uuid', KEYRING)).toThrow() + }) + + it('mintChallengeToken throws for a malformed keyring', () => { + expect(() => + mintChallengeToken('authentication', null, { current: { keyId: 'k1', secret: 'short' } }), + ).toThrow() + }) + + it('never verifies against a different token type (htsu.)', () => { + const stepUp = mintStepUpToken(AGENT_ID, KEYRING) + expect(verifyChallengeToken(stepUp.token, KEYRING)).toBeNull() + }) +}) + +describe('mintStepUpToken / verifyStepUpToken', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('a freshly minted token verifies and recovers the agentId', () => { + const minted = mintStepUpToken(AGENT_ID, KEYRING) + expect(verifyStepUpToken(minted.token, KEYRING)).toEqual({ + agentId: AGENT_ID, + nonce: minted.nonce, + }) + }) + + it('is shaped htsu.{keyId}.{payload}.{sig} — four dot-separated segments, htsu prefix', () => { + const minted = mintStepUpToken(AGENT_ID, KEYRING) + const segments = minted.token.split('.') + expect(segments).toHaveLength(4) + expect(segments[0]).toBe('htsu') + }) + + it('rejects an expired token (past the TTL)', () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')) + const minted = mintStepUpToken(AGENT_ID, KEYRING) + vi.setSystemTime(new Date('2026-01-01T00:05:01Z')) + expect(verifyStepUpToken(minted.token, KEYRING)).toBeNull() + }) + + it('a tampered agentId does not verify', () => { + const mine = mintStepUpToken(AGENT_ID, KEYRING) + const theirs = mintStepUpToken(OTHER_AGENT_ID, KEYRING) + const segments = mine.token.split('.') + segments[2] = theirs.token.split('.')[2] + expect(verifyStepUpToken(segments.join('.'), KEYRING)).toBeNull() + }) + + it('mintStepUpToken throws for a non-uuid agentId', () => { + expect(() => mintStepUpToken('not-a-uuid', KEYRING)).toThrow() + }) + + it('never verifies against a different token type (htw.)', () => { + const challenge = mintChallengeToken('registration', AGENT_ID, KEYRING) + expect(verifyStepUpToken(challenge.token, KEYRING)).toBeNull() + }) +}) diff --git a/src/auth/webauthn-token.ts b/src/auth/webauthn-token.ts new file mode 100644 index 0000000..e96adfe --- /dev/null +++ b/src/auth/webauthn-token.ts @@ -0,0 +1,324 @@ +/** + * Signed WebAuthn ceremony tokens (HT-75; specs/auth/passkeys.md §5, §7) — + * the stateless HMAC half of the two-layer challenge/step-up discipline + * those sections specify. Mirrors `src/auth/invite-token.ts`'s shape (a + * single base64url JSON payload segment, current+retired key rotation, + * constant-time verification) off the same {@link Keyring} — the same + * pattern reused for a new domain, per this codebase's standing convention + * (`invite-token.ts`'s own module doc makes the same move off + * `gmail-connect.ts`'s `state` token). + * + * Two DISTINCT token types, two DISTINCT domain-separator prefixes, in one + * file because both are pure HMAC mint/verify pairs over the same + * `Keyring` with near-identical mechanics — keeping them together avoids + * duplicating the shared `sign`/`candidateKeys`/`signatureMatches` helpers + * a third time (they already exist once each in `invite-token.ts` and + * `gmail-connect.ts`). + * + * ## `htw.` — the WebAuthn ceremony challenge token (spec §7) + * + * ``` + * htw.{keyId}.{payload-b64url}.{sig-b64url} + * ``` + * + * Payload: `{ ceremony, challengeB64, agentId, nonce, issuedAtMs }`. + * `challengeB64` is the actual WebAuthn ceremony challenge (32 random + * bytes) handed to `generateRegistrationOptions`/`generateAuthenticationOptions` + * and checked byte-for-byte at verify time; `nonce` is a SEPARATE random + * value that is the primary key of the `webauthn_challenges` DB row + * (`src/store/webauthn.ts`) — the token proves freshness and carries the + * challenge bytes back to the verify step, but the DB row is what actually + * enforces single-use (spec §7: "a bare signature+TTL check can be + * satisfied twice"). Default TTL 5 minutes (spec §7). + * + * ## `htsu.` — the step-up proof token (spec §5.1) + * + * ``` + * htsu.{keyId}.{payload-b64url}.{sig-b64url} + * ``` + * + * Payload: `{ agentId, issuedAtMs, nonce }` — proof that the ACTING Agent + * recently demonstrated an existing factor (password or an existing + * passkey). `nonce` is likewise the `webauthn_stepup_tokens` row's primary + * key. Default TTL 5 minutes (spec §5.1). + * + * ## Security properties (mirrors invite-token.ts / gmail-connect.ts) + * + * - Both `mint*` functions are STRICT: throw on a malformed keyring + * ({@link assertValidKeyring}) — minting an unverifiable token is a bug. + * - Both `verify*` functions are TOTAL over the token string: every + * rejection path returns `null`, never throws, since both are reachable + * with fully untrusted input (a webauthn ceremony verify endpoint, or a + * step-up-spending endpoint). + * - Signature verification happens BEFORE the payload is ever JSON-parsed — + * same "no parse-oracle independent of the signature" property + * `invite-token.ts` documents. + * - `htw.` and `htsu.` can never verify as each other, or as `hti.` + * (invite), `gmc.` (Gmail state), or `ht.` (reply tokens) — the literal + * prefix is part of the signed canonical string, not just the wire + * format (spec §7). + */ + +import { createHmac, randomBytes, timingSafeEqual } from 'node:crypto' +import { assertValidKeyring, type Keyring, type SigningKey } from '../mail/reply-token.js' + +/** The three WebAuthn ceremonies (spec §2.2, §7). */ +export type WebAuthnCeremony = 'registration' | 'authentication' | 'step-up' + +const UUID_PATTERN = /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/ + +function sign(secret: string, canonical: string): string { + return createHmac('sha256', secret).update(canonical).digest('base64url') +} + +/** Keys in the ring (current first, then retired) whose keyId matches the token's. Same helper `invite-token.ts`/`gmail-connect.ts` each keep a local copy of. */ +function candidateKeys(keyring: Keyring, keyId: string): SigningKey[] { + const all = keyring.retired ? [keyring.current, ...keyring.retired] : [keyring.current] + return all.filter((key) => key.keyId === keyId) +} + +/** Constant-time signature check — same length-guarded pattern as `invite-token.ts`/`gmail-connect.ts`. */ +function signatureMatches(secret: string, canonical: string, providedSig: string): boolean { + const expected = Buffer.from(sign(secret, canonical)) + const provided = Buffer.from(providedSig) + if (expected.length !== provided.length) return false + return timingSafeEqual(expected, provided) +} + +// --- htw. challenge token ---------------------------------------------- + +const CHALLENGE_PREFIX = 'htw' +const CHALLENGE_SEGMENT_COUNT = 4 +const CHALLENGE_BYTES = 32 +const CHALLENGE_NONCE_BYTES = 16 + +/** Default challenge-token TTL: 5 minutes (spec §7 — generous slack above the ceremony's own client-side 60s default timeout). */ +export const DEFAULT_CHALLENGE_TOKEN_TTL_MS = 5 * 60 * 1000 + +interface ChallengeTokenPayload { + ceremony: WebAuthnCeremony + challengeB64: string + agentId: string | null + nonce: string + issuedAtMs: number +} + +function challengeCanonicalString(keyId: string, payloadB64: string): string { + return `${CHALLENGE_PREFIX}.${keyId}.${payloadB64}` +} + +/** What a freshly minted challenge token carries — everything both the caller (to build ceremony options) and the DB row (`webauthn_challenges`) need. */ +export interface MintedChallengeToken { + token: string + nonce: string + /** base64url WebAuthn challenge bytes — pass verbatim as `options.challenge`. */ + challengeB64: string +} + +/** + * Mint an `htw.` challenge token for `ceremony`. `agentId` is the acting + * Agent for `registration`/`step-up` (bound at mint time from the session) + * and `null` for `authentication` (pre-identification, spec §6.2). STRICT: + * throws on a malformed keyring (module doc). + */ +export function mintChallengeToken( + ceremony: WebAuthnCeremony, + agentId: string | null, + keyring: Keyring, +): MintedChallengeToken { + assertValidKeyring(keyring) + if (agentId !== null && (typeof agentId !== 'string' || !UUID_PATTERN.test(agentId))) { + throw new Error( + `mintChallengeToken: agentId must be a uuid or null (got ${JSON.stringify(agentId)})`, + ) + } + + const { keyId, secret } = keyring.current + const challengeB64 = randomBytes(CHALLENGE_BYTES).toString('base64url') + const payload: ChallengeTokenPayload = { + ceremony, + challengeB64, + agentId, + nonce: randomBytes(CHALLENGE_NONCE_BYTES).toString('base64url'), + issuedAtMs: Date.now(), + } + const payloadB64 = Buffer.from(JSON.stringify(payload), 'utf8').toString('base64url') + const sig = sign(secret, challengeCanonicalString(keyId, payloadB64)) + return { + token: `${CHALLENGE_PREFIX}.${keyId}.${payloadB64}.${sig}`, + nonce: payload.nonce, + challengeB64, + } +} + +/** A verified `htw.` token's payload. */ +export interface VerifiedChallengeToken { + ceremony: WebAuthnCeremony + challengeB64: string + agentId: string | null + nonce: string +} + +/** + * Verify a candidate `htw.` token: well-formed, correctly signed by a known + * (current or retired) key, minted no more than `ttlMs` ago (default + * {@link DEFAULT_CHALLENGE_TOKEN_TTL_MS}). TOTAL over `token` — never + * throws. This is signature+TTL only — it does NOT check single-use (the + * `webauthn_challenges` DB consume, `src/store/webauthn.ts`, is that layer + * — spec §7) and does NOT check that `ceremony` matches what the caller's + * endpoint expects (the caller does that itself, per-endpoint — spec §7's + * "application-level" check). + */ +export function verifyChallengeToken( + token: string, + keyring: Keyring, + ttlMs: number = DEFAULT_CHALLENGE_TOKEN_TTL_MS, +): VerifiedChallengeToken | null { + assertValidKeyring(keyring) + + if (typeof token !== 'string') return null + const segments = token.split('.') + if (segments.length !== CHALLENGE_SEGMENT_COUNT) return null + const [prefix, keyId, payloadB64, sig] = segments + if (prefix !== CHALLENGE_PREFIX) return null + if (keyId.length === 0 || payloadB64.length === 0 || sig.length === 0) return null + + const canonical = challengeCanonicalString(keyId, payloadB64) + let signatureOk = false + for (const key of candidateKeys(keyring, keyId)) { + if (signatureMatches(key.secret, canonical, sig)) { + signatureOk = true + break + } + } + if (!signatureOk) return null + + let parsed: unknown + try { + parsed = JSON.parse(Buffer.from(payloadB64, 'base64url').toString('utf8')) + } catch { + return null + } + if (typeof parsed !== 'object' || parsed === null) return null + const { ceremony, challengeB64, agentId, nonce, issuedAtMs } = parsed as Record + + if (ceremony !== 'registration' && ceremony !== 'authentication' && ceremony !== 'step-up') { + return null + } + if (typeof challengeB64 !== 'string' || challengeB64.length === 0) return null + if (agentId !== null && (typeof agentId !== 'string' || !UUID_PATTERN.test(agentId))) return null + if (typeof nonce !== 'string' || nonce.length === 0) return null + + const now = Date.now() + if (typeof issuedAtMs !== 'number' || !Number.isFinite(issuedAtMs) || issuedAtMs < 0) return null + if (issuedAtMs > now) return null + if (now - issuedAtMs > ttlMs) return null + + return { ceremony, challengeB64, agentId, nonce } +} + +// --- htsu. step-up token ------------------------------------------------- + +const STEPUP_PREFIX = 'htsu' +const STEPUP_SEGMENT_COUNT = 4 +const STEPUP_NONCE_BYTES = 16 + +/** Default step-up-token TTL: 5 minutes (spec §5.1). */ +export const DEFAULT_STEPUP_TOKEN_TTL_MS = 5 * 60 * 1000 + +interface StepUpTokenPayload { + agentId: string + issuedAtMs: number + nonce: string +} + +function stepUpCanonicalString(keyId: string, payloadB64: string): string { + return `${STEPUP_PREFIX}.${keyId}.${payloadB64}` +} + +/** What a freshly minted step-up token carries. */ +export interface MintedStepUpToken { + token: string + nonce: string +} + +/** + * Mint an `htsu.` step-up token for `agentId` — proof this Agent just + * demonstrated an existing factor (spec §5.1). STRICT: throws on a + * malformed keyring or non-uuid `agentId` (module doc). + */ +export function mintStepUpToken(agentId: string, keyring: Keyring): MintedStepUpToken { + assertValidKeyring(keyring) + if (typeof agentId !== 'string' || !UUID_PATTERN.test(agentId)) { + throw new Error(`mintStepUpToken: agentId must be a uuid (got ${JSON.stringify(agentId)})`) + } + + const { keyId, secret } = keyring.current + const payload: StepUpTokenPayload = { + agentId, + issuedAtMs: Date.now(), + nonce: randomBytes(STEPUP_NONCE_BYTES).toString('base64url'), + } + const payloadB64 = Buffer.from(JSON.stringify(payload), 'utf8').toString('base64url') + const sig = sign(secret, stepUpCanonicalString(keyId, payloadB64)) + return { token: `${STEPUP_PREFIX}.${keyId}.${payloadB64}.${sig}`, nonce: payload.nonce } +} + +/** A verified `htsu.` token's payload. */ +export interface VerifiedStepUpToken { + agentId: string + nonce: string +} + +/** + * Verify a candidate `htsu.` token: well-formed, correctly signed, minted + * no more than `ttlMs` ago (default {@link DEFAULT_STEPUP_TOKEN_TTL_MS}). + * TOTAL over `token` — never throws. Signature+TTL only, same + * "single-use is the DB row's job, ceremony/agent binding is the caller's + * job" split as {@link verifyChallengeToken} — see `src/store/webauthn.ts` + * for the consume side and spec §5.2 for the "verify re-validates but does + * not re-consume" discipline. + */ +export function verifyStepUpToken( + token: string, + keyring: Keyring, + ttlMs: number = DEFAULT_STEPUP_TOKEN_TTL_MS, +): VerifiedStepUpToken | null { + assertValidKeyring(keyring) + + if (typeof token !== 'string') return null + const segments = token.split('.') + if (segments.length !== STEPUP_SEGMENT_COUNT) return null + const [prefix, keyId, payloadB64, sig] = segments + if (prefix !== STEPUP_PREFIX) return null + if (keyId.length === 0 || payloadB64.length === 0 || sig.length === 0) return null + + const canonical = stepUpCanonicalString(keyId, payloadB64) + let signatureOk = false + for (const key of candidateKeys(keyring, keyId)) { + if (signatureMatches(key.secret, canonical, sig)) { + signatureOk = true + break + } + } + if (!signatureOk) return null + + let parsed: unknown + try { + parsed = JSON.parse(Buffer.from(payloadB64, 'base64url').toString('utf8')) + } catch { + return null + } + if (typeof parsed !== 'object' || parsed === null) return null + const { agentId, issuedAtMs, nonce } = parsed as Record + + if (typeof agentId !== 'string' || !UUID_PATTERN.test(agentId)) return null + if (typeof nonce !== 'string' || nonce.length === 0) return null + + const now = Date.now() + if (typeof issuedAtMs !== 'number' || !Number.isFinite(issuedAtMs) || issuedAtMs < 0) return null + if (issuedAtMs > now) return null + if (now - issuedAtMs > ttlMs) return null + + return { agentId, nonce } +} diff --git a/src/composition/app.test.ts b/src/composition/app.test.ts index 5730de0..9332a28 100644 --- a/src/composition/app.test.ts +++ b/src/composition/app.test.ts @@ -25,6 +25,7 @@ const HEALTHY_REPORT: HealthReport = { forgedTokens: { deliveriesLast24h: 0, tokensLast24h: 0, alertThreshold: 5 }, mailboxes: [], webhooks: { autoDisabled: [], deliveryFailuresLast24h: 0 }, + webauthn: { counterRegressionsLast24h: 0 }, } /** Build a handler over spy deps; the inbox API spy returns a recognizable 299 so delegation is observable. */ diff --git a/src/composition/health.test.ts b/src/composition/health.test.ts index 3a8da84..a534b0a 100644 --- a/src/composition/health.test.ts +++ b/src/composition/health.test.ts @@ -125,6 +125,7 @@ describe('runHealthCheck', () => { }) expect(report.mailboxes).toEqual([]) expect(report.webhooks).toEqual({ autoDisabled: [], deliveryFailuresLast24h: 0 }) + expect(report.webauthn).toEqual({ counterRegressionsLast24h: 0 }) expect(new Date(report.generatedAt).getTime()).not.toBeNaN() }) @@ -345,4 +346,53 @@ describe('runHealthCheck', () => { expect(alert).toContain('1 webhook delivery(ies)') }) }) + + describe('passkey counter regressions (HT-75; specs/auth/passkeys.md §8)', () => { + /** Insert an Agent + a `webauthn_credentials` row directly (no `AgentStore`/`WebAuthnStore` needed for this fixture). */ + async function seedRegressedCredential( + database: Db, + regressedAgoHours: number | null, + ): Promise { + const [agent] = await database.query<{ id: string }>( + `INSERT INTO agents (email, name, role, status) VALUES ($1, 'Agent', 'agent', 'active') RETURNING id`, + [`agent-${Math.random()}@example.test`], + ) + await database.query( + `INSERT INTO webauthn_credentials + (agent_id, credential_id, public_key, sign_count, backup_eligible, backup_state, name, sign_count_regression_at) + VALUES ($1, $2, $3, 10, false, false, 'Key', ${ + regressedAgoHours === null ? 'NULL' : `now() - interval '${regressedAgoHours} hours'` +})`, + [agent.id, `cred-${Math.random()}`, new Uint8Array([1])], + ) + } + + it('a credential regressed in the last 24h trips webauthn-counter-regression; one with no regression is silent', async () => { + const { database, check } = await fresh() + await seedRegressedCredential(database, null) // never regressed + + const clean = await check() + expect(clean.webauthn.counterRegressionsLast24h).toBe(0) + expect(clean.alerts.some((a) => a.startsWith('webauthn-counter-regression'))).toBe(false) + + await seedRegressedCredential(database, 1) // regressed 1h ago + const report = await check() + + expect(report.ok).toBe(false) + expect(report.webauthn.counterRegressionsLast24h).toBe(1) + const alert = report.alerts.find((a) => a.startsWith('webauthn-counter-regression: ')) + expect(alert).toBeDefined() + expect(alert).toContain('1 credential(s)') + }) + + it('a regression OLDER than 24h does not trip the alert (growth, not standing count)', async () => { + const { database, check } = await fresh() + await seedRegressedCredential(database, 30) // regressed 30h ago — outside the window + + const report = await check() + expect(report.ok).toBe(true) + expect(report.webauthn.counterRegressionsLast24h).toBe(0) + expect(report.alerts.some((a) => a.startsWith('webauthn-counter-regression'))).toBe(false) + }) + }) }) diff --git a/src/composition/health.ts b/src/composition/health.ts index dbfbaaf..b1da55d 100644 --- a/src/composition/health.ts +++ b/src/composition/health.ts @@ -55,6 +55,15 @@ * delivery.ts`'s module doc — so this is a SEPARATE signal from the * auto-disable alert: an endpoint can shed individual failed deliveries * for a while before crossing 20 consecutive and auto-disabling). + * - **Passkey counter regressions** (HT-75; specs/auth/passkeys.md §8): + * `webauthn-counter-regression` for any `webauthn_credentials` row whose + * `sign_count_regression_at` (set by `src/auth/webauthn-ceremony.ts` on a + * Tier-2 clone-signal rejection) falls in the last 24h — a directly + * analogous check to `forged-token-burst` above (a per-row marker column, + * not a log table; the signal is growth, never the standing count), + * except tripped on ANY count `> 0`, not a threshold: spec §8 is explicit + * this is a "high-quality clone signal for a non-synced credential," not + * noise to average over a burst window. * * ## What it deliberately does NOT check * @@ -140,6 +149,11 @@ export interface HealthReport { /** `queue_jobs` rows on `WEBHOOK_DELIVERY_TOPIC` dead-lettered in the last 24h. */ deliveryFailuresLast24h: number } + /** HT-75 (specs/auth/passkeys.md §8) — see the module doc's Passkey counter regressions section. */ + webauthn: { + /** `webauthn_credentials` rows whose `sign_count_regression_at` falls in the last 24h. Any value `> 0` trips the `webauthn-counter-regression` alert. */ + counterRegressionsLast24h: number + } } /** Every ledger status, for zero-filling {@link HealthReport.ingest}'s per-status map (a status with no 24h rows must still appear, as `0`). */ @@ -301,6 +315,20 @@ export async function runHealthCheck(deps: HealthCheckDeps): Promise( + `SELECT count(*)::int AS count FROM webauthn_credentials + WHERE sign_count_regression_at > now() - interval '24 hours'`, + ) + const webauthnCounterRegressionsLast24h = webauthnRegressionRows[0]?.count ?? 0 + if (webauthnCounterRegressionsLast24h > 0) { + alerts.push( + `webauthn-counter-regression: ${webauthnCounterRegressionsLast24h} credential(s) rejected ` + + 'for signature-counter regression in the last 24h — a high-quality clone signal for a ' + + 'non-synced credential; inspect and consider revoking (runbook Part G)', + ) + } + return { ok: alerts.length === 0, alerts, @@ -314,6 +342,7 @@ export async function runHealthCheck(deps: HealthCheckDeps): Promise { expect(body.error.code).toBe('gmail_push_rejected') }) }) + +describe('buildApp — passkeys (HT-75; specs/auth/passkeys.md §3)', () => { + let db: Db + + afterEach(async () => { + await db.close() + }) + + async function buildWithUiBaseUrl(uiBaseUrl: string | undefined) { + db = await createPgliteDb() + await migrate(db) + return buildApp( + { ...testConfig(), ...(uiBaseUrl !== undefined ? { uiBaseUrl } : {}) }, + { db, blobStore: fakeBlobStore() }, + ) + } + + async function providerKinds( + handler: (request: Request) => Promise, + ): Promise { + const res = await handler( + new Request(`${ORIGIN}/api/v1/auth/providers`, { + headers: { Authorization: `Bearer ${API_TOKEN}` }, + }), + ) + const body = (await res.json()) as { providers: { kind: string }[] } + return body.providers.map((p) => p.kind) + } + + it('with no uiBaseUrl configured, webauthn is absent: GET /auth/providers omits it, and every webauthn route 404s', async () => { + const handler = await buildWithUiBaseUrl(undefined) + expect(await providerKinds(handler)).toEqual(['credentials']) + + const res = await handler( + new Request(`${ORIGIN}/api/v1/auth/webauthn/authentication/options`, { + method: 'POST', + headers: { Authorization: `Bearer ${API_TOKEN}` }, + }), + ) + expect(res.status).toBe(404) + }) + + it('with a valid domain-form uiBaseUrl, webauthn is wired: GET /auth/providers includes it', async () => { + const handler = await buildWithUiBaseUrl('https://inbox.example.test') + expect(await providerKinds(handler)).toEqual(['credentials', 'webauthn']) + }) + + it('with an IP-literal uiBaseUrl, buildApp does NOT crash — it degrades to webauthn-absent, exactly like an unset uiBaseUrl (a deliberate choice: a passkeys-only misconfiguration must not take down the whole engine)', async () => { + const handler = await buildWithUiBaseUrl('http://127.0.0.1:3000') + expect(await providerKinds(handler)).toEqual(['credentials']) + + // Every other feature is unaffected — the inbox API still works. + const res = await handler( + new Request(`${ORIGIN}/api/v1/conversations`, { + headers: { Authorization: `Bearer ${API_TOKEN}` }, + }), + ) + expect(res.status).toBe(200) + }) +}) diff --git a/src/composition/root.ts b/src/composition/root.ts index 7fe6a14..0846e0d 100644 --- a/src/composition/root.ts +++ b/src/composition/root.ts @@ -47,8 +47,11 @@ import { type GmailReconcileJob, } from '../api/gmail-webhook.js' import { createInboxApi } from '../api/index.js' +import type { WebAuthnApiDeps } from '../api/webauthn.js' import { createPasswordAuthProvider } from '../auth/password-provider.js' import type { AuthProvider } from '../auth/provider.js' +import { createWebAuthnAuthProvider } from '../auth/webauthn-provider.js' +import { resolveWebAuthnRp } from '../auth/webauthn-rp.js' import type { Db } from '../db/client.js' import { createPostgresDb } from '../db/postgres.js' import { createGmailConnectService } from '../mail/gmail-connect.js' @@ -86,6 +89,7 @@ import { createThreadAttachmentStore, createWebhookEndpointStore, } from '../store/index.js' +import { createWebAuthnStore } from '../store/webauthn.js' import { createWebhookDeliveryHandler, WEBHOOK_DELIVERY_TOPIC, @@ -118,6 +122,9 @@ const GMAIL_SCOPES = [ */ const SIGNING_KEY_ID = 'ht1' +/** Deployment display name shown in the OS passkey UI (HT-75; specs/auth/passkeys.md §6.1's `rpName` — "config or a fixed string"; no `AppConfig` field exists for this yet, so a fixed string is used). */ +const WEBAUTHN_RP_NAME = 'Helpthread' + /** Overrides for {@link buildApp}, injected only by tests so the wiring is exercised without real Postgres/Supabase or any network. */ export interface BuildAppOverrides { /** A `Db` to use instead of constructing a `PostgresDb` from `config.databaseUrl` (e.g. an in-memory PGlite `Db`). */ @@ -160,6 +167,7 @@ export async function buildApp( const agentStore = createAgentStore(db) const assistantStore = createAssistantStore(db) const savedReplyStore = createSavedReplyStore(db) + const webAuthnStore = createWebAuthnStore(db) // --- Module substrate (HT-69; specs/modules/substrate-v1.md §4/§5): the // event outbox and webhook endpoint stores. `webhookEndpointStore` reuses @@ -170,15 +178,16 @@ export async function buildApp( const eventOutboxStore = createEventOutboxStore(db) const webhookEndpointStore = createWebhookEndpointStore(db, config.tokenEncryptionKey) + // --- The HMAC keyring backing reply/state/view/webauthn tokens (single current key). --- + const keyring: Keyring = { current: { keyId: SIGNING_KEY_ID, secret: config.signingSecret } } + // --- Agents & Authentication (HT-54): the core provider registry is just // `[password]` — an ordered list, no discovery mechanism (spec §4's - // honest-scope note). A marketplace module adds a provider HERE, in a - // future ticket, not via any plugin loader this build ships. --- + // honest-scope note). HT-75 (specs/auth/passkeys.md §3) pushes a SECOND + // provider onto this SAME array below, once `sender` exists — see that + // block's own comment for why it's conditional on `config.uiBaseUrl`. --- const authProviders: AuthProvider[] = [createPasswordAuthProvider({ agentStore })] - // --- The HMAC keyring backing reply/state/view tokens (single current key). --- - const keyring: Keyring = { current: { keyId: SIGNING_KEY_ID, secret: config.signingSecret } } - // --- Durable job queue — ONE instance shared by the webhook enqueue, the // maintenance sweep enqueue, and the drain, so they share tunables. --- const queue = createPostgresQueue(db) @@ -207,6 +216,57 @@ export async function buildApp( }, }) + // --- Passkeys (HT-75; specs/auth/passkeys.md §3) — ONLY when + // `config.uiBaseUrl` is set AND resolves to a domain-form hostname: there + // is no safe fallback origin to bind WebAuthn ceremonies to, so a + // deployment with no known (or WebAuthn-unusable) UI origin simply never + // gets the passkey login option (`GET /auth/providers` omits the + // descriptor, and `webauthn` stays `undefined` — every route in + // `src/api/webauthn.ts` 404s) — the same degrade-by-omission shape + // `agents.uiBaseUrl` already uses for invites. + // + // The `uiBaseUrl`-set-but-IP-literal case is a deliberate judgment call, + // not directly pinned by the spec: `resolveWebAuthnRp` THROWS for that + // case (module doc), and letting that throw propagate would crash + // `buildApp()` entirely — taking down the WHOLE engine (mail ingestion, + // conversations, everything) over a passkeys-only misconfiguration that + // spec §3 itself frames as "every other HT-54 feature working, passkeys + // silently failing at the first ceremony," not "the deployment doesn't + // boot." Caught here and treated identically to `uiBaseUrl` being unset — + // the SAME degrade-by-omission this block already applies, just reached + // by a different path — rather than an unbounded-blast-radius boot crash. + // authProviders.push below mutates the SAME array reference already + // passed to `createPasswordAuthProvider`'s sibling above and threaded + // into both `webauthn.providers` (step-up/password reuses the registered + // `password` provider) and `agents.providers` below. --- + let webauthn: WebAuthnApiDeps | undefined + if (config.uiBaseUrl !== undefined) { + let rp: ReturnType | undefined + try { + rp = resolveWebAuthnRp(config.uiBaseUrl) + } catch (err) { + console.error( + '[composition] HELPTHREAD_UI_BASE_URL is set but not WebAuthn-usable (an IP literal, not a domain) — passkeys are disabled for this deployment; every other feature is unaffected', + err, + ) + } + if (rp !== undefined) { + webauthn = { + db, + store: webAuthnStore, + agentStore, + providers: authProviders, + keyring, + rp, + rpName: WEBAUTHN_RP_NAME, + sender, + mailDomain: config.mailDomain, + supportAddress: config.supportAddress, + } + authProviders.push(createWebAuthnAuthProvider({ db, store: webAuthnStore, keyring, rp })) + } + } + // --- Gmail push webhook deps. The JWKS key source is built ONCE here and // reused across every request (its fetch cache only caches if reused — see // createGooglePushKeySource's doc). --- @@ -289,6 +349,9 @@ export async function buildApp( // MailboxStore instance every other mailbox-scoped feature in this root // shares — no second store needed. savedReplies: { store: savedReplyStore, mailboxStore }, + // Passkeys (HT-75) — spread in only when configured (uiBaseUrl set), + // matching agents.uiBaseUrl's own optional-field convention above. + ...(webauthn !== undefined ? { webauthn } : {}), // HT-49 review fix: Gmail delivers a sent reply's own copy back into the // SAME mailbox it was sent from, where reconcile would otherwise re-ingest // it as a phantom inbound message (src/mail/send.ts's "The reply token's diff --git a/src/db/migrate.test.ts b/src/db/migrate.test.ts index 1e4ebb1..ed39bec 100644 --- a/src/db/migrate.test.ts +++ b/src/db/migrate.test.ts @@ -65,6 +65,7 @@ describe('migrate', () => { { id: 23, name: 'event_outbox' }, { id: 24, name: 'saved_replies' }, { id: 25, name: 'conversation_snooze' }, + { id: 26, name: 'webauthn' }, ]) }) @@ -100,6 +101,7 @@ describe('migrate', () => { { id: 23 }, { id: 24 }, { id: 25 }, + { id: 26 }, ]) }) diff --git a/src/db/migrate.ts b/src/db/migrate.ts index 35627e6..a50a3a6 100644 --- a/src/db/migrate.ts +++ b/src/db/migrate.ts @@ -1351,6 +1351,101 @@ ALTER TABLE conversations ADD CONSTRAINT conversations_snoozed_until_pending_onl ); ` +/** + * Migration 026 — passkey (WebAuthn) login (HT-75; specs/auth/passkeys.md + * §2). Three new tables; neither `agents` nor `agent_auth_identities` + * changes (spec §1: "additive only"). + * + * ## `webauthn_credentials` (spec §2.1) + * + * A provider-owned credential table, deliberately NOT a row shape inside + * `agent_auth_identities` — the spec's own §2.1 argues this at length + * (mutable per-use state that would turn a low-write table into a + * mixed-traffic one; a public key / counter / transports / backup-flags + * shape that doesn't fit `agent_auth_identities`' one `secret_hash` column). + * `credential_id` is the WebAuthn authenticator's own id, globally unique + * (not scoped to `agent_id`) and NOT NULL — it is the lookup key the + * discoverable-credential authentication ceremony resolves an Agent from + * BEFORE it knows who is signing in (spec §6.2). `name` is NOT NULL at the + * database even though optional on the wire (spec §6.1, §9): the API layer + * defaults a blank/omitted name to `"Passkey — {date}"` before the INSERT + * ever runs. `sign_count_regression_at` is the HT-44 health-check signal + * (spec §8) — a marker column, not an audit trail, overwritten on each new + * Tier-2 regression. `ON DELETE CASCADE` mirrors `agent_auth_identities`' + * own cascade (migration 018): a deleted Agent's credentials go with them. + * + * ## `webauthn_challenges` (spec §2.2, §7) + * + * One row per minted WebAuthn ceremony challenge, keyed by its `nonce` + * (the SAME nonce embedded in the signed `htw.` token, `src/auth/ + * webauthn-token.ts`) — the DB-backed single-use layer a bare signed token + * cannot provide on its own (spec §7: "a bare signature+TTL check can be + * satisfied twice"). `ceremony` has three values (`registration` / + * `authentication` / `step-up`) and is part of BOTH the mint and the + * consume statement (`AND ceremony = $2`) — the database-level half of + * spec §7's "ceremony discriminator enforced, not just recorded" fix. + * `agent_id` is set for registration/step-up (session-bound at mint) and + * NULL for authentication (pre-identification, spec §6.2's discoverable + * flow) — nullable, `ON DELETE CASCADE` so a deleted Agent's in-flight + * challenges go with them same as their credentials. + * + * ## `webauthn_stepup_tokens` (spec §2.3, §5) + * + * Backs the enrollment-hardening step-up proof (spec §5): a credential + * mints a durable, independent factor, so registering one requires fresh + * evidence of an EXISTING factor, not just a live session. Same shape and + * discipline as `webauthn_challenges` (nonce PK, `expires_at`, + * `consumed_at`) — a separate table because a step-up token proves a + * DIFFERENT thing (an existing factor was just demonstrated) than a + * WebAuthn ceremony challenge does, even though both reuse the identical + * signed-token-plus-DB-row mechanism. `agent_id` is NOT NULL here (unlike + * `webauthn_challenges`) — a step-up token always proves step-up for a + * SPECIFIC, already-session-identified Agent; there is no anonymous case. + * + * ## No cron — opportunistic purge on mint (spec §2.2) + * + * Neither challenge table gets a cleanup job. `WebAuthnStore`'s mint + * methods (`src/store/webauthn.ts`) precede every INSERT with `DELETE ... + * WHERE expires_at < now()` in the SAME transaction, piggybacking the + * cleanup on a write that was happening anyway (spec §2.2's "the cost of + * self-cleaning is one indexed DELETE"). The two `_expires` indexes below + * are what makes that DELETE cheap. + */ +const MIGRATION_026_WEBAUTHN = ` +CREATE TABLE webauthn_credentials ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + agent_id uuid NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + credential_id text NOT NULL, + public_key bytea NOT NULL, + sign_count bigint NOT NULL DEFAULT 0, + transports text[] NOT NULL DEFAULT '{}', + backup_eligible boolean NOT NULL, + backup_state boolean NOT NULL, + name text NOT NULL, + sign_count_regression_at timestamptz, + created_at timestamptz NOT NULL DEFAULT now(), + last_used_at timestamptz, + updated_at timestamptz NOT NULL DEFAULT now() +); +CREATE UNIQUE INDEX webauthn_credentials_credential_id_key ON webauthn_credentials (credential_id); +CREATE INDEX webauthn_credentials_agent ON webauthn_credentials (agent_id); +CREATE TABLE webauthn_challenges ( + nonce text PRIMARY KEY, + ceremony text NOT NULL CHECK (ceremony IN ('registration', 'authentication', 'step-up')), + agent_id uuid REFERENCES agents(id) ON DELETE CASCADE, + expires_at timestamptz NOT NULL, + consumed_at timestamptz +); +CREATE INDEX webauthn_challenges_expires ON webauthn_challenges (expires_at); +CREATE TABLE webauthn_stepup_tokens ( + nonce text PRIMARY KEY, + agent_id uuid NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + expires_at timestamptz NOT NULL, + consumed_at timestamptz +); +CREATE INDEX webauthn_stepup_tokens_expires ON webauthn_stepup_tokens (expires_at); +` + /** * Every migration, in the order they must apply. `id` is the sole ordering * key (ascending) — array position is not relied upon, so re-sorting this @@ -1478,6 +1573,11 @@ const MIGRATIONS: Migration[] = [ name: 'conversation_snooze', sql: MIGRATION_025_CONVERSATION_SNOOZE, }, + { + id: 26, + name: 'webauthn', + sql: MIGRATION_026_WEBAUTHN, + }, ] /** diff --git a/src/db/postgres.test.ts b/src/db/postgres.test.ts index 636edc0..cec5ed2 100644 --- a/src/db/postgres.test.ts +++ b/src/db/postgres.test.ts @@ -310,6 +310,9 @@ describe('createPostgresDb with a schema option', () => { 'saved_replies', 'thread_attachments', 'threads', + 'webauthn_challenges', + 'webauthn_credentials', + 'webauthn_stepup_tokens', 'webhook_endpoints', ]) diff --git a/src/store/webauthn.test.ts b/src/store/webauthn.test.ts new file mode 100644 index 0000000..fe490d7 --- /dev/null +++ b/src/store/webauthn.test.ts @@ -0,0 +1,399 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { createPgliteDb, type Db } from '../db/client.js' +import { migrate } from '../db/migrate.js' +import type { AgentStore } from './agents.js' +import { createAgentStore } from './agents.js' +import { createWebAuthnStore, type WebAuthnStore } from './webauthn.js' + +describe('WebAuthnStore', () => { + let db: Db | undefined + let store: WebAuthnStore | undefined + let agentStore: AgentStore | undefined + + afterEach(async () => { + await db?.close() + db = undefined + store = undefined + agentStore = undefined + }) + + async function freshStore(): Promise<{ db: Db; store: WebAuthnStore; agentStore: AgentStore }> { + db = await createPgliteDb() + await migrate(db) + store = createWebAuthnStore(db) + agentStore = createAgentStore(db) + return { db, store, agentStore } + } + + async function makeAgent(a: AgentStore, email: string): Promise { + const result = await a.createAgent({ + name: 'Agent', + email, + role: 'agent', + status: 'active', + passwordHash: 'scrypt$hash', + }) + if (!result.ok) throw new Error('expected ok') + return result.agent.id + } + + const CREDENTIAL_1 = { + credentialId: 'cred-1', + publicKey: new Uint8Array([1, 2, 3, 4]), + signCount: 0, + transports: ['internal'], + backupEligible: true, + backupState: false, + name: 'MacBook Touch ID', + } + + // --- insertCredential / lookups -------------------------------------------- + + describe('insertCredential / getCredentialByCredentialId / getCredentialById', () => { + it('inserts and reads a credential back with every field intact', async () => { + const { store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + + const result = await store.insertCredential({ agentId, ...CREDENTIAL_1 }) + expect(result.ok).toBe(true) + if (!result.ok) throw new Error('expected ok') + expect(result.credential.agentId).toBe(agentId) + expect(result.credential.credentialId).toBe('cred-1') + expect(Array.from(result.credential.publicKey)).toEqual([1, 2, 3, 4]) + expect(result.credential.signCount).toBe(0) + expect(result.credential.transports).toEqual(['internal']) + expect(result.credential.backupEligible).toBe(true) + expect(result.credential.backupState).toBe(false) + expect(result.credential.signCountRegressionAt).toBeNull() + expect(result.credential.lastUsedAt).toBeNull() + + const byCredentialId = await store.getCredentialByCredentialId('cred-1') + expect(byCredentialId?.id).toBe(result.credential.id) + + const byId = await store.getCredentialById(result.credential.id) + expect(byId?.credentialId).toBe('cred-1') + }) + + it('returns null for an unknown credential_id / id', async () => { + const { store } = await freshStore() + expect(await store.getCredentialByCredentialId('nope')).toBeNull() + expect(await store.getCredentialById('00000000-0000-4000-8000-000000000000')).toBeNull() + }) + + it("'credential_taken' on a duplicate credential_id — even across different Agents (spec §6.1: the UNIQUE index enforces it either way)", async () => { + const { store, agentStore } = await freshStore() + const agent1 = await makeAgent(agentStore, 'a1@example.test') + const agent2 = await makeAgent(agentStore, 'a2@example.test') + + const first = await store.insertCredential({ agentId: agent1, ...CREDENTIAL_1 }) + expect(first.ok).toBe(true) + + const second = await store.insertCredential({ agentId: agent2, ...CREDENTIAL_1 }) + expect(second).toEqual({ ok: false, reason: 'credential_taken' }) + }) + }) + + // --- FOR UPDATE / counter persistence --------------------------------------- + + describe('getCredentialForUpdateInTx / updateAfterSuccessfulAuth / markCounterRegression', () => { + it('reads and updates the counter inside a transaction', async () => { + const { db: testDb, store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + const inserted = await store.insertCredential({ agentId, ...CREDENTIAL_1 }) + if (!inserted.ok) throw new Error('expected ok') + + await testDb.transaction(async (tx) => { + const locked = await store.getCredentialForUpdateInTx('cred-1', tx) + expect(locked?.signCount).toBe(0) + await store.updateAfterSuccessfulAuth( + inserted.credential.id, + { signCount: 5, backupState: true }, + tx, + ) + }) + + const after = await store.getCredentialByCredentialId('cred-1') + expect(after?.signCount).toBe(5) + expect(after?.backupState).toBe(true) + expect(after?.lastUsedAt).not.toBeNull() + }) + + it('marks a counter regression, and the marker survives the transaction committing', async () => { + const { db: testDb, store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + const inserted = await store.insertCredential({ agentId, ...CREDENTIAL_1 }) + if (!inserted.ok) throw new Error('expected ok') + + await testDb.transaction(async (tx) => { + await store.markCounterRegression(inserted.credential.id, tx) + }) + + const after = await store.getCredentialByCredentialId('cred-1') + expect(after?.signCountRegressionAt).not.toBeNull() + }) + }) + + // --- listCredentialsForAgent / renameCredential / deleteCredential -------- + + describe('listCredentialsForAgent / renameCredential', () => { + it('lists only the given Agent’s credentials', async () => { + const { store, agentStore } = await freshStore() + const agent1 = await makeAgent(agentStore, 'a1@example.test') + const agent2 = await makeAgent(agentStore, 'a2@example.test') + await store.insertCredential({ agentId: agent1, ...CREDENTIAL_1, credentialId: 'c1' }) + await store.insertCredential({ agentId: agent2, ...CREDENTIAL_1, credentialId: 'c2' }) + + const list = await store.listCredentialsForAgent(agent1) + expect(list).toHaveLength(1) + expect(list[0].credentialId).toBe('c1') + }) + + it('renames a credential scoped to its owning Agent; null if the id belongs to someone else', async () => { + const { store, agentStore } = await freshStore() + const agent1 = await makeAgent(agentStore, 'a1@example.test') + const agent2 = await makeAgent(agentStore, 'a2@example.test') + const inserted = await store.insertCredential({ agentId: agent1, ...CREDENTIAL_1 }) + if (!inserted.ok) throw new Error('expected ok') + + const renamed = await store.renameCredential(inserted.credential.id, agent1, 'New name') + expect(renamed?.name).toBe('New name') + + const wrongOwner = await store.renameCredential(inserted.credential.id, agent2, 'Nope') + expect(wrongOwner).toBeNull() + }) + }) + + describe('deleteCredential — the last-credential guard (spec §9.1)', () => { + it('deletes a credential when the Agent still has a password identity', async () => { + const { store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') // createAgent gives a password identity + const inserted = await store.insertCredential({ agentId, ...CREDENTIAL_1 }) + if (!inserted.ok) throw new Error('expected ok') + + const result = await store.deleteCredential(inserted.credential.id, agentId) + expect(result).toBe('ok') + expect(await store.getCredentialById(inserted.credential.id)).toBeNull() + }) + + it('deletes a credential when the Agent has another credential (even without a password identity)', async () => { + const { db: testDb, store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + // Strip the password identity to force the "no password" branch. + await testDb.query('DELETE FROM agent_auth_identities WHERE agent_id = $1', [agentId]) + const first = await store.insertCredential({ agentId, ...CREDENTIAL_1, credentialId: 'c1' }) + const second = await store.insertCredential({ agentId, ...CREDENTIAL_1, credentialId: 'c2' }) + if (!first.ok || !second.ok) throw new Error('expected ok') + + const result = await store.deleteCredential(first.credential.id, agentId) + expect(result).toBe('ok') + }) + + it("refuses ('last_credential') to delete the Agent's only credential once they have no password identity", async () => { + const { db: testDb, store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + await testDb.query('DELETE FROM agent_auth_identities WHERE agent_id = $1', [agentId]) + const inserted = await store.insertCredential({ agentId, ...CREDENTIAL_1 }) + if (!inserted.ok) throw new Error('expected ok') + + const result = await store.deleteCredential(inserted.credential.id, agentId) + expect(result).toBe('last_credential') + // The row must still exist — refused, not partially deleted. + expect(await store.getCredentialById(inserted.credential.id)).not.toBeNull() + }) + + it("'not_found' for an id that doesn't belong to this Agent", async () => { + const { store, agentStore } = await freshStore() + const agent1 = await makeAgent(agentStore, 'a1@example.test') + const agent2 = await makeAgent(agentStore, 'a2@example.test') + const inserted = await store.insertCredential({ agentId: agent1, ...CREDENTIAL_1 }) + if (!inserted.ok) throw new Error('expected ok') + + expect(await store.deleteCredential(inserted.credential.id, agent2)).toBe('not_found') + }) + + // --- The account-lockout TOCTOU (CodeRabbit, PR #94) --------------------- + // + // Bug: the original `FOR UPDATE` scoped only to the TARGET row + // (`id = $1 AND agent_id = $2`); the "does this Agent have another + // credential" count was a separate, UNLOCKED read. Two concurrent + // `deleteCredential` calls for DIFFERENT credentials of the same + // passwordless Agent could each see the other's row as still present, + // both pass `otherCount === 0` as false, and both commit — leaving the + // Agent with zero credentials and no password: locked out. + // + // Fix: lock EVERY credential row for the Agent (`WHERE agent_id = $1 + // FOR UPDATE`) before computing the guard, so a second, real concurrent + // Postgres transaction targeting the SAME Agent blocks on this SELECT + // until the first commits, then re-reads the Agent's CURRENT row set — + // never a stale one. + // + // What CANNOT be proven here: a literal `Promise.all` of two + // `deleteCredential()` calls does NOT reproduce genuine interleaving + // against PGlite — verified empirically (a `SELECT ... FOR UPDATE` + // inside one `db.transaction()` held open while a second + // `db.transaction()` is kicked off: the second call's callback does not + // even START running until the first's `db.transaction()` call has + // fully COMMITTED). PGlite is a single, in-process connection that + // serializes whole transactions, not just individual row locks — the + // exact same limitation `src/store/agents.test.ts` already documents + // for `createFirstAdmin`'s advisory-lock guard ("true concurrency isn't + // reproducible against single-connection PGlite... waits for a + // Supabase-backed Db"). A naive concurrent-call test would pass + // identically against the OLD, buggy code too (PGlite's own + // serialization already prevents interleaving, independent of this + // fix), so it would prove nothing about THIS bug specifically. + // + // What IS proven instead, matching that same file's own precedent for + // this exact limitation: (1) an instrumented `Db` asserting the lock + // SQL actually targets the Agent's WHOLE credential set, not just the + // target row — the structural fact a real concurrent Postgres session + // relies on to serialize two deletes on the SAME lock; and (2) that the + // guard's arithmetic, now computed from that locked row set rather than + // a separate count query, is correct. + it('the row lock targets EVERY credential for the Agent, not just the one being deleted (instrumented Db)', async () => { + const { db: testDb, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + await testDb.query('DELETE FROM agent_auth_identities WHERE agent_id = $1', [agentId]) + const first = await createWebAuthnStore(testDb).insertCredential({ + agentId, + ...CREDENTIAL_1, + credentialId: 'c1', + }) + if (!first.ok) throw new Error('expected ok') + + const statements: { sql: string; params: unknown[] }[] = [] + const instrumented: Db = { + query: (sql, params = []) => { + statements.push({ sql, params }) + return testDb.query(sql, params) + }, + transaction: (fn) => + testDb.transaction((tx) => + fn({ + query: (sql, params = []) => { + statements.push({ sql, params }) + return tx.query(sql, params) + }, + }), + ), + close: () => testDb.close(), + } + const instrumentedStore = createWebAuthnStore(instrumented) + + await instrumentedStore.deleteCredential(first.credential.id, agentId) + + const lockStatement = statements.find((s) => s.sql.includes('FOR UPDATE')) + expect(lockStatement).toBeDefined() + // The load-bearing fix: parameterized on agent_id ALONE (locks the + // whole set) — never additionally scoped to the target credential id. + expect(lockStatement?.sql).toMatch(/WHERE agent_id = \$1\s+FOR UPDATE/) + expect(lockStatement?.sql).not.toMatch(/id = \$1 AND agent_id = \$2/) + expect(lockStatement?.params).toEqual([agentId]) + }) + + it('computes the last-credential guard from the SAME locked row set — deleting one of two leaves exactly one, and THAT delete then correctly refuses', async () => { + const { store, agentStore, db: testDb } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + await testDb.query('DELETE FROM agent_auth_identities WHERE agent_id = $1', [agentId]) + const first = await store.insertCredential({ agentId, ...CREDENTIAL_1, credentialId: 'c1' }) + const second = await store.insertCredential({ agentId, ...CREDENTIAL_1, credentialId: 'c2' }) + if (!first.ok || !second.ok) throw new Error('expected ok') + + // Deleting the first while the second still exists succeeds. + expect(await store.deleteCredential(first.credential.id, agentId)).toBe('ok') + + // The Agent must never reach zero: the SAME store, immediately after, + // refuses to delete the now-only-remaining credential. + expect(await store.deleteCredential(second.credential.id, agentId)).toBe('last_credential') + expect(await store.listCredentialsForAgent(agentId)).toHaveLength(1) + }) + }) + + // --- challenges ------------------------------------------------------------- + + describe('mintChallenge / consumeChallenge', () => { + it('consumes a minted challenge exactly once (single-use)', async () => { + const { store } = await freshStore() + const expiresAt = new Date(Date.now() + 5 * 60 * 1000) + await store.mintChallenge({ + nonce: 'n1', + ceremony: 'authentication', + agentId: null, + expiresAt, + }) + + expect(await store.consumeChallenge('n1', 'authentication')).toBe(true) + expect(await store.consumeChallenge('n1', 'authentication')).toBe(false) // already consumed + }) + + it('the ceremony discriminator is enforced at the database layer (spec §7)', async () => { + const { store } = await freshStore() + const expiresAt = new Date(Date.now() + 5 * 60 * 1000) + await store.mintChallenge({ nonce: 'n2', ceremony: 'registration', agentId: null, expiresAt }) + + // A validly-signed token minted for one ceremony must not consume the + // row under a DIFFERENT caller-hardcoded ceremony expectation. + expect(await store.consumeChallenge('n2', 'authentication')).toBe(false) + expect(await store.consumeChallenge('n2', 'step-up')).toBe(false) + expect(await store.consumeChallenge('n2', 'registration')).toBe(true) + }) + + it('an expired challenge cannot be consumed', async () => { + const { store } = await freshStore() + const alreadyExpired = new Date(Date.now() - 1000) + await store.mintChallenge({ + nonce: 'n3', + ceremony: 'authentication', + agentId: null, + expiresAt: alreadyExpired, + }) + expect(await store.consumeChallenge('n3', 'authentication')).toBe(false) + }) + + it('a mint opportunistically purges every already-expired row (spec §2.2)', async () => { + const { db: testDb, store } = await freshStore() + const alreadyExpired = new Date(Date.now() - 1000) + await store.mintChallenge({ + nonce: 'stale', + ceremony: 'authentication', + agentId: null, + expiresAt: alreadyExpired, + }) + const [{ count: before }] = await testDb.query<{ count: number }>( + 'SELECT count(*)::int AS count FROM webauthn_challenges', + ) + expect(before).toBe(1) + + // The next mint purges the stale row before inserting its own. + await store.mintChallenge({ + nonce: 'fresh', + ceremony: 'authentication', + agentId: null, + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + }) + const rows = await testDb.query<{ nonce: string }>('SELECT nonce FROM webauthn_challenges') + expect(rows.map((r) => r.nonce)).toEqual(['fresh']) + }) + }) + + // --- step-up tokens ----------------------------------------------------- + + describe('mintStepUpToken / consumeStepUpToken', () => { + it('consumes a minted step-up token exactly once', async () => { + const { store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + const expiresAt = new Date(Date.now() + 5 * 60 * 1000) + await store.mintStepUpToken({ nonce: 'su1', agentId, expiresAt }) + + expect(await store.consumeStepUpToken('su1')).toBe(true) + expect(await store.consumeStepUpToken('su1')).toBe(false) + }) + + it('an expired step-up token cannot be consumed', async () => { + const { store, agentStore } = await freshStore() + const agentId = await makeAgent(agentStore, 'a@example.test') + await store.mintStepUpToken({ nonce: 'su2', agentId, expiresAt: new Date(Date.now() - 1000) }) + expect(await store.consumeStepUpToken('su2')).toBe(false) + }) + }) +}) diff --git a/src/store/webauthn.ts b/src/store/webauthn.ts new file mode 100644 index 0000000..3f12dcb --- /dev/null +++ b/src/store/webauthn.ts @@ -0,0 +1,424 @@ +/** + * `WebAuthnStore` — persistence for `webauthn_credentials`, + * `webauthn_challenges`, and `webauthn_stepup_tokens` (migration 026, HT-75; + * specs/auth/passkeys.md §2). Three tables, one store file — the same + * "closely-related tables share a store module" precedent + * `src/store/agents.ts` already sets (`agents` + `agent_auth_identities` + + * `agent_mailbox_access`). + * + * Follows this codebase's standing store convention: an interface + a + * `create*Store(db)` factory, raw parameterized SQL over the `Db`/`Queryable` + * seam, expected failures as discriminated outcomes rather than thrown + * exceptions. + * + * ## The TOCTOU fix lives here, not in the caller (spec §6.2, §8) + * + * {@link WebAuthnStore.getCredentialForUpdateInTx} REQUIRES a `tx` — it is + * only ever meaningful inside a transaction, since its whole purpose is the + * `SELECT ... FOR UPDATE` re-read spec §6.2 mandates: the counter/clone + * comparison and the write that updates it must happen against the SAME + * locked row, not the earlier, unlocked read used for cryptographic + * verification. `src/auth/webauthn-ceremony.ts` is the one caller. + * + * ## Opportunistic purge, no cron (spec §2.2) + * + * {@link WebAuthnStore.mintChallenge}/{@link WebAuthnStore.mintStepUpToken} + * each precede their INSERT with `DELETE ... WHERE expires_at < now()` in + * the SAME transaction — the only cleanup mechanism either table gets. + */ + +import type { WebAuthnCeremony } from '../auth/webauthn-token.js' +import type { Db, Queryable, SqlValue } from '../db/client.js' + +// --- webauthn_credentials ---------------------------------------------- + +/** A stored WebAuthn credential, as read back from `webauthn_credentials`. Never exposes anything beyond what `GET .../webauthn-credentials` is allowed to return PLUS what verification needs — `publicKey`/`credentialId` are present here (verification needs them) but the API layer (`src/api/webauthn.ts`) never serializes them onto the wire (spec §9, §10: "no secret ever leaves the server"). */ +export interface WebAuthnCredentialRecord { + id: string + agentId: string + credentialId: string + publicKey: Uint8Array + signCount: number + transports: string[] + backupEligible: boolean + backupState: boolean + name: string + signCountRegressionAt: Date | null + createdAt: Date + lastUsedAt: Date | null + updatedAt: Date +} + +/** Input to {@link WebAuthnStore.insertCredential} — everything `registrationInfo` (`@simplewebauthn/server`) plus the client-reported `transports` and the (already-defaulted) `name` yield. */ +export interface InsertCredentialInput { + agentId: string + credentialId: string + publicKey: Uint8Array + signCount: number + transports: string[] + backupEligible: boolean + backupState: boolean + name: string +} + +/** The outcome of {@link WebAuthnStore.insertCredential}. `'credential_taken'` covers BOTH a different Agent's credential and a same-Agent re-registration (spec §6.1: the UNIQUE index enforces it server-side either way; `excludeCredentials` is what stops the same-Agent case client-side). */ +export type InsertCredentialResult = + | { ok: true; credential: WebAuthnCredentialRecord } + | { ok: false; reason: 'credential_taken' } + +/** The outcome of {@link WebAuthnStore.deleteCredential} (spec §9.1's revoke-last-credential guard). */ +export type DeleteCredentialResult = 'ok' | 'not_found' | 'last_credential' + +// --- webauthn_challenges / webauthn_stepup_tokens ------------------------ + +/** Input to {@link WebAuthnStore.mintChallenge}. */ +export interface MintChallengeInput { + nonce: string + ceremony: WebAuthnCeremony + agentId: string | null + expiresAt: Date +} + +/** Input to {@link WebAuthnStore.mintStepUpToken}. */ +export interface MintStepUpTokenInput { + nonce: string + agentId: string + expiresAt: Date +} + +export interface WebAuthnStore { + /** + * Insert a new credential row. `'credential_taken'` on a `credential_id` + * unique-index conflict (spec §6.1) — never a thrown constraint-violation + * error, so the API layer maps it to `409` without parsing a raw pg error. + */ + insertCredential(input: InsertCredentialInput): Promise + + /** Look up a credential by its OWN row id (the API-facing rename/revoke handle, spec §9). `null` if no row has that id. */ + getCredentialById(id: string): Promise + + /** Look up a credential by the WebAuthn authenticator's own `credential_id` — the authentication ceremony's discoverable-credential lookup key (spec §6.2, §4.3). Unlocked: safe for the pre-verification read. `null` if none matches. */ + getCredentialByCredentialId(credentialId: string): Promise + + /** + * The SAME lookup as {@link getCredentialByCredentialId}, but + * `SELECT ... FOR UPDATE` inside `tx` — the locked re-read spec §6.2 + * requires BEFORE the counter/clone comparison and the write that + * updates it (module doc). Only meaningful inside a transaction; `tx` is + * required, not optional. + */ + getCredentialForUpdateInTx( + credentialId: string, + tx: Queryable, + ): Promise + + /** After a successful, non-regressing authentication (spec §6.2): persist the new counter, refreshed `backup_state`, and `last_used_at = now()`. Must run in the SAME transaction as the `FOR UPDATE` read that authorized it. */ + updateAfterSuccessfulAuth( + id: string, + patch: { signCount: number; backupState: boolean }, + tx: Queryable, + ): Promise + + /** Stamp `sign_count_regression_at = now()` — the HT-44 health-check signal (spec §8). Must run in the SAME transaction as the `FOR UPDATE` read that detected the regression. */ + markCounterRegression(id: string, tx: Queryable): Promise + + /** Every credential belonging to `agentId`, ordered by `created_at` — `GET .../webauthn-credentials` (spec §9). */ + listCredentialsForAgent(agentId: string): Promise + + /** Rename credential `id`, scoped to `agentId` (the row must belong to that Agent) — `null` if no such row for that Agent. Not step-up-gated (spec §5.4). */ + renameCredential( + id: string, + agentId: string, + name: string, + ): Promise + + /** + * Delete credential `id`, scoped to `agentId`. `'not_found'` if no such + * row for that Agent. `'last_credential'` — the spec §9.1 defensive + * guard — if the Agent would be left with neither a `password` identity + * nor any OTHER `webauthn_credentials` row (normally unreachable given + * the "passkeys are additive" invariant §1 establishes; added anyway, + * same reasoning as the last-admin guard in `src/store/agents.ts`). + */ + deleteCredential(id: string, agentId: string): Promise + + /** + * Mint a `webauthn_challenges` row, preceded (same transaction) by the + * opportunistic purge of every expired row (spec §2.2) — the ONLY + * cleanup mechanism this table gets. + */ + mintChallenge(input: MintChallengeInput): Promise + + /** + * Consume a `webauthn_challenges` row: `UPDATE ... SET consumed_at = + * now() WHERE nonce = $1 AND ceremony = $2 AND consumed_at IS NULL AND + * expires_at > now()`. Returns whether the row was found and consumed — + * `false` covers "never existed / already consumed / expired / wrong + * ceremony" uniformly (spec §7: the database-level half of the ceremony + * discriminator, and the actual single-use enforcement for + * `authentication`). + */ + consumeChallenge(nonce: string, ceremony: WebAuthnCeremony): Promise + + /** Mint a `webauthn_stepup_tokens` row, same opportunistic-purge discipline as {@link mintChallenge} (spec §2.2). */ + mintStepUpToken(input: MintStepUpTokenInput): Promise + + /** + * Consume a `webauthn_stepup_tokens` row (spec §5.2): `UPDATE ... SET + * consumed_at = now() WHERE nonce = $1 AND consumed_at IS NULL AND + * expires_at > now()`. Called ONLY at `registration/options` time — + * `registration/verify` re-validates the token's signature/TTL/agent + * match but does NOT call this again (spec §5.2's "two independent + * layers, not duplicated logic" — a second consume attempt would always + * fail, which is not the property wanted there). + */ + consumeStepUpToken(nonce: string): Promise +} + +/** Raw `webauthn_credentials` row shape, before mapping to {@link WebAuthnCredentialRecord}. */ +interface CredentialRow { + id: string + agent_id: string + credential_id: string + public_key: Uint8Array + sign_count: string | number + transports: string[] + backup_eligible: boolean + backup_state: boolean + name: string + sign_count_regression_at: Date | string | null + created_at: Date | string + last_used_at: Date | string | null + updated_at: Date | string +} + +const CREDENTIAL_COLUMNS = + 'id, agent_id, credential_id, public_key, sign_count, transports, backup_eligible, backup_state, name, sign_count_regression_at, created_at, last_used_at, updated_at' + +function toDate(value: Date | string): Date { + return value instanceof Date ? value : new Date(value) +} + +function toNullableDate(value: Date | string | null): Date | null { + return value === null ? null : toDate(value) +} + +/** `sign_count` is a `bigint` column — `pg`/PGlite may hand it back as a string to avoid an appearance of precision loss. Every value this codebase ever writes is a WebAuthn authenticator counter (a 32-bit field per the WebAuthn spec), always well within `Number.MAX_SAFE_INTEGER`, so a plain `Number()` conversion is exact. */ +function toSignCount(value: string | number): number { + return typeof value === 'number' ? value : Number(value) +} + +function toCredentialRecord(row: CredentialRow): WebAuthnCredentialRecord { + return { + id: row.id, + agentId: row.agent_id, + credentialId: row.credential_id, + publicKey: row.public_key, + signCount: toSignCount(row.sign_count), + transports: row.transports, + backupEligible: row.backup_eligible, + backupState: row.backup_state, + name: row.name, + signCountRegressionAt: toNullableDate(row.sign_count_regression_at), + createdAt: toDate(row.created_at), + lastUsedAt: toNullableDate(row.last_used_at), + updatedAt: toDate(row.updated_at), + } +} + +/** Is `err` the `webauthn_credentials.credential_id` unique-index violation (SQLSTATE 23505)? Total over non-object input, same shape as `src/store/agents.ts`'s FK-violation guards. */ +function isCredentialIdConflict(err: unknown): boolean { + if (typeof err !== 'object' || err === null) return false + return (err as { code?: unknown }).code === '23505' +} + +/** Create a {@link WebAuthnStore} backed by `db`. */ +export function createWebAuthnStore(db: Db): WebAuthnStore { + return { + async insertCredential(input) { + try { + const rows = await db.query( + `INSERT INTO webauthn_credentials + (agent_id, credential_id, public_key, sign_count, transports, backup_eligible, backup_state, name) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + RETURNING ${CREDENTIAL_COLUMNS}`, + [ + input.agentId, + input.credentialId, + input.publicKey, + input.signCount, + input.transports as unknown as SqlValue, + input.backupEligible, + input.backupState, + input.name, + ], + ) + return { ok: true, credential: toCredentialRecord(rows[0]) } + } catch (err) { + if (isCredentialIdConflict(err)) return { ok: false, reason: 'credential_taken' } + throw err + } + }, + + async getCredentialById(id) { + const rows = await db.query( + `SELECT ${CREDENTIAL_COLUMNS} FROM webauthn_credentials WHERE id = $1`, + [id], + ) + const row = rows[0] + return row === undefined ? null : toCredentialRecord(row) + }, + + async getCredentialByCredentialId(credentialId) { + const rows = await db.query( + `SELECT ${CREDENTIAL_COLUMNS} FROM webauthn_credentials WHERE credential_id = $1`, + [credentialId], + ) + const row = rows[0] + return row === undefined ? null : toCredentialRecord(row) + }, + + async getCredentialForUpdateInTx(credentialId, tx) { + const rows = await tx.query( + `SELECT ${CREDENTIAL_COLUMNS} FROM webauthn_credentials WHERE credential_id = $1 FOR UPDATE`, + [credentialId], + ) + const row = rows[0] + return row === undefined ? null : toCredentialRecord(row) + }, + + async updateAfterSuccessfulAuth(id, patch, tx) { + await tx.query( + `UPDATE webauthn_credentials + SET sign_count = $2, backup_state = $3, last_used_at = now(), updated_at = now() + WHERE id = $1`, + [id, patch.signCount, patch.backupState], + ) + }, + + async markCounterRegression(id, tx) { + await tx.query( + `UPDATE webauthn_credentials SET sign_count_regression_at = now(), updated_at = now() WHERE id = $1`, + [id], + ) + }, + + async listCredentialsForAgent(agentId) { + const rows = await db.query( + `SELECT ${CREDENTIAL_COLUMNS} FROM webauthn_credentials WHERE agent_id = $1 ORDER BY created_at`, + [agentId], + ) + return rows.map(toCredentialRecord) + }, + + async renameCredential(id, agentId, name) { + const rows = await db.query( + `UPDATE webauthn_credentials SET name = $3, updated_at = now() + WHERE id = $1 AND agent_id = $2 + RETURNING ${CREDENTIAL_COLUMNS}`, + [id, agentId, name], + ) + const row = rows[0] + return row === undefined ? null : toCredentialRecord(row) + }, + + async deleteCredential(id, agentId) { + return db.transaction(async (tx) => { + // Lock EVERY credential row belonging to this Agent — not just the + // target row being deleted — so two concurrent `deleteCredential` + // calls for the SAME Agent (even on DIFFERENT credential ids) + // serialize on this SELECT rather than each computing the + // last-credential guard against a stale, unlocked count. + // + // The bug this closes (CodeRabbit, PR #94): with a `FOR UPDATE` + // scoped only to `id = $1`, two concurrent deletes of DIFFERENT + // credentials for the same passwordless Agent each locked a + // DIFFERENT row and then ran an UNLOCKED `count(*) WHERE id <> $2` + // — each transaction saw the OTHER's row as still present, so both + // computed `other_count > 0`, both passed the guard, and both + // committed. Net effect: the Agent reached zero credentials with no + // password identity — locked out, exactly the outcome §9.1's guard + // exists to make unreachable. + // + // Locking the whole set fixes this: the second transaction's + // `SELECT ... FOR UPDATE` blocks until the first commits (or rolls + // back), then re-reads under READ COMMITTED — so by the time it + // computes the guard, the first transaction's delete is already + // visible (or the whole set already reflects fewer rows), and the + // guard is evaluated against the Agent's REAL, current credential + // count rather than a snapshot from before either delete ran. + const rows = await tx.query<{ id: string }>( + 'SELECT id FROM webauthn_credentials WHERE agent_id = $1 FOR UPDATE', + [agentId], + ) + if (!rows.some((row) => row.id === id)) return 'not_found' + + // Spec §9.1: refuse if this Agent would be left with neither a + // password identity nor any OTHER webauthn credential. Read AFTER + // the lock above is acquired, so it's part of the same consistent + // view the guard decides against — there is no code path in this + // codebase that removes a password identity independently of a + // full Agent delete (only `setPassword`/`acceptInvite` write one, + // and only ever add/replace it), so no analogous lock is needed on + // `agent_auth_identities` for this guard to be race-free. + const [{ has_password }] = await tx.query<{ has_password: boolean }>( + `SELECT EXISTS( + SELECT 1 FROM agent_auth_identities WHERE agent_id = $1 AND provider = 'password' + ) AS has_password`, + [agentId], + ) + if (!has_password) { + const otherCount = rows.filter((row) => row.id !== id).length + if (otherCount === 0) return 'last_credential' + } + + await tx.query('DELETE FROM webauthn_credentials WHERE id = $1', [id]) + return 'ok' + }) + }, + + async mintChallenge(input) { + await db.transaction(async (tx) => { + await tx.query('DELETE FROM webauthn_challenges WHERE expires_at < now()') + await tx.query( + `INSERT INTO webauthn_challenges (nonce, ceremony, agent_id, expires_at) + VALUES ($1, $2, $3, $4)`, + [input.nonce, input.ceremony, input.agentId, input.expiresAt], + ) + }) + }, + + async consumeChallenge(nonce, ceremony) { + const rows = await db.query<{ nonce: string }>( + `UPDATE webauthn_challenges + SET consumed_at = now() + WHERE nonce = $1 AND ceremony = $2 AND consumed_at IS NULL AND expires_at > now() + RETURNING nonce`, + [nonce, ceremony], + ) + return rows.length > 0 + }, + + async mintStepUpToken(input) { + await db.transaction(async (tx) => { + await tx.query('DELETE FROM webauthn_stepup_tokens WHERE expires_at < now()') + await tx.query( + `INSERT INTO webauthn_stepup_tokens (nonce, agent_id, expires_at) + VALUES ($1, $2, $3)`, + [input.nonce, input.agentId, input.expiresAt], + ) + }) + }, + + async consumeStepUpToken(nonce) { + const rows = await db.query<{ nonce: string }>( + `UPDATE webauthn_stepup_tokens + SET consumed_at = now() + WHERE nonce = $1 AND consumed_at IS NULL AND expires_at > now() + RETURNING nonce`, + [nonce], + ) + return rows.length > 0 + }, + } +}