fix(oidc): emit redirect full authorize URL to login bridge, not return_to

longjacksonle commented

2026-06-05 04:21:12 +02:00

Owner

What

Corrects the OP's side of the SSO login-redirect bridge. The /oauth2/authorize handler, on NeedsLogin, redirected to the login UI (bunyip-web) with ?return_to=<bare query string>. bunyip-web's login implements the bridge as ?redirect=<absolute URL> (bunyip-web/src/handlers/auth_pages.rs): it reads a redirect param, and safe_redirect only accepts a /-relative path or an absolute URL whose origin equals its configured issuer, falling back to /dashboard otherwise. So the OP's return_to was ignored and its bare-query value would have been rejected regardless, stranding the user on the bunyip dashboard instead of completing the round-trip.

This emits ?redirect=<issuer>/oauth2/authorize?<serialized request>: correct param name, full absolute URL on the OP issuer's origin that bunyip's allowlist accepts and bounces back to.

Scope: PARTIAL FIX (necessary, not sufficient)

This fixes the contract mismatch but does NOT by itself make the federated login complete, because mokosh and bunyip do not share an OP session in the current dev-sso topology:

mokosh-server OP sessions live in its own mokosh database; OP cookie domain .a8n.run.
bunyip-api authenticates against a separate bunyip database and sets host-scoped access_token/refresh_token cookies; it never creates a mokosh OP sid session.

With this change applied in isolation, the post-login bounce to mokosh-api/oauth2/authorize still finds no session, re-enters NeedsLogin, and would redirect-loop against bunyip rather than land on the dashboard. Completing the flow needs session federation between the two services (bunyip login proxying into mokosh /v1/auth/login, or mokosh trusting a bunyip-issued token) - out of scope here and tracked separately. First-party SPAs can authenticate directly via /v1/auth/login today and skip this bridge entirely.

Why merge the partial anyway

The param-name + value-shape mismatch is a genuine latent bug in the bridge contract; fixing it now is a prerequisite for the eventual session-federation work and removes one of the two blockers. It is intentionally not wired into a live flow by this PR.

Verification

cargo fmt -p mokosh-auth-http --check, cargo clippy -p mokosh-auth-http -- -D warnings, and cargo build -p mokosh-auth-http all clean. No behavior is exercised by an integration test because the round-trip cannot complete until session federation lands; this is a contract-only correction.

🤖 Generated with Claude Code

## What Corrects the OP's side of the SSO login-redirect bridge. The `/oauth2/authorize` handler, on `NeedsLogin`, redirected to the login UI (bunyip-web) with `?return_to=<bare query string>`. bunyip-web's login implements the bridge as `?redirect=<absolute URL>` (`bunyip-web/src/handlers/auth_pages.rs`): it reads a `redirect` param, and `safe_redirect` only accepts a `/`-relative path or an absolute URL whose origin equals its configured issuer, falling back to `/dashboard` otherwise. So the OP's `return_to` was ignored and its bare-query value would have been rejected regardless, stranding the user on the bunyip dashboard instead of completing the round-trip. This emits `?redirect=<issuer>/oauth2/authorize?<serialized request>`: correct param name, full absolute URL on the OP issuer's origin that bunyip's allowlist accepts and bounces back to. ## Scope: PARTIAL FIX (necessary, not sufficient) This fixes the contract mismatch but does NOT by itself make the federated login complete, because mokosh and bunyip do not share an OP session in the current dev-sso topology: - mokosh-server OP sessions live in its own `mokosh` database; OP cookie domain `.a8n.run`. - bunyip-api authenticates against a separate `bunyip` database and sets host-scoped `access_token`/`refresh_token` cookies; it never creates a mokosh OP `sid` session. With this change applied in isolation, the post-login bounce to `mokosh-api/oauth2/authorize` still finds no session, re-enters `NeedsLogin`, and would redirect-loop against bunyip rather than land on the dashboard. Completing the flow needs session federation between the two services (bunyip login proxying into mokosh `/v1/auth/login`, or mokosh trusting a bunyip-issued token) - out of scope here and tracked separately. First-party SPAs can authenticate directly via `/v1/auth/login` today and skip this bridge entirely. ## Why merge the partial anyway The param-name + value-shape mismatch is a genuine latent bug in the bridge contract; fixing it now is a prerequisite for the eventual session-federation work and removes one of the two blockers. It is intentionally not wired into a live flow by this PR. ## Verification `cargo fmt -p mokosh-auth-http --check`, `cargo clippy -p mokosh-auth-http -- -D warnings`, and `cargo build -p mokosh-auth-http` all clean. No behavior is exercised by an integration test because the round-trip cannot complete until session federation lands; this is a contract-only correction. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

longjacksonle added 1 commit

2026-06-05 04:21:12 +02:00

fix(oidc): emit redirect full authorize URL to login bridge, not return_to

Check / fmt + clippy + compile + tests (pull_request) Successful in 1m6s

Details

Build OCI container / Build and push mokosh-api image (push) Successful in 3m14s

Details

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

Details

8b11ea85fc

The OP's authorize handler, when a user is unauthenticated, redirects to the login UI (bunyip-web) so they can sign in and resume the flow. It was appending the original request as `?return_to=<bare query string>`, but bunyip-web's login implements the redirect-bridge contract as `?redirect=<absolute URL>`: it reads a `redirect` param and its `safe_redirect` only accepts a `/`-relative path or an absolute URL whose origin equals the configured issuer, falling back to its own `/dashboard` for anything else. So the OP's `return_to` param was ignored and its bare-query-string value would have been rejected anyway, stranding the user on the bunyip dashboard instead of completing the OIDC round-trip.

This emits `?redirect=<issuer>/oauth2/authorize?<serialized request>` instead: the correct param name, and a full absolute URL on the OP issuer's origin that bunyip's allowlist accepts and bounces back to so the authorize flow resumes.

PARTIAL FIX - necessary but not sufficient. This corrects the OP's side of the bridge contract, but the round-trip still cannot complete in the current dev-sso topology because mokosh and bunyip do not share an OP session: mokosh-server's OP sessions live in its own `mokosh` database with cookie domain `.a8n.run`, while bunyip-api authenticates against its separate `bunyip` database and sets host-scoped `access_token`/`refresh_token` cookies, never creating a mokosh OP `sid` session. With this change applied in isolation, the post-login bounce to mokosh-api `/oauth2/authorize` still finds no session and re-enters `NeedsLogin`, which would redirect-loop against bunyip rather than land on the dashboard. Completing the federated flow requires wiring session federation between the two services (bunyip login proxying into mokosh `/v1/auth/login`, or mokosh trusting a bunyip-issued token) - tracked separately. First-party SPAs can authenticate directly via `/v1/auth/login` today and skip this bridge entirely.

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

longjacksonle referenced this pull request

2026-06-05 19:36:39 +02:00

feat(dev-sso): validate bunyip-issued tokens as a Resource Server #113

longjacksonle commented

2026-06-05 19:37:11 +02:00

Author

Owner

Superseded by #113 (Resource Server cutover). Re-analysis: dev-sso was wired with mokosh-server as the OP, but staging already runs bunyip-as-OP (api.a8n.systems serves the live discovery doc; msp-api 404s). This PR patched mokosh-server's own OP login bridge, the wrong layer once bunyip-api is the sole OP. Closing; the fix is the three-repo alignment (#113 here, bunyip #66, mokosh-apps #67).