fix(passkey): 删除 passkey 先确认并显示名称与注册时间,邮箱未验证时后端拒删最后一把,错误内联显示

This commit is contained in:
Lemon-miaow committed 2026-09-25 10:04:31 +08:00
1 parent 90c39afb0c
commit ea425cffa4
16 files changed
+355 -59

No files matched your search

+22 -5
View File
@@ -539,13 +539,30 @@ func (f *fakeRepo) PasskeyCredentialsForUser(_ context.Context, userID string) (
}
// DeletePasskeyCredential mirrors PGRepo: scoped to userID so a caller can only unbind
// their OWN credential; no matching (user, id) row → ErrNotFound.
// their OWN credential; no matching (user, id) row → ErrNotFound; the last passkey of
// a user whose email is unverified (or who has no user row here) → ErrLastPasskey.
func (f *fakeRepo) DeletePasskeyCredential(_ context.Context, userID, id string) error {
if c, ok := f.passkeyCreds[id]; ok && c.UserID == userID {
delete(f.passkeyCreds, id)
return nil
c, ok := f.passkeyCreds[id]
if !ok || c.UserID != userID {
return ErrNotFound
}
return ErrNotFound
total := 0
for _, other := range f.passkeyCreds {
if other.UserID == userID {
total++
}
}
verified := false
for _, u := range f.staff {
if u.ID == userID {
verified = u.EmailVerified
}
}
if total == 1 && !verified {
return ErrLastPasskey
}
delete(f.passkeyCreds, id)
return nil
}
// DeleteAllPasskeyCredentialsForUser mirrors PGRepo: unbind every passkey the user holds,
+6
View File
@@ -57,6 +57,12 @@ var (
// finish endpoint exists; the ceremony state is gone (never begun, already
// consumed, or expired) — so handlers map it to 400, not 404.
ErrPasskeyChallengeInvalid = errors.New("passkey challenge invalid or expired")
// ErrLastPasskey means a passkey delete would remove the account's only one while
// its email is unverified. That passkey is then the account's only durable way
// in (setupRequired: no verified email and no passkey puts it back behind the
// setup gate, and a staff account has no other self-service door at all), so the
// delete is refused; handlers map it to 409 last_passkey.
ErrLastPasskey = errors.New("cannot remove the only passkey of an account without a verified email")
// ErrPlayerBindForbidden means a public Bind-Code redemption resolved to a STAFF
// account (admin or owner), which the player-console bootstrap refuses
// (console-tier access model). Staff authenticate at op.console behind Zero Trust,
+7
View File
@@ -400,6 +400,8 @@ func (a *API) handlePasskeyList(w http.ResponseWriter, r *http.Request) {
// handlePasskeyDelete unbinds one of the caller's passkeys (spec §14, external app
// face). The delete is scoped to the principal, so a caller can only remove their OWN
// credential; an unknown or cross-user id → 404 (it never silently no-ops as success).
// The last passkey of an account without a verified email → 409 last_passkey: it is
// that account's only durable way in (ErrLastPasskey).
func (a *API) handlePasskeyDelete(w http.ResponseWriter, r *http.Request) {
p := principalFromContext(r.Context())
id := r.PathValue("id")
@@ -412,6 +414,11 @@ func (a *API) handlePasskeyDelete(w http.ResponseWriter, r *http.Request) {
writeError(w, r, newError(http.StatusNotFound, "not_found", "no such passkey"))
return
}
if errors.Is(err, ErrLastPasskey) {
writeError(w, r, newError(http.StatusConflict, "last_passkey",
"this is your only passkey and your email is not verified; add another passkey or verify an email first"))
return
}
writeError(w, r, err)
return
}
+61
View File
@@ -46,6 +46,8 @@ func plantPasskeyChallenge(repo *fakeRepo, id string, expiresAt time.Time, sessi
func TestPasskeyRegisterVertical(t *testing.T) {
user := &Principal{UserID: "u1", Email: "[email protected]", Role: "user"}
repo := newFakeRepo()
// A verified email keeps a door open, so step 5 may remove the only passkey.
repo.staff["u1"] = &StaffUser{ID: "u1", Username: "u1", Email: "[email protected]", Role: "user", EmailVerified: true}
v := &fakePasskeyVerifier{
options: json.RawMessage(`{"publicKey":{"challenge":"Y2hhbGxlbmdl"}}`),
credential: VerifiedCredential{
@@ -294,6 +296,65 @@ func TestPasskeyDeleteScoping(t *testing.T) {
}
}
// TestPasskeyDeleteLastGuard pins the last-passkey guard: without a verified email
// the only passkey is the account's way in, so its delete is a 409 that leaves it
// bound; a second passkey or a verified email lets the delete through.
func TestPasskeyDeleteLastGuard(t *testing.T) {
cred := func(id string) PasskeyCredential {
return PasskeyCredential{ID: id, UserID: "u1", CredentialID: "c-" + id, CreatedAt: frozenNow}
}
for _, tc := range []struct {
name string
verified bool
creds []string
want int
}{
{"only passkey, email unverified", false, []string{"a"}, http.StatusConflict},
{"only passkey, email verified", true, []string{"a"}, http.StatusNoContent},
{"two passkeys, email unverified", false, []string{"a", "b"}, http.StatusNoContent},
} {
t.Run(tc.name, func(t *testing.T) {
for _, role := range []string{"user", "admin"} {
user := &Principal{UserID: "u1", Email: "[email protected]", Role: role}
repo := newFakeRepo()
repo.staff["u1"] = &StaffUser{ID: "u1", Username: "u1", Email: "[email protected]", Role: role, EmailVerified: tc.verified}
for _, id := range tc.creds {
repo.passkeyCreds[id] = cred(id)
}
eh := newPasskeyAPI(repo, &fakePasskeyVerifier{}, user)
w := do(eh, "DELETE", "/api/v1/account/passkey/credentials/a", "", nil)
if w.Code != tc.want {
t.Fatalf("%s: code = %d body %s, want %d", role, w.Code, w.Body.String(), tc.want)
}
_, kept := repo.passkeyCreds["a"]
if tc.want == http.StatusConflict {
if got := decodeErr(t, w); got != "last_passkey" {
t.Errorf("%s: error code = %q, want last_passkey", role, got)
}
if !kept {
t.Errorf("%s: a refused delete must leave the passkey bound", role)
}
} else if kept {
t.Errorf("%s: an allowed delete must remove the passkey", role)
}
}
})
}
// Removing one of two leaves the other as the last one, which is then guarded.
user := &Principal{UserID: "u1", Email: "[email protected]", Role: "user"}
repo := newFakeRepo()
repo.staff["u1"] = &StaffUser{ID: "u1", Username: "u1", Email: "[email protected]", Role: "user"}
repo.passkeyCreds["a"], repo.passkeyCreds["b"] = cred("a"), cred("b")
eh := newPasskeyAPI(repo, &fakePasskeyVerifier{}, user)
if w := do(eh, "DELETE", "/api/v1/account/passkey/credentials/a", "", nil); w.Code != http.StatusNoContent {
t.Fatalf("first delete: code = %d, want 204", w.Code)
}
if w := do(eh, "DELETE", "/api/v1/account/passkey/credentials/b", "", nil); w.Code != http.StatusConflict {
t.Fatalf("second delete: code = %d, want 409 (it is now the last one)", w.Code)
}
}
// TestPasskeyDeleteUnknown pins the unknown-id path: deleting an id that does not exist
// is a 404, never a silent 204.
func TestPasskeyDeleteUnknown(t *testing.T) {
+25 -6
View File
@@ -1412,19 +1412,38 @@ func (p *PGRepo) AdvanceCredentialSignCount(ctx context.Context, credentialID st
// only unbind their OWN credential. No matching (user, id) row → ErrNotFound via a zero
// RowsAffected, so a stale or cross-user id cannot silently no-op as success.
func (p *PGRepo) DeletePasskeyCredential(ctx context.Context, userID, id string) error {
res, err := p.db.ExecContext(ctx,
`DELETE FROM webauthn_credentials WHERE id = $1 AND user_id = $2`, id, userID)
tx, err := p.db.BeginTx(ctx, nil)
if err != nil {
return err
}
n, err := res.RowsAffected()
if err != nil {
defer func() { _ = tx.Rollback() }()
// Lock the user row first: every delete for this user queues here, so the count
// below cannot go stale between the check and the DELETE.
var verified bool
switch err := tx.QueryRowContext(ctx,
`SELECT email_verified FROM users WHERE id = $1 FOR UPDATE`, userID).Scan(&verified); {
case errors.Is(err, sql.ErrNoRows):
return ErrNotFound
case err != nil:
return err
}
if n == 0 {
var mine, total int
if err := tx.QueryRowContext(ctx,
`SELECT count(*) FILTER (WHERE id = $2), count(*) FROM webauthn_credentials WHERE user_id = $1`,
userID, id).Scan(&mine, &total); err != nil {
return err
}
if mine == 0 {
return ErrNotFound
}
return nil
if total == 1 && !verified {
return ErrLastPasskey
}
if _, err := tx.ExecContext(ctx,
`DELETE FROM webauthn_credentials WHERE id = $1 AND user_id = $2`, id, userID); err != nil {
return err
}
return tx.Commit()
}
// DeleteAllPasskeyCredentialsForUser unbinds every passkey a user holds. Unlike the
+4 -1
View File
@@ -459,7 +459,10 @@ type Repo interface {
PasskeyCredentialsForUser(ctx context.Context, userID string) ([]PasskeyCredential, error)
// DeletePasskeyCredential removes the passkey row id, scoped to userID so a caller
// 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.
// so a stale or cross-user id cannot silently no-op as success. When the row is
// the user's last passkey and their email is unverified it returns ErrLastPasskey
// and deletes nothing; the check and the delete hold the user row locked, so two
// concurrent deletes of a user's last two passkeys cannot both pass.
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
+84
View File
@@ -722,6 +722,90 @@ func TestDeadAccountsAreLockedOutInPG(t *testing.T) {
}
}
// DeletePasskeyCredential keeps the last passkey of an account whose email is
// unverified: removing it would leave no durable way in. The guard reads the count
// under the user-row lock, so two concurrent deletes of an account's last two
// passkeys resolve to exactly one delete and one ErrLastPasskey, never zero left.
func TestLastPasskeyGuardInPG(t *testing.T) {
ctx := context.Background()
seed := func(t *testing.T, userID string) string {
t.Helper()
id := "cred-" + suffix(t)
if _, err := db.ExecContext(ctx,
`INSERT INTO webauthn_credentials (id, user_id, credential_id, public_key) VALUES ($1,$2,$3,'pk')`,
id, userID, "cid-"+suffix(t)); err != nil {
t.Fatalf("seed passkey: %v", err)
}
return id
}
count := func(t *testing.T, userID string) int {
t.Helper()
var n int
if err := db.QueryRowContext(ctx,
`SELECT count(*) FROM webauthn_credentials WHERE user_id = $1`, userID).Scan(&n); err != nil {
t.Fatalf("count: %v", err)
}
return n
}
for i := 0; i < 5; i++ {
u := newUser(t, "user", "lastpk")
a, b := seed(t, u.ID), seed(t, u.ID)
var wg sync.WaitGroup
errs := make([]error, 2)
for j, id := range []string{a, b} {
wg.Add(1)
go func(j int, id string) {
defer wg.Done()
errs[j] = repo.DeletePasskeyCredential(ctx, u.ID, id)
}(j, id)
}
wg.Wait()
ok, refused := 0, 0
for _, err := range errs {
switch {
case err == nil:
ok++
case errors.Is(err, api.ErrLastPasskey):
refused++
default:
t.Fatalf("concurrent delete: %v", err)
}
}
if ok != 1 || refused != 1 || count(t, u.ID) != 1 {
t.Fatalf("round %d: %d deleted, %d refused, %d left; want 1, 1, 1", i, ok, refused, count(t, u.ID))
}
}
u := newUser(t, "admin", "lastpk")
last := seed(t, u.ID)
if err := repo.DeletePasskeyCredential(ctx, u.ID, last); !errors.Is(err, api.ErrLastPasskey) {
t.Fatalf("last passkey, unverified: %v, want ErrLastPasskey", err)
}
if err := repo.DeletePasskeyCredential(ctx, u.ID, "cred-none-"+suffix(t)); !errors.Is(err, api.ErrNotFound) {
t.Fatalf("unknown id: %v, want ErrNotFound", err)
}
other := newUser(t, "user", "lastpk")
if err := repo.DeletePasskeyCredential(ctx, other.ID, last); !errors.Is(err, api.ErrNotFound) {
t.Fatalf("another user's passkey: %v, want ErrNotFound", err)
}
// A verified email is another door, so the last passkey may go.
now := mustNow()
if err := repo.CreateEmailOTP(ctx, "lp-"+suffix(t), u.ID, "lastpk-"+suffix(t)+"@example.net", "h", "onboard_email", now.Add(5*time.Minute)); err != nil {
t.Fatalf("CreateEmailOTP: %v", err)
}
if _, err := repo.VerifyEmailOTP(ctx, u.ID, "onboard_email", "h", now); err != nil {
t.Fatalf("VerifyEmailOTP: %v", err)
}
if err := repo.DeletePasskeyCredential(ctx, u.ID, last); err != nil {
t.Fatalf("last passkey, verified: %v", err)
}
if n := count(t, u.ID); n != 0 {
t.Fatalf("%d passkeys left after the allowed delete, want 0", n)
}
}
// VerifyLinkCode's takeover rule: a fresh in-game code (proof the caller holds the
// UUID) lets a live account take over a link whose account was SOFT-DELETED — the
// migrated-source case, whose retire keeps the link but kills the account — while a