security: bind OIDC login flow to the browser (login-CSRF guard)

CHANGELOG.md

@@ -4,6 +4,17 @@ All notable changes to Plutus are documented here.
 
 ## [Unreleased]
 
+### Security
+- **OIDC login-CSRF: bind the OAuth flow to the browser that started it.** The
+  authorization `state` was held only in a process-global pool, so an attacker
+  could complete their own Google sign-in, capture their `code`+`state`, and feed
+  a victim's browser a `/auth/callback?code=…&state=…` link — planting an
+  attacker-owned session in the victim's browser (the victim would then operate
+  inside, and leak API keys to, the attacker's tenant). `/auth/login` now sets a
+  short-lived `HttpOnly; SameSite=Lax` `plutus_oauth_state` cookie and
+  `handle_callback` requires the callback `state` to match it (constant-time),
+  in addition to the existing `_pending` nonce check. (2026-07-05 security review)
+
 ## [1.0.0] — 2026-06-27
 
 **Plutus 1.0 — the billing loop is closed and the contract is frozen.** The

plutus_agent/server/app.py

@@ -255,11 +255,14 @@ def _authz_org(self, conn, requested, *, strict=False):
 
     def _auth_login(self):
         try:
-            url = authmod.login_url(self.ctx.cfg)
+            url, state = authmod.login_url(self.ctx.cfg)
         except authmod.AuthError as e:
             return self._send(500, views.simple_page(
                 "Sign in", "Sign-in is misconfigured", html.escape(str(e)), ok=False))
-        return self._send(200, views.login_page(url))
+        # Bind this login attempt to the browser (login-CSRF guard): the callback
+        # state must match this cookie.
+        return self._send(200, views.login_page(url), headers={
+            "Set-Cookie": authmod.set_state_cookie_header(state, self.ctx.cfg)})
 
     def _client_ip(self) -> str:
         """Best-effort client IP for rate limiting (fix #59). Honors the first
@@ -277,14 +280,16 @@ def _client_ip(self) -> str:
     def _auth_callback(self, conn, q):
         flat = {k: (v[0] if isinstance(v, list) else v) for k, v in q.items()}
         try:
-            token = authmod.handle_callback(conn, self.ctx.cfg, flat,
-                                            client_ip=self._client_ip())
+            token = authmod.handle_callback(
+                conn, self.ctx.cfg, flat, client_ip=self._client_ip(),
+                cookie_state=authmod.read_cookie(self, authmod.STATE_COOKIE))
         except authmod.AuthError as e:
             return self._send(403, views.simple_page(
                 "Sign in", "Could not sign you in", html.escape(str(e)), ok=False))
         self.send_response(303)
         self.send_header("Location", "/")
         self.send_header("Set-Cookie", authmod.set_cookie_header(token, self.ctx.cfg))
+        self.send_header("Set-Cookie", authmod.clear_state_cookie_header(self.ctx.cfg))
         self.end_headers()
 
     def _auth_logout(self, conn):

plutus_agent/server/auth.py

@@ -39,6 +39,7 @@
 JWKS_ENDPOINT = "https://www.googleapis.com/oauth2/v3/certs"
 SCOPES = "openid email profile"
 COOKIE = "plutus_session"
+STATE_COOKIE = "plutus_oauth_state"  # binds an in-flight OAuth login to the browser that began it (login-CSRF guard)
 _STATE_TTL = 600  # how long a login attempt's state/nonce stays valid (seconds)
 
 # Self-serve signup rate limiting (per hour globally)
@@ -240,8 +241,12 @@ def _take_state(state: str) -> Optional[str]:
 
 
 # ---------------------------------------------------------------- the flow ---
-def login_url(cfg) -> str:
-    """Build the Google authorization URL and stash its state/nonce."""
+def login_url(cfg) -> tuple[str, str]:
+    """Build the Google authorization URL and stash its state/nonce.
+
+    Returns ``(url, state)``; the caller sets ``state`` as a short-lived cookie
+    on the browser so :func:`handle_callback` can bind the callback to it.
+    """
     state = secrets.token_urlsafe(24)
     nonce = secrets.token_urlsafe(24)
     _remember_state(state, nonce)
@@ -255,7 +260,7 @@ def login_url(cfg) -> str:
         "access_type": "online",
         "prompt": "select_account",
     }
-    return AUTH_ENDPOINT + "?" + urllib.parse.urlencode(params)
+    return AUTH_ENDPOINT + "?" + urllib.parse.urlencode(params), state
 
 
 def _exchange_code(cfg, code: str) -> dict:
@@ -405,18 +410,27 @@ def _authorize_email(conn, cfg, email: str, name: Optional[str] = None,
     return None
 
 
-def handle_callback(conn, cfg, q: dict, client_ip: Optional[str] = None) -> str:
+def handle_callback(conn, cfg, q: dict, client_ip: Optional[str] = None,
+                    cookie_state: Optional[str] = None) -> str:
     """Process /auth/callback query params; return a fresh session token.
 
-    ``client_ip`` (fix #59) feeds the per-IP signup throttle. Raises
-    :class:`AuthError` on any problem.
+    ``client_ip`` (fix #59) feeds the per-IP signup throttle. ``cookie_state`` is
+    the value of the ``plutus_oauth_state`` cookie set on this browser at
+    ``/auth/login``; the callback ``state`` must match it (login-CSRF guard).
+    Raises :class:`AuthError` on any problem.
     """
     if q.get("error"):
         raise AuthError(f"Google returned an error: {q['error']}")
     code = q.get("code")
     state = q.get("state")
     if not code or not state:
         raise AuthError("missing code or state")
+    # Login-CSRF guard: the callback state must match the state cookie this
+    # browser received at /auth/login. Without this an attacker could feed a
+    # victim's browser their own code+state and plant an attacker-owned session
+    # (the global _pending pool alone does not bind the flow to a browser).
+    if not cookie_state or not secrets.compare_digest(str(state), str(cookie_state)):
+        raise AuthError("sign-in state did not match this browser — start again")
     nonce = _take_state(state)
     if nonce is None:
         raise AuthError("invalid or expired sign-in state — try again")
@@ -454,7 +468,7 @@ def csrf_token(session_token: str) -> str:
 
 
 # ----------------------------------------------------------- cookie helpers ---
-def read_cookie(handler) -> str:
+def read_cookie(handler, name: str = COOKIE) -> str:
     raw = handler.headers.get("Cookie", "")
     if not raw:
         return ""
@@ -463,7 +477,7 @@ def read_cookie(handler) -> str:
         jar.load(raw)
     except Exception:
         return ""
-    m = jar.get(COOKIE)
+    m = jar.get(name)
     return m.value if m else ""
 
 
@@ -483,6 +497,16 @@ def clear_cookie_header(cfg) -> str:
     return f"{COOKIE}=; Path=/; HttpOnly; SameSite=Lax{_secure_attr(cfg)}; Max-Age=0"
 
 
+def set_state_cookie_header(state: str, cfg) -> str:
+    """Short-lived cookie binding an in-flight OAuth login to this browser."""
+    return (f"{STATE_COOKIE}={state}; Path=/; HttpOnly; SameSite=Lax"
+            f"{_secure_attr(cfg)}; Max-Age={int(_STATE_TTL)}")
+
+
+def clear_state_cookie_header(cfg) -> str:
+    return f"{STATE_COOKIE}=; Path=/; HttpOnly; SameSite=Lax{_secure_attr(cfg)}; Max-Age=0"
+
+
 def current_user(handler, conn):
     """The signed-in user row for this request, or None."""
     return db.session_user(conn, read_cookie(handler))

tests/test_auth.py

@@ -177,36 +177,62 @@ def test_claims_validation_rejects_bad_token(self):
                 authmod._claims_from_id_token(_jwt(claims), self.cfg, nonce)
 
     def test_callback_creates_session(self):
-        url = authmod.login_url(self.cfg)
+        url, state = authmod.login_url(self.cfg)
         qs = urllib.parse.parse_qs(urllib.parse.urlparse(url).query)
-        state, nonce = qs["state"][0], qs["nonce"][0]
+        nonce = qs["nonce"][0]
 
         orig = authmod._exchange_code
         authmod._exchange_code = lambda cfg, code: {"id_token": _jwt(self._good_claims(nonce))}
         try:
             token = authmod.handle_callback(
-                self.conn, self.cfg, {"code": "abc", "state": state})
+                self.conn, self.cfg, {"code": "abc", "state": state},
+                cookie_state=state)
         finally:
             authmod._exchange_code = orig
 
         self.assertEqual(db.session_user(self.conn, token)["email"], "owner@example.com")
 
     def test_callback_rejects_unknown_state(self):
+        # cookie_state matches the (never-issued) callback state, so this exercises
+        # the _pending lookup rejection rather than the login-CSRF guard.
         with self.assertRaises(authmod.AuthError):
             authmod.handle_callback(self.conn, self.cfg,
-                                    {"code": "abc", "state": "never-issued"})
+                                    {"code": "abc", "state": "never-issued"},
+                                    cookie_state="never-issued")
+
+    def test_callback_rejects_state_cookie_mismatch(self):
+        """Login-CSRF guard: a valid pending state with a missing/mismatched
+        browser cookie is rejected, so an attacker's code+state replayed into a
+        victim's browser cannot plant an attacker-owned session."""
+        url, state = authmod.login_url(self.cfg)
+        qs = urllib.parse.parse_qs(urllib.parse.urlparse(url).query)
+        nonce = qs["nonce"][0]
+        orig = authmod._exchange_code
+        authmod._exchange_code = lambda cfg, code: {"id_token": _jwt(self._good_claims(nonce))}
+        try:
+            with self.assertRaises(authmod.AuthError):
+                authmod.handle_callback(self.conn, self.cfg,
+                                        {"code": "abc", "state": state},
+                                        cookie_state="attacker-different")
+            with self.assertRaises(authmod.AuthError):
+                authmod.handle_callback(self.conn, self.cfg,
+                                        {"code": "abc", "state": state},
+                                        cookie_state=None)
+        finally:
+            authmod._exchange_code = orig
 
     def test_callback_open_signup_creates_session_for_stranger(self):
         cfg = _auth_cfg(allow_signup=True)
-        url = authmod.login_url(cfg)
+        url, state = authmod.login_url(cfg)
         qs = urllib.parse.parse_qs(urllib.parse.urlparse(url).query)
-        state, nonce = qs["state"][0], qs["nonce"][0]
+        nonce = qs["nonce"][0]
         claims = self._good_claims(nonce, email="newbie@elsewhere.com", name="Newbie")
         orig = authmod._exchange_code
         authmod._exchange_code = lambda c, code: {"id_token": _jwt(claims)}
         try:
             token = authmod.handle_callback(self.conn, cfg,
-                                            {"code": "abc", "state": state})
+                                            {"code": "abc", "state": state},
+                                            cookie_state=state)
         finally:
             authmod._exchange_code = orig
         self.assertEqual(db.session_user(self.conn, token)["email"], "newbie@elsewhere.com")