fix(api): rate-limit email-OTP sends to close the email-bomb vector

handleEmailOTPStart minted and mailed a code on every call, so an
authenticated caller could drive unbounded mail to any address they
typed — an email-bomb primitive against arbitrary mailboxes.

Add a separate otpLimiter (its own sync.Once and map, distinct from the
wake limiter) and throttle each send on two keys before anything is
minted: the caller (user:<id>) and the recipient (email:<lower>). A
refused send mints no code and mails nothing; both cooldowns are
recorded only after delivery succeeds, mirroring the wake path so a
failed mint or delivery never consumes the throttle. The two-key design
stops both one account fanning out across addresses and many accounts
converging on one mailbox.
This commit is contained in:
flyemoji committed 2026-06-30 20:04:23 +09:00
1 parent 2a4a81b9b2
commit 6c3999a067
3 files changed
+115

No files matched your search

+14
View File
@@ -95,6 +95,9 @@ type API struct {
cooldownOnce sync.Once
cooldown *cooldownLimiter
otpCooldownOnce sync.Once
otpCooldown *cooldownLimiter
}
// now returns the current time using the injected clock.
@@ -113,6 +116,17 @@ func (a *API) limiter() *cooldownLimiter {
return a.cooldown
}
// otpLimiter lazily builds a SEPARATE cooldown limiter for email-OTP sends, so an
// OTP resend throttle never shares state with the wake throttle. Keyed by
// principal and by recipient (see handleEmailOTPStart), it bounds how often a code
// may be mailed and closes the email-bomb vector.
func (a *API) otpLimiter() *cooldownLimiter {
a.otpCooldownOnce.Do(func() {
a.otpCooldown = &cooldownLimiter{now: a.now, last: map[string]time.Time{}}
})
return a.otpCooldown
}
// apiRoute is one served HTTP route. Each face exposes its routes as a single
// table (internalAPIRoutes / externalAPIRoutes) so that one declaration drives
// BOTH handler construction here AND the OpenAPI parity test (openapi_test.go):
+20
View File
@@ -41,6 +41,12 @@ const (
// a value in [0, otpCodeBound) zero-pads to exactly otpCodeDigits digits.
otpCodeDigits = 6
otpCodeBound = 1_000_000
// otpResendCooldown is the minimum spacing between OTP sends. Without it,
// handleEmailOTPStart is an email-bomb primitive: an authenticated caller could
// drive unbounded mail to any address they type. The cooldown is enforced on two
// 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
)
// OTPMailer delivers a one-time code to an email address. It is a seam, not a
@@ -115,6 +121,16 @@ func (a *API) handleEmailOTPStart(w http.ResponseWriter, r *http.Request) {
writeError(w, r, newError(http.StatusBadRequest, "bad_request", "a valid email is required"))
return
}
// Throttle sends on both the caller and the recipient before minting anything,
// so a refused request mints no code and mails nothing. The recipient key is
// lower-cased so case variants of one address can't sidestep the per-mailbox cap.
userKey, emailKey := "user:"+p.UserID, "email:"+strings.ToLower(email)
lim := a.otpLimiter()
if !lim.allowed(userKey, otpResendCooldown) || !lim.allowed(emailKey, otpResendCooldown) {
writeError(w, r, newError(http.StatusTooManyRequests, "otp_resend_cooldown",
"a code was sent recently; wait a moment before requesting another"))
return
}
code, err := newEmailOTP()
if err != nil {
writeError(w, r, err)
@@ -134,6 +150,10 @@ func (a *API) handleEmailOTPStart(w http.ResponseWriter, r *http.Request) {
writeError(w, r, err)
return
}
// Start both cooldowns only after a code was actually sent: a failed mint or
// delivery above must not consume the throttle, mirroring the wake path.
lim.record(userKey)
lim.record(emailKey)
a.audit(r, auditActor(p), "account.email.otp_sent", "")
writeJSON(w, http.StatusAccepted, map[string]any{
"sent": true,
+81
View File
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"net/http"
"net/http/httptest"
"testing"
"time"
)
@@ -150,6 +151,81 @@ func TestEmailOTPStartValidation(t *testing.T) {
})
}
// TestEmailOTPStartRateLimited closes the email-bomb vector: handleEmailOTPStart is
// an authenticated primitive that mails arbitrary addresses, so a resend cooldown
// bounds it on both the caller (a fan-out across many addresses) and the recipient
// (a convergence of many accounts on one mailbox).
func TestEmailOTPStartRateLimited(t *testing.T) {
const emailA, emailB = "[email protected]", "[email protected]"
u1 := &Principal{UserID: "u1", Email: "[email protected]", Role: "user"}
u2 := &Principal{UserID: "u2", Email: "[email protected]", Role: "user"}
start := func(eh http.Handler, email string) *httptest.ResponseRecorder {
return do(eh, "POST", "/api/v1/account/email/start", `{"email":"`+email+`"}`, nil)
}
t.Run("same caller and recipient is throttled, then recovers after the cooldown", func(t *testing.T) {
repo := newFakeRepo()
mailer := &captureMailer{}
api := newTestAPI(repo, newFakeCluster())
api.External = staticExternal{p: u1}
api.Mailer = mailer
clock := time.Unix(1_700_000_000, 0)
api.Now = func() time.Time { return clock }
eh := api.ExternalHandler()
if w := start(eh, emailA); w.Code != http.StatusAccepted {
t.Fatalf("first send: code = %d, want 202 (%s)", w.Code, w.Body.String())
}
// An immediate resend is refused with 429 — and mints/mails nothing.
if w := start(eh, emailA); w.Code != http.StatusTooManyRequests || decodeErr(t, w) != "otp_resend_cooldown" {
t.Fatalf("immediate resend: code = %d body %s, want 429 otp_resend_cooldown", w.Code, w.Body.String())
}
if mailer.calls != 1 {
t.Errorf("mailer calls = %d, want 1 (the throttled resend must not mail)", mailer.calls)
}
if len(repo.otps) != 1 {
t.Errorf("persisted codes = %d, want 1 (the throttled resend must not mint)", len(repo.otps))
}
// Once the cooldown elapses the same address may be mailed again.
clock = clock.Add(otpResendCooldown + time.Second)
if w := start(eh, emailA); w.Code != http.StatusAccepted {
t.Fatalf("post-cooldown send: code = %d, want 202 (%s)", w.Code, w.Body.String())
}
})
t.Run("one caller cannot fan out across addresses", func(t *testing.T) {
api := newTestAPI(newFakeRepo(), newFakeCluster())
api.External = staticExternal{p: u1}
api.Mailer = &captureMailer{}
eh := api.ExternalHandler()
if w := start(eh, emailA); w.Code != http.StatusAccepted {
t.Fatalf("send to A: code = %d, want 202", w.Code)
}
// A different recipient, same caller, same instant: the per-caller key throttles.
if w := start(eh, emailB); w.Code != http.StatusTooManyRequests {
t.Fatalf("fan-out to B: code = %d, want 429", w.Code)
}
})
t.Run("many callers cannot converge on one recipient", func(t *testing.T) {
api := newTestAPI(newFakeRepo(), newFakeCluster())
api.External = staticExternal{p: u1}
api.Mailer = &captureMailer{}
eh := api.ExternalHandler()
if w := start(eh, emailA); w.Code != http.StatusAccepted {
t.Fatalf("u1 send to A: code = %d, want 202", w.Code)
}
// A different caller targeting the same address, same instant: the per-recipient
// key throttles even though u2 has never sent before.
api.External = staticExternal{p: u2}
if w := start(eh, emailA); w.Code != http.StatusTooManyRequests {
t.Fatalf("u2 converge on A: code = %d, want 429", w.Code)
}
})
}
// TestEmailOTPVerifyRejections is the redeem-side failure matrix. The expired and
// locked cases plant rows directly: the test clock is frozen, so an already-expired
// or already-exhausted row is the only way to reach those branches deterministically.
@@ -275,10 +351,15 @@ func TestEmailOTPSupersede(t *testing.T) {
api := newTestAPI(repo, newFakeCluster())
api.External = staticExternal{p: user}
api.Mailer = mailer
// A re-request is a fresh send, so it must clear the resend cooldown: advance the
// clock past it between the two starts (supersede is orthogonal to the throttle).
clock := time.Unix(1_700_000_000, 0)
api.Now = func() time.Time { return clock }
eh := api.ExternalHandler()
do(eh, "POST", "/api/v1/account/email/start", `{"email":"[email protected]"}`, nil)
first := mailer.code
clock = clock.Add(otpResendCooldown + time.Second)
do(eh, "POST", "/api/v1/account/email/start", `{"email":"[email protected]"}`, nil)
second := mailer.code