fix(api): rate-limit email-OTP sends to close the email-bomb vector (6c3999a0) · Commits · FelisMC / Felis · GitLab

internal/api/api.go

+14 −0

Changes for internal/api/api.go: 14 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -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):

internal/api/handlers_email_otp.go

+20 −0

Changes for internal/api/handlers_email_otp.go: 20 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -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,

internal/api/handlers_email_otp_test.go

+81 −0

Changes for internal/api/handlers_email_otp_test.go: 81 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -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