From 9ab7b27a30400c0239e16667a96806984aff9bc0 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sat, 26 Sep 2026 07:12:20 +0800 Subject: [PATCH] =?UTF-8?q?fix(passkey):=20=E6=96=AD=E8=A8=80=E6=97=B6?= =?UTF-8?q?=E6=8C=89=E5=87=AD=E6=8D=AE=E6=A0=A1=E9=AA=8C=20UV=EF=BC=8C?= =?UTF-8?q?=E5=B9=B6=E6=8A=8A=E5=AD=98=E5=82=A8=E7=9A=84=20BE/BS=20?= =?UTF-8?q?=E6=A0=87=E5=BF=97=E4=BA=A4=E7=BB=99=E6=A0=A1=E9=AA=8C=E5=99=A8?= =?UTF-8?q?=EF=BC=8C=E4=BA=91=E5=90=8C=E6=AD=A5=20passkey=20=E5=8F=AF?= =?UTF-8?q?=E4=BB=A5=E7=99=BB=E5=BD=95=20(#9)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/openapi.yaml | 43 ++++--- internal/api/audit.go | 33 +++-- internal/api/handlers_account_migrate_test.go | 6 +- internal/api/handlers_passkey.go | 67 +++++++---- internal/api/handlers_passkey_discoverable.go | 14 +-- .../api/handlers_passkey_discoverable_test.go | 2 +- internal/api/handlers_passkey_login_test.go | 6 +- internal/api/passkey_uv_test.go | 113 ++++++++++++++++++ internal/api/reauth.go | 13 +- internal/api/reauth_test.go | 18 +-- internal/passkey/verifier.go | 25 +++- internal/passkey/verifier_test.go | 59 ++++++++- panel/src/lib/openapi.gen.ts | 16 +-- 13 files changed, 319 insertions(+), 96 deletions(-) create mode 100644 internal/api/passkey_uv_test.go diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 8a3b24f..00ededb 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2528,9 +2528,12 @@ paths: is consumed atomically and the assertion is verified against it; on success a host-only felis_session cookie is minted. Both players and staff may log in this way — a passkey is a two-factor authenticator (possession + user verification), strong enough to stand alone without the - in-game approval op-login requires. Every failure mode (unknown address, no - live challenge for the signed value, expired challenge, bad assertion) collapses into one uniform - passkey_login_invalid, so the door reveals nothing. + in-game approval op-login requires. User verification is checked per + credential: the passkey must have verified the user when it was bound, and this + assertion must verify the user now. Every failure mode (unknown address, no + live challenge for the signed value, expired challenge, bad assertion, a + credential or assertion without user verification, a cloned authenticator) + collapses into one uniform passkey_login_invalid, so the door reveals nothing. x-felis-face: [external] x-felis-tier: public security: [] @@ -2563,8 +2566,9 @@ paths: '400': description: >- Invalid email or missing assertion (bad_request); or the login could not be - completed — unknown address, no live or expired challenge, or a failed - assertion, all uniform (passkey_login_invalid). + completed — unknown address, no live or expired challenge, a failed + assertion, no user verification, or a cloned authenticator, all uniform + (passkey_login_invalid). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -2674,10 +2678,12 @@ paths: verified against it; the account is resolved from the authenticator-revealed userHandle (the account's stable id), never from anything the client supplied, and the session is minted for the account the assertion actually resolved AND - verified to. Both players and staff may log in this way. Every failure mode — a - missing/expired/consumed login_id, a bad assertion, AND a userHandle that - resolves to no account — collapses into one uniform passkey_login_invalid, so - the door reveals nothing (not even whether the handle was well-formed). + verified to. Both players and staff may log in this way, with the same + per-credential user-verification check as the username-first door. Every failure + mode — a missing/expired/consumed login_id, a bad assertion, no user + verification, a cloned authenticator, AND a userHandle that resolves to no + account — collapses into one uniform passkey_login_invalid, so the door reveals + nothing (not even whether the handle was well-formed). x-felis-face: [external] x-felis-tier: public security: [] @@ -2713,8 +2719,9 @@ paths: '400': description: >- Missing login_id or assertion (bad_request); or the login could not be - completed — no live/expired/consumed challenge, a failed assertion, or a - userHandle that resolves to no account, all uniform (passkey_login_invalid). + completed — no live/expired/consumed challenge, a failed assertion, no user + verification, a cloned authenticator, or a userHandle that resolves to no + account, all uniform (passkey_login_invalid). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -4823,7 +4830,8 @@ paths: summary: Finish the passkey assertion and mark this session reauthed for 5 minutes. description: > Verifies the assertion against the reauth challenge with the login door's - clone check (a cloned authenticator is 400 passkey_login_invalid). + user-verification and clone checks (a credential or assertion without user + verification, or a cloned authenticator, is 400 passkey_login_invalid). x-felis-face: [external] x-felis-tier: app x-felis-setup-allowed: true @@ -4843,7 +4851,7 @@ paths: '200': $ref: '#/components/responses/Reauthed' '400': - description: Assertion invalid, challenge stale, or a cloned authenticator (passkey_login_invalid); no browser session (no_session). + description: Assertion invalid, challenge stale, no user verification, or a cloned authenticator (passkey_login_invalid); no browser session (no_session). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -5226,9 +5234,10 @@ paths: summary: Finish the passkey assertion and confirm the migration (spec §B3 step-up). description: > Verifies the WebAuthn assertion against the fresh migrate-purpose challenge and, - like the login door, applies the authenticator sign-count clone check: a cloned - authenticator is rejected fail-closed (400 passkey_login_invalid) and audited. On - success the migration advances to confirmed with confirm_factor passkey. + like the login door, applies the per-credential user-verification check and the + authenticator sign-count clone check: either refusal fails closed (400 + passkey_login_invalid) and is audited. On success the migration advances to + confirmed with confirm_factor passkey. x-felis-face: [external] x-felis-tier: app security: [{ sessionCookie: [] }] @@ -5254,7 +5263,7 @@ paths: properties: confirmed: { type: boolean, const: true } '400': - description: Assertion invalid, challenge stale, or a cloned authenticator was detected (passkey_login_invalid). + description: Assertion invalid, challenge stale, no user verification, or a cloned authenticator was detected (passkey_login_invalid). content: application/json: schema: { $ref: '#/components/schemas/Error' } diff --git a/internal/api/audit.go b/internal/api/audit.go index 4b9e8f9..a96646c 100644 --- a/internal/api/audit.go +++ b/internal/api/audit.go @@ -134,17 +134,30 @@ func (a *API) authFailure(r *http.Request, door, reason string, u *StaffUser) { a.auditEntry(r, e) } -// passkeyCloneRejected records an assertion refused for a regressed signature -// counter. It keeps its own action so a cloned authenticator stands out from -// ordinary failures, and counts as a failure of its door. u nil: a signed-in -// step-up, attributed to the caller. -func (a *API) passkeyCloneRejected(r *http.Request, door string, u *StaffUser, credentialID string) { - metrics.AuthFailuresTotal.WithLabelValues(door, "clone_rejected").Inc() - if u == nil { - a.audit(r, "auth.passkey_clone_rejected", credentialID) - return +// passkeyAssertionRejected records an assertion applyAssertion refused on policy: a +// regressed signature counter (auth.passkey_clone_rejected) or a user the credential +// or the assertion did not verify (auth.passkey_uv_rejected). Each keeps its own action +// so it stands out from ordinary failures, and counts as a failure of its door. u nil: +// a signed-in step-up, attributed to the caller. It reports false, recording nothing, +// for any other error, which the caller answers as a fault. +func (a *API) passkeyAssertionRejected(r *http.Request, door string, u *StaffUser, credentialID string, err error) bool { + var reason string + switch { + case errors.Is(err, errPasskeyClonedAuthenticator): + reason = "clone_rejected" + case errors.Is(err, errPasskeyUserNotVerified): + reason = "uv_rejected" + default: + return false } - a.auditAccount(r, u, "auth.passkey_clone_rejected", credentialID) + metrics.AuthFailuresTotal.WithLabelValues(door, reason).Inc() + action := "auth.passkey_" + reason + if u == nil { + a.audit(r, action, credentialID) + } else { + a.auditAccount(r, u, action, credentialID) + } + return true } // isOTPRefusal reports whether err is a refused code (wrong, spent, or the diff --git a/internal/api/handlers_account_migrate_test.go b/internal/api/handlers_account_migrate_test.go index dadcbc9..37c2e3a 100644 --- a/internal/api/handlers_account_migrate_test.go +++ b/internal/api/handlers_account_migrate_test.go @@ -149,7 +149,7 @@ func TestMigrateForcePasskey(t *testing.T) { repo := newFakeRepo() repo.seedUser(UserView{ID: "u1", Username: "old", Email: "old@example.net", Role: "user"}) repo.links[uuid] = "u1" - repo.passkeyCreds["row1"] = PasskeyCredential{ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", CreatedAt: frozenNow} + repo.passkeyCreds["row1"] = PasskeyCredential{ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", UserVerified: true, CreatedAt: frozenNow} mk, mailer, _ := migrateEnv(repo) ih := mk(src).InternalHandler() @@ -178,7 +178,7 @@ func TestMigratePasskeyConfirm(t *testing.T) { repo := newFakeRepo() repo.seedUser(UserView{ID: "u1", Username: "old", Email: "old@example.net", Role: "user"}) repo.links[uuid] = "u1" - repo.passkeyCreds["row1"] = PasskeyCredential{ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", CreatedAt: frozenNow} + repo.passkeyCreds["row1"] = PasskeyCredential{ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", UserVerified: true, CreatedAt: frozenNow} mk, _, v := migrateEnv(repo) ih := mk(src).InternalHandler() @@ -227,7 +227,7 @@ func TestMigratePasskeyCloneRejected(t *testing.T) { repo := newFakeRepo() repo.seedUser(UserView{ID: "u1", Username: "old", Email: "old@example.net", Role: "user"}) repo.links[uuid] = "u1" - repo.passkeyCreds["row1"] = PasskeyCredential{ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", CreatedAt: frozenNow} + repo.passkeyCreds["row1"] = PasskeyCredential{ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", UserVerified: true, CreatedAt: frozenNow} mk, _, v := migrateEnv(repo) v.assertion = VerifiedAssertion{CredentialID: "cred-1", UserVerified: true, CloneWarning: true} diff --git a/internal/api/handlers_passkey.go b/internal/api/handlers_passkey.go index 61718fb..6551014 100644 --- a/internal/api/handlers_passkey.go +++ b/internal/api/handlers_passkey.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "net/http" + "slices" "strings" "time" ) @@ -161,11 +162,11 @@ type VerifiedCredential struct { // and whether that counter regressed (a possible clone). Like VerifiedCredential it carries // no secret. SignCount is a raw ceremony fact, NOT a policy verdict; CloneWarning IS the // verifier's regression verdict, but the refuse-vs-allow decision is the handler's. Clone -// policy therefore lives in one place with the stored counter (applyAssertionCounter, which -// both login doors call). SignCount is legitimately 0 for authenticators that keep no counter. +// policy therefore lives in one place with the stored counter (applyAssertion, which +// every assertion door calls). SignCount is legitimately 0 for authenticators that keep no counter. // -// applyAssertionCounter is that single consumer: it refuses a CloneWarning fail-closed and, -// on success, advances the stored counter and stamps last_used_at. This is the stable seam +// applyAssertion is that single consumer: it refuses an unverified user or a CloneWarning +// fail-closed and, on success, advances the stored counter and stamps last_used_at. This is the stable seam // output the production adapter (internal/passkey) produces and its Oracle test asserts on, // so handler and adapter agree on shape without either reshaping the other. type VerifiedAssertion struct { @@ -177,9 +178,10 @@ type VerifiedAssertion struct { // assertion and structurally never raise it. The login handlers refuse it fail-closed. CloneWarning bool // UserVerified records that a PIN/biometric (not mere presence) was performed - // during the assertion ceremony. The verifier enforces UV=required at BeginLogin, - // so this is always true for a successful assertion; persisting it makes the - // guarantee auditable and survives a future policy that permits UV=preferred. + // during this assertion. The verifier asks for UV=required today, so a successful + // assertion carries it; applyAssertion checks it anyway, together with the + // credential's stored bind-time flag, so a later UV=preferred policy or a door that + // asks for less cannot let a presence-only assertion through. UserVerified bool } @@ -188,22 +190,40 @@ type VerifiedAssertion struct { var errPasskeyUnavailable = newError(http.StatusServiceUnavailable, "passkey_unavailable", "passkey subsystem is not configured") -// errPasskeyClonedAuthenticator is the internal signal from applyAssertionCounter that a +// errPasskeyClonedAuthenticator is the internal signal from applyAssertion that a // verified assertion carried a clone warning (its signature counter did not advance past the // stored value). It never reaches the client verbatim: the login doors map it to the generic // passkey_login_invalid envelope — no clone oracle to a prober — and audit it distinctly. var errPasskeyClonedAuthenticator = errors.New("passkey assertion rejected: clone warning") -// applyAssertionCounter is the single consumer of a verified assertion's signature-counter -// facts, shared by the username-first (handlePasskeyLoginFinish) and discoverable -// (handlePasskeyLoginDiscoverableFinish) login doors so clone policy lives in one place with -// the stored counter. A CloneWarning fails closed with errPasskeyClonedAuthenticator; -// otherwise it advances the stored counter to the asserted value and stamps last_used_at. -// Counter-less/synced authenticators report 0 and never warn, so they pass through and simply -// re-stamp 0 — the check gates only counter-keeping authenticators, where a rollback is the -// meaningful clone signal. It runs BEFORE the session is minted, so a clone or a persist -// failure denies the login rather than leaving an advanced counter with no session. -func (a *API) applyAssertionCounter(ctx context.Context, va VerifiedAssertion) error { +// errPasskeyUserNotVerified is the internal signal from applyAssertion that the assertion, +// or the credential it came from, did not verify the user. Like the clone signal it is +// answered with the opaque envelope and audited distinctly. +var errPasskeyUserNotVerified = errors.New("passkey assertion rejected: user not verified") + +// applyAssertion is the single consumer of a verified assertion, shared by the +// username-first and discoverable login doors and the step-up confirmation, so assertion +// policy lives in one place with the stored credential. creds are the account's bound +// passkeys the verifier checked the assertion against. +// +// Every one of those doors grants a session or confirms one, so each needs user +// verification, per credential (migration 0009): the credential must have verified the +// user when it was bound, and this assertion must have verified the user now. Either +// missing fails closed with errPasskeyUserNotVerified. A credential bound presence-only +// (a pre-0009 row, or a future UV=preferred enrollment) therefore cannot sign in; its +// owner still has the email-code door. +// +// A CloneWarning fails closed with errPasskeyClonedAuthenticator; otherwise it advances +// the stored counter to the asserted value and stamps last_used_at. Counter-less/synced +// authenticators report 0 and never warn, so they pass through and simply re-stamp 0 — the +// check gates only counter-keeping authenticators, where a rollback is the meaningful clone +// signal. It runs BEFORE the session is minted, so a refusal or a persist failure denies +// the login rather than leaving an advanced counter with no session. +func (a *API) applyAssertion(ctx context.Context, va VerifiedAssertion, creds []PasskeyCredential) error { + bound := slices.IndexFunc(creds, func(c PasskeyCredential) bool { return c.CredentialID == va.CredentialID }) + if !va.UserVerified || bound < 0 || !creds[bound].UserVerified { + return errPasskeyUserNotVerified + } if va.CloneWarning { return errPasskeyClonedAuthenticator } @@ -710,12 +730,11 @@ func (a *API) handlePasskeyLoginFinish(w http.ResponseWriter, r *http.Request) { "passkey login could not be completed; begin again")) return } - // Clone policy + counter advance, in one place shared with the discoverable door. A - // regressed counter is refused with the same opaque envelope (no clone oracle) but audited - // distinctly; a successful assertion advances the stored counter and stamps last_used_at. - if err := a.applyAssertionCounter(r.Context(), va); err != nil { - if errors.Is(err, errPasskeyClonedAuthenticator) { - a.passkeyCloneRejected(r, "passkey", u, va.CredentialID) + // UV + clone policy + counter advance, in one place shared with the discoverable door. A + // refusal is answered with the same opaque envelope (no oracle) but audited distinctly; a + // successful assertion advances the stored counter and stamps last_used_at. + if err := a.applyAssertion(r.Context(), va, creds); err != nil { + if a.passkeyAssertionRejected(r, "passkey", u, va.CredentialID, err) { writeError(w, r, newError(http.StatusBadRequest, "passkey_login_invalid", "passkey login could not be completed; begin again")) return diff --git a/internal/api/handlers_passkey_discoverable.go b/internal/api/handlers_passkey_discoverable.go index b29ecea..749d07f 100644 --- a/internal/api/handlers_passkey_discoverable.go +++ b/internal/api/handlers_passkey_discoverable.go @@ -148,6 +148,7 @@ func (a *API) handlePasskeyLoginDiscoverableFinish(w http.ResponseWriter, r *htt // the session below is minted for the account the assertion actually resolved AND verified to // — not anything the client supplied (the body carries only a challenge handle). var resolved *StaffUser + var resolvedCreds []PasskeyCredential resolve := func(userHandle []byte) (PasskeyUser, error) { u, err := a.Repo.UserByID(r.Context(), string(userHandle)) if err != nil { @@ -164,7 +165,7 @@ func (a *API) handlePasskeyLoginDiscoverableFinish(w http.ResponseWriter, r *htt if err != nil { return PasskeyUser{}, err } - resolved = u + resolved, resolvedCreds = u, creds // Name/DisplayName are cosmetic at assertion time (nothing is shown to the user); use the // stable username so a nil email never matters. return PasskeyUser{ID: u.ID, Name: u.Username, DisplayName: u.Username, Credentials: creds}, nil @@ -187,12 +188,11 @@ func (a *API) handlePasskeyLoginDiscoverableFinish(w http.ResponseWriter, r *htt "passkey login could not be completed; begin again")) return } - // Same clone policy + counter advance as the username-first door (applyAssertionCounter): a - // regressed counter is refused with the identical opaque envelope but audited under the - // resolved account; a successful assertion advances the stored counter and stamps last_used_at. - if err := a.applyAssertionCounter(r.Context(), va); err != nil { - if errors.Is(err, errPasskeyClonedAuthenticator) { - a.passkeyCloneRejected(r, "passkey_discoverable", resolved, va.CredentialID) + // Same UV + clone policy + counter advance as the username-first door (applyAssertion): a + // refusal gets the identical opaque envelope but is audited under the resolved account; a + // successful assertion advances the stored counter and stamps last_used_at. + if err := a.applyAssertion(r.Context(), va, resolvedCreds); err != nil { + if a.passkeyAssertionRejected(r, "passkey_discoverable", resolved, va.CredentialID, err) { writeError(w, r, newError(http.StatusBadRequest, "passkey_login_invalid", "passkey login could not be completed; begin again")) return diff --git a/internal/api/handlers_passkey_discoverable_test.go b/internal/api/handlers_passkey_discoverable_test.go index 4057747..65f0522 100644 --- a/internal/api/handlers_passkey_discoverable_test.go +++ b/internal/api/handlers_passkey_discoverable_test.go @@ -44,7 +44,7 @@ func seedDiscoverableLoginAPI(t *testing.T) (*API, *fakeRepo, *fakePasskeyVerifi Role: "user", EmailVerified: true, } repo.passkeyCreds["row1"] = PasskeyCredential{ - ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", CreatedAt: frozenNow, + ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", UserVerified: true, CreatedAt: frozenNow, } v := &fakePasskeyVerifier{ options: json.RawMessage(`{"publicKey":{"challenge":"ZGlzYw"}}`), diff --git a/internal/api/handlers_passkey_login_test.go b/internal/api/handlers_passkey_login_test.go index fd5fb23..22b27f5 100644 --- a/internal/api/handlers_passkey_login_test.go +++ b/internal/api/handlers_passkey_login_test.go @@ -46,7 +46,7 @@ func seedLoginPasskeyAPI(t *testing.T) (*API, *fakeRepo, *fakePasskeyVerifier) { Role: "user", EmailVerified: true, } repo.passkeyCreds["row1"] = PasskeyCredential{ - ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", CreatedAt: frozenNow, + ID: "row1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k", UserVerified: true, CreatedAt: frozenNow, } v := &fakePasskeyVerifier{ options: json.RawMessage(`{"publicKey":{"challenge":"YXNzZXJ0"}}`), @@ -494,7 +494,7 @@ func TestPasskeyLoginAllowsStaff(t *testing.T) { Role: "admin", EmailVerified: true, } repo.passkeyCreds["row1"] = PasskeyCredential{ - ID: "row1", UserID: "a1", CredentialID: "cred-a1", PublicKey: "k", CreatedAt: frozenNow, + ID: "row1", UserID: "a1", CredentialID: "cred-a1", PublicKey: "k", UserVerified: true, CreatedAt: frozenNow, } v := &fakePasskeyVerifier{ options: json.RawMessage(`{"publicKey":{"challenge":"YXNzZXJ0"}}`), @@ -534,7 +534,7 @@ func TestPasskeyLoginFaceSeparation(t *testing.T) { } // TestPasskeyLoginFinishCloneRejected is the username-first mirror of the discoverable door's -// clone refusal (task #40 item 5). Both doors share applyAssertionCounter, but each WIRES it +// clone refusal (task #40 item 5). Both doors share applyAssertion, but each WIRES it // independently, so this proves the username-first finish also fails closed on a CloneWarning: // the same opaque passkey_login_invalid envelope (no clone oracle), no session minted, the stored // credential left untouched at its enrollment-time counter, and a distinct auth.passkey_clone_ diff --git a/internal/api/passkey_uv_test.go b/internal/api/passkey_uv_test.go new file mode 100644 index 0000000..666a727 --- /dev/null +++ b/internal/api/passkey_uv_test.go @@ -0,0 +1,113 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// uvCases are the ways an assertion can lack user verification (#9): the credential +// was bound presence-only (a pre-0009 row, or a future UV=preferred enrollment), this +// assertion itself did not verify the user, or it names a credential the account does +// not hold. Each must be refused at every door that grants or confirms a session. +var uvCases = []struct { + name string + storedUV bool + assertUV bool + assertCred string +}{ + {"credential bound without UV", false, true, ""}, + {"assertion without UV", true, false, ""}, + {"credential not bound to the account", true, true, "cred-elsewhere"}, +} + +func TestPasskeyLoginRefusesUnverifiedUser(t *testing.T) { + for _, tc := range uvCases { + t.Run(tc.name, func(t *testing.T) { + api, repo, v := seedLoginPasskeyAPI(t) + row := repo.passkeyCreds["row1"] + row.UserVerified = tc.storedUV + repo.passkeyCreds["row1"] = row + v.assertion = VerifiedAssertion{CredentialID: "cred-1", UserVerified: tc.assertUV, SignCount: 3} + if tc.assertCred != "" { + v.assertion.CredentialID = tc.assertCred + } + plantLoginChallenge(repo, "live", frozenNow.Add(passkeyChallengeTTL)) + + w := do(api.ExternalHandler(), "POST", "/api/v1/auth/passkey/login/finish", finishBody("player@example.net", "YXNzZXJ0"), jsonHeader) + assertUVRefused(t, w, repo, "player") + }) + } +} + +func TestPasskeyDiscoverableLoginRefusesUnverifiedUser(t *testing.T) { + for _, tc := range uvCases { + t.Run(tc.name, func(t *testing.T) { + api, repo, v := seedDiscoverableLoginAPI(t) + row := repo.passkeyCreds["row1"] + row.UserVerified = tc.storedUV + repo.passkeyCreds["row1"] = row + v.assertion = VerifiedAssertion{CredentialID: "cred-1", UserVerified: tc.assertUV, SignCount: 3} + if tc.assertCred != "" { + v.assertion.CredentialID = tc.assertCred + } + eh := api.ExternalHandler() + + loginID := beginDiscoverableLogin(t, eh) + w := do(eh, "POST", "/api/v1/auth/passkey/login/discoverable/finish", + `{"login_id":"`+loginID+`","assertion":{"id":"cred-1","type":"public-key"}}`, jsonHeader) + assertUVRefused(t, w, repo, "player") + }) + } +} + +func TestReauthByPasskeyRefusesUnverifiedUser(t *testing.T) { + for _, tc := range uvCases { + t.Run(tc.name, func(t *testing.T) { + f := reauthFixture(t) + pv := f.api.Passkey.(*fakePasskeyVerifier) + f.repo.passkeyCreds["row1"] = PasskeyCredential{ID: "row1", UserID: "u1", CredentialID: "cred-1", + UserVerified: tc.storedUV, CreatedAt: frozenNow} + pv.assertion = VerifiedAssertion{CredentialID: "cred-1", UserVerified: tc.assertUV, SignCount: 3} + if tc.assertCred != "" { + pv.assertion.CredentialID = tc.assertCred + } + if w := do(f.eh, "POST", "/api/v1/account/reauth/passkey/begin", "", asCookie(laptopTok)); w.Code != http.StatusOK { + t.Fatalf("begin = %d (%s)", w.Code, w.Body.String()) + } + f.repo.audits = nil + w := do(f.eh, "POST", "/api/v1/account/reauth/passkey/finish", `{"assertion":{"id":"cred-1"}}`, jsonCookie(laptopTok)) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "passkey_login_invalid" { + t.Fatalf("finish = %d (%s), want 400 passkey_login_invalid", w.Code, w.Body.String()) + } + if !f.reauthAt(laptopTok).IsZero() { + t.Fatal("an unverified assertion marked the session reauthenticated") + } + if got := f.repo.passkeyCreds["row1"]; got.SignCount != 0 || got.LastUsedAt != nil { + t.Errorf("refusal advanced the credential: SignCount=%d LastUsedAt=%v", got.SignCount, got.LastUsedAt) + } + if n := len(f.repo.audits); n != 1 || f.repo.audits[0].Action != "auth.passkey_uv_rejected" { + t.Fatalf("want exactly 1 auth.passkey_uv_rejected audit, got %+v", f.repo.audits) + } + }) + } +} + +// assertUVRefused checks a sign-in door's refusal: the opaque envelope every finish +// failure shares, no session, the stored credential untouched, and one distinct audit +// under the account. +func assertUVRefused(t *testing.T, rec *httptest.ResponseRecorder, repo *fakeRepo, actor string) { + t.Helper() + if rec.Code != http.StatusBadRequest || decodeErr(t, rec) != "passkey_login_invalid" { + t.Fatalf("finish = %d (%s), want 400 passkey_login_invalid", rec.Code, rec.Body.String()) + } + if len(repo.sessions) != 0 || len(rec.Result().Cookies()) != 0 { + t.Fatalf("refusal minted a session: sessions=%d cookies=%v", len(repo.sessions), rec.Result().Cookies()) + } + if got := repo.passkeyCreds["row1"]; got.SignCount != 0 || got.LastUsedAt != nil { + t.Errorf("refusal advanced the credential: SignCount=%d LastUsedAt=%v", got.SignCount, got.LastUsedAt) + } + if n := len(repo.audits); n != 1 || repo.audits[0].Action != "auth.passkey_uv_rejected" || repo.audits[0].Actor != actor { + t.Fatalf("want exactly 1 auth.passkey_uv_rejected audit by %s, got %+v", actor, repo.audits) + } +} diff --git a/internal/api/reauth.go b/internal/api/reauth.go index 971267a..69e7a7b 100644 --- a/internal/api/reauth.go +++ b/internal/api/reauth.go @@ -329,13 +329,12 @@ func (a *API) finishStepUpPasskey(w http.ResponseWriter, r *http.Request, p *Pri a.authFailure(r, door, "bad_assertion", nil) return invalid() } - // Same clone policy as the login door (applyAssertionCounter): a rolled-back - // counter fails closed with the opaque envelope, so a step-up never accepts an - // authenticator that login refuses. A clean assertion advances the stored - // sign-count, keeping the clone signal meaningful for the next login. - if err := a.applyAssertionCounter(r.Context(), va); err != nil { - if errors.Is(err, errPasskeyClonedAuthenticator) { - a.passkeyCloneRejected(r, door, nil, va.CredentialID) + // Same UV and clone policy as the login door (applyAssertion): an unverified + // user or a rolled-back counter fails closed with the opaque envelope, so a + // step-up never accepts an authenticator that login refuses. A clean assertion + // advances the stored sign-count, keeping the clone signal meaningful. + if err := a.applyAssertion(r.Context(), va, creds); err != nil { + if a.passkeyAssertionRejected(r, door, nil, va.CredentialID, err) { return invalid() } writeError(w, r, err) diff --git a/internal/api/reauth_test.go b/internal/api/reauth_test.go index d01dc33..177346a 100644 --- a/internal/api/reauth_test.go +++ b/internal/api/reauth_test.go @@ -113,7 +113,7 @@ func TestGuardedChangesNeedARecentReauth(t *testing.T) { t.Run(tc.name+"/"+g.name, func(t *testing.T) { f := reauthFixture(t) f.api.Mailer = &captureMailer{} - f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", CreatedAt: frozenNow} + f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", UserVerified: true, CreatedAt: frozenNow} if tc.proof >= 0 { f.setReauth(laptopTok, f.api.now().Add(-tc.proof)) } @@ -172,7 +172,7 @@ func TestPasskeyAloneNeedsReauth(t *testing.T) { func TestAccessCallerNeedsNoReauth(t *testing.T) { repo := newFakeRepo() repo.staff["op"] = &StaffUser{ID: "u1", Username: "op", Role: "admin", Email: "op@example.net", EmailVerified: true} - repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", CreatedAt: frozenNow} + repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", UserVerified: true, CreatedAt: frozenNow} api := newTestAPI(repo, newFakeCluster()) api.Passkey = &fakePasskeyVerifier{} api.External = staticExternal{p: &Principal{UserID: "u1", Email: "op@example.net", Role: "admin", EmailVerified: true}} @@ -203,7 +203,7 @@ func getReauthStatus(t *testing.T, f *sessionsFixture, tok string) reauthStatusB func TestReauthStatusNamesTheFactors(t *testing.T) { f := reauthFixture(t) f.api.Mailer = &captureMailer{} - f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", CreatedAt: frozenNow} + f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", UserVerified: true, CreatedAt: frozenNow} f.repo.passkeyCreds["p"] = PasskeyCredential{ID: "p", UserID: "u3", CredentialID: "c-p", CreatedAt: frozenNow} steve := getReauthStatus(t, f, laptopTok) @@ -350,7 +350,7 @@ func TestReauthByEmailNeedsAVerifiedAddress(t *testing.T) { func TestReauthByPasskey(t *testing.T) { f := reauthFixture(t) pv := f.api.Passkey.(*fakePasskeyVerifier) - f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", SignCount: 4, CreatedAt: frozenNow} + f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", SignCount: 4, UserVerified: true, CreatedAt: frozenNow} finish := func() int { t.Helper() if w := do(f.eh, "POST", "/api/v1/account/reauth/passkey/begin", "", asCookie(laptopTok)); w.Code != http.StatusOK { @@ -367,7 +367,7 @@ func TestReauthByPasskey(t *testing.T) { t.Fatalf("bad assertion = %d, want 400", code) } pv.failErr = nil - pv.assertion = VerifiedAssertion{CredentialID: "c-a", SignCount: 2, CloneWarning: true} + pv.assertion = VerifiedAssertion{CredentialID: "c-a", SignCount: 2, CloneWarning: true, UserVerified: true} if code := finish(); code != http.StatusBadRequest { t.Fatalf("cloned authenticator = %d, want 400", code) } @@ -375,7 +375,7 @@ func TestReauthByPasskey(t *testing.T) { t.Fatal("a failed assertion marked the session") } - pv.assertion = VerifiedAssertion{CredentialID: "c-a", SignCount: 5} + pv.assertion = VerifiedAssertion{CredentialID: "c-a", SignCount: 5, UserVerified: true} if code := finish(); code != http.StatusOK { t.Fatalf("finish = %d, want 200", code) } @@ -396,8 +396,8 @@ func TestReauthByPasskey(t *testing.T) { func TestReauthPasskeyChallengeIsItsOwnPurpose(t *testing.T) { f := reauthFixture(t) pv := f.api.Passkey.(*fakePasskeyVerifier) - pv.assertion = VerifiedAssertion{CredentialID: "c-a", SignCount: 5} - f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", CreatedAt: frozenNow} + pv.assertion = VerifiedAssertion{CredentialID: "c-a", SignCount: 5, UserVerified: true} + f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", UserVerified: true, CreatedAt: frozenNow} if err := f.repo.CreatePasskeyChallenge(t.Context(), "m1", "u1", passkeyPurposeMigrate, []byte("s"), f.api.now().Add(time.Minute)); err != nil { t.Fatal(err) } @@ -466,7 +466,7 @@ func onlyNotice(t *testing.T, m *noticeMailer) (to, subject, body string) { func TestRemovingAPasskeyMailsTheAccount(t *testing.T) { f, mailer := noticeFixture(t) - f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", CreatedAt: frozenNow} + f.repo.passkeyCreds["a"] = PasskeyCredential{ID: "a", UserID: "u1", CredentialID: "c-a", UserVerified: true, CreatedAt: frozenNow} if w := do(f.eh, "DELETE", "/api/v1/account/passkey/credentials/a", "", fromIP(asCookie(laptopTok))); w.Code != http.StatusNoContent { t.Fatalf("delete = %d (%s)", w.Code, w.Body.String()) } diff --git a/internal/passkey/verifier.go b/internal/passkey/verifier.go index a665adf..ae1a0a1 100644 --- a/internal/passkey/verifier.go +++ b/internal/passkey/verifier.go @@ -210,11 +210,7 @@ func (v *Verifier) FinishLogin(user api.PasskeyUser, sessionData []byte, asserti if err != nil { return api.VerifiedAssertion{}, err } - return api.VerifiedAssertion{ - CredentialID: base64.RawURLEncoding.EncodeToString(cred.ID), - SignCount: cred.Authenticator.SignCount, - CloneWarning: cred.Authenticator.CloneWarning, - }, nil + return verifiedAssertion(cred), nil } // BeginDiscoverableLogin starts a USERNAMELESS assertion ceremony (task #40): the caller is @@ -273,11 +269,19 @@ func (v *Verifier) FinishDiscoverableLogin(resolveUser func(userHandle []byte) ( if err != nil { return api.VerifiedAssertion{}, err } + return verifiedAssertion(cred), nil +} + +// verifiedAssertion reports a validated login. go-webauthn has replaced cred.Flags with +// the flags of this assertion, so UserVerified says whether the user was verified now, +// which the handler checks alongside the credential's stored bind-time flag. +func verifiedAssertion(cred *webauthn.Credential) api.VerifiedAssertion { return api.VerifiedAssertion{ CredentialID: base64.RawURLEncoding.EncodeToString(cred.ID), SignCount: cred.Authenticator.SignCount, CloneWarning: cred.Authenticator.CloneWarning, - }, nil + UserVerified: cred.Flags.UserVerified, + } } // excludeDescriptors turns the principal's already-bound passkeys into the @@ -353,6 +357,15 @@ func (w webauthnUser) WebAuthnCredentials() []webauthn.Credential { cred.PublicKey = key } cred.Authenticator.SignCount = c.SignCount + // ValidateLogin refuses an assertion whose backup-eligible flag differs from the + // stored credential's. A synced passkey reports BE on every ceremony, so without + // the stored flags it would enroll and then never log in. + cred.Flags = webauthn.CredentialFlags{ + UserPresent: true, + UserVerified: c.UserVerified, + BackupEligible: c.BackupEligible, + BackupState: c.BackupState, + } out = append(out, cred) } return out diff --git a/internal/passkey/verifier_test.go b/internal/passkey/verifier_test.go index 97225d0..59059b1 100644 --- a/internal/passkey/verifier_test.go +++ b/internal/passkey/verifier_test.go @@ -227,7 +227,10 @@ func enrollCredential(t *testing.T, v *Verifier, rp virtualwebauthn.RelyingParty if err != nil { t.Fatalf("FinishRegistration: %v", err) } - return api.PasskeyCredential{CredentialID: vc.CredentialID, PublicKey: vc.PublicKey, SignCount: vc.SignCount} + // The ceremony flags travel with the row exactly as PGRepo stores and reloads them + // (migration 0009), so a login test validates against what production would hold. + return api.PasskeyCredential{CredentialID: vc.CredentialID, PublicKey: vc.PublicKey, SignCount: vc.SignCount, + UserVerified: vc.UserVerified, BackupEligible: vc.BackupEligible, BackupState: vc.BackupState} } // TestLoginRoundTrip is the PARITY check for the assertion (login) half, deliberately @@ -583,3 +586,57 @@ func TestEnrollmentRequestsResidentKey(t *testing.T) { t.Errorf("authenticatorSelection.residentKey = %q, want %q", got, "preferred") } } + +// TestSyncedPasskeyLogsIn covers a passkey kept in a cloud keychain (iCloud Keychain, +// Google Password Manager), which reports backup-eligible on every ceremony. go-webauthn +// refuses an assertion whose BE flag differs from the stored credential's, so the stored +// flags must reach it: dropped, every synced passkey would enroll and then never log in. +// The asserted UV flag must come back too, for the handler's per-credential check. +func TestSyncedPasskeyLogsIn(t *testing.T) { + for _, door := range []string{"username-first", "discoverable"} { + t.Run(door, func(t *testing.T) { + v := newTestVerifier(t) + rp := virtualRP() + authenticator := virtualwebauthn.NewAuthenticatorWithOptions(virtualwebauthn.AuthenticatorOptions{BackupEligible: true, BackupState: true}) + cred := virtualwebauthn.NewCredential(virtualwebauthn.KeyTypeEC2) + stored := enrollCredential(t, v, rp, authenticator, cred) + if !stored.BackupEligible || !stored.BackupState { + t.Fatalf("enrollment recorded BE=%v BS=%v, want both true", stored.BackupEligible, stored.BackupState) + } + + var va api.VerifiedAssertion + var err error + if door == "username-first" { + options, sessionData, berr := v.BeginLogin(testUser(stored)) + if berr != nil { + t.Fatalf("BeginLogin: %v", berr) + } + opts, perr := virtualwebauthn.ParseAssertionOptions(string(options)) + if perr != nil { + t.Fatalf("ParseAssertionOptions: %v", perr) + } + resp := virtualwebauthn.CreateAssertionResponse(rp, authenticator, cred, *opts) + va, err = v.FinishLogin(testUser(stored), sessionData, strings.NewReader(resp)) + } else { + authenticator.Options.UserHandle = []byte(testUserID) + options, sessionData, berr := v.BeginDiscoverableLogin() + if berr != nil { + t.Fatalf("BeginDiscoverableLogin: %v", berr) + } + opts, perr := virtualwebauthn.ParseAssertionOptions(string(options)) + if perr != nil { + t.Fatalf("ParseAssertionOptions: %v", perr) + } + resp := virtualwebauthn.CreateAssertionResponse(rp, authenticator, cred, *opts) + va, err = v.FinishDiscoverableLogin(func([]byte) (api.PasskeyUser, error) { return testUser(stored), nil }, + sessionData, strings.NewReader(resp)) + } + if err != nil { + t.Fatalf("finish: %v (a synced passkey must log in)", err) + } + if !va.UserVerified { + t.Error("va.UserVerified = false, want true (the authenticator verified the user)") + } + }) + } +} diff --git a/panel/src/lib/openapi.gen.ts b/panel/src/lib/openapi.gen.ts index f53a6f1..562c469 100644 --- a/panel/src/lib/openapi.gen.ts +++ b/panel/src/lib/openapi.gen.ts @@ -717,7 +717,7 @@ export interface paths { put?: never; /** * Complete a passkey (WebAuthn) login and mint a session (spec §14, §B). - * @description Second leg of the public passkey door: the caller returns the email (to re-select the account) and the raw navigator.credentials.get() assertion. The live login challenge whose value the assertion signed (response.clientDataJSON) is consumed atomically and the assertion is verified against it; on success a host-only felis_session cookie is minted. Both players and staff may log in this way — a passkey is a two-factor authenticator (possession + user verification), strong enough to stand alone without the in-game approval op-login requires. Every failure mode (unknown address, no live challenge for the signed value, expired challenge, bad assertion) collapses into one uniform passkey_login_invalid, so the door reveals nothing. + * @description Second leg of the public passkey door: the caller returns the email (to re-select the account) and the raw navigator.credentials.get() assertion. The live login challenge whose value the assertion signed (response.clientDataJSON) is consumed atomically and the assertion is verified against it; on success a host-only felis_session cookie is minted. Both players and staff may log in this way — a passkey is a two-factor authenticator (possession + user verification), strong enough to stand alone without the in-game approval op-login requires. User verification is checked per credential: the passkey must have verified the user when it was bound, and this assertion must verify the user now. Every failure mode (unknown address, no live challenge for the signed value, expired challenge, bad assertion, a credential or assertion without user verification, a cloned authenticator) collapses into one uniform passkey_login_invalid, so the door reveals nothing. */ post: operations["passkeyLoginFinish"]; delete?: never; @@ -757,7 +757,7 @@ export interface paths { put?: never; /** * Complete a usernameless (discoverable) passkey login and mint a session (spec §14, §B, task - * @description Second leg of the from-zero door: the caller returns the opaque login_id from begin (the only link to the stashed challenge, since it is not user-keyed) and the raw navigator.credentials.get() assertion — and NOTHING that names an account. The stashed challenge is consumed atomically and the assertion is verified against it; the account is resolved from the authenticator-revealed userHandle (the account's stable id), never from anything the client supplied, and the session is minted for the account the assertion actually resolved AND verified to. Both players and staff may log in this way. Every failure mode — a missing/expired/consumed login_id, a bad assertion, AND a userHandle that resolves to no account — collapses into one uniform passkey_login_invalid, so the door reveals nothing (not even whether the handle was well-formed). + * @description Second leg of the from-zero door: the caller returns the opaque login_id from begin (the only link to the stashed challenge, since it is not user-keyed) and the raw navigator.credentials.get() assertion — and NOTHING that names an account. The stashed challenge is consumed atomically and the assertion is verified against it; the account is resolved from the authenticator-revealed userHandle (the account's stable id), never from anything the client supplied, and the session is minted for the account the assertion actually resolved AND verified to. Both players and staff may log in this way, with the same per-credential user-verification check as the username-first door. Every failure mode — a missing/expired/consumed login_id, a bad assertion, no user verification, a cloned authenticator, AND a userHandle that resolves to no account — collapses into one uniform passkey_login_invalid, so the door reveals nothing (not even whether the handle was well-formed). */ post: operations["passkeyLoginDiscoverableFinish"]; delete?: never; @@ -1586,7 +1586,7 @@ export interface paths { put?: never; /** * Finish the passkey assertion and mark this session reauthed for 5 minutes. - * @description Verifies the assertion against the reauth challenge with the login door's clone check (a cloned authenticator is 400 passkey_login_invalid). + * @description Verifies the assertion against the reauth challenge with the login door's user-verification and clone checks (a credential or assertion without user verification, or a cloned authenticator, is 400 passkey_login_invalid). */ post: operations["reauthPasskeyFinish"]; delete?: never; @@ -1780,7 +1780,7 @@ export interface paths { put?: never; /** * Finish the passkey assertion and confirm the migration (spec §B3 step-up). - * @description Verifies the WebAuthn assertion against the fresh migrate-purpose challenge and, like the login door, applies the authenticator sign-count clone check: a cloned authenticator is rejected fail-closed (400 passkey_login_invalid) and audited. On success the migration advances to confirmed with confirm_factor passkey. + * @description Verifies the WebAuthn assertion against the fresh migrate-purpose challenge and, like the login door, applies the per-credential user-verification check and the authenticator sign-count clone check: either refusal fails closed (400 passkey_login_invalid) and is audited. On success the migration advances to confirmed with confirm_factor passkey. */ post: operations["migrateConfirmPasskeyFinish"]; delete?: never; @@ -4475,7 +4475,7 @@ export interface operations { }; }; }; - /** @description Invalid email or missing assertion (bad_request); or the login could not be completed — unknown address, no live or expired challenge, or a failed assertion, all uniform (passkey_login_invalid). */ + /** @description Invalid email or missing assertion (bad_request); or the login could not be completed — unknown address, no live or expired challenge, a failed assertion, no user verification, or a cloned authenticator, all uniform (passkey_login_invalid). */ 400: { headers: { [name: string]: unknown; @@ -4622,7 +4622,7 @@ export interface operations { }; }; }; - /** @description Missing login_id or assertion (bad_request); or the login could not be completed — no live/expired/consumed challenge, a failed assertion, or a userHandle that resolves to no account, all uniform (passkey_login_invalid). */ + /** @description Missing login_id or assertion (bad_request); or the login could not be completed — no live/expired/consumed challenge, a failed assertion, no user verification, a cloned authenticator, or a userHandle that resolves to no account, all uniform (passkey_login_invalid). */ 400: { headers: { [name: string]: unknown; @@ -6784,7 +6784,7 @@ export interface operations { }; responses: { 200: components["responses"]["Reauthed"]; - /** @description Assertion invalid, challenge stale, or a cloned authenticator (passkey_login_invalid); no browser session (no_session). */ + /** @description Assertion invalid, challenge stale, no user verification, or a cloned authenticator (passkey_login_invalid); no browser session (no_session). */ 400: { headers: { [name: string]: unknown; @@ -7222,7 +7222,7 @@ export interface operations { }; }; }; - /** @description Assertion invalid, challenge stale, or a cloned authenticator was detected (passkey_login_invalid). */ + /** @description Assertion invalid, challenge stale, no user verification, or a cloned authenticator was detected (passkey_login_invalid). */ 400: { headers: { [name: string]: unknown;