Skip to content

feat(security): make the auth-flow user scope independently enforceable (currently subsumed by IP scope) #506

Description

@ericfitz

Summary

The auth-flow rate limiter (api/auth_flow_rate_limiter.go) enforces three scopes — session, IP, and user — but all three currently use the same 100/60s limit, and checkRateLimitWithIPLimit evaluates them in the order session → IP → user. Because every request from a single client shares one source IP, the IP-scope counter accumulates at least as fast as the user-scope counter, and (being checked first, at the same limit) the IP scope always trips first. As a result, the user scope can never be the attributed blocker for real traffic originating from a single IP — it is effectively subsumed by the IP scope.

This was surfaced while implementing the multi-scope integration test in #505. That test can only exercise the user scope by spoofing a unique X-Forwarded-For per request (the same technique the unit tests use), which isolates the user counter from the IP counter. That is a valid test, but it does not reflect how the limiter behaves against a real single-IP attacker.

Why it matters

The user scope exists to throttle attacks keyed on a single account/login_hint (e.g. password-spray against one victim) independently of the coarse shared-IP limit — important for the corporate-NAT case where you'd rather throttle the specific user than the whole egress IP. As currently configured it provides no independent protection beyond what the IP scope already gives.

Options to weigh (needs a product/security decision)

  1. Differentiate the limits so the per-user limit is lower than the per-IP limit (e.g. user 100, IP higher), letting the user scope bite before the IP scope. Changes production limit values.
  2. Reorder the checks to most-specific-first (session → user → IP) so the user scope is attributed before the shared-IP scope when both would trip. This is a near-zero blocking-behavior change (it only changes the attributed X-RateLimit-Scope in the pure same-user/same-IP case, where user and IP counts tie), and is arguably the more correct attribution.
  3. Accept the current behavior and document that, for single-IP traffic, the user scope is intentionally subsumed by the IP scope (in which case the per-user counter is mainly meaningful for proxied/XFF traffic).

Acceptance Criteria

  • Decide which option (or combination) to adopt, with security sign-off on any limit/order change.
  • If behavior changes: update checkRateLimitWithIPLimit and the unit tests in api/rate_limit_middleware_test.go.
  • Update the integration test TestAuthFlowRateLimiting_MultiScope so the user-scope assertion reflects real (not XFF-isolated) behavior, if applicable.

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions