fix(oidc): honor checked= flag on /login to break authorize redirect loop #74

Merged

David merged 1 commit from fix/oidc-login-redirect-loop-honor-checked into main

2026-06-08 14:42:18 +02:00

David commented

2026-06-08 14:28:24 +02:00

Owner

Problem

Returning users hit an infinite redirect loop on RP (e.g. Mokosh) login: /oauth2/authorize (bunyip-api) and /login (bunyip-web) bounce forever until the browser aborts with NS_ERROR_REDIRECT_LOOP.

bunyip has two independent "signed in" checks. bunyip-web's is_signed_in() succeeds whenever /v1/users/me works with the hub access_token, auto-refreshed from the 30-day "remember me" refresh_token cookie. bunyip-api's authorize gates only on a server-side op_sessions row, which hard-expires after 7 days with no sliding renewal. Once the OP session expires (or is cleared by a logout-elsewhere or an api redeploy) while the hub refresh cookie is still valid, the two checks disagree: authorize 302s to /login?...&checked=1, login_get sees the hub session and 303s back to authorize, repeat.

authorize already appends &checked=1 as the intended loop-breaker, but RedirectQuery never deserialized checked, so it was silently dropped.

Fix

Add checked: Option<String> to RedirectQuery and guard the signed-in shortcut in login_get with q.checked.is_none(). When checked is present the IdP has already proven there is no OP session, so login_get renders the login form instead of redirecting back. The form carries the original ?redirect= (the authorize URL), so the POST mints a fresh OP session via establish_op_session and the authorize round-trip then completes. Behavior without checked is unchanged.

Testing

cargo fmt --check and cargo clippy --workspace --all-targets -D warnings clean in the pinned rust-builder image.
cargo test -p bunyip-web --bins: 12/12 pass (including the safe_redirect suite in the touched file).
The only failing workspace test, config::tests::test_config_defaults, is a pre-existing in-suite env leak (the test never removes APP_PORT/PORT) in a crate this PR does not touch.

Fixes #BUNYIP-56

🤖 Generated with Claude Code

## Problem Returning users hit an infinite redirect loop on RP (e.g. Mokosh) login: `/oauth2/authorize` (bunyip-api) and `/login` (bunyip-web) bounce forever until the browser aborts with `NS_ERROR_REDIRECT_LOOP`. bunyip has two independent "signed in" checks. bunyip-web's `is_signed_in()` succeeds whenever `/v1/users/me` works with the hub `access_token`, auto-refreshed from the 30-day "remember me" `refresh_token` cookie. bunyip-api's `authorize` gates only on a server-side `op_sessions` row, which hard-expires after 7 days with no sliding renewal. Once the OP session expires (or is cleared by a logout-elsewhere or an api redeploy) while the hub refresh cookie is still valid, the two checks disagree: `authorize` 302s to `/login?...&checked=1`, `login_get` sees the hub session and 303s back to `authorize`, repeat. `authorize` already appends `&checked=1` as the intended loop-breaker, but `RedirectQuery` never deserialized `checked`, so it was silently dropped. ## Fix Add `checked: Option<String>` to `RedirectQuery` and guard the signed-in shortcut in `login_get` with `q.checked.is_none()`. When `checked` is present the IdP has already proven there is no OP session, so `login_get` renders the login form instead of redirecting back. The form carries the original `?redirect=` (the authorize URL), so the POST mints a fresh OP session via `establish_op_session` and the authorize round-trip then completes. Behavior without `checked` is unchanged. ## Testing - `cargo fmt --check` and `cargo clippy --workspace --all-targets -D warnings` clean in the pinned rust-builder image. - `cargo test -p bunyip-web --bins`: 12/12 pass (including the `safe_redirect` suite in the touched file). - The only failing workspace test, `config::tests::test_config_defaults`, is a pre-existing in-suite env leak (the test never removes `APP_PORT`/`PORT`) in a crate this PR does not touch. Fixes #BUNYIP-56 🤖 Generated with [Claude Code](https://claude.com/claude-code)

David added 1 commit

2026-06-08 14:28:24 +02:00

fix(oidc): honor checked= flag on /login to break authorize loop

Check / fmt / clippy / build / test (pull_request) Successful in 1m2s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

8d9e4de233

/oauth2/authorize redirects to /login?redirect=<authorize>&checked=1 when it finds no valid OP session, but RedirectQuery never deserialized checked, so login_get redirected straight back to authorize whenever the hub session still looked signed in. With the 30-day refresh cookie outliving the 7-day OP session (or after a logout-elsewhere / api redeploy), the two checks disagree and the browser loops until NS_ERROR_REDIRECT_LOOP.

Add checked to RedirectQuery and guard the signed-in shortcut with q.checked.is_none(). When checked is present the IdP has already proven there is no OP session, so login_get falls through to the login form; the POST then mints a fresh OP session and the authorize round-trip completes.

#BUNYIP-56

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>