From 4f59d5128a76bf0f2e3b7a61b9c733a934980227 Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Sat, 4 Jul 2026 21:23:48 +0900 Subject: [PATCH] feat(auth): add owner-tier passkey-unbind remediation endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add DELETE /api/v1/users/{id}/passkeys (owner-only) to unbind every passkey a target account holds — the authenticator remediation that stops a passkey planted or retained via a transiently-hijacked session from surviving as a standing login foothold. It wires the previously-uncalled DeleteAllPasskeyCredentialsForUser and is deliberately not a lockout: the account re-enters via the email-OTP door (players) or op-login's in-game approval (staff), then re-enrolls. Documented in the OpenAPI, so the served/documented parity gate covers it. Remove RevokeUserSessionsExcept: a change-password-era orphan with no callers since the passwordless migration. Its keep-one ("log out my other devices") semantics is inherently self-service, and no such slice is on the roadmap; the admin remediation path already uses RevokeAllUserSessions. --- docs/openapi.yaml | 25 +++++++++++++++ internal/api/api.go | 1 + internal/api/api_test.go | 58 +++++++++++++++++++++++++++++----- internal/api/handlers_users.go | 27 ++++++++++++++++ internal/api/pgrepo.go | 12 ------- internal/api/repo.go | 3 -- 6 files changed, 103 insertions(+), 23 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index b777ff0..6f1798f 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2715,6 +2715,31 @@ paths: '403': $ref: '#/components/responses/Forbidden' + /api/v1/users/{id}/passkeys: + delete: + tags: [users] + operationId: unbindUserPasskeys + summary: Unbind every passkey of a user (owner only) — authenticator remediation. + x-felis-face: [external] + x-felis-tier: owner + security: [{ accessJWT: [] }] + parameters: + - { name: id, in: path, required: true, schema: { type: string } } + responses: + '200': + description: All passkeys unbound (a no-op 200 when the user had none). + content: + application/json: + schema: + type: object + required: [ok] + properties: + ok: { type: boolean, const: true } + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + /api/v1/users/{id}/links: post: tags: [users] diff --git a/internal/api/api.go b/internal/api/api.go index 97462d3..f4fc767 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -426,6 +426,7 @@ func (a *API) externalAPIRoutes() []apiRoute { {Method: "GET", Pattern: "/api/v1/users/{id}/sessions", Owner: true, h: a.handleListUserSessions}, {Method: "DELETE", Pattern: "/api/v1/users/{id}/sessions", Owner: true, h: a.handleRevokeUserSessions}, {Method: "DELETE", Pattern: "/api/v1/users/{id}/sessions/{hash}", Owner: true, h: a.handleRevokeUserSession}, + {Method: "DELETE", Pattern: "/api/v1/users/{id}/passkeys", Owner: true, h: a.handleUnbindUserPasskeys}, {Method: "DELETE", Pattern: "/api/v1/users/{id}/links/{mc_uuid}", Owner: true, h: a.handleUnlinkAccount}, {Method: "POST", Pattern: "/api/v1/users/{id}/links", Owner: true, h: a.handleLinkAccount}, } diff --git a/internal/api/api_test.go b/internal/api/api_test.go index a869cf0..1bd7784 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -662,14 +662,6 @@ func (f *fakeRepo) RevokeSession(_ context.Context, tokenHash string) error { } return nil } -func (f *fakeRepo) RevokeUserSessionsExcept(_ context.Context, userID, keepTokenHash string) error { - for h, s := range f.sessions { - if s.userID == userID && h != keepTokenHash { - s.revoked = true - } - } - return nil -} func (f *fakeRepo) GetSetting(_ context.Context, key string) ([]byte, error) { if v, ok := f.settings[key]; ok { return v, nil @@ -1228,6 +1220,56 @@ func TestExternalFaceRequiresPrincipal(t *testing.T) { } } +// TestUnbindUserPasskeys proves the authenticator-remediation door +// (DELETE /users/{id}/passkeys) severs every passkey a target account holds, is +// gated to the owner role (an Operator-grade admin is refused, so it is stricter +// than the app-admin surface), and treats an account with no passkeys as a 200 +// no-op rather than a 404 — remediation must be idempotent. +func TestUnbindUserPasskeys(t *testing.T) { + repo := newFakeRepo() + api := newTestAPI(repo, newFakeCluster()) + + // Seed the target account with two bound passkeys. + ctx := context.Background() + for _, id := range []string{"pk1", "pk2"} { + if err := repo.CreatePasskeyCredential(ctx, PasskeyCredential{ + ID: id, UserID: "victim", CredentialID: "cred-" + id, PublicKey: "pub", + }); err != nil { + t.Fatalf("seed %s: %v", id, err) + } + } + + owner := &Principal{UserID: "owner1", Email: "owner@mc.example.net", Role: "owner", ViaAdminAccess: true} + + t.Run("owner unbinds every passkey", func(t *testing.T) { + api.External = staticExternal{p: owner} + w := do(api.ExternalHandler(), "DELETE", "/api/v1/users/victim/passkeys", "", nil) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + creds, _ := repo.PasskeyCredentialsForUser(ctx, "victim") + if len(creds) != 0 { + t.Fatalf("passkeys remaining = %d, want 0", len(creds)) + } + }) + + t.Run("no passkeys is a 200 no-op, not a 404", func(t *testing.T) { + api.External = staticExternal{p: owner} + w := do(api.ExternalHandler(), "DELETE", "/api/v1/users/ghost/passkeys", "", nil) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + }) + + t.Run("an Operator-grade admin is refused (owner-only)", func(t *testing.T) { + api.External = staticExternal{p: &Principal{UserID: "op1", Role: "admin", ViaAdminAccess: true}} + w := do(api.ExternalHandler(), "DELETE", "/api/v1/users/victim/passkeys", "", nil) + if w.Code != http.StatusForbidden { + t.Fatalf("code = %d, want 403", w.Code) + } + }) +} + // TestMeIdentity proves GET /api/v1/me reports the server-computed identity the // panel uses to gate its Admin / SysAdmin navigation. The load-bearing assertion // is the third subtest: is_admin tracks Principal.IsAdmin(), so the admin ROLE is diff --git a/internal/api/handlers_users.go b/internal/api/handlers_users.go index 97928dc..f86ea23 100644 --- a/internal/api/handlers_users.go +++ b/internal/api/handlers_users.go @@ -368,6 +368,33 @@ func (a *API) handleRevokeUserSession(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]any{"ok": true}) } +// handleUnbindUserPasskeys unbinds every passkey a user holds +// (DELETE /users/{id}/passkeys). It is the admin account-remediation for a +// compromised authenticator: a passkey planted (or retained) via a transiently +// hijacked session is a standing login foothold that outlives a mere session +// revoke, so severing it needs its own owner-tier action. It is deliberately NOT a +// lockout — the account keeps every other way back in: a player re-enters through +// the email-OTP door and re-enrolls, an operator through op-login's in-game +// approval — so an owner can cut a bad credential without stranding the account. +// DeleteAllPasskeyCredentialsForUser treats removing zero rows as success, so +// unbinding an account that holds no passkeys is a 200 no-op, not a 404. +func (a *API) handleUnbindUserPasskeys(w http.ResponseWriter, r *http.Request) { + p := principalFromContext(r.Context()) + id := r.PathValue("id") + if id == "" { + writeError(w, r, errBadRequest) + return + } + + if err := a.Repo.DeleteAllPasskeyCredentialsForUser(r.Context(), id); err != nil { + writeError(w, r, err) + return + } + + a.audit(r, p.Email, "user.unbind_passkeys", id) + writeJSON(w, http.StatusOK, map[string]any{"ok": true}) +} + // ---- account-link admin ---- // handleUnlinkAccount removes a single (user_id, mc_uuid) binding diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index d88aacb..f05dabb 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -836,18 +836,6 @@ func (p *PGRepo) RevokeSession(ctx context.Context, tokenHash string) error { return err } -// RevokeUserSessionsExcept revokes every live session of a user except keepTokenHash -// — logs out an account's other devices while keeping the current one. Its original -// caller (the change-password flow) was removed in the passwordless migration; it is -// retained for the account-remediation path (P5, #78) and currently has no caller. -func (p *PGRepo) RevokeUserSessionsExcept(ctx context.Context, userID, keepTokenHash string) error { - _, err := p.db.ExecContext(ctx, - `UPDATE sessions SET revoked_at = now() - WHERE user_id = $1 AND token_hash <> $2 AND revoked_at IS NULL`, - userID, keepTokenHash) - return err -} - // ---- runtime platform settings (spec §B platform_settings) ---- // GetSetting reads a setting's raw jsonb value as bytes, or ErrNotFound. diff --git a/internal/api/repo.go b/internal/api/repo.go index bf4e79f..e8b4c1f 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -440,9 +440,6 @@ type Repo interface { // RevokeSession marks a session revoked (logout). It is idempotent: revoking an // absent or already-revoked session is not an error. RevokeSession(ctx context.Context, tokenHash string) error - // RevokeUserSessionsExcept revokes every live session of a user except the one - // whose hash is keepTokenHash. Used to log out other devices on a security event. - RevokeUserSessionsExcept(ctx context.Context, userID, keepTokenHash string) error // ConsumeSetupToken atomically marks a one-time setup token consumed and returns // its user_id, or ErrNotFound when the token is absent, already consumed, or