fix(studio): anon /studio nav → 302 /login, not a raw 401

The IAM auth gate refused any unauthenticated request whose Accept lacked `text/html` with a 401, so a plain browser/curl navigation to a protected page (e.g. /studio) hit a dead 401 instead of the login flow. Invert the rule and decomplect it: a top-level browser navigation is now redirected (302) to the one server-owned login entry point, `/login?next=<path>` — which starts the OIDC code flow and returns the user where they were. Only a request that self-identifies as API/XHR/JSON (Sec-Fetch-Mode != navigate, X-Requested-With, or Accept: application/json without HTML) still gets 401, so the SPA's own fetch calls handle re-auth instead of following a redirect into HTML. An ambiguous client (plain curl, Accept: */*) counts as a navigation and is redirected — a human never sees a raw 401. handle_login honors a sanitized ?next (open-redirect/loop-safe). Bump 0.17.18.
2026-07-18 17:31:25 -07:00
parent 4b128ccfc8
commit d45067db4d
3 changed files with 190 additions and 14 deletions
@@ -9,10 +9,13 @@ signing keys are cached. Signature (RS256 by default), ``exp``, ``iss`` and
 tenancy / rate-limit / billing paths.

 Browser sessions use the standard OIDC Authorization Code flow: an
-unauthenticated HTML request is redirected to the IAM ``authorize`` endpoint;
-IAM bounces back to ``/callback`` which exchanges the code for a token and sets
-an httpOnly session cookie. No frontend changes are required — the prebuilt SPA
-just rides the cookie.
+unauthenticated browser navigation is redirected (302) to ``/login`` — the one
+server-owned entry point — which starts the flow at the IAM ``authorize``
+endpoint; IAM bounces back to ``/callback`` which exchanges the code for a token
+and sets an httpOnly session cookie. A programmatic API/XHR/JSON request instead
+gets a 401 (it handles its own re-auth), so the SPA's ``fetch`` calls never
+follow a redirect and parse HTML as JSON. No frontend changes are required — the
+prebuilt SPA just rides the cookie.

 Localhost connections bypass auth (single-user dev). Static assets, health
 checks and the worker coordinator endpoints are exempt.
@@ -94,6 +97,48 @@ def _is_public_path(path: str) -> bool:
    ))


+def _is_api_request(request: web.Request) -> bool:
+    """A programmatic API/XHR/JSON caller (answered with 401) as opposed to a
+    top-level browser navigation (answered with a 302 into the login flow).
+
+    A human who types a URL or follows a link must never be shown a raw 401, so
+    an *ambiguous* non-browser client (plain ``curl``, ``Accept: */*``, no
+    fetch-metadata) counts as a navigation and is redirected. Only a request that
+    self-identifies as fetch/XHR/JSON is refused — otherwise the SPA's own
+    ``fetch`` calls would follow the redirect to the IdP and parse HTML as JSON.
+    """
+    # Fetch-metadata (every evergreen browser stamps it): a user navigation is
+    # ``Sec-Fetch-Mode: navigate``; ``fetch()``/XHR carry cors|same-origin|no-cors.
+    mode = request.headers.get("Sec-Fetch-Mode", "").lower()
+    if mode:
+        return mode != "navigate"
+    # Legacy AJAX marker (jQuery / axios / htmx).
+    if request.headers.get("X-Requested-With", "").lower() == "xmlhttprequest":
+        return True
+    # Content negotiation: an explicit JSON preference with no HTML acceptance.
+    accept = request.headers.get("Accept", "")
+    return "application/json" in accept and "text/html" not in accept
+
+
+def _safe_next(raw: str | None) -> str:
+    """A same-origin return path, or ``/``. Rejects absolute URLs, protocol- and
+    backslash-relative forms (open-redirect vectors) and the login route itself
+    (which would loop)."""
+    if not raw or not raw.startswith("/") or raw[:2] in ("//", "/\\"):
+        return "/"
+    if raw == "/login" or raw.startswith(("/login?", "/login/")):
+        return "/"
+    return raw
+
+
+def _login_redirect(request: web.Request) -> web.Response:
+    """Send an unauthenticated browser to the ONE server-owned login entry point
+    (``/login``), preserving where it was headed via ``?next`` so the OIDC round
+    trip returns it there. The authorize-URL construction lives in ``/login``
+    (``handle_login``) alone — this gate only decides *who* is sent there."""
+    return web.HTTPFound("/login?" + urlencode({"next": str(request.rel_url)}))
+
+
 def _cache_key(token: str) -> str:
    return hashlib.sha256(token.encode()).hexdigest()[:32]

@@ -339,7 +384,9 @@ def create_iam_auth_middleware(iam_url: str, localhost_bypass: bool = True):
        Authorization Code flow (PKCE + signed state + ``/callback``) is built
        server-side — there is exactly one way to start a login, and the client
        never hand-rolls an authorize URL. Already-authenticated callers skip
-        straight to the app; everyone else is bounced to IAM and returns to ``/``.
+        straight to the app; everyone else is bounced to IAM and returns to the
+        sanitized ``?next`` target (default ``/``) — this is where the auth gate
+        sends an unauthenticated browser navigation, carrying its original path.
        """
        token = None
        auth_header = request.headers.get("Authorization", "")
@@ -353,7 +400,7 @@ def create_iam_auth_middleware(iam_url: str, localhost_bypass: bool = True):
                    return web.HTTPFound("/")
            except Exception:
                pass  # IAM/JWKS hiccup — fall through and re-authenticate cleanly.
-        return _authorize_redirect(request, target="/")
+        return _authorize_redirect(request, target=_safe_next(request.query.get("next")))

    async def handle_callback(request: web.Request) -> web.Response:
        """Exchange the authorization code for a token and set the session cookie."""
@@ -428,12 +475,16 @@ def create_iam_auth_middleware(iam_url: str, localhost_bypass: bool = True):
        if not token and path == "/ws":
            token = request.rel_url.query.get("token")

-        wants_html = "text/html" in request.headers.get("Accept", "")
+        # A browser navigation is redirected into the login flow (302 → /login,
+        # carrying ?next); a programmatic API/XHR/JSON caller gets a 401 it can
+        # handle itself. One predicate decides, one way, for both no-token and
+        # bad-token cases below.
+        api = _is_api_request(request)

        if not token:
-            if wants_html:
-                return _authorize_redirect(request)
-            return web.json_response({"error": "Authentication required", "iam_url": iam_url}, status=401)
+            if api:
+                return web.json_response({"error": "Authentication required", "iam_url": iam_url}, status=401)
+            return _login_redirect(request)

        try:
            user = await _validate(token)
@@ -441,9 +492,9 @@ def create_iam_auth_middleware(iam_url: str, localhost_bypass: bool = True):
            return web.json_response({"error": "Authentication service unavailable"}, status=503)

        if user is None:
-            if wants_html:
-                return _authorize_redirect(request)
-            return web.json_response({"error": "Invalid or expired token"}, status=401)
+            if api:
+                return web.json_response({"error": "Invalid or expired token"}, status=401)
+            return _login_redirect(request)

        request["iam_user"] = _with_active_org(user, request.cookies.get(_ACTIVE_ORG_COOKIE))
        # The verified raw access token. Studio records a completed render in the
@@ -1,6 +1,6 @@
 [project]
 name = "hanzo-studio"
-version = "0.17.17"
+version = "0.17.18"
 readme = "README.md"
 license = { file = "LICENSE" }
 requires-python = ">=3.10"
@@ -4,10 +4,14 @@ The JWT path is exercised end to end with a locally generated RSA keypair:
 sign an access token like Hanzo IAM would, then verify it through the same
 code the middleware uses. Covers signature, exp, iss, aud and the org claim.
 """
+import asyncio
 import time
+from urllib.parse import parse_qs, urlencode, urlsplit

 import jwt
 import pytest
+from aiohttp import web
+from aiohttp.test_utils import make_mocked_request
 from cryptography.hazmat.primitives.asymmetric import rsa

 from middleware import iam_auth_middleware as m
@@ -204,3 +208,124 @@ def test_handle_login_returns_to_app_root_not_the_login_path():
    # State is HMAC-signed with the client_id key (see _sign_state usage).
    payload = m._verify_state(cookie, "hanzo-studio")
    assert payload is not None and payload["t"] == "/"
+
+
+# --- browser-vs-API gate: anon navigation → 302 /login, XHR/JSON → 401 ---
+
+def _gate(headers):
+    """Run the auth middleware for an unauthenticated GET /studio with the given
+    request headers; return the response. localhost_bypass is off so the token
+    gate actually runs (prod sits behind the ingress, never loopback)."""
+    mw = m.create_iam_auth_middleware("https://hanzo.id", localhost_bypass=False)
+
+    async def _ok(_request):
+        return web.Response(text="ok")
+
+    req = make_mocked_request("GET", "/studio", headers=headers)
+    return asyncio.run(mw(req, _ok))
+
+
+def test_anon_browser_navigation_redirects_to_login():
+    """A top-level browser navigation (Accept: text/html) to a protected page is
+    bounced to the server-owned /login entry point, carrying ?next so the OIDC
+    round trip returns to /studio — never a raw 401."""
+    resp = _gate({"Accept": "text/html,application/xhtml+xml"})
+    assert resp.status == 302
+    loc = resp.headers["Location"]
+    assert loc.startswith("/login?")
+    assert parse_qs(urlsplit(loc).query)["next"] == ["/studio"]
+
+
+def test_anon_plain_curl_redirects_to_login():
+    """The confirmed repro: a bare client (Accept: */*, no fetch-metadata) is
+    treated as a navigation and redirected, not 401'd."""
+    resp = _gate({"Accept": "*/*"})
+    assert resp.status == 302
+    assert resp.headers["Location"].startswith("/login?next=")
+
+
+def test_anon_no_accept_header_redirects_to_login():
+    """No Accept header at all is still ambiguous, not an API call → redirect."""
+    resp = _gate({})
+    assert resp.status == 302
+    assert "/login" in resp.headers["Location"]
+
+
+def test_anon_sec_fetch_navigate_redirects():
+    """Modern browsers stamp Sec-Fetch-Mode: navigate on top-level loads."""
+    resp = _gate({"Accept": "*/*", "Sec-Fetch-Mode": "navigate"})
+    assert resp.status == 302
+    assert "/login" in resp.headers["Location"]
+
+
+def test_anon_xhr_gets_401():
+    """An XHR (X-Requested-With) must get a JSON 401 so the SPA can react, not a
+    redirect it would follow into HTML."""
+    resp = _gate({"Accept": "*/*", "X-Requested-With": "XMLHttpRequest"})
+    assert resp.status == 401
+
+
+def test_anon_json_accept_gets_401():
+    """An explicit JSON client (Accept: application/json) is an API caller."""
+    resp = _gate({"Accept": "application/json"})
+    assert resp.status == 401
+
+
+def test_anon_fetch_cors_gets_401():
+    """A programmatic fetch() carries Sec-Fetch-Mode: cors — 401, not redirect."""
+    resp = _gate({"Accept": "*/*", "Sec-Fetch-Mode": "cors"})
+    assert resp.status == 401
+
+
+def test_is_api_request_predicate():
+    def api(headers):
+        return m._is_api_request(make_mocked_request("GET", "/studio", headers=headers))
+
+    assert api({"Accept": "*/*"}) is False                      # plain curl → nav
+    assert api({"Accept": "text/html"}) is False                # browser → nav
+    assert api({}) is False                                     # unknown → nav
+    assert api({"Sec-Fetch-Mode": "navigate"}) is False         # top-level nav
+    assert api({"Sec-Fetch-Mode": "cors"}) is True              # fetch/XHR
+    assert api({"Sec-Fetch-Mode": "same-origin"}) is True       # fetch/XHR
+    assert api({"X-Requested-With": "XMLHttpRequest"}) is True  # legacy AJAX
+    assert api({"Accept": "application/json"}) is True          # JSON client
+
+
+def test_safe_next_predicate():
+    assert m._safe_next("/studio") == "/studio"
+    assert m._safe_next("/studio?org=acme&run=1") == "/studio?org=acme&run=1"
+    assert m._safe_next(None) == "/"
+    assert m._safe_next("") == "/"
+    assert m._safe_next("//evil.example/x") == "/"       # protocol-relative
+    assert m._safe_next("https://evil.example") == "/"   # absolute
+    assert m._safe_next("/\\evil.example") == "/"        # backslash-relative
+    assert m._safe_next("/login") == "/"                 # would loop
+    assert m._safe_next("/login?next=/x") == "/"         # would loop
+
+
+def test_handle_login_honors_next_param():
+    """A redirect from the auth gate carries ?next=/studio; login must return the
+    user there after the OIDC round trip (the signed state proves the target)."""
+    mw = m.create_iam_auth_middleware("https://hanzo.id")
+    req = make_mocked_request(
+        "GET", "/login?" + urlencode({"next": "/studio"}),
+        headers={"X-Forwarded-Proto": "https", "X-Forwarded-Host": "studio.hanzo.ai"},
+    )
+    resp = asyncio.run(mw.handle_login(req))
+    assert resp.status == 302
+    payload = m._verify_state(resp.cookies[m._STATE_COOKIE].value, "hanzo-studio")
+    assert payload["t"] == "/studio"
+
+
+def test_handle_login_rejects_open_redirect_next():
+    """An off-origin, protocol-relative or self-referential next is dropped to
+    '/', closing the open-redirect and login-loop vectors."""
+    mw = m.create_iam_auth_middleware("https://hanzo.id")
+    for bad in ("//evil.example/x", "https://evil.example", "/login"):
+        req = make_mocked_request(
+            "GET", "/login?" + urlencode({"next": bad}),
+            headers={"X-Forwarded-Proto": "https", "X-Forwarded-Host": "studio.hanzo.ai"},
+        )
+        resp = asyncio.run(mw.handle_login(req))
+        payload = m._verify_state(resp.cookies[m._STATE_COOKIE].value, "hanzo-studio")
+        assert payload["t"] == "/", f"next={bad!r} should be rejected"