From c20b12c655b2670441ca4676a1753631a30c7b24 Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Sat, 4 Jul 2026 21:46:35 +0900 Subject: [PATCH] refactor(api): drop dead password-era ResetMailer, reconcile passkey-unbind docs The passwordless migration left ResetMailer (SendPasswordReset) and its API field with zero callers and no wiring; the web console authenticates via email-OTP and passkey only. Remove both, plus the now-orphaned context import that the interface was the last user of in handlers_users.go. Reconcile the DeleteAllPasskeyCredentialsForUser docs in repo.go and pgrepo.go: they claimed there was no production caller, but 2f22027 wired the owner-tier DELETE /users/{id}/passkeys. Both now note that a complete authenticator remediation pairs the unbind with a session revoke (unbinding alone leaves the live hijacked session; revoking alone leaves a re-enrollable credential), and the OpenAPI operation carries the same guidance in a new description. Reword the stale local-password test-fake header, since the passwordless fakes carry no must_change_password field. No behavior change. gofmt, build, and the full test tree are green; OpenAPI parity and passkey-unbind tests pass; a grep confirms ResetMailer/SendPasswordReset are gone from the Go tree. --- docs/openapi.yaml | 8 ++++++++ internal/api/api.go | 5 ----- internal/api/api_test.go | 4 ++-- internal/api/handlers_users.go | 9 --------- internal/api/pgrepo.go | 6 +++--- internal/api/repo.go | 11 ++++++----- 6 files changed, 19 insertions(+), 24 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 6f1798f..54e10cd 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2720,6 +2720,14 @@ paths: tags: [users] operationId: unbindUserPasskeys summary: Unbind every passkey of a user (owner only) — authenticator remediation. + description: >- + Severs a compromised or planted authenticator that would otherwise outlive a + session revoke. A complete remediation pairs this with revoking the user's + sessions (DELETE /users/{id}/sessions/{hash}): unbinding the credential alone + leaves the live hijacked session, and revoking sessions alone leaves a + re-enrollable credential. It is not a lockout — the account re-enters via the + email-OTP door or op-login and re-enrolls. Removing zero passkeys is a 200 + no-op, not a 404. x-felis-face: [external] x-felis-tier: owner security: [{ accessJWT: [] }] diff --git a/internal/api/api.go b/internal/api/api.go index f4fc767..5c5ed06 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -73,11 +73,6 @@ type API struct { // sender. The code is never returned to the client on either path. Mailer OTPMailer - // ResetMailer delivers admin-generated password-reset passwords to the user's - // verified email address. Same nil→server-side-log pattern as Mailer; the - // password is never returned to the admin caller. Production wires a real sender. - ResetMailer ResetMailer - // Passkey verifies WebAuthn credential-creation ceremonies (spec §14 / Phase 6 // passkey bind). It is optional: when nil the passkey register routes report 503 // rather than panic, so the authenticated enrollment boundary is exercised before diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 1bd7784..01a811b 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -605,10 +605,10 @@ func (f *fakeRepo) BackupByID(_ context.Context, id string) (*BackupRecord, erro return nil, ErrNotFound } -// ---- local-password auth fakes (spec §B) ---- +// ---- staff / session auth fakes (spec §B, passwordless) ---- // Each method mirrors the PGRepo contract: a returned StaffUser is copied so a // test cannot mutate the stored row by reference, SessionUser re-reads the -// CURRENT staff flags (so a password change clears must_change_password for live +// CURRENT staff row (so a role change or a deleted account takes effect on live // sessions just as the PG JOIN does), and the settings/sessions semantics match. func (f *fakeRepo) UserByUsername(_ context.Context, username string) (*StaffUser, error) { diff --git a/internal/api/handlers_users.go b/internal/api/handlers_users.go index f86ea23..e086891 100644 --- a/internal/api/handlers_users.go +++ b/internal/api/handlers_users.go @@ -1,21 +1,12 @@ package api import ( - "context" "errors" "net/http" "strconv" "strings" ) -// ResetMailer delivers a freshly-generated admin-reset password to the user's -// verified email address. nil means the password is logged server-side (the -// KNOWN-LIMITATION pattern from OTPMailer — production wires a real sender). -// The password is never returned to the admin caller. -type ResetMailer interface { - SendPasswordReset(ctx context.Context, email, password string) error -} - // ---- user CRUD ---- // handleListUsers is the admin-tier user list (GET /users). It gates on diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index f05dabb..807b7e8 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -1020,9 +1020,9 @@ func (p *PGRepo) DeletePasskeyCredential(ctx context.Context, userID, id string) // 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. It is the remediation that stops a -// passkey planted through a transiently-hijacked session from surviving; its original -// caller (the change-password flow) was removed in the passwordless migration, so it is -// currently uncalled, retained for the account-remediation/reset path (P5, #78). +// passkey planted through a transiently-hijacked session from surviving; its production +// caller is the owner-tier DELETE /users/{id}/passkeys, which a complete remediation +// pairs with a session revoke (unbinding alone leaves the live hijacked session). func (p *PGRepo) DeleteAllPasskeyCredentialsForUser(ctx context.Context, userID string) error { _, err := p.db.ExecContext(ctx, `DELETE FROM webauthn_credentials WHERE user_id = $1`, userID) diff --git a/internal/api/repo.go b/internal/api/repo.go index e8b4c1f..e3c5c5b 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -354,11 +354,12 @@ type Repo interface { DeletePasskeyCredential(ctx context.Context, userID, id string) error // DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds — the // remediation that stops a passkey planted via a transiently-hijacked session from - // surviving as a standing login foothold. Its original caller, the change-password - // flow, was removed in the passwordless migration, so it currently has no production - // caller; it is retained for the account-remediation/reset path (P5, #78). Removing - // zero rows is success, not an error — an account with no passkeys is the intended - // post-condition either way. + // surviving as a standing login foothold. Its production caller is the owner-tier + // DELETE /users/{id}/passkeys (handleUnbindUserPasskeys); a complete remediation + // pairs it with a session revoke, since unbinding the credential without revoking + // live sessions leaves the hijacked session itself, and revoking sessions without + // unbinding leaves a re-enrollable credential. 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) ----