From 15f729ffea2cec818f591e4e1d523f94fce38228 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Thu, 24 Sep 2026 15:37:01 +0800 Subject: [PATCH] =?UTF-8?q?fix(auth):=20=E9=82=AE=E7=AE=B1=E9=AA=8C?= =?UTF-8?q?=E8=AF=81=E7=A0=81=E6=8C=89=E8=B4=A6=E5=8F=B7=E7=B4=AF=E8=AE=A1?= =?UTF-8?q?=E9=94=99=E8=AF=AF=E6=AC=A1=E6=95=B0=E5=B0=81=E9=A1=B6=E5=B9=B6?= =?UTF-8?q?=E9=80=9A=E7=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/openapi.yaml | 47 +++-- internal/api/api_test.go | 46 ++++- internal/api/errors.go | 20 ++ internal/api/handlers_account_migrate.go | 12 ++ internal/api/handlers_auth_email.go | 17 +- internal/api/handlers_email_otp.go | 22 +++ internal/api/handlers_op_login.go | 12 +- internal/api/otp_lock.go | 105 ++++++++++ internal/api/otp_lock_test.go | 183 ++++++++++++++++++ internal/api/pgrepo.go | 101 ++++++++-- internal/api/repo.go | 15 +- internal/metrics/metrics.go | 10 + internal/pgint/pgint_test.go | 93 +++++++++ .../migrations/0022_otp_failure_budget.sql | 14 ++ panel/src/i18n/resources/en-US/errors.json | 1 + panel/src/i18n/resources/zh-CN/errors.json | 1 + panel/src/lib/api.ts | 2 + 17 files changed, 664 insertions(+), 37 deletions(-) create mode 100644 internal/api/otp_lock.go create mode 100644 internal/api/otp_lock_test.go create mode 100644 internal/store/migrations/0022_otp_failure_budget.sql diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 371b856..f53d27d 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2209,7 +2209,9 @@ paths: purpose. An address with no account returns the SAME 202 with no code minted, and the per-recipient cooldown is kept on that path too, so probing reveals nothing (existence is learnt only at the sanctioned /auth/options oracle). - Gated on local_auth_enabled. + An account that spent its daily wrong-code budget (10 per 24h, across every + code) also gets the same 202 and no mail until the window ends. Gated on + local_auth_enabled. x-felis-face: [external] x-felis-tier: public security: [] @@ -2268,7 +2270,10 @@ paths: under the login purpose, and on success mints a host-only felis_session. An unknown address, a wrong or expired code, and an attempt-exhausted code all return the IDENTICAL 400 invalid_code, so the door is not an existence or - lockout oracle. Staff are refused (403) — but only AFTER a valid code is + lockout oracle. The 10th wrong code in 24h locks the door for that account + until the window ends (the right code then also reads as invalid_code); the + owner is told by mail once, and the lock is audited as auth.otp.locked. + Staff are refused (403) — but only AFTER a valid code is redeemed, so only the account owner can ever reach that refusal. x-felis-face: [external] x-felis-tier: public @@ -2324,7 +2329,8 @@ paths: staff address, opens an op_login request, and mails a one-time code under the op_login purpose, returning the request handle the browser polls. A non-staff or unknown address gets the SAME 202 with a random, non-persisted handle and no - mail, so this never becomes a staff-enumeration oracle. Gated on + mail, so this never becomes a staff-enumeration oracle. A staff account that + spent its daily wrong-code budget gets the same neutral 202. Gated on local_auth_enabled. x-felis-face: [external] x-felis-tier: public @@ -2415,7 +2421,7 @@ paths: Public, pre-session final leg: mints a host-only staff session only when BOTH factors have landed — the request is approved-and-live AND the mailed code verifies. Every failure (unknown handle, not-yet-approved, wrong or locked code, - lost race) collapses into one uniform 400 op_login_invalid, so a code-less + an account past its daily wrong-code budget, lost race) collapses into one uniform 400 op_login_invalid, so a code-less caller learns nothing. Admin is re-asserted before the session is issued. x-felis-face: [external] x-felis-tier: public @@ -3768,6 +3774,14 @@ paths: schema: { $ref: '#/components/schemas/Error' } '401': $ref: '#/components/responses/Unauthorized' + '429': + description: >- + Resend requested before the cooldown elapsed (otp_resend_cooldown); or the + account spent its daily wrong-code budget (otp_account_locked, with + Retry-After). + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } '502': $ref: '#/components/responses/MailUndeliverable' @@ -3779,8 +3793,10 @@ paths: description: > Consumes a previously delivered code for the authenticated principal. On success the user's email is written and email_verified is set true. Too many - incorrect attempts lock the code (429); an unknown, expired, consumed, or - mismatched code is a 400. + incorrect attempts lock the code (429 otp_locked); 10 wrong codes in 24h, + counted across every code, lock the account's email-code door until the + window ends (429 otp_account_locked with Retry-After). An unknown, expired, + consumed, or mismatched code is a 400. x-felis-face: [external] x-felis-tier: app security: [{ accessJWT: [] }] @@ -3812,7 +3828,9 @@ paths: '401': $ref: '#/components/responses/Unauthorized' '429': - description: Too many incorrect attempts; the code is locked. + description: >- + Too many incorrect attempts on this code (otp_locked), or the account's + daily wrong-code budget is spent (otp_account_locked, with Retry-After). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -4070,7 +4088,10 @@ paths: application/json: schema: { $ref: '#/components/schemas/Error' } '429': - description: Resend requested before the cooldown elapsed. + description: >- + Resend requested before the cooldown elapsed (otp_resend_cooldown), or the + account's daily wrong-code budget is spent (otp_account_locked, with + Retry-After). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -4085,8 +4106,9 @@ paths: description: > Consumes the fresh migrate-purpose email code for the caller's initiated migration and advances it to confirmed with confirm_factor email_otp. Too many - wrong attempts lock the code (429 otp_locked); an unknown, expired, consumed, or - mismatched code is a 400 invalid_code. + wrong attempts lock the code (429 otp_locked), and 10 wrong codes in 24h lock + the account's email-code door (429 otp_account_locked with Retry-After); an + unknown, expired, consumed, or mismatched code is a 400 invalid_code. x-felis-face: [external] x-felis-tier: app security: [{ accessJWT: [] }] @@ -4127,7 +4149,10 @@ paths: application/json: schema: { $ref: '#/components/schemas/Error' } '429': - description: The code is locked after too many wrong attempts (otp_locked). + description: >- + The code is locked after too many wrong attempts (otp_locked), or the + account's daily wrong-code budget is spent (otp_account_locked, with + Retry-After). content: application/json: schema: { $ref: '#/components/schemas/Error' } diff --git a/internal/api/api_test.go b/internal/api/api_test.go index f73c166..c0531ff 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -74,6 +74,8 @@ type fakeRepo struct { // player email OTPs (spec §B2). Keyed by row id; the verify path scans for the // newest live (user, purpose) just as the PG query does. otps map[string]*fakeEmailOTP + // otpBudget mirrors otp_failure_windows, keyed user|purpose. + otpBudget map[string]*fakeOTPBudget // op-login requests (spec §B op-login). opLogins mirrors op_login_requests keyed // by id; the in-game approve/finish paths mutate status/consumed in place, and // tests plant rows directly to drive the status/finish/pending-list paths. @@ -151,6 +153,37 @@ type fakeDataHold struct { expiresAt time.Time } +// fakeOTPBudget mirrors an otp_failure_windows row. +type fakeOTPBudget struct { + windowStart time.Time + failures int +} + +// OTPLockedUntil mirrors PGRepo.OTPLockedUntil through the shared otpLockEnd rule. +func (f *fakeRepo) OTPLockedUntil(_ context.Context, userID, purpose string, now time.Time) (time.Time, error) { + b := f.otpBudget[userID+"|"+purpose] + if b == nil { + return time.Time{}, nil + } + return otpLockEnd(b.windowStart, b.failures, now), nil +} + +// chargeOTP mirrors chargeOTPMismatch: one wrong guess on the code and the budget. +func (f *fakeRepo) chargeOTP(live *fakeEmailOTP, now time.Time) error { + live.attempts++ + key := live.userID + "|" + live.purpose + b := f.otpBudget[key] + if b == nil || !b.windowStart.Add(otpFailureWindow).After(now) { + b = &fakeOTPBudget{windowStart: now} + f.otpBudget[key] = b + } + b.failures++ + if b.failures == otpFailureBudget { + return &OTPAccountLockedError{Until: b.windowStart.Add(otpFailureWindow), JustLocked: true} + } + return ErrOTPInvalid +} + // fakeEmailOTP mirrors an email_otps row: only the code hash is held (never the // digits), attempts caps brute force, consumed marks single-use, and createdAt // orders the newest-live lookup. @@ -227,6 +260,7 @@ func newFakeRepo() *fakeRepo { sessions: map[string]*fakeSession{}, settings: map[string][]byte{}, otps: map[string]*fakeEmailOTP{}, + otpBudget: map[string]*fakeOTPBudget{}, opLogins: map[string]*fakeOpLogin{}, setupTokens: map[string]fakeSetupToken{}, blacklist: map[string]bool{}, @@ -373,12 +407,14 @@ func (f *fakeRepo) VerifyEmailOTP(_ context.Context, userID, purpose, codeHash s if !live.expiresAt.After(now) { return "", ErrOTPInvalid } + if until, _ := f.OTPLockedUntil(context.Background(), userID, purpose, now); !until.IsZero() { + return "", &OTPAccountLockedError{Until: until} + } if live.attempts >= otpMaxAttempts { return "", ErrOTPLocked } if live.codeHash != codeHash { - live.attempts++ // a typo costs an attempt but does not consume the code - return "", ErrOTPInvalid + return "", f.chargeOTP(live, now) // a typo costs an attempt but does not consume the code } // A DIFFERENT verified holder of the same address → ErrEmailTaken, code left // live — mirrors PGRepo's guard + the users_verified_email_unique index. @@ -1319,12 +1355,14 @@ func (f *fakeRepo) ConsumeLoginEmailOTP(_ context.Context, userID, purpose, code if live == nil || !live.expiresAt.After(now) { return ErrOTPInvalid } + if until, _ := f.OTPLockedUntil(context.Background(), userID, purpose, now); !until.IsZero() { + return &OTPAccountLockedError{Until: until} + } if live.attempts >= otpMaxAttempts { return ErrOTPLocked } if live.codeHash != codeHash { - live.attempts++ // a typo costs an attempt but does not consume the code - return ErrOTPInvalid + return f.chargeOTP(live, now) // a typo costs an attempt but does not consume the code } live.consumed = true return nil diff --git a/internal/api/errors.go b/internal/api/errors.go index 5fa34a9..9dea2ed 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "net/http" + "time" ) // Sentinel errors the repository and cluster layers return so handlers can map @@ -44,6 +45,11 @@ var ( // can answer 429 (back off / request a new code) rather than inviting another // guess against a code that will never accept one. ErrOTPLocked = errors.New("email code locked: too many attempts") + // ErrOTPAccountLocked means the (user, purpose) has spent its wrong-code budget + // for the current window (otpFailureBudget): every code for that door is refused, + // the right one included, until the window ends. The repo returns it as an + // *OTPAccountLockedError carrying the end of the lock. + ErrOTPAccountLocked = errors.New("email codes locked for this account: too many wrong codes") // ErrPasskeyChallengeInvalid means a passkey enrollment ceremony cannot be // finished: there is no live (unconsumed, unexpired) challenge for the caller and // purpose (Phase 6 WebAuthn bind). Like ErrOTPInvalid it is a client error — the @@ -102,6 +108,20 @@ func (e *MaintenanceBusyError) Error() string { func (e *MaintenanceBusyError) Is(target error) bool { return target == ErrMaintenanceInProgress } +// OTPAccountLockedError is ErrOTPAccountLocked with its detail. JustLocked is set +// only on the wrong guess that spent the budget, so the handler notifies and +// audits the lock exactly once. +type OTPAccountLockedError struct { + Until time.Time + JustLocked bool +} + +func (e *OTPAccountLockedError) Error() string { + return ErrOTPAccountLocked.Error() + " until " + e.Until.UTC().Format(time.RFC3339) +} + +func (e *OTPAccountLockedError) Is(target error) bool { return target == ErrOTPAccountLocked } + // apiError is a handler-level error carrying an HTTP status and a stable, // machine-readable code. The error envelope matches the platform convention: // diff --git a/internal/api/handlers_account_migrate.go b/internal/api/handlers_account_migrate.go index 4dd3a1e..2e490cd 100644 --- a/internal/api/handlers_account_migrate.go +++ b/internal/api/handlers_account_migrate.go @@ -206,6 +206,13 @@ func (a *API) handleMigrateConfirmOTPStart(w http.ResponseWriter, r *http.Reques } // Per-recipient cooldown, namespaced apart from the other OTP doors so they never // perturb each other's throttle. + if until, err := a.Repo.OTPLockedUntil(r.Context(), p.UserID, otpPurposeMigrate, a.now()); err != nil { + writeError(w, r, err) + return + } else if !until.IsZero() { + writeOTPAccountLocked(w, r, until, a.now()) + return + } emailKey := "migrate:confirm:" + strings.ToLower(p.Email) lim := a.otpLimiter() emailAt, ok := lim.reserve(emailKey, otpResendCooldown) @@ -267,7 +274,12 @@ func (a *API) handleMigrateConfirmOTPVerify(w http.ResponseWriter, r *http.Reque if _, ok := a.requireInitiatedMigration(w, r, p.UserID); !ok { return } + var lock *OTPAccountLockedError switch err := a.Repo.ConsumeLoginEmailOTP(r.Context(), p.UserID, otpPurposeMigrate, otpCodeHash(code), a.now()); { + case errors.As(err, &lock): + a.noteOTPLock(r, err, p.UserID, otpPurposeMigrate) + writeOTPAccountLocked(w, r, lock.Until, a.now()) + return case errors.Is(err, ErrOTPLocked): writeError(w, r, newError(http.StatusTooManyRequests, "otp_locked", "too many incorrect attempts; request a new code")) diff --git a/internal/api/handlers_auth_email.go b/internal/api/handlers_auth_email.go index ae6339a..6e75964 100644 --- a/internal/api/handlers_auth_email.go +++ b/internal/api/handlers_auth_email.go @@ -126,6 +126,19 @@ func (a *API) handleLoginEmailStart(w http.ResponseWriter, r *http.Request) { return } + // A locked door (wrong-code budget spent) gets the same neutral 202 and no + // mail: the owner was told by the lock notice, and a distinct answer here + // would tell a prober the address has an account. + switch until, err := a.Repo.OTPLockedUntil(r.Context(), u.ID, otpPurposeLogin, a.now()); { + case err != nil: + writeError(w, r, err) + return + case !until.IsZero(): + committed = true + writeJSON(w, http.StatusAccepted, map[string]any{"sent": true, "expires_at": expiresAt.UTC()}) + return + } + code, err := newEmailOTP() if err != nil { writeError(w, r, err) @@ -220,7 +233,9 @@ func (a *API) handleLoginEmailVerify(w http.ResponseWriter, r *http.Request) { // verified; touching the row here would let a stale OTP-snapshot address overwrite // the live one and could 500 a correct code on a spurious collision. switch err := a.Repo.ConsumeLoginEmailOTP(r.Context(), u.ID, otpPurposeLogin, otpCodeHash(code), a.now()); { - case errors.Is(err, ErrOTPInvalid), errors.Is(err, ErrOTPLocked): + case errors.Is(err, ErrOTPInvalid), errors.Is(err, ErrOTPLocked), errors.Is(err, ErrOTPAccountLocked): + // The account lock answers the same way; its owner hears about it by mail. + a.noteOTPLock(r, err, u.ID, otpPurposeLogin) // Both a wrong/expired code and an attempt-exhausted one return the SAME 400 // invalid_code, byte-identical to the unknown-account branch above. Surfacing // otp_locked as a distinct 429 (as the authenticated onboarding door does) would diff --git a/internal/api/handlers_email_otp.go b/internal/api/handlers_email_otp.go index dfc2fbd..a9fc1b6 100644 --- a/internal/api/handlers_email_otp.go +++ b/internal/api/handlers_email_otp.go @@ -47,6 +47,14 @@ const ( // keys (principal and recipient) so neither one account fanning out across many // addresses, nor many accounts converging on one address, can flood a mailbox. otpResendCooldown = 60 * time.Second + // otpFailureBudget caps wrong codes per (user, purpose) inside otpFailureWindow, + // across every code minted in it. The per-code cap alone resets on each resend, + // and the public login door can mint a code a minute, so without this an + // attacker gets ~7,200 guesses a day at one account. Ten a day puts a blind hit + // at 1e-5 per day; a person who mistypes that often can wait out the window or + // use a passkey. + otpFailureBudget = 10 + otpFailureWindow = 24 * time.Hour ) // OTPMailer delivers a one-time code to an email address. It is a seam, not a @@ -129,6 +137,15 @@ func (a *API) handleEmailOTPStart(w http.ResponseWriter, r *http.Request) { // can't sidestep the per-mailbox cap. If any later step fails the deferred rollback // frees both windows, so a failed mint or delivery never consumes the cooldown — // the same property the old record-after-send gave, now race-free. + // A spent wrong-code budget locks the door; mailing another code into it + // would only spend a send. + if until, err := a.Repo.OTPLockedUntil(r.Context(), p.UserID, otpPurposeOnboard, a.now()); err != nil { + writeError(w, r, err) + return + } else if !until.IsZero() { + writeOTPAccountLocked(w, r, until, a.now()) + return + } userKey, emailKey := "user:"+p.UserID, "email:"+strings.ToLower(email) lim := a.otpLimiter() userAt, ok := lim.reserve(userKey, otpResendCooldown) @@ -205,7 +222,12 @@ func (a *API) handleEmailOTPVerify(w http.ResponseWriter, r *http.Request) { return } email, err := a.Repo.VerifyEmailOTP(r.Context(), p.UserID, otpPurposeOnboard, otpCodeHash(code), a.now()) + var lock *OTPAccountLockedError switch { + case errors.As(err, &lock): + a.noteOTPLock(r, err, p.UserID, otpPurposeOnboard) + writeOTPAccountLocked(w, r, lock.Until, a.now()) + return case errors.Is(err, ErrOTPLocked): writeError(w, r, newError(http.StatusTooManyRequests, "otp_locked", "too many incorrect attempts; request a new code")) diff --git a/internal/api/handlers_op_login.go b/internal/api/handlers_op_login.go index dc872e0..2e840ba 100644 --- a/internal/api/handlers_op_login.go +++ b/internal/api/handlers_op_login.go @@ -144,6 +144,15 @@ func (a *API) handleOpLoginStart(w http.ResponseWriter, r *http.Request) { neutral() return } + // A locked door (wrong-code budget spent) is neutral too: no request, no mail. + switch until, err := a.Repo.OTPLockedUntil(r.Context(), u.ID, otpPurposeOpLogin, a.now()); { + case err != nil: + writeError(w, r, err) + return + case !until.IsZero(): + neutral() + return + } id, err := newOTPID() if err != nil { @@ -272,7 +281,8 @@ func (a *API) handleOpLoginFinish(w http.ResponseWriter, r *http.Request) { // attempt without minting anything and returns the uniform failure — the code, not // the request, is the problem, and the request stays approved for a retry. switch err := a.Repo.ConsumeLoginEmailOTP(r.Context(), loginReq.UserID, otpPurposeOpLogin, otpCodeHash(code), now); { - case errors.Is(err, ErrOTPInvalid), errors.Is(err, ErrOTPLocked): + case errors.Is(err, ErrOTPInvalid), errors.Is(err, ErrOTPLocked), errors.Is(err, ErrOTPAccountLocked): + a.noteOTPLock(r, err, loginReq.UserID, otpPurposeOpLogin) writeError(w, r, invalid) return case err != nil: diff --git a/internal/api/otp_lock.go b/internal/api/otp_lock.go new file mode 100644 index 0000000..d09e8e0 --- /dev/null +++ b/internal/api/otp_lock.go @@ -0,0 +1,105 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log" + "net/http" + "strconv" + "time" + + "felis.lolicon.best/internal/metrics" +) + +// The account-level wrong-code budget (otpFailureBudget per otpFailureWindow) +// is enforced in the repo; this file is what the doors do with it. The two +// pre-session doors (email login, op-login) answer a locked account exactly +// like a wrong code, so they never become an existence oracle; the owner +// learns about the lock from a notice mail instead. The signed-in doors +// (onboarding, migration step-up) answer 429 otp_account_locked with the time +// the lock ends. + +// noticeSender is the optional half of Mailer that sends a free-form notice; +// internal/mail.SMTP implements it (the reaper uses the same method). +type noticeSender interface { + SendNotice(ctx context.Context, email, subject, body string) error +} + +// otpDoorName names each purpose in the lock notice, in both languages. +var otpDoorName = map[string][2]string{ + otpPurposeLogin: {"邮箱验证码登录", "email-code sign-in"}, + otpPurposeOpLogin: {"管理员登录", "staff sign-in"}, +} + +// writeOTPAccountLocked answers a signed-in door whose budget is spent. +func writeOTPAccountLocked(w http.ResponseWriter, r *http.Request, until, now time.Time) { + secs := int64(until.Sub(now).Round(time.Second) / time.Second) + if secs < 1 { + secs = 1 + } + w.Header().Set("Retry-After", strconv.FormatInt(secs, 10)) + writeError(w, r, newError(http.StatusTooManyRequests, "otp_account_locked", + "too many wrong codes on this account; email codes work again after %s", + until.UTC().Format(time.RFC3339))) +} + +// noteOTPLock handles a redeem that met the account lock. Only the guess that +// spent the budget (JustLocked) does anything: count, audit, log, and for the +// pre-session doors mail the account so its owner learns why the right code +// stopped working. Everything here is best effort; the lock already holds. +func (a *API) noteOTPLock(r *http.Request, err error, userID, purpose string) { + var lock *OTPAccountLockedError + if !errors.As(err, &lock) || !lock.JustLocked { + return + } + metrics.OTPLockoutsTotal.WithLabelValues(purpose).Inc() + log.Printf("auth: %s codes for user %s locked until %s after %d wrong codes (request_id=%s)", + purpose, userID, lock.Until.UTC().Format(time.RFC3339), otpFailureBudget, requestIDFromContext(r.Context())) + + ctx := r.Context() + actor := userID + u, uerr := a.Repo.UserByID(ctx, userID) + if uerr == nil && u.Username != "" { + actor = u.Username + } + payload, _ := json.Marshal(map[string]any{ + "user_id": userID, "purpose": purpose, "until": lock.Until.UTC(), "failures": otpFailureBudget, + }) + if aerr := a.Repo.Audit(ctx, AuditEntry{ + Actor: actor, Source: "external", Action: "auth.otp.locked", + RequestID: requestIDFromContext(ctx), Payload: payload, + }); aerr != nil { + log.Printf("auth: audit of otp lock for user %s failed: %v", userID, aerr) + } + + door, notify := otpDoorName[purpose] + if !notify || uerr != nil || u.Email == "" { + return + } + sender, ok := a.Mailer.(noticeSender) + if !ok { + log.Printf("auth: no notice mailer; user %s was not told their %s is locked", userID, purpose) + return + } + subject, body := otpLockNotice(door, lock.Until) + if err := sender.SendNotice(ctx, u.Email, subject, body); err != nil { + log.Printf("auth: otp lock notice to user %s failed: %v", userID, err) + } +} + +// otpLockNotice renders the bilingual lock notice. +func otpLockNotice(door [2]string, until time.Time) (subject, body string) { + at := until.UTC().Format("2006-01-02 15:04 MST") + subject = "Felis " + door[0] + "已暂停 · " + door[1] + " paused" + body = fmt.Sprintf(`Felis 在 24 小时内收到了 %[1]d 次错误的邮箱验证码,已暂停这个账户的%[2]s,%[3]s 自动恢复。 +如果不是你本人在尝试,说明有人在猜你的验证码。账户仍然安全:暂停期间任何验证码都无法登录。 +你仍然可以用已绑定的 Passkey 登录。 + +Felis received %[1]d wrong email codes for this account within 24 hours and paused %[4]s until %[3]s. +If this wasn't you, someone is guessing your code. Your account is safe: no code works while it is paused. +You can still sign in with a passkey you have registered. +`, otpFailureBudget, door[0], at, door[1]) + return subject, body +} diff --git a/internal/api/otp_lock_test.go b/internal/api/otp_lock_test.go new file mode 100644 index 0000000..f01944d --- /dev/null +++ b/internal/api/otp_lock_test.go @@ -0,0 +1,183 @@ +package api + +import ( + "context" + "net/http" + "strings" + "testing" + "time" +) + +// The per-code attempt cap resets on every resend; these pin the account-level +// budget that does not. The public login door must stay uniform (a locked +// account reads like a wrong code and mails nothing), tell the owner by mail +// once, and reopen when the window ends. The signed-in onboarding door answers +// 429 with Retry-After instead. + +type noticeMailer struct { + captureMailer + notices []string // "to|subject|body" +} + +func (m *noticeMailer) SendNotice(_ context.Context, email, subject, body string) error { + m.notices = append(m.notices, email+"|"+subject+"|"+body) + return nil +} + +func TestLoginDoorLocksAfterDailyWrongCodeBudget(t *testing.T) { + api, repo, _ := seedLoginEmailAPI(t) + mailer := ¬iceMailer{} + api.Mailer = mailer + clock := time.Unix(1_700_000_000, 0) + api.Now = func() time.Time { return clock } + eh := api.ExternalHandler() + + start := func() { + t.Helper() + clock = clock.Add(otpResendCooldown + time.Second) + if w := do(eh, "POST", "/api/v1/auth/email/start", `{"email":"player@example.net"}`, jsonHeader); w.Code != http.StatusAccepted { + t.Fatalf("start = %d (%s)", w.Code, w.Body.String()) + } + } + verify := func(code string) int { + t.Helper() + w := do(eh, "POST", "/api/v1/auth/email/verify", `{"email":"player@example.net","code":"`+code+`"}`, jsonHeader) + if w.Code != http.StatusOK { + if c, _ := errEnvelope(t, w); c != "invalid_code" { + t.Fatalf("verify answered %d %s; the login door must only ever say invalid_code", w.Code, c) + } + } + return w.Code + } + + // Ten wrong codes over three sends; no single code reaches its own cap. + for i := 0; i < otpFailureBudget; i++ { + if i%4 == 0 { + start() + } + if code := verify("not-the-code"); code != http.StatusBadRequest { + t.Fatalf("wrong guess %d = %d", i+1, code) + } + } + sends := mailer.calls + liveCode := mailer.code + + if len(mailer.notices) != 1 || !strings.HasPrefix(mailer.notices[0], "Player@Example.NET|") { + t.Fatalf("lock notices = %q, want one to the stored address", mailer.notices) + } + var lockAudits int + for _, a := range repo.audits { + if a.Action == "auth.otp.locked" { + lockAudits++ + if a.Actor != "player" || !strings.Contains(string(a.Payload), `"purpose":"login_email"`) { + t.Fatalf("lock audit = %+v", a) + } + } + } + if lockAudits != 1 { + t.Fatalf("lock audits = %d, want 1", lockAudits) + } + + // Locked: the right code fails like a wrong one, and a resend mails nothing. + if code := verify(liveCode); code != http.StatusBadRequest { + t.Fatalf("right code while locked = %d, want 400", code) + } + start() + if mailer.calls != sends { + t.Fatalf("a locked door mailed a code (%d sends, want %d)", mailer.calls, sends) + } + if len(mailer.notices) != 1 { + t.Fatalf("a standing lock re-sent the notice: %d", len(mailer.notices)) + } + + // The window ends: a fresh send and its code work again. + clock = clock.Add(otpFailureWindow) + start() + if mailer.calls != sends+1 { + t.Fatalf("no code mailed after the window (%d sends)", mailer.calls) + } + if code := verify(mailer.code); code != http.StatusOK { + t.Fatalf("right code after the window = %d, want 200", code) + } +} + +func TestOnboardDoorLockAnswers429(t *testing.T) { + repo := newFakeRepo() + repo.staff["player"] = &StaffUser{ID: "u1", Username: "player", Email: "old@example.net"} + mailer := ¬iceMailer{} + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"}} + api.Mailer = mailer + clock := time.Unix(1_700_000_000, 0) + api.Now = func() time.Time { return clock } + eh := api.ExternalHandler() + + var last int + for i := 0; i < otpFailureBudget; i++ { + if i%4 == 0 { + clock = clock.Add(otpResendCooldown + time.Second) + if w := do(eh, "POST", "/api/v1/account/email/start", `{"email":"player@example.net"}`, nil); w.Code != http.StatusAccepted { + t.Fatalf("start = %d (%s)", w.Code, w.Body.String()) + } + } + w := do(eh, "POST", "/api/v1/account/email/verify", `{"code":"not-the-code"}`, nil) + last = w.Code + if i == otpFailureBudget-1 { + if c, _ := errEnvelope(t, w); w.Code != http.StatusTooManyRequests || c != "otp_account_locked" { + t.Fatalf("10th wrong code = %d %s, want 429 otp_account_locked", w.Code, c) + } + if w.Header().Get("Retry-After") == "" { + t.Fatal("locked answer carries no Retry-After") + } + } + } + if last != http.StatusTooManyRequests { + t.Fatalf("last = %d", last) + } + // The signed-in owner sees the 429; no notice mail is needed. + if len(mailer.notices) != 0 { + t.Fatalf("onboarding lock mailed a notice: %q", mailer.notices) + } + clock = clock.Add(otpResendCooldown + time.Second) + sends := mailer.calls + w := do(eh, "POST", "/api/v1/account/email/start", `{"email":"player@example.net"}`, nil) + if c, _ := errEnvelope(t, w); w.Code != http.StatusTooManyRequests || c != "otp_account_locked" || mailer.calls != sends { + t.Fatalf("start while locked = %d %s (sends %d→%d), want 429 and no mail", w.Code, c, sends, mailer.calls) + } +} + +func TestOTPLockEnd(t *testing.T) { + t0 := time.Date(2026, 9, 24, 8, 0, 0, 0, time.UTC) + for _, tc := range []struct { + name string + failures int + now time.Time + locked bool + }{ + {"under budget", otpFailureBudget - 1, t0.Add(time.Hour), false}, + {"budget spent", otpFailureBudget, t0.Add(time.Hour), true}, + {"window just ended", otpFailureBudget, t0.Add(otpFailureWindow), false}, + } { + got := otpLockEnd(t0, tc.failures, tc.now) + if got.IsZero() == tc.locked { + t.Errorf("%s: otpLockEnd = %v, locked want %v", tc.name, got, tc.locked) + } + if tc.locked && !got.Equal(t0.Add(otpFailureWindow)) { + t.Errorf("%s: lock ends %v, want window end", tc.name, got) + } + } +} + +func TestOTPLockNoticeNamesDoorAndTime(t *testing.T) { + subject, body := otpLockNotice(otpDoorName[otpPurposeLogin], time.Date(2026, 9, 25, 8, 0, 0, 0, time.UTC)) + for _, want := range []string{"邮箱验证码登录", "email-code sign-in"} { + if !strings.Contains(subject, want) { + t.Errorf("subject %q missing %q", subject, want) + } + } + for _, want := range []string{"2026-09-25 08:00 UTC", "Passkey", "passkey", "10"} { + if !strings.Contains(body, want) { + t.Errorf("body missing %q:\n%s", want, body) + } + } +} diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index ffd706a..77e0467 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -820,18 +820,16 @@ func (p *PGRepo) VerifyEmailOTP(ctx context.Context, userID, purpose, codeHash s if !expiresAt.After(now) { return "", ErrOTPInvalid } + if until, err := otpLockedUntil(ctx, tx, userID, purpose, now, true); err != nil { + return "", err + } else if !until.IsZero() { + return "", &OTPAccountLockedError{Until: until} + } if attempts >= otpMaxAttempts { return "", ErrOTPLocked } if storedHash != codeHash { - if _, err := tx.ExecContext(ctx, - `UPDATE email_otps SET attempts = attempts + 1 WHERE id = $1`, id); err != nil { - return "", fmt.Errorf("record otp attempt: %w", err) - } - if err := tx.Commit(); err != nil { - return "", err - } - return "", ErrOTPInvalid + return "", chargeOTPMismatch(ctx, tx, id, userID, purpose, now) } // A DIFFERENT account may not also prove this address: the pre-session login @@ -2135,18 +2133,16 @@ func (p *PGRepo) ConsumeLoginEmailOTP(ctx context.Context, userID, purpose, code if !expiresAt.After(now) { return ErrOTPInvalid } + if until, err := otpLockedUntil(ctx, tx, userID, purpose, now, true); err != nil { + return err + } else if !until.IsZero() { + return &OTPAccountLockedError{Until: until} + } if attempts >= otpMaxAttempts { return ErrOTPLocked } if storedHash != codeHash { - if _, err := tx.ExecContext(ctx, - `UPDATE email_otps SET attempts = attempts + 1 WHERE id = $1`, id); err != nil { - return fmt.Errorf("record otp attempt: %w", err) - } - if err := tx.Commit(); err != nil { - return err - } - return ErrOTPInvalid + return chargeOTPMismatch(ctx, tx, id, userID, purpose, now) } if _, err := tx.ExecContext(ctx, @@ -2156,6 +2152,79 @@ func (p *PGRepo) ConsumeLoginEmailOTP(ctx context.Context, userID, purpose, code return tx.Commit() } +// OTPLockedUntil reads the (user, purpose) wrong-code lock for a start door. +func (p *PGRepo) OTPLockedUntil(ctx context.Context, userID, purpose string, now time.Time) (time.Time, error) { + return otpLockedUntil(ctx, p.db, userID, purpose, now, false) +} + +// otpQuerier is the read half shared by *sql.DB and *sql.Tx. +type otpQuerier interface { + QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row +} + +// otpLockedUntil returns when the (user, purpose) lock ends, or zero when the +// budget is not spent in the current window. forUpdate takes the row lock so a +// redeem serialises its check with its own charge. +func otpLockedUntil(ctx context.Context, q otpQuerier, userID, purpose string, now time.Time, forUpdate bool) (time.Time, error) { + query := `SELECT window_start, failures FROM otp_failure_windows WHERE user_id = $1 AND purpose = $2` + if forUpdate { + query += ` FOR UPDATE` + } + var ( + start time.Time + failures int + ) + switch err := q.QueryRowContext(ctx, query, userID, purpose).Scan(&start, &failures); { + case errors.Is(err, sql.ErrNoRows): + return time.Time{}, nil + case err != nil: + return time.Time{}, fmt.Errorf("read otp failure budget: %w", err) + } + return otpLockEnd(start, failures, now), nil +} + +// otpLockEnd is the lock rule on one budget row: spent inside a live window. +func otpLockEnd(windowStart time.Time, failures int, now time.Time) time.Time { + end := windowStart.Add(otpFailureWindow) + if failures < otpFailureBudget || !end.After(now) { + return time.Time{} + } + return end +} + +// chargeOTPMismatch records one wrong guess against the code and the account +// budget, commits, and returns what the caller should answer: ErrOTPInvalid, or +// the lock this guess just tripped. A window that has ended starts over at 1. +func chargeOTPMismatch(ctx context.Context, tx *sql.Tx, codeID, userID, purpose string, now time.Time) error { + if _, err := tx.ExecContext(ctx, + `UPDATE email_otps SET attempts = attempts + 1 WHERE id = $1`, codeID); err != nil { + return fmt.Errorf("record otp attempt: %w", err) + } + var ( + start time.Time + failures int + ) + if err := tx.QueryRowContext(ctx, + `INSERT INTO otp_failure_windows (user_id, purpose, window_start, failures) + VALUES ($1, $2, $3, 1) + ON CONFLICT (user_id, purpose) DO UPDATE SET + failures = CASE WHEN otp_failure_windows.window_start + $4 * interval '1 second' <= $3 + THEN 1 ELSE otp_failure_windows.failures + 1 END, + window_start = CASE WHEN otp_failure_windows.window_start + $4 * interval '1 second' <= $3 + THEN $3 ELSE otp_failure_windows.window_start END + RETURNING window_start, failures`, + userID, purpose, now, int64(otpFailureWindow/time.Second)).Scan(&start, &failures); err != nil { + return fmt.Errorf("charge otp failure budget: %w", err) + } + if err := tx.Commit(); err != nil { + return err + } + if failures == otpFailureBudget { + return &OTPAccountLockedError{Until: start.Add(otpFailureWindow), JustLocked: true} + } + return ErrOTPInvalid +} + // ---- op.console staff login: in-game approval state machine (spec §B op-login) ---- // CreateOpLoginRequest records a fresh pending op.console login attempt for a staff diff --git a/internal/api/repo.go b/internal/api/repo.go index 715b198..3a55a78 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -314,9 +314,10 @@ type Repo interface { CreateEmailOTP(ctx context.Context, id, userID, email, codeHash, purpose string, expiresAt time.Time) error // VerifyEmailOTP redeems the newest live code for (userID, purpose) against // codeHash, atomically (spec §B2). No live code, an expired one, or a consumed - // one → ErrOTPInvalid; an exhausted attempt budget → ErrOTPLocked; a hash - // mismatch increments attempts and returns ErrOTPInvalid WITHOUT consuming the - // code (so a typo does not burn it). On a match the code is consumed and the + // one → ErrOTPInvalid; an exhausted attempt budget → ErrOTPLocked; a spent + // (user, purpose) wrong-code budget → *OTPAccountLockedError, even for the right + // code; a hash mismatch increments attempts, charges that budget, and returns + // ErrOTPInvalid WITHOUT consuming the code (so a typo does not burn it). On a match the code is consumed and the // user row is flipped to email=, email_verified=true; the // proven email is returned — unless a DIFFERENT account has already proven the // same address, which is ErrEmailTaken with the code left unconsumed (the @@ -343,8 +344,14 @@ type Repo interface { // settled — re-proving control of a code this session must not re-touch the row. // Returning only an error is deliberate: unlike onboarding, login has nothing to // prove about the address, so there is no email to hand back. Errors are exactly - // ErrOTPInvalid / ErrOTPLocked (ErrEmailTaken is structurally impossible here). + // ErrOTPInvalid / ErrOTPLocked / *OTPAccountLockedError (ErrEmailTaken is + // structurally impossible here). ConsumeLoginEmailOTP(ctx context.Context, userID, purpose, codeHash string, now time.Time) error + // OTPLockedUntil reports when the (userID, purpose) wrong-code lock ends, or the + // zero time when that door is open. Both redeem paths enforce the lock + // themselves (returning *OTPAccountLockedError, and charging every mismatch to + // the budget); the start doors read it so a locked door mails no code. + OTPLockedUntil(ctx context.Context, userID, purpose string, now time.Time) (time.Time, error) // ---- op.console staff login: in-game approval state machine (spec §B op-login) ---- diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 7db2331..0baf7fc 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -55,6 +55,15 @@ var ( Name: "reaper_worlds_deleted_total", Help: "Total number of world volumes deleted by the reaper.", }) + + // OTPLockoutsTotal counts accounts whose email-code door locked after the + // daily wrong-code budget was spent, by code purpose. Outside a person + // fumbling codes, a lockout means someone is guessing at that account. + OTPLockoutsTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Name: "auth_otp_lockouts_total", + Help: "Email-code doors locked after too many wrong codes, by purpose.", + }, []string{"purpose"}) ) // SyncServerGauge republishes felis_servers_total from a full snapshot of the @@ -87,6 +96,7 @@ func Collectors() []prometheus.Collector { StartDurationSeconds, ImageBuildFailuresTotal, ReaperWorldsDeletedTotal, + OTPLockoutsTotal, } } diff --git a/internal/pgint/pgint_test.go b/internal/pgint/pgint_test.go index 1186ab5..38dc90d 100644 --- a/internal/pgint/pgint_test.go +++ b/internal/pgint/pgint_test.go @@ -324,6 +324,99 @@ func TestConsumeLoginEmailOTPContract(t *testing.T) { } } +// The account-level wrong-code budget must survive supersede: minting a fresh +// code resets the per-code attempts, and the public login door can mint one a +// minute, so only a counter outside email_otps bounds guessing per account. +func TestOTPFailureBudgetContract(t *testing.T) { + ctx := context.Background() + u := newUser(t, "user", "otp-budget") + purpose := "login_email" + addr := "budget-" + suffix(t) + "@example.net" + start := mustNow().Truncate(time.Second) + mint := func(hash string, at time.Time) { + t.Helper() + if err := repo.CreateEmailOTP(ctx, "bud-"+suffix(t), u.ID, addr, hash, purpose, at.Add(5*time.Minute)); err != nil { + t.Fatalf("CreateEmailOTP: %v", err) + } + } + + // Ten wrong guesses spread over three codes; the per-code cap (5) is never hit. + var tripped *api.OTPAccountLockedError + for i := 0; i < 10; i++ { + at := start.Add(time.Duration(i) * time.Minute) + if i%4 == 0 { + mint("h-good", at) + } + err := repo.ConsumeLoginEmailOTP(ctx, u.ID, purpose, "h-wrong", at) + if i < 9 { + if !errors.Is(err, api.ErrOTPInvalid) { + t.Fatalf("wrong guess %d = %v, want ErrOTPInvalid", i+1, err) + } + continue + } + if !errors.As(err, &tripped) || !tripped.JustLocked { + t.Fatalf("10th wrong guess = %v, want a fresh *OTPAccountLockedError", err) + } + } + if want := start.Add(24 * time.Hour); !tripped.Until.Equal(want) { + t.Fatalf("lock until %v, want %v (window opened by the first wrong guess)", tripped.Until, want) + } + + // The right code on a live, barely-used code is refused while locked. + at := start.Add(10 * time.Minute) + var lock *api.OTPAccountLockedError + if err := repo.ConsumeLoginEmailOTP(ctx, u.ID, purpose, "h-good", at); !errors.As(err, &lock) || lock.JustLocked { + t.Fatalf("right code while locked = %v, want a standing *OTPAccountLockedError", err) + } + if until, err := repo.OTPLockedUntil(ctx, u.ID, purpose, at); err != nil || until.IsZero() { + t.Fatalf("OTPLockedUntil during lock = %v, %v", until, err) + } + // Other purposes of the same account are separate doors. + if until, err := repo.OTPLockedUntil(ctx, u.ID, "onboard_email", at); err != nil || !until.IsZero() { + t.Fatalf("onboard door locked too: %v, %v", until, err) + } + + // After the window the door reopens and the count starts over. + later := start.Add(25 * time.Hour) + if until, err := repo.OTPLockedUntil(ctx, u.ID, purpose, later); err != nil || !until.IsZero() { + t.Fatalf("OTPLockedUntil after window = %v, %v", until, err) + } + mint("h-good", later) + if err := repo.ConsumeLoginEmailOTP(ctx, u.ID, purpose, "h-wrong", later); !errors.Is(err, api.ErrOTPInvalid) { + t.Fatalf("first wrong guess of a new window = %v, want ErrOTPInvalid", err) + } + var failures int + if err := db.QueryRowContext(ctx, + `SELECT failures FROM otp_failure_windows WHERE user_id = $1 AND purpose = $2`, u.ID, purpose).Scan(&failures); err != nil { + t.Fatalf("read budget row: %v", err) + } + if failures != 1 { + t.Fatalf("failures after window reset = %d, want 1", failures) + } + if err := repo.ConsumeLoginEmailOTP(ctx, u.ID, purpose, "h-good", later); err != nil { + t.Fatalf("right code after the window = %v, want nil", err) + } + + // The onboarding primitive shares the rule. + onboard := "onboard_email" + for i := 0; i < 10; i++ { + at := start.Add(time.Duration(i) * time.Minute) + if i%4 == 0 { + if err := repo.CreateEmailOTP(ctx, "bud-on-"+suffix(t), u.ID, addr, "h-good", onboard, at.Add(5*time.Minute)); err != nil { + t.Fatalf("CreateEmailOTP onboard: %v", err) + } + } + _, err := repo.VerifyEmailOTP(ctx, u.ID, onboard, "h-wrong", at) + if i == 9 && !errors.Is(err, api.ErrOTPAccountLocked) { + t.Fatalf("10th wrong onboarding guess = %v, want ErrOTPAccountLocked", err) + } + } + if _, err := repo.VerifyEmailOTP(ctx, u.ID, onboard, "h-good", start.Add(10*time.Minute)); !errors.Is(err, api.ErrOTPAccountLocked) { + t.Fatalf("right onboarding code while locked = %v, want ErrOTPAccountLocked", err) + } + assertEmailProven(t, u.ID, "", false) +} + // ---- user admin (spec §7) ------------------------------------------------------- // The claim gate must hold under concurrency (audit #4): two simultaneous claims diff --git a/internal/store/migrations/0022_otp_failure_budget.sql b/internal/store/migrations/0022_otp_failure_budget.sql new file mode 100644 index 0000000..d5a6abf --- /dev/null +++ b/internal/store/migrations/0022_otp_failure_budget.sql @@ -0,0 +1,14 @@ +-- Account-level budget on wrong one-time codes. The per-code attempts cap +-- (email_otps.attempts) resets whenever a new code is minted, and the public +-- login door lets anyone mint one per minute, so on its own it allows about +-- 7,200 guesses a day against one account. This row survives supersede: it +-- counts every wrong code for a (user, purpose) inside a fixed window, and once +-- the budget is spent that door refuses even the right code until the window +-- ends (internal/api otpFailureBudget / otpFailureWindow). +CREATE TABLE otp_failure_windows ( + user_id text NOT NULL REFERENCES users(id) ON DELETE CASCADE, + purpose text NOT NULL, + window_start timestamptz NOT NULL, + failures int NOT NULL DEFAULT 0, + PRIMARY KEY (user_id, purpose) +); diff --git a/panel/src/i18n/resources/en-US/errors.json b/panel/src/i18n/resources/en-US/errors.json index 96c4f08..8656c93 100644 --- a/panel/src/i18n/resources/en-US/errors.json +++ b/panel/src/i18n/resources/en-US/errors.json @@ -22,6 +22,7 @@ "generic": "Something went wrong.", "otp_resend_cooldown": "Verification code requested too frequently, please try again later.", "otp_locked": "Too many incorrect attempts, please request a new verification code.", + "otp_account_locked": "Too many wrong codes in 24 hours — email codes for this account are paused. Try again later or use a passkey.", "passkey_challenge_invalid": "Passkey challenge is invalid or expired, please try again.", "invalid_attestation": "Could not verify this Passkey, please try again.", "passkey_already_bound": "This Passkey is already bound to another account.", diff --git a/panel/src/i18n/resources/zh-CN/errors.json b/panel/src/i18n/resources/zh-CN/errors.json index 19cc253..c1c8d47 100644 --- a/panel/src/i18n/resources/zh-CN/errors.json +++ b/panel/src/i18n/resources/zh-CN/errors.json @@ -22,6 +22,7 @@ "generic": "出了点问题,请稍后重试。", "otp_resend_cooldown": "验证码发送频繁,请稍后再试。", "otp_locked": "验证码错误次数过多,请重新获取验证码。", + "otp_account_locked": "24 小时内验证码错误次数已达上限,此账号的邮箱验证码暂时停用,请稍后再试或改用 Passkey。", "passkey_challenge_invalid": "验证挑战无效或已过期,请重试。", "invalid_attestation": "无法验证此 Passkey,请重试。", "passkey_already_bound": "此 Passkey 已被其他账户绑定。", diff --git a/panel/src/lib/api.ts b/panel/src/lib/api.ts index cd0f91e..b057924 100644 --- a/panel/src/lib/api.ts +++ b/panel/src/lib/api.ts @@ -677,6 +677,8 @@ export function humanizeError(e: unknown): string { return t("otp_resend_cooldown"); case "otp_locked": return t("otp_locked"); + case "otp_account_locked": + return t("otp_account_locked"); case "passkey_challenge_invalid": return t("passkey_challenge_invalid"); case "invalid_attestation":