fix(oidc): per-IP rate limit on /oauth2/{authorize,token,userinfo,revoke} #281

Merged
YousifShkara merged 1 commit from fix/BUNYIP-264-oauth-rate-limiters into main 2026-06-30 06:39:33 +02:00
Owner

BUNYIP-264: the /oauth2/* family was unmetered, leaving the token endpoint as an open brute-force surface for client_secret_basic and giving authorize / userinfo / revoke no cap against junk traffic. The audit named this medium-severity.

Reuses the existing rate-limit infrastructure (rate_limits table, RateLimitRepository::check_and_increment). A new convenience wrapper RateLimitRepository::check_rate_limit mirrors the bunyip-api check_rate_limit so bunyip-oidc handlers can call it directly without crossing the api -> oidc dependency direction.

Four new RateLimitConfig constants, keyed per-IP:

  • OAUTH_TOKEN: 60 / min (covers SPA refresh + hub silent-SSO + e2e bursts).
  • OAUTH_AUTHORIZE: 120 / min (multi-tab power users juggling apps).
  • OAUTH_USERINFO: 240 / min (silent-SSO + RP profile hydration are chatty).
  • OAUTH_REVOKE: 60 / min (one revoke per logout; sustained traffic is abuse).

Both /oauth2/authorize and /oauth2/token enforce the limit BEFORE any DB / discovery work, so a flood at the cap shape never reaches the rest of the pipeline. /oauth2/userinfo enforces before bearer parsing. /oauth2/revoke enforces before client authentication. The exceeded-cap response is the existing AppError::RateLimited { retry_after } (429 + Retry-After header).

Discovery (/.well-known/openid-configuration) and JWKS (/.well-known/jwks.json) are deliberately out of scope here: their handler signatures lack req and pool extractors, the responses are cached aggressively by every conforming RP (5-minute Cache-Control already), and they're not a typical brute-force surface. A follow-up can shape that work separately.

#BUNYIP-264

BUNYIP-264: the `/oauth2/*` family was unmetered, leaving the token endpoint as an open brute-force surface for `client_secret_basic` and giving authorize / userinfo / revoke no cap against junk traffic. The audit named this medium-severity. Reuses the existing rate-limit infrastructure (`rate_limits` table, `RateLimitRepository::check_and_increment`). A new convenience wrapper `RateLimitRepository::check_rate_limit` mirrors the bunyip-api `check_rate_limit` so bunyip-oidc handlers can call it directly without crossing the api -> oidc dependency direction. Four new `RateLimitConfig` constants, keyed per-IP: - `OAUTH_TOKEN`: 60 / min (covers SPA refresh + hub silent-SSO + e2e bursts). - `OAUTH_AUTHORIZE`: 120 / min (multi-tab power users juggling apps). - `OAUTH_USERINFO`: 240 / min (silent-SSO + RP profile hydration are chatty). - `OAUTH_REVOKE`: 60 / min (one revoke per logout; sustained traffic is abuse). Both `/oauth2/authorize` and `/oauth2/token` enforce the limit BEFORE any DB / discovery work, so a flood at the cap shape never reaches the rest of the pipeline. `/oauth2/userinfo` enforces before bearer parsing. `/oauth2/revoke` enforces before client authentication. The exceeded-cap response is the existing `AppError::RateLimited { retry_after }` (429 + `Retry-After` header). Discovery (`/.well-known/openid-configuration`) and JWKS (`/.well-known/jwks.json`) are deliberately out of scope here: their handler signatures lack `req` and `pool` extractors, the responses are cached aggressively by every conforming RP (5-minute Cache-Control already), and they're not a typical brute-force surface. A follow-up can shape that work separately. #BUNYIP-264
fix(oidc): per-IP rate limit on /oauth2/{authorize,token,userinfo,revoke}
All checks were successful
Check / fmt + clippy + build + tests (pull_request) Successful in 36m36s
E2E / Playwright against deployment (pull_request) Successful in 54s
Create release / Create release from merged PR (pull_request) Has been skipped
791c6b4aab
BUNYIP-264: the `/oauth2/*` family was unmetered, leaving the token endpoint as an open brute-force surface for `client_secret_basic` and giving authorize / userinfo / revoke no cap against junk traffic. The audit named this medium-severity.

Reuses the existing rate-limit infrastructure (`rate_limits` table, `RateLimitRepository::check_and_increment`). A new convenience wrapper `RateLimitRepository::check_rate_limit` mirrors the bunyip-api `check_rate_limit` so bunyip-oidc handlers can call it directly without crossing the api -> oidc dependency direction.

Four new `RateLimitConfig` constants, keyed per-IP:

- `OAUTH_TOKEN`: 60 / min (covers SPA refresh + hub silent-SSO + e2e bursts).
- `OAUTH_AUTHORIZE`: 120 / min (multi-tab power users juggling apps).
- `OAUTH_USERINFO`: 240 / min (silent-SSO + RP profile hydration are chatty).
- `OAUTH_REVOKE`: 60 / min (one revoke per logout; sustained traffic is abuse).

Both `/oauth2/authorize` and `/oauth2/token` enforce the limit BEFORE any DB / discovery work, so a flood at the cap shape never reaches the rest of the pipeline. `/oauth2/userinfo` enforces before bearer parsing. `/oauth2/revoke` enforces before client authentication. The exceeded-cap response is the existing `AppError::RateLimited { retry_after }` (429 + `Retry-After` header).

Discovery (`/.well-known/openid-configuration`) and JWKS (`/.well-known/jwks.json`) are deliberately out of scope here: their handler signatures lack `req` and `pool` extractors, the responses are cached aggressively by every conforming RP (5-minute Cache-Control already), and they're not a typical brute-force surface. A follow-up can shape that work separately.

#BUNYIP-264
YousifShkara deleted branch fix/BUNYIP-264-oauth-rate-limiters 2026-06-30 06:39:33 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/bunyip!281
No description provided.