diff --git a/SECURITY.md b/SECURITY.md index 8807fe4d..d464dd8d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -61,3 +61,48 @@ We thank everyone who has reported issues responsibly. Confirmed reporters who opt in are listed in the published advisory and our release notes. + + ## Production hardening notes + + ### Backchannel (CIBA) rate limiting + + `/oauth2/bc-authorize` is rate-limited per `client_id` and per + `login_hint` to prevent end-user notification spam, + `backchannel_auth_requests` table flooding, and `login_hint` + enumeration. + + **Bucket key composition.** Buckets are keyed on `client_id` and + `login_hint` alone — NOT on `(client_id, tenant)` or + `(login_hint, tenant)`. `account_id` / `project_id` arrive in the + request body and are not bound to the client (clients in ZeroID are + global). Including them in the bucket key would let an attacker + bypass the cap by supplying random tenant values per request. Tenant + fields are still included in the WARN log event for forensics. + + The default implementation is an **in-process, in-memory token bucket**. + This is correct for single-instance deployments. **Multi-replica + deployments need a shared-store backend** — without one, each replica + enforces the limit independently and a fleet of N replicas effectively + permits N× the documented cap. + + - **Default config:** 10 req/min per client, 5 req/min per user. Tunable + via `BackchannelConfig.PerClientRateLimitPerMinute` and + `PerUserRateLimitPerMinute` (set to `0` to disable a dimension). + - **Memory bound.** The in-memory limiter caps tracked keys at 100k by + default; beyond that, requests for new keys fail open. Operators can + inspect `TokenBucketLimiter.BucketCount()` and alert before the cap + is reached. The cap exists because `login_hint` is attacker-supplied + and otherwise unbounded. + - **Multi-replica:** implement `zeroid.RateLimiter` against your shared + store (Redis, Memcached, hosted KV) and install via + `Server.SetBackchannelRateLimiters(perClient, perUser)`. The interface + has two methods (`Allow`, `Stop`); the fail-open contract is documented + on the interface. + - **Defense-in-depth:** run an edge limiter (CDN, nginx, Envoy) for + per-IP throttling regardless. The in-process limiter exists for + per-actor semantic enforcement that an edge limiter cannot express. + - **Detection:** rate-limit rejections emit a structured WARN log with + `event=bc_authorize_rate_limited`, `reason`, `client_id`, tenant IDs, + and a `login_hint_hash` (SHA-256 prefix, no PII). Backend failures + (which trigger fail-open) emit `event=bc_authorize_rate_limiter_backend_error`. + Both events are designed for log-aggregation alerting. diff --git a/config.go b/config.go index 49463cf5..9aae41fe 100644 --- a/config.go +++ b/config.go @@ -60,6 +60,17 @@ type BackchannelConfig struct { // register endpoints like https://localhost:9000/. Production deployments // MUST keep this false (see GHSA-599q-j34m-33vc). AllowPrivateNotificationEndpoints bool `koanf:"allow_private_notification_endpoints"` + + // PerClientRateLimitPerMinute caps /oauth2/bc-authorize per + // (client_id, account_id, project_id). nil uses the service default + // (10/min); 0 explicitly disables. Pointer because 0 is meaningful. + PerClientRateLimitPerMinute *int `koanf:"per_client_rate_limit_per_minute"` + + // PerUserRateLimitPerMinute caps /oauth2/bc-authorize per + // (login_hint, account_id, project_id). nil uses the service default + // (5/min); 0 explicitly disables. Prevents one user being spammed + // across multiple clients in the same tenant. + PerUserRateLimitPerMinute *int `koanf:"per_user_rate_limit_per_minute"` } // AttestationConfig governs the attestation verification subsystem. The diff --git a/hooks.go b/hooks.go index ac579e99..c91c3b53 100644 --- a/hooks.go +++ b/hooks.go @@ -104,3 +104,21 @@ type BackchannelNotification struct { // debuggability but does not block request creation — the user may approve // through another channel. type BackchannelNotifier func(ctx context.Context, n BackchannelNotification) error + +// RateLimiter gates an /oauth2/bc-authorize request keyed by an opaque +// string. ZeroID ships with an in-memory implementation; multi-replica +// deployments should plug in a shared-store backend (Redis, Memcached, +// hosted KV) via Server.SetBackchannelRateLimiters to avoid the per-replica +// bypass where each instance enforces independently. +// +// Implementations must be safe for concurrent use. Result shape: +// - (true, 0, nil) request permitted +// - (false, retryAfter, nil) request rejected; retryAfter is surfaced +// via the Retry-After header, rounded up to whole seconds (RFC 7231 §7.1.3) +// - (_, _, err) backend failed; ZeroID fails open and logs +// a WARN event so the operator notices the degraded posture +type RateLimiter interface { + Allow(ctx context.Context, key string) (allowed bool, retryAfter time.Duration, err error) + // Stop releases resources held by the limiter. Idempotent. + Stop() +} diff --git a/internal/handler/oauth.go b/internal/handler/oauth.go index b87992cd..7f0e1e7e 100644 --- a/internal/handler/oauth.go +++ b/internal/handler/oauth.go @@ -3,7 +3,9 @@ package handler import ( "context" "errors" + "math" "net/http" + "strconv" "github.com/danielgtaylor/huma/v2" "github.com/rs/zerolog/log" @@ -97,6 +99,21 @@ func extractOAuthError(err error) (code, description string, status int) { return "server_error", "an unexpected error occurred", http.StatusInternalServerError } +// retryAfterHeader formats an *OAuthError's RetryAfter as integer +// delta-seconds (RFC 7231 §7.1.3). Returns "" when the error carries no +// hint. Rounds up so the advertised wait is never shorter than the refill. +func retryAfterHeader(err error) string { + var oauthErr *service.OAuthError + if !errors.As(err, &oauthErr) || oauthErr.RetryAfter <= 0 { + return "" + } + secs := int(math.Ceil(oauthErr.RetryAfter.Seconds())) + if secs < 1 { + secs = 1 + } + return strconv.Itoa(secs) +} + type IntrospectInput struct { Body struct { Token string `json:"token" required:"true" minLength:"1" doc:"JWT to introspect"` @@ -341,9 +358,12 @@ type BcAuthorizeInput struct { } // BcAuthorizeOutput mirrors the success response in CIBA Core §7.3. +// RetryAfter is set on rate-limit rejections; Huma renders it as the +// Retry-After header. type BcAuthorizeOutput struct { - Status int - Body any // service.CreateAuthRequestOutput on success; oauthErrorBody on error + Status int + RetryAfter string `header:"Retry-After"` + Body any // service.CreateAuthRequestOutput on success; oauthErrorBody on error } func (a *API) bcAuthorizeOp(ctx context.Context, input *BcAuthorizeInput) (*BcAuthorizeOutput, error) { @@ -367,9 +387,18 @@ func (a *API) bcAuthorizeOp(ctx context.Context, input *BcAuthorizeInput) (*BcAu ClientNotificationToken: input.Body.ClientNotificationToken, }) if err != nil { - log.Error().Err(err).Str("client_id", input.Body.ClientID).Msg("bc-authorize failed") code, desc, status := extractOAuthError(err) - return &BcAuthorizeOutput{Status: status, Body: oauthErrorBody{Error: code, ErrorDescription: desc}}, nil + // 429s are an expected signal already logged at Warn in the service; + // skip the Error-level handler line so error-rate alerts aren't + // noisy under sustained rate-limited traffic. + if status != http.StatusTooManyRequests { + log.Error().Err(err).Str("client_id", input.Body.ClientID).Msg("bc-authorize failed") + } + return &BcAuthorizeOutput{ + Status: status, + RetryAfter: retryAfterHeader(err), + Body: oauthErrorBody{Error: code, ErrorDescription: desc}, + }, nil } return &BcAuthorizeOutput{Status: http.StatusOK, Body: out}, nil } diff --git a/internal/service/backchannel.go b/internal/service/backchannel.go index 7dd150f9..04dea70e 100644 --- a/internal/service/backchannel.go +++ b/internal/service/backchannel.go @@ -4,7 +4,9 @@ import ( "bytes" "context" "crypto/rand" + "crypto/sha256" "encoding/base64" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -58,6 +60,12 @@ type BackchannelService struct { // with a 10-second timeout enforced by pingClient.Timeout). pingClient *http.Client pingDispatchAsync bool // overridable for tests + + // Rate limiters for /oauth2/bc-authorize. Nil disables the dimension. + // Held as the interface so a shared-store backend can be plugged in via + // SetRateLimiters. + perClientLimiter RateLimiter + perUserLimiter RateLimiter } // BackchannelNotifierFunc is the internal alias for the public @@ -108,6 +116,15 @@ type BackchannelServiceConfig struct { // setter; both should be set from the same source in the deployer's // server construction code. AllowPrivateNotificationEndpoints bool + + // PerClientRateLimit caps /oauth2/bc-authorize requests per minute keyed + // on (client_id, account_id, project_id). 0 disables; default 10. + PerClientRateLimit int + // PerUserRateLimit caps /oauth2/bc-authorize requests per minute keyed + // on (login_hint, account_id, project_id). 0 disables; default 5. + // Prevents one user being spammed across multiple clients in the same + // tenant. + PerUserRateLimit int } // DefaultBackchannelConfig returns sensible defaults for production deployments. @@ -121,6 +138,8 @@ func DefaultBackchannelConfig() BackchannelServiceConfig { PingTimeout: 10 * time.Second, PingMaxRetries: 3, PingBaseDelay: 500 * time.Millisecond, + PerClientRateLimit: 10, + PerUserRateLimit: 5, } } @@ -163,7 +182,18 @@ func NewBackchannelService( svcCancel: svcCancel, pingClient: &http.Client{Timeout: cfg.PingTimeout}, pingDispatchAsync: true, + perClientLimiter: newPerMinuteLimiter(cfg.PerClientRateLimit), + perUserLimiter: newPerMinuteLimiter(cfg.PerUserRateLimit), + } +} + +// newPerMinuteLimiter builds the default in-memory limiter: capacity=n +// (burst), refill=n/60 tokens/sec. Returns nil when n <= 0. +func newPerMinuteLimiter(n int) RateLimiter { + if n <= 0 { + return nil } + return NewTokenBucketLimiter(float64(n), float64(n)/60.0) } // Stop cancels the service's lifecycle context, signalling in-flight detached @@ -179,6 +209,52 @@ func (s *BackchannelService) Stop() { s.svcCancel = nil s.svcCtx = nil } + if s.perClientLimiter != nil { + s.perClientLimiter.Stop() + } + if s.perUserLimiter != nil { + s.perUserLimiter.Stop() + } +} + +// SetRateLimits swaps in fresh in-memory limiters at the supplied +// requests-per-minute; 0 disables a dimension. Atomic with the cfg update +// so concurrent callers can't see torn state. For non-default backends, +// use SetRateLimiters. +func (s *BackchannelService) SetRateLimits(perClientRPM, perUserRPM int) { + s.mu.Lock() + defer s.mu.Unlock() + s.cfg.PerClientRateLimit = perClientRPM + s.cfg.PerUserRateLimit = perUserRPM + s.replaceLimitersLocked( + newPerMinuteLimiter(perClientRPM), + newPerMinuteLimiter(perUserRPM), + ) +} + +// SetRateLimiters installs RateLimiter implementations for the per-client +// and per-user dimensions. Nil disables a dimension. Previously-installed +// limiters are stopped — except when the new instance equals the existing +// one (calling Stop() on the default in-memory impl kills its reaper +// goroutine, which cannot be restarted). +func (s *BackchannelService) SetRateLimiters(perClient, perUser RateLimiter) { + s.mu.Lock() + defer s.mu.Unlock() + s.replaceLimitersLocked(perClient, perUser) +} + +// replaceLimitersLocked swaps limiters in atomically. Caller holds s.mu. +// Skips Stop() when the new instance equals the existing one to avoid +// rendering the supplied limiter non-functional. +func (s *BackchannelService) replaceLimitersLocked(perClient, perUser RateLimiter) { + if s.perClientLimiter != nil && s.perClientLimiter != perClient { + s.perClientLimiter.Stop() + } + if s.perUserLimiter != nil && s.perUserLimiter != perUser { + s.perUserLimiter.Stop() + } + s.perClientLimiter = perClient + s.perUserLimiter = perUser } // SetNotifier wires the deployer's BackchannelNotifier. Safe to call any time @@ -268,6 +344,15 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut return nil, oauthBadRequestCause("invalid_client", fmt.Sprintf("unknown client %s", in.ClientID), err) } + // Rate-limit BEFORE notifier dispatch and BEFORE persistence: rejecting + // after the notifier would still spam the user; rejecting after + // persistence would still flood backchannel_auth_requests. Tenant is + // part of the key because clients in ZeroID are global — keying on + // client_id alone would be a cross-tenant key. + if err := s.checkRateLimit(ctx, in); err != nil { + return nil, err + } + // Determine notification mode. CIBA Core §10 makes the delivery mode a // property of the client registration, not the per-request — so we read // the row off the client and let the per-request client_notification_token @@ -364,6 +449,83 @@ func (s *BackchannelService) CreateAuthRequest(ctx context.Context, in CreateAut }, nil } +// checkRateLimit applies the per-client and per-user limiters. On reject, +// returns 429 + error="slow_down" + Retry-After (CIBA Core §11 polling-side +// semantics). On a limiter backend error the call fails open — a +// malfunctioning limiter must not DoS the auth service it is protecting — +// and a distinct WARN event records the degraded posture. +// +// Bucket keys are the raw client_id and login_hint — tenant fields are NOT +// in the key because they are caller-supplied JSON-body strings unbound to +// the client; including them would let an attacker bypass the cap by +// varying tenant per request. Tenant remains in the WARN event for +// forensics. +func (s *BackchannelService) checkRateLimit(ctx context.Context, in CreateAuthRequestInput) error { + s.mu.RLock() + perClient := s.perClientLimiter + perUser := s.perUserLimiter + s.mu.RUnlock() + + if perClient != nil { + ok, retryAfter, err := perClient.Allow(ctx, in.ClientID) + if err != nil { + logRateLimitBackendError("per_client", in, err) + } else if !ok { + logRateLimitRejection("per_client", in, retryAfter) + return oauthTooManyRequests( + "slow_down", + "too many bc-authorize requests for this client; retry after Retry-After seconds", + retryAfter, + ) + } + } + if perUser != nil { + ok, retryAfter, err := perUser.Allow(ctx, in.LoginHint) + if err != nil { + logRateLimitBackendError("per_user", in, err) + } else if !ok { + logRateLimitRejection("per_user", in, retryAfter) + return oauthTooManyRequests( + "slow_down", + "too many bc-authorize requests for this user; retry after Retry-After seconds", + retryAfter, + ) + } + } + return nil +} + +func logRateLimitRejection(reason string, in CreateAuthRequestInput, retryAfter time.Duration) { + log.Warn(). + Str("event", "bc_authorize_rate_limited"). + Str("reason", reason). + Str("client_id", in.ClientID). + Str("account_id", in.AccountID). + Str("project_id", in.ProjectID). + Str("login_hint_hash", hashedLoginHint(in.LoginHint)). + Dur("retry_after", retryAfter). + Msg("bc-authorize rate limit exceeded") +} + +func logRateLimitBackendError(reason string, in CreateAuthRequestInput, err error) { + log.Warn(). + Err(err). + Str("event", "bc_authorize_rate_limiter_backend_error"). + Str("reason", reason). + Str("client_id", in.ClientID). + Str("account_id", in.AccountID). + Str("project_id", in.ProjectID). + Str("login_hint_hash", hashedLoginHint(in.LoginHint)). + Msg("bc-authorize rate limiter backend error; failing open") +} + +// hashedLoginHint returns a 16-hex-char SHA-256 prefix — enough to cluster +// events by victim without leaking the raw login_hint (often PII) to logs. +func hashedLoginHint(hint string) string { + sum := sha256.Sum256([]byte(hint)) + return hex.EncodeToString(sum[:])[:16] +} + // ApproveInput resolves a pending request positively. type ApproveInput struct { AuthReqID string diff --git a/internal/service/oauth_error.go b/internal/service/oauth_error.go index ba92b8c3..aeef764e 100644 --- a/internal/service/oauth_error.go +++ b/internal/service/oauth_error.go @@ -1,6 +1,9 @@ package service -import "net/http" +import ( + "net/http" + "time" +) // OAuthError is the structured error type returned by OAuthService methods. // @@ -14,8 +17,11 @@ type OAuthError struct { Code string // Description is the human-readable message returned in error_description. Description string - // HTTPStatus is the HTTP response status code (400, 401, or 500). + // HTTPStatus is the HTTP response status code (400, 401, 429, or 500). HTTPStatus int + // RetryAfter, when non-zero, is surfaced as the Retry-After response + // header (RFC 7231 §7.1.3). + RetryAfter time.Duration // err is the underlying cause; preserved for logging, not sent to clients. err error } @@ -55,3 +61,14 @@ func oauthUnauthorized(description string, cause error) *OAuthError { func oauthServerError(description string, cause error) *OAuthError { return &OAuthError{Code: "server_error", Description: description, HTTPStatus: http.StatusInternalServerError, err: cause} } + +// oauthTooManyRequests returns an *OAuthError for HTTP 429 with the given +// Retry-After hint. +func oauthTooManyRequests(code, description string, retryAfter time.Duration) *OAuthError { + return &OAuthError{ + Code: code, + Description: description, + HTTPStatus: http.StatusTooManyRequests, + RetryAfter: retryAfter, + } +} diff --git a/internal/service/rate_limiter.go b/internal/service/rate_limiter.go new file mode 100644 index 00000000..7934be2d --- /dev/null +++ b/internal/service/rate_limiter.go @@ -0,0 +1,210 @@ +package service + +import ( + "context" + "sync" + "time" +) + +// RateLimiter gates a request keyed by an opaque string. Implementations +// must be safe for concurrent use. +// +// Result shape: +// - (true, 0, nil) request permitted +// - (false, retryAfter, nil) request rejected; retryAfter is the hint +// surfaced via the Retry-After header, rounded up to whole seconds +// (RFC 7231 §7.1.3) +// - (_, _, err) backend itself failed; the caller decides +// fail-open vs fail-closed +type RateLimiter interface { + Allow(ctx context.Context, key string) (allowed bool, retryAfter time.Duration, err error) + // Stop releases resources held by the limiter (goroutines, connection + // pools). Idempotent. + Stop() +} + +// DefaultMaxBuckets is the default cap on tracked keys. Bounds memory +// against unbounded-key inputs (e.g. attacker-supplied login_hints) at the +// cost of failing open once the cap is reached. Operators can raise via +// the MaxBuckets field on a custom-constructed limiter. +const DefaultMaxBuckets = 100_000 + +// maxReapBatch caps the number of map entries scanned per reap pass. +// Bounds the worst-case lock-hold time so a large bucket map cannot block +// concurrent Allow calls for an unbounded duration; successive ticks pick +// up where the previous left off (Go randomises map iteration order). +const maxReapBatch = 4096 + +// TokenBucketLimiter is an in-process token-bucket RateLimiter. Buckets are +// keyed by string and self-pruning. Per-instance only — multi-replica +// deployments should plug a shared-store implementation in via +// Server.SetBackchannelRateLimiters. +type TokenBucketLimiter struct { + // Capacity is the burst size. A fresh bucket starts full. + Capacity float64 + // RefillPerSec is the steady-state allowance. For "N requests per + // minute" use N/60. + RefillPerSec float64 + // IdleTTL bounds how long an unused bucket lingers before the reaper + // drops it. Defaults to 10× the time to refill a full bucket so a + // bucket cannot be reaped while it would still observe drained state + // on the next request. + IdleTTL time.Duration + // MaxBuckets caps the number of distinct keys tracked. When the map + // reaches this size, requests for new keys are silently allowed + // (fail-open) — the alternative is allocating memory without bound on + // attacker-supplied keys. 0 disables the cap; default DefaultMaxBuckets. + MaxBuckets int + + // now is overridable for deterministic tests; defaults to time.Now().UTC(). + now func() time.Time + + mu sync.Mutex + buckets map[string]*bucketState + + stop chan struct{} + stopOnce sync.Once +} + +var _ RateLimiter = (*TokenBucketLimiter)(nil) + +type bucketState struct { + tokens float64 + lastRefill time.Time + lastSeen time.Time +} + +// NewTokenBucketLimiter starts the background reaper; callers must Stop() +// it during shutdown. Returns nil when capacity or refillPerSec is +// non-positive — the call site treats nil as disabled. +func NewTokenBucketLimiter(capacity, refillPerSec float64) *TokenBucketLimiter { + if capacity <= 0 || refillPerSec <= 0 { + return nil + } + // 1-minute floor keeps fast limiters from reaping themselves under + // steady traffic. + refillSeconds := capacity / refillPerSec + idleTTL := time.Duration(refillSeconds*10) * time.Second + if idleTTL < time.Minute { + idleTTL = time.Minute + } + l := &TokenBucketLimiter{ + Capacity: capacity, + RefillPerSec: refillPerSec, + IdleTTL: idleTTL, + MaxBuckets: DefaultMaxBuckets, + now: func() time.Time { return time.Now().UTC() }, + buckets: make(map[string]*bucketState), + stop: make(chan struct{}), + } + go l.reapLoop() + return l +} + +// Allow is concurrency-safe. The in-memory implementation never returns a +// non-nil error; the signature carries one to satisfy the RateLimiter +// interface (network-backed implementations can fail). +func (l *TokenBucketLimiter) Allow(_ context.Context, key string) (bool, time.Duration, error) { + if l == nil { + return true, 0, nil + } + now := l.now() + + l.mu.Lock() + defer l.mu.Unlock() + + b, ok := l.buckets[key] + if !ok { + // Fail open at capacity — refusing to track a new key is safer + // than allocating without bound when an attacker can supply + // arbitrary keys. + if l.MaxBuckets > 0 && len(l.buckets) >= l.MaxBuckets { + return true, 0, nil + } + // Start full so a legitimate first request isn't punished. + b = &bucketState{tokens: l.Capacity, lastRefill: now} + l.buckets[key] = b + } else { + elapsed := now.Sub(b.lastRefill).Seconds() + if elapsed > 0 { + b.tokens += elapsed * l.RefillPerSec + if b.tokens > l.Capacity { + b.tokens = l.Capacity + } + b.lastRefill = now + } + } + b.lastSeen = now + + if b.tokens >= 1 { + b.tokens -= 1 + return true, 0, nil + } + // Fractional tokens carry into the wait so a second 429 in a burst + // doesn't advertise a needlessly long Retry-After. + missing := 1 - b.tokens + waitSec := missing / l.RefillPerSec + // Round up — RFC 7231 §7.1.3 requires integer delta-seconds. + retryAfter := time.Duration(waitSec*float64(time.Second)) + time.Second - 1 + retryAfter = retryAfter.Truncate(time.Second) + if retryAfter < time.Second { + retryAfter = time.Second + } + return false, retryAfter, nil +} + +// BucketCount returns the current number of tracked keys. Exposed for +// operator observability — alert when this approaches MaxBuckets. +func (l *TokenBucketLimiter) BucketCount() int { + if l == nil { + return 0 + } + l.mu.Lock() + defer l.mu.Unlock() + return len(l.buckets) +} + +// Stop is idempotent and nil-safe. +func (l *TokenBucketLimiter) Stop() { + if l == nil { + return + } + l.stopOnce.Do(func() { close(l.stop) }) +} + +func (l *TokenBucketLimiter) reapLoop() { + // IdleTTL/2 cadence keeps reaper lock contention with Allow negligible. + interval := l.IdleTTL / 2 + if interval < 30*time.Second { + interval = 30 * time.Second + } + t := time.NewTicker(interval) + defer t.Stop() + for { + select { + case <-l.stop: + return + case <-t.C: + l.reap() + } + } +} + +func (l *TokenBucketLimiter) reap() { + cutoff := l.now().Add(-l.IdleTTL) + l.mu.Lock() + defer l.mu.Unlock() + // Bound the per-pass work so a large map cannot stall Allow callers + // for an unbounded duration. Go's randomised map iteration order + // means successive passes cover what this pass misses. + scanned := 0 + for k, b := range l.buckets { + if scanned >= maxReapBatch { + return + } + scanned++ + if b.lastSeen.Before(cutoff) { + delete(l.buckets, k) + } + } +} diff --git a/internal/service/rate_limiter_test.go b/internal/service/rate_limiter_test.go new file mode 100644 index 00000000..12e39dfa --- /dev/null +++ b/internal/service/rate_limiter_test.go @@ -0,0 +1,288 @@ +package service + +import ( + "context" + "errors" + "strconv" + "sync" + "testing" + "time" +) + +// fakeClock returns a controllable time source for deterministic limiter +// tests. Advance() moves the clock forward; concurrent reads via the now() +// closure are safe. +type fakeClock struct { + mu sync.Mutex + t time.Time +} + +func newFakeClock(start time.Time) *fakeClock { return &fakeClock{t: start} } + +func (c *fakeClock) now() time.Time { + c.mu.Lock() + defer c.mu.Unlock() + return c.t +} + +func (c *fakeClock) advance(d time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + c.t = c.t.Add(d) +} + +// newTestLimiter builds a limiter and immediately stops its background +// reaper — tests drive elapsed time via the fake clock, so the reaper +// goroutine would be a flakiness source if left running. +func newTestLimiter(t *testing.T, capacity, refillPerSec float64, clock *fakeClock) *TokenBucketLimiter { + t.Helper() + l := NewTokenBucketLimiter(capacity, refillPerSec) + l.Stop() + l.now = clock.now + return l +} + +// TestTokenBucketLimiter_BurstAllowance proves a fresh bucket permits exactly +// `capacity` requests in immediate succession, then rejects with a positive +// Retry-After. +func TestTokenBucketLimiter_BurstAllowance(t *testing.T) { + clock := newFakeClock(time.Unix(0, 0)) + l := newTestLimiter(t, 5, 5.0/60.0, clock) // 5/min + + for i := 1; i <= 5; i++ { + ok, retry, err := l.Allow(context.Background(), "k") + if err != nil { + t.Fatalf("burst slot %d: unexpected error: %v", i, err) + } + if !ok || retry != 0 { + t.Fatalf("burst slot %d: want (true, 0); got (%v, %v)", i, ok, retry) + } + } + ok, retry, err := l.Allow(context.Background(), "k") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok { + t.Fatalf("6th request must be rejected once the bucket is empty") + } + if retry < time.Second { + t.Fatalf("Retry-After must be ≥ 1s per RFC 7231 §7.1.3; got %v", retry) + } +} + +// TestTokenBucketLimiter_RefillsOverTime proves a bucket regenerates a token +// after exactly capacity/refillPerSec seconds. +func TestTokenBucketLimiter_RefillsOverTime(t *testing.T) { + clock := newFakeClock(time.Unix(0, 0)) + // 1 token/sec — single-token refill window is exactly 1 second. + l := newTestLimiter(t, 1, 1.0, clock) + + if ok, _, _ := l.Allow(context.Background(), "k"); !ok { + t.Fatal("first request must succeed") + } + if ok, _, _ := l.Allow(context.Background(), "k"); ok { + t.Fatal("second immediate request must be rejected") + } + clock.advance(time.Second) + if ok, _, _ := l.Allow(context.Background(), "k"); !ok { + t.Fatal("after 1s the bucket must have refilled one token") + } +} + +// TestTokenBucketLimiter_KeysAreIndependent proves two different keys share +// no bucket state — important because per-client and per-user buckets both +// use the same limiter type and a leak would let abuse on one dimension +// silently consume the other dimension's allowance. +func TestTokenBucketLimiter_KeysAreIndependent(t *testing.T) { + clock := newFakeClock(time.Unix(0, 0)) + l := newTestLimiter(t, 1, 1.0, clock) + + if ok, _, _ := l.Allow(context.Background(), "alice"); !ok { + t.Fatal("alice's first request must succeed") + } + if ok, _, _ := l.Allow(context.Background(), "bob"); !ok { + t.Fatal("bob's first request must succeed (independent bucket)") + } + if ok, _, _ := l.Allow(context.Background(), "alice"); ok { + t.Fatal("alice's second request must be rejected (her bucket is empty)") + } +} + +// TestTokenBucketLimiter_NilReceiverAllows confirms the nil-receiver +// "disabled" contract — callers can pass nil when config sets the limit to +// 0 without sprinkling guards through the call site. +func TestTokenBucketLimiter_NilReceiverAllows(t *testing.T) { + var l *TokenBucketLimiter + ok, retry, err := l.Allow(context.Background(), "anything") + if err != nil { + t.Fatalf("nil limiter must not error; got %v", err) + } + if !ok || retry != 0 { + t.Fatalf("nil limiter must always allow; got (%v, %v)", ok, retry) + } + l.Stop() // must not panic +} + +// TestTokenBucketLimiter_DisabledOnZeroConfig confirms the constructor +// returns nil when either parameter is non-positive — the kill-switch +// guarantee operators rely on. +func TestTokenBucketLimiter_DisabledOnZeroConfig(t *testing.T) { + cases := []struct { + capacity, refill float64 + }{ + {0, 5.0 / 60.0}, + {5, 0}, + {-1, 1}, + {1, -1}, + } + for _, c := range cases { + if got := NewTokenBucketLimiter(c.capacity, c.refill); got != nil { + got.Stop() + t.Errorf("NewTokenBucketLimiter(%v, %v) = non-nil; want nil (disabled)", c.capacity, c.refill) + } + } +} + +// TestTokenBucketLimiter_RetryAfterRoundsUp proves Retry-After is never +// shorter than the actual refill wait — clients honouring the header must +// not poll back before a token is available, otherwise the 429 storm +// continues indefinitely. +func TestTokenBucketLimiter_RetryAfterRoundsUp(t *testing.T) { + clock := newFakeClock(time.Unix(0, 0)) + // 30/min = 0.5 tokens/sec — a single missing token takes 2s to refill. + l := newTestLimiter(t, 1, 0.5, clock) + _, _, _ = l.Allow(context.Background(), "k") + _, retry, _ := l.Allow(context.Background(), "k") + if retry < 2*time.Second { + t.Fatalf("Retry-After must round up to ≥ 2s for a 0.5 tokens/sec refill; got %v", retry) + } +} + +// failingLimiter is a RateLimiter that always returns a backend error. +// Used to exercise the fail-open path in BackchannelService.checkRateLimit. +type failingLimiter struct { + err error + calls int +} + +func (f *failingLimiter) Allow(_ context.Context, _ string) (bool, time.Duration, error) { + f.calls++ + return false, 0, f.err +} +func (f *failingLimiter) Stop() {} + +// TestRateLimiterInterface_FailOpen documents the contract the +// BackchannelService relies on: an implementation returning a non-nil error +// signals a backend failure, and the call site must NOT block the request +// (fail-open). This test exercises the failingLimiter shape itself — the +// service-level fail-open assertion lives in the integration test where +// Server wiring is in scope. +func TestRateLimiterInterface_FailOpen(t *testing.T) { + want := errors.New("redis: connection refused") + l := &failingLimiter{err: want} + + ok, retry, err := l.Allow(context.Background(), "any-key") + if !errors.Is(err, want) { + t.Fatalf("want %v; got %v", want, err) + } + if ok { + t.Fatal("a backend-error response should not assert 'allowed'; the caller decides whether to fail open") + } + if retry != 0 { + t.Fatalf("backend-error retryAfter should be zero (no advice possible); got %v", retry) + } + if l.calls != 1 { + t.Fatalf("limiter called %d times; want 1", l.calls) + } +} + +// TestRateLimiterInterface_Compatibility is a compile-time-style check that +// any future RateLimiter implementation can be passed where the interface +// is expected. The compile-time `var _ RateLimiter = ...` assertion in +// rate_limiter.go already does this for TokenBucketLimiter; this test +// guarantees the same for an ad-hoc stub, ensuring the interface stays +// small enough that third-party implementations are not coupled to +// internal types. +func TestRateLimiterInterface_Compatibility(t *testing.T) { + var _ RateLimiter = (*TokenBucketLimiter)(nil) + var _ RateLimiter = (*failingLimiter)(nil) +} + +// TestTokenBucketLimiter_MaxBucketsFailsOpen proves the limiter fails open +// at capacity rather than allocating memory without bound. Critical when +// keys come from attacker-controllable input (login_hint). +func TestTokenBucketLimiter_MaxBucketsFailsOpen(t *testing.T) { + clock := newFakeClock(time.Unix(0, 0)) + l := newTestLimiter(t, 1, 1.0, clock) + l.MaxBuckets = 3 + + // Fill the map up to the cap with three distinct keys. + for _, k := range []string{"a", "b", "c"} { + ok, _, err := l.Allow(context.Background(), k) + if err != nil || !ok { + t.Fatalf("first request for key %q must be allowed; got (%v, _, %v)", k, ok, err) + } + } + if got := l.BucketCount(); got != 3 { + t.Fatalf("BucketCount = %d; want 3", got) + } + + // A fourth distinct key cannot be tracked. The limiter must fail + // open rather than allocating — and the bucket count must not grow. + ok, retry, err := l.Allow(context.Background(), "d") + if err != nil { + t.Fatalf("at-capacity Allow must not error; got %v", err) + } + if !ok { + t.Fatalf("at-capacity Allow must fail open (return true); got false") + } + if retry != 0 { + t.Fatalf("at-capacity Allow retryAfter must be 0; got %v", retry) + } + if got := l.BucketCount(); got != 3 { + t.Fatalf("BucketCount after at-capacity request = %d; want 3 (no growth)", got) + } + + // Existing keys must continue to be rate-limited normally. + if ok, _, _ := l.Allow(context.Background(), "a"); ok { + t.Fatal("existing key 'a' must still be rate-limited (its bucket is empty)") + } +} + +// TestTokenBucketLimiter_ReapIsBounded proves reap caps its per-pass work +// so a very large bucket map cannot stall Allow callers for an unbounded +// duration. Successive ticks pick up where the previous left off. +func TestTokenBucketLimiter_ReapIsBounded(t *testing.T) { + clock := newFakeClock(time.Unix(0, 0)) + l := newTestLimiter(t, 1, 1.0, clock) + l.MaxBuckets = maxReapBatch * 3 // populate well past the per-pass cap + + // Populate maxReapBatch*2 buckets (well above the per-pass cap). + total := maxReapBatch * 2 + for i := 0; i < total; i++ { + key := keyN(i) + if _, _, err := l.Allow(context.Background(), key); err != nil { + t.Fatalf("populate iter %d: %v", i, err) + } + } + if got := l.BucketCount(); got != total { + t.Fatalf("BucketCount = %d; want %d", got, total) + } + + // Age every bucket past IdleTTL so reap considers them all expired. + clock.advance(l.IdleTTL + time.Second) + + // One reap pass must scan at most maxReapBatch entries — so the count + // drops by at most that many, regardless of map size. + l.reap() + dropped := total - l.BucketCount() + if dropped > maxReapBatch { + t.Fatalf("single reap pass dropped %d entries; per-pass cap is %d", + dropped, maxReapBatch) + } + if dropped == 0 { + t.Fatal("reap dropped nothing — expected at least some entries to expire") + } +} + +func keyN(i int) string { return "k-" + strconv.Itoa(i) } diff --git a/scripts/demo/issue_139_bc_authorize_rate_limit.sh b/scripts/demo/issue_139_bc_authorize_rate_limit.sh new file mode 100755 index 00000000..32f399b0 --- /dev/null +++ b/scripts/demo/issue_139_bc_authorize_rate_limit.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# Demo script for issue #139 — bc-authorize rate limiting. +# +# Walks through: +# 1. Register a public OAuth client. +# 2. Fire 12 bc-authorize requests in quick succession. +# 3. Show the first 10 return 200 (within the 10/min per-client cap) +# and the remaining 2 return 429 + slow_down + Retry-After. +# +# Run zeroid first: make setup-keys && docker compose up -d +# Then run this: bash scripts/demo/issue_139_bc_authorize_rate_limit.sh + +set -euo pipefail + +BASE_URL="${BASE_URL:-http://localhost:8899}" +ACCOUNT_ID="${ACCOUNT_ID:-acct-demo}" +PROJECT_ID="${PROJECT_ID:-proj-demo}" +CLIENT_ID="${CLIENT_ID:-demo-ciba-$(date +%s)}" +LOGIN_HINT="${LOGIN_HINT:-victim@example.com}" + +bold() { printf '\033[1m%s\033[0m\n' "$*"; } +green() { printf '\033[32m%s\033[0m' "$*"; } +red() { printf '\033[31m%s\033[0m' "$*"; } +gray() { printf '\033[90m%s\033[0m' "$*"; } + +bold "─── 1. Register a public OAuth client ─────────────────────────────────" +echo "POST $BASE_URL/api/v1/oauth/clients" +echo " X-Account-ID: $ACCOUNT_ID" +echo " X-Project-ID: $PROJECT_ID" +echo " client_id: $CLIENT_ID" +echo +curl -sS -X POST "$BASE_URL/api/v1/oauth/clients" \ + -H "Content-Type: application/json" \ + -H "X-Account-ID: $ACCOUNT_ID" \ + -H "X-Project-ID: $PROJECT_ID" \ + -d "{ + \"client_id\": \"$CLIENT_ID\", + \"name\": \"issue-139-demo\", + \"grant_types\": [\"urn:openid:params:grant-type:ciba\", \"client_credentials\"], + \"backchannel_token_delivery_mode\": \"poll\" + }" | head -c 400 +echo +echo + +bold "─── 2. Fire 12 bc-authorize requests as the same client ───────────────" +echo "Per-client cap is 10/min by default → first 10 should pass, last 2 should be rate-limited." +echo "Per-user (login_hint) cap is 5/min, but the per-client check fires first." +echo "(Setting login_hint to a unique value per request to isolate the per-client dimension.)" +echo + +successes=0 +ratelimits=0 + +for i in $(seq 1 12); do + hint="iter-$i-$LOGIN_HINT" + # -w writes status + Retry-After header to stderr so the loop output stays + # readable. -D - dumps response headers; we grep just the ones we want. + response=$(curl -sS -i -X POST "$BASE_URL/oauth2/bc-authorize" \ + -H "Content-Type: application/json" \ + -d "{ + \"client_id\": \"$CLIENT_ID\", + \"account_id\": \"$ACCOUNT_ID\", + \"project_id\": \"$PROJECT_ID\", + \"login_hint\": \"$hint\", + \"scope\": \"openid\" + }") + + status=$(printf '%s' "$response" | awk 'NR==1 {print $2}') + retry=$(printf '%s' "$response" | awk 'tolower($1)=="retry-after:" {print $2}' | tr -d '\r') + body=$(printf '%s' "$response" | awk 'BEGIN{b=0} /^\r?$/ {b=1; next} b') + + case "$status" in + 200) + successes=$((successes+1)) + printf ' request %2d → %s ' "$i" "$(green "200 OK")" + auth_id=$(printf '%s' "$body" | python3 -c 'import json,sys;print(json.load(sys.stdin).get("auth_req_id",""))' 2>/dev/null || true) + printf 'auth_req_id=%s\n' "$(gray "${auth_id:0:24}…")" + ;; + 429) + ratelimits=$((ratelimits+1)) + err=$(printf '%s' "$body" | python3 -c 'import json,sys;print(json.load(sys.stdin).get("error",""))' 2>/dev/null || true) + printf ' request %2d → %s error=%s retry-after=%ss\n' \ + "$i" "$(red "429 Too Many Requests")" "$err" "$retry" + ;; + *) + printf ' request %2d → unexpected status %s\n' "$i" "$status" + printf '%s\n' "$body" | head -c 300 + ;; + esac +done + +echo +bold "─── 3. Result ─────────────────────────────────────────────────────────" +echo " successes : $successes (expected: 10)" +echo " rate-limits: $ratelimits (expected: 2)" +echo +if [[ "$successes" -eq 10 && "$ratelimits" -eq 2 ]]; then + printf '%s issue #139 rate limit is enforced as documented.\n' "$(green '✓')" + exit 0 +else + printf '%s unexpected — check zeroid logs for the bc_authorize_rate_limited WARN event.\n' "$(red '✗')" + exit 1 +fi diff --git a/server.go b/server.go index 819f0297..0dc725de 100644 --- a/server.go +++ b/server.go @@ -216,6 +216,12 @@ func NewServer(cfg Config) (*Server, error) { // otherwise-circular dependency cleanly. backchannelCfg := service.DefaultBackchannelConfig() backchannelCfg.AllowPrivateNotificationEndpoints = cfg.Backchannel.AllowPrivateNotificationEndpoints + if cfg.Backchannel.PerClientRateLimitPerMinute != nil { + backchannelCfg.PerClientRateLimit = *cfg.Backchannel.PerClientRateLimitPerMinute + } + if cfg.Backchannel.PerUserRateLimitPerMinute != nil { + backchannelCfg.PerUserRateLimit = *cfg.Backchannel.PerUserRateLimitPerMinute + } // Mirror the SSRF-guard relaxation flag onto OAuthClientService so the // registration-time check (in OAuthClientService.RegisterClient) and the // request-time check (in BackchannelService.CreateAuthRequest) agree. @@ -584,6 +590,59 @@ func (s *Server) SetBackchannelPingDispatchSync(sync bool) { s.backchannelSvc.SetPingDispatchSync(sync) } +// SetBackchannelRateLimits replaces the in-memory bc-authorize limit +// thresholds at runtime; 0 disables a dimension. Production deployments +// should set these via cfg.Backchannel at NewServer time. For non-default +// backends, use SetBackchannelRateLimiters. +func (s *Server) SetBackchannelRateLimits(perClientRPM, perUserRPM int) { + if s.backchannelSvc == nil { + return + } + s.backchannelSvc.SetRateLimits(perClientRPM, perUserRPM) +} + +// SetBackchannelRateLimiters installs deployer-supplied RateLimiter +// implementations for the per-client and per-user dimensions. Nil disables +// a dimension. Previously-installed limiters are stopped. +// +// This is the extension point for multi-replica deployments — see the +// RateLimiter doc for the interface contract. +func (s *Server) SetBackchannelRateLimiters(perClient, perUser RateLimiter) { + if s.backchannelSvc == nil { + return + } + s.backchannelSvc.SetRateLimiters( + wrapRateLimiter(perClient), + wrapRateLimiter(perUser), + ) +} + +// wrapRateLimiter adapts a public RateLimiter to the internal +// service.RateLimiter. Pass-through when the caller already supplies the +// internal type (e.g. *service.TokenBucketLimiter) to skip the wrapper. +func wrapRateLimiter(l RateLimiter) service.RateLimiter { + if l == nil { + return nil + } + if inner, ok := l.(service.RateLimiter); ok { + return inner + } + return rateLimiterAdapter{inner: l} +} + +// rateLimiterAdapter bridges the public and internal RateLimiter +// interfaces. They are signature-identical today; the adapter exists so +// either side can evolve independently. +type rateLimiterAdapter struct { + inner RateLimiter +} + +func (a rateLimiterAdapter) Allow(ctx context.Context, key string) (bool, time.Duration, error) { + return a.inner.Allow(ctx, key) +} + +func (a rateLimiterAdapter) Stop() { a.inner.Stop() } + // SetTrustedServiceValidator sets the validator used during external principal // token exchange (RFC 8693) to verify the caller is a trusted internal service. // The validator reads from context (populated by deployer-provided global middleware diff --git a/tests/integration/ciba_rate_limit_test.go b/tests/integration/ciba_rate_limit_test.go new file mode 100644 index 00000000..9b692a56 --- /dev/null +++ b/tests/integration/ciba_rate_limit_test.go @@ -0,0 +1,357 @@ +package integration_test + +import ( + "context" + "errors" + "net/http" + "strconv" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" + + zeroid "github.com/highflame-ai/zeroid" +) + +// TestCIBA_BcAuthorize_PerClientRateLimit covers the per-client token bucket +// on /oauth2/bc-authorize (issue #139 acceptance criteria): +// +// - 20 rapid requests as one client → first N succeed, the rest receive +// HTTP 429 with error="slow_down" and a Retry-After header. +// - The rate-limit check fires BEFORE the BackchannelNotifier dispatches — +// a client that exceeds its cap cannot spam end-user notifications. +// - The rate-limit check fires BEFORE persistence — the +// backchannel_auth_requests table does not accumulate rows from the +// rejected requests. +func TestCIBA_BcAuthorize_PerClientRateLimit(t *testing.T) { + clientID := uid("ciba-rl-perclient") + registerTestOAuthClient(clientID, []string{"client_credentials"}) + + // Drive limits to a small, deterministic value so 20 requests cross the + // threshold inside a single sub-second test step. Per-user is set high + // so this test isolates the per-client path; the next test inverts it. + const allowed = 5 + testZeroIDServer.SetBackchannelRateLimits(allowed, 1000) + t.Cleanup(func() { + // Restore production defaults for subsequent tests. + testZeroIDServer.SetBackchannelRateLimits(10, 5) + }) + + var notifyCount int64 + testZeroIDServer.SetBackchannelNotifier(func(_ context.Context, _ zeroid.BackchannelNotification) error { + atomic.AddInt64(¬ifyCount, 1) + return nil + }) + testZeroIDServer.SetBackchannelNotifyDispatchSync(true) + t.Cleanup(func() { + testZeroIDServer.SetBackchannelNotifyDispatchSync(false) + testZeroIDServer.SetBackchannelNotifier(nil) + }) + + preRows := countBackchannelRowsForClient(t, clientID) + + const ( + burst = 20 + loginHint = "ratelimit-victim@example.com" + ) + + var ( + successes int + rateLimits int + ) + for i := 0; i < burst; i++ { + resp := post(t, "/oauth2/bc-authorize", map[string]any{ + "client_id": clientID, + "account_id": testAccountID, + "project_id": testProjectID, + "login_hint": loginHint, + "scope": "openid", + }, nil) + + switch resp.StatusCode { + case http.StatusOK: + successes++ + _ = resp.Body.Close() + case http.StatusTooManyRequests: + rateLimits++ + body := decode(t, resp) + require.Equal(t, "slow_down", body["error"], + "429 responses must carry error=slow_down per CIBA Core §11; body=%v", body) + + ra := resp.Header.Get("Retry-After") + require.NotEmpty(t, ra, "429 responses must include a Retry-After header (RFC 7231 §7.1.3)") + raSecs, err := strconv.Atoi(ra) + require.NoError(t, err, "Retry-After must be integer delta-seconds; got %q", ra) + require.GreaterOrEqual(t, raSecs, 1, "Retry-After must be ≥ 1s") + default: + t.Fatalf("unexpected status %d on bc-authorize request %d", resp.StatusCode, i) + } + } + + // First N succeed, remainder are rate-limited — the primary acceptance + // criterion. Token-bucket bursting means we get exactly `allowed` + // successes when traffic arrives faster than the refill rate (which a + // sub-second loop trivially does). + require.Equal(t, allowed, successes, + "first %d requests must succeed (token-bucket capacity)", allowed) + require.Equal(t, burst-allowed, rateLimits, + "remaining %d requests must be rate-limited with 429", burst-allowed) + + // Rate limit fires BEFORE the notifier — attacker cannot spam end-user + // notifications even with high request volume. + require.Equal(t, int64(allowed), atomic.LoadInt64(¬ifyCount), + "notifier must fire only for the %d allowed requests; rejected requests must not invoke the notifier", allowed) + + // Rate limit fires BEFORE persistence — the table is protected from + // the DoS-via-pending-rows attack surface. + postRows := countBackchannelRowsForClient(t, clientID) + require.Equal(t, int64(allowed), postRows-preRows, + "backchannel_auth_requests must accumulate only the %d allowed rows; rejected requests must not persist", allowed) +} + +// TestCIBA_BcAuthorize_PerUserRateLimit covers the per-user (login_hint) cap: +// a single user cannot be spammed across many clients in the same tenant. +// +// Two distinct clients post to the same login_hint. The per-user cap is +// driven below the per-client cap so the per-user limiter fires first. +func TestCIBA_BcAuthorize_PerUserRateLimit(t *testing.T) { + clientA := uid("ciba-rl-userA") + clientB := uid("ciba-rl-userB") + registerTestOAuthClient(clientA, []string{"client_credentials"}) + registerTestOAuthClient(clientB, []string{"client_credentials"}) + + // per-user = 3, per-client = high → per-user trips before per-client. + const userCap = 3 + testZeroIDServer.SetBackchannelRateLimits(1000, userCap) + t.Cleanup(func() { + testZeroIDServer.SetBackchannelRateLimits(10, 5) + }) + + const ( + burst = 10 + loginHint = "shared-victim@example.com" + ) + + successes := 0 + rejections := 0 + for i := 0; i < burst; i++ { + // Alternate between clients so the per-client bucket is never + // approached and only the per-user bucket can be the cause of any + // 429. + cid := clientA + if i%2 == 1 { + cid = clientB + } + resp := post(t, "/oauth2/bc-authorize", map[string]any{ + "client_id": cid, + "account_id": testAccountID, + "project_id": testProjectID, + "login_hint": loginHint, + }, nil) + + switch resp.StatusCode { + case http.StatusOK: + successes++ + case http.StatusTooManyRequests: + rejections++ + body := decode(t, resp) + require.Equal(t, "slow_down", body["error"], "body=%v", body) + default: + t.Fatalf("unexpected status %d on iteration %d", resp.StatusCode, i) + } + _ = resp.Body.Close() + } + + require.Equal(t, userCap, successes, + "per-user cap must throttle requests targeting one login_hint regardless of client_id") + require.Equal(t, burst-userCap, rejections) +} + +// TestCIBA_BcAuthorize_CustomRateLimiter proves the extension point: +// deployers can plug a custom zeroid.RateLimiter into the bc-authorize +// pipeline via Server.SetBackchannelRateLimiters without any changes to +// zeroid internals. The same hook is what production deployments use to +// swap in a Redis-backed (or any other shared-store) limiter for multi- +// instance correctness. +// +// This test installs a deny-everything stub on the per-client dimension +// and asserts every request gets 429 + slow_down + Retry-After through the +// deployer-supplied limiter. The reverse case (backend error → fail open) +// is asserted in TestCIBA_BcAuthorize_CustomRateLimiter_FailOpen. +func TestCIBA_BcAuthorize_CustomRateLimiter(t *testing.T) { + clientID := uid("ciba-rl-custom") + registerTestOAuthClient(clientID, []string{"client_credentials"}) + + stub := &recordingLimiter{ + decide: func(_ context.Context, _ string) (bool, time.Duration, error) { + return false, 7 * time.Second, nil + }, + } + testZeroIDServer.SetBackchannelRateLimiters(stub, nil) // per-user disabled + t.Cleanup(func() { + // Restore the default in-memory backend at production defaults. + testZeroIDServer.SetBackchannelRateLimits(10, 5) + }) + + resp := post(t, "/oauth2/bc-authorize", map[string]any{ + "client_id": clientID, + "account_id": testAccountID, + "project_id": testProjectID, + "login_hint": "custom-limiter@example.com", + }, nil) + defer resp.Body.Close() + + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode, + "custom limiter denial must surface as 429") + body := decode(t, resp) + require.Equal(t, "slow_down", body["error"]) + require.Equal(t, "7", resp.Header.Get("Retry-After"), + "custom limiter's retryAfter must be plumbed verbatim through the Retry-After header") + require.Equal(t, int64(1), stub.calls(), "custom limiter must be invoked exactly once per request") +} + +// TestCIBA_BcAuthorize_TenantBypassClosed proves that varying tenant IDs in +// the request body does NOT reset the per-client rate-limit bucket. The +// previous design keyed buckets on (client_id, account_id, project_id), +// which let an attacker bypass the cap by supplying random tenant values. +// The bucket key is now just client_id. +func TestCIBA_BcAuthorize_TenantBypassClosed(t *testing.T) { + clientID := uid("ciba-rl-tenant-bypass") + registerTestOAuthClient(clientID, []string{"client_credentials"}) + + const allowed = 3 + testZeroIDServer.SetBackchannelRateLimits(allowed, 1000) + t.Cleanup(func() { testZeroIDServer.SetBackchannelRateLimits(10, 5) }) + + // Fire `allowed+3` requests, each with a unique account_id + project_id + // pair. If the limiter keyed on the tenant tuple, every request would + // look like a fresh bucket and all would succeed (the bypass). With the + // fix, only the first `allowed` succeed. + successes := 0 + rejections := 0 + for i := 0; i < allowed+3; i++ { + resp := post(t, "/oauth2/bc-authorize", map[string]any{ + "client_id": clientID, + "account_id": "acct-bypass-" + strconv.Itoa(i), + "project_id": "proj-bypass-" + strconv.Itoa(i), + "login_hint": "victim-" + strconv.Itoa(i) + "@example.com", + }, nil) + switch resp.StatusCode { + case http.StatusOK: + successes++ + case http.StatusTooManyRequests: + rejections++ + default: + t.Fatalf("unexpected status %d on iteration %d", resp.StatusCode, i) + } + _ = resp.Body.Close() + } + + require.Equal(t, allowed, successes, + "per-client cap must hold even when account_id/project_id vary per request") + require.Equal(t, 3, rejections) +} + +// TestCIBA_BcAuthorize_SetRateLimitersSelfStop proves the limiter passed +// into Server.SetBackchannelRateLimiters is NOT stopped when it equals the +// existing instance. The default in-memory impl's reaper goroutine cannot +// be restarted, so an unconditional Stop would brick a re-installed limiter. +func TestCIBA_BcAuthorize_SetRateLimitersSelfStop(t *testing.T) { + stub := &recordingLimiter{ + decide: func(_ context.Context, _ string) (bool, time.Duration, error) { + return true, 0, nil + }, + } + + testZeroIDServer.SetBackchannelRateLimiters(stub, nil) + t.Cleanup(func() { testZeroIDServer.SetBackchannelRateLimits(10, 5) }) + + // Install the same instance again. The implementation must NOT call + // Stop() on a limiter it's reinstalling. + testZeroIDServer.SetBackchannelRateLimiters(stub, nil) + + stub.mu.Lock() + stopped := stub.stopped + stub.mu.Unlock() + require.False(t, stopped, + "limiter must not be Stop()ped when SetBackchannelRateLimiters reinstalls the same instance") +} + +// TestCIBA_BcAuthorize_CustomRateLimiter_FailOpen proves the documented +// fail-open contract: when a deployer-supplied RateLimiter returns a +// backend error (e.g. Redis unreachable), the service permits the request +// rather than locking out legitimate traffic. Critical for production +// deployments — the rate limiter must never DoS the authentication service +// it is supposed to protect. +func TestCIBA_BcAuthorize_CustomRateLimiter_FailOpen(t *testing.T) { + clientID := uid("ciba-rl-failopen") + registerTestOAuthClient(clientID, []string{"client_credentials"}) + + stub := &recordingLimiter{ + decide: func(_ context.Context, _ string) (bool, time.Duration, error) { + return false, 0, errors.New("simulated backend outage") + }, + } + testZeroIDServer.SetBackchannelRateLimiters(stub, nil) + t.Cleanup(func() { + testZeroIDServer.SetBackchannelRateLimits(10, 5) + }) + + resp := post(t, "/oauth2/bc-authorize", map[string]any{ + "client_id": clientID, + "account_id": testAccountID, + "project_id": testProjectID, + "login_hint": "failopen@example.com", + }, nil) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode, + "limiter backend error must fail open — the auth service must not be locked out by a dependency outage") + require.Equal(t, int64(1), stub.calls()) +} + +// recordingLimiter is a deployer-supplied zeroid.RateLimiter used to drive +// the extension-point integration tests. The decide closure is invoked +// inside Allow so each test can return the precise (allowed, retryAfter, +// err) triple it needs. +type recordingLimiter struct { + mu sync.Mutex + nCalls int64 + stopped bool + decide func(ctx context.Context, key string) (bool, time.Duration, error) +} + +// Compile-time guarantee the test stub satisfies the public RateLimiter +// contract. Catches signature drift if the interface evolves. +var _ zeroid.RateLimiter = (*recordingLimiter)(nil) + +func (r *recordingLimiter) Allow(ctx context.Context, key string) (bool, time.Duration, error) { + atomic.AddInt64(&r.nCalls, 1) + return r.decide(ctx, key) +} + +func (r *recordingLimiter) Stop() { + r.mu.Lock() + defer r.mu.Unlock() + r.stopped = true +} + +func (r *recordingLimiter) calls() int64 { return atomic.LoadInt64(&r.nCalls) } + +// countBackchannelRowsForClient returns the current row count in +// backchannel_auth_requests for the given client_id. Used to verify that +// rate-limited bc-authorize requests do not persist rows (criterion: rate +// check happens before insertion). +func countBackchannelRowsForClient(t *testing.T, clientID string) int64 { + t.Helper() + var n int64 + err := testDB.NewSelect(). + TableExpr("backchannel_auth_requests"). + ColumnExpr("count(*)"). + Where("client_id = ?", clientID). + Scan(context.Background(), &n) + require.NoError(t, err) + return n +}