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:
3 files changed
+115
No files matched your search
@@ -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):
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in new issue
Block a user