diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 4d235b1..b715e92 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -359,6 +359,17 @@ func (f *fakeRepo) DeletePasskeyCredential(_ context.Context, userID, id string) return ErrNotFound } +// DeleteAllPasskeyCredentialsForUser mirrors PGRepo: unbind every passkey the user holds, +// and removing zero is a successful no-op (never ErrNotFound). +func (f *fakeRepo) DeleteAllPasskeyCredentialsForUser(_ context.Context, userID string) error { + for id, c := range f.passkeyCreds { + if c.UserID == userID { + delete(f.passkeyCreds, id) + } + } + return nil +} + // fakePasskeyVerifier is the hermetic PasskeyVerifier: it performs no real attestation // crypto, so it exercises the enrollment STATE MACHINE (challenge persistence, consume, // conflict, audit) without go-webauthn. BeginRegistration returns a fixed options blob diff --git a/internal/api/handlers_auth.go b/internal/api/handlers_auth.go index 47b3fe3..63da54f 100644 --- a/internal/api/handlers_auth.go +++ b/internal/api/handlers_auth.go @@ -218,6 +218,16 @@ func (a *API) handleChangePassword(w http.ResponseWriter, r *http.Request) { return } + // Revoking sessions is not enough: a passkey needs no password, so one planted + // through a transiently-hijacked session would outlive the reset as a standing login + // foothold. A password change is a possible-compromise signal, so unbind every passkey + // as part of the same remediation. The user re-enrolls afterward if they want one; the + // email-OTP factor stays available in the meantime, so this never locks anyone out. + if err := a.Repo.DeleteAllPasskeyCredentialsForUser(r.Context(), u.ID); err != nil { + writeError(w, r, err) + return + } + a.audit(r, u.Username, "auth.password_change", "") writeJSON(w, http.StatusOK, map[string]any{"ok": true}) } diff --git a/internal/api/handlers_auth_test.go b/internal/api/handlers_auth_test.go index eb59a18..3dc7ea1 100644 --- a/internal/api/handlers_auth_test.go +++ b/internal/api/handlers_auth_test.go @@ -244,6 +244,10 @@ func TestHandleChangePasswordSuccess(t *testing.T) { // no felis_session cookie, so keep="" and every session of u1 is revoked — the // safe direction the handler documents. repo.sessions["other-device"] = &fakeSession{userID: "u1", expiresAt: api.now().Add(time.Hour)} + // A bound passkey for u1: the change must unbind it too. A passkey planted through a + // hijacked session needs no password, so it would otherwise survive the reset as a + // standing login foothold. + repo.passkeyCreds["pk1"] = PasskeyCredential{ID: "pk1", UserID: "u1", CredentialID: "cred-1", PublicKey: "k"} w := do(api.ExternalHandler(), "POST", "/api/v1/auth/change-password", `{"current_password":"old-password","new_password":"brand-new-password"}`, jsonHeader) @@ -261,6 +265,9 @@ func TestHandleChangePasswordSuccess(t *testing.T) { if !repo.sessions["other-device"].revoked { t.Fatalf("other sessions should be revoked on a password change") } + if len(repo.passkeyCreds) != 0 { + t.Fatalf("password change left %d passkeys, want 0 — a planted passkey must not survive remediation", len(repo.passkeyCreds)) + } } // TestHandleChangePasswordContentTypeGuard pins the defense-in-depth guard on the diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index e8580a1..988b12d 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -997,3 +997,14 @@ func (p *PGRepo) DeletePasskeyCredential(ctx context.Context, userID, id string) } return nil } + +// DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds. Unlike the +// single-credential delete this does NOT report ErrNotFound on zero rows: removing all of +// a user's passkeys when they have none is a successful no-op, since "the user holds no +// passkeys" is exactly the intended post-condition. The change-password flow calls it so a +// passkey planted through a transiently-hijacked session cannot survive the remediation. +func (p *PGRepo) DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID string) error { + _, err := p.db.ExecContext(ctx, + `DELETE FROM webauthn_credentials WHERE user_id = $1`, userID) + return err +} diff --git a/internal/api/repo.go b/internal/api/repo.go index 3f7859a..5b83e91 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -291,6 +291,12 @@ type Repo interface { // can only unbind their OWN credential. No matching (user, id) row → ErrNotFound, // so a stale or cross-user id cannot silently no-op as success. DeletePasskeyCredential(ctx context.Context, userID, id string) error + // DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds. The + // change-password flow calls it so a passkey planted via a transiently-hijacked + // session does not survive the remediation (password reset + session revoke) as a + // standing login foothold. Removing zero rows is success, not an error — an account + // with no passkeys is the intended post-condition either way. + DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID string) error // ---- player game-login: username-collision reclaim (spec §B3) ----