Serve /oauth/authorize as a 302 instead of a 200 + JS redirect

[aterga]

What

The authorize hop to the Internet Identity connect link was served as a 200 HTML page whose inline script did location.replace(...). This makes it a real HTTP 302 (Location) instead: one fewer place a redirect can be interfered with, and it works with JavaScript disabled.

Why the old code used JS, and why the 302 is safe

js_redirect's doc said it used a script hop so "a Location redirect drops the fragment in some clients" — the II link carries its params in the URL fragment (#callback=…&state=…&registration_key=…). That concern does not affect the real path:

• Modern browsers preserve a fragment present in a Location header (RFC 7231 §7.1.2); the fragment never goes on the wire, so beta II's frontend reads it from location.hash exactly as before.
• The clients that would drop the fragment are non-browser redirect-followers, which can't complete an interactive II sign-in anyway (the fragment exists for browser JS at beta.id.ai to consume).

Changes

• authorize now returns redirect_302(&ii_url) (still setting the mcp_connect cookie on that response). redirect_302 also sets Referrer-Policy: no-referrer — the authorize query carries only non-secret OAuth params (client_id, redirect_uri, PKCE challenge, state), so that's tidiness, not a leak fix. Its doc comment is generalized to cover both top-level hops (out to II, and back to the client).
• Removed the now-unused js_redirect helper. js_escape stays (still used by the finishing page and the pinned page's redeem URL).

Testing

• cargo build --locked --all-targets + cargo test --locked --all-targets green (111 tests). No test asserted the authorize response shape, so none needed changing.
• Server: GET /oauth/authorize now returns 302 Found with Location: https://beta.id.ai/mcp#callback=…&state=…&registration_key=… (fragment intact), plus the connect cookie and Referrer-Policy: no-referrer.
• Browser: a headless-Chromium run following a 302 whose Location carries a fragment confirmed the full fragment (callback, state, registration_key) survives into location.hash — i.e. beta II will read it unchanged.

Note (as discussed): this is a general robustness nicety and does not address a blocked-popup failure; it just removes the script-driven interposition on the authorize hop.

🤖 Generated with Claude Code

https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym

---

Generated by Claude Code

[Copilot]

Pull request overview

This PR replaces the /oauth/authorize “JS-driven redirect page” with a real HTTP 302 Found redirect to the Internet Identity /mcp URL (including its fragment), improving robustness and working with JavaScript disabled.

Changes:

• Switch /oauth/authorize from js_redirect(&ii_url) (200 + inline JS) to redirect_302(&ii_url) (302 + Location).
• Generalize and expand the redirect_302 doc comment to cover both redirect hops and ensure Referrer-Policy: no-referrer is set.
• Remove the now-unused js_redirect helper (keeping js_escape for other pages).

Comments suppressed due to low confidence (1)

src/auth.rs:503

• The authorize redirect behavior changed from an HTML + JS hop to an HTTP 302, but there’s still no test coverage asserting the response shape (status + Location + Referrer-Policy + Set-Cookie). Since this file already has unit tests, adding a small test around the redirect helper (and/or the authorize handler) would prevent regressions (e.g., accidentally reintroducing a 200 page or dropping the Referrer-Policy header).

    // Redirect the consenting browser to the II connect link with a real HTTP
    // 302 (`Location`). The link's params ride in the URL fragment
    // (`#callback=…&state=…&registration_key=…`); modern browsers preserve a
    // fragment present in a `Location` header (RFC 7231 §7.1.2), and the fragment
    // never goes on the wire, so beta II's frontend still reads it from
    // `location.hash` exactly as before, with one fewer interposition point than
    // a script-driven hop, and it works with JS disabled. `redirect_302` also
    // sets `Referrer-Policy: no-referrer` (the authorize query carries only
    // non-secret OAuth params, so that is tidiness, not a leak fix).
    let mut resp = redirect_302(&ii_url);
    resp.headers_mut().insert(
        axum::http::header::SET_COOKIE,
        axum::http::HeaderValue::from_str(&set_cookie).expect("valid cookie"),
    );

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.