fix(auth): validate issuer, audience and expiry on the token response; tell users to retry a rejected code

app/auth.py

@@ -275,7 +275,9 @@ def complete_login(
             or not isinstance(nonce, str)
             or not isinstance(next_path, str)
             or not isinstance(created_at, int)
-            or not hmac.compare_digest(expected_state, state)
+            or not hmac.compare_digest(
+                expected_state.encode("utf-8"), state.encode("utf-8")
+            )
             or now < created_at
             or now - created_at > LOGIN_TRANSACTION_SECONDS
         ):

app/central_auth.py

@@ -57,6 +57,25 @@ class AuthTransactionError(CentralAuthError):
     """The browser callback did not match a live login transaction."""
 
 
+class CodeExchangeRejectedError(CentralAuthError):
+    """The auth service refused the code: expired, replayed, or superseded.
+
+    This is the user's problem to retry, not an outage: a second tab, a slow
+    click, or a stale bookmark produces it. Callers should say "try again",
+    not "the service is unavailable".
+    """
+
+
+# Tolerated skew between this host's clock and the auth service's when
+# checking the assertion expiry. The assertion lives five minutes.
+CLOCK_SKEW_SECONDS = 60
+
+
+def _constant_time_equal(expected: str, actual: str) -> bool:
+    """compare_digest on str raises TypeError for non-ASCII; compare bytes."""
+    return hmac.compare_digest(expected.encode("utf-8"), actual.encode("utf-8"))
+
+
 @dataclass(frozen=True)
 class AuthenticatedPrincipal:
     subject: str
@@ -229,12 +248,19 @@ def exchange(
         try:
             with _open_token_request(request, self.timeout_seconds) as response:
                 raw = response.read(MAX_TOKEN_RESPONSE_BYTES + 1)
-        except (
-            urllib.error.HTTPError,
-            urllib.error.URLError,
-            TimeoutError,
-            OSError,
-        ) as exc:
+        except urllib.error.HTTPError as exc:
+            # 400 is the auth service's invalid_grant: the code was consumed,
+            # expired, or superseded by a newer /authorize for this browser.
+            # Anything else (401 invalid_client, 5xx) is a deployment or
+            # availability problem.
+            if exc.code == 400:
+                raise CodeExchangeRejectedError(
+                    "The authentication service rejected the sign-in code"
+                ) from exc
+            raise CentralAuthError(
+                "The authentication service rejected the code exchange"
+            ) from exc
+        except (urllib.error.URLError, TimeoutError, OSError) as exc:
             raise CentralAuthError(
                 "The authentication service rejected the code exchange"
             ) from exc
@@ -250,13 +276,28 @@ def exchange(
         subject = payload.get("sub")
         email = payload.get("email")
         nonce = payload.get("nonce")
+        issuer = payload.get("iss")
+        audience = payload.get("aud")
+        expires_at = payload.get("exp")
+        now = int(time.time())
+        # The response arrives over an authenticated TLS backchannel, so these
+        # checks defend against misconfiguration rather than an attacker: a
+        # response minted by a different issuer, for a different client, or
+        # replayed after its assertion window must not create a session.
         if (
             not isinstance(subject, str)
             or not subject
             or len(subject) > 255
             or not isinstance(email, str)
             or not isinstance(nonce, str)
-            or not hmac.compare_digest(nonce, expected_nonce)
+            or not _constant_time_equal(expected_nonce, nonce)
+            or not isinstance(issuer, str)
+            or issuer.rstrip("/") != self.issuer_url
+            or not isinstance(audience, str)
+            or audience != self.client_id
+            or isinstance(expires_at, bool)
+            or not isinstance(expires_at, int)
+            or expires_at + CLOCK_SKEW_SECONDS <= now
         ):
             raise CentralAuthError("The authentication response was invalid")
         try:

app/main.py

@@ -33,6 +33,7 @@
     AccessDeniedError,
     AuthTransactionError,
     CentralAuthError,
+    CodeExchangeRejectedError,
     verify_csrf_token,
 )
 from app.config import settings
@@ -616,6 +617,12 @@ def auth_callback(
             status_code=403,
             detail="Your account does not have access to this Explorer instance.",
         ) from exc
+    except CodeExchangeRejectedError as exc:
+        # A consumed, expired, or superseded code (for example a second tab
+        # that started its own sign-in) is a retry, not an outage.
+        raise HTTPException(
+            status_code=400, detail="Sign-in expired. Try again."
+        ) from exc
     except CentralAuthError as exc:
         raise HTTPException(
             status_code=502, detail="The authentication service is unavailable."

scripts/bootstrap.sh

@@ -237,7 +237,9 @@ EMAIL_S3_BUCKET="$EMAIL_S3_BUCKET"
 # AWS_SECRET_ACCESS_KEY=
 EOF
 chown "$APP_USER:$APP_USER" "$ENV_FILE"
-chmod 0640 "$ENV_FILE"
+# Owner-only: this file carries the session secret, the central-auth client
+# secret, and any AWS keys.
+chmod 0600 "$ENV_FILE"
 ok "env seeded at $ENV_FILE"
 
 # ── step 4b: optional encrypted config bundle ───────────────────────────

tests/test_central_auth.py

@@ -9,6 +9,8 @@
 import json
 import sqlite3
 import stat
+import time
+import urllib.error
 import urllib.parse
 
 import pytest
@@ -18,6 +20,7 @@
     CentralAuthClient,
     CentralAuthError,
     CentralAuthStore,
+    CodeExchangeRejectedError,
     csrf_token_for_session,
     verify_csrf_token,
 )
@@ -117,6 +120,9 @@ def read(self, _limit):
                     "sub": "account-123",
                     "email": "Alice@Example.com",
                     "nonce": "expected-nonce",
+                    "iss": "https://auth.example.com",
+                    "aud": "explorer",
+                    "exp": int(time.time()) + 300,
                 }
             ).encode()
 
@@ -192,7 +198,16 @@ def __exit__(self, *args):
             return None
 
         def read(self, _limit):
-            return b'{"sub":"account-123","email":"alice@example.com","nonce":"wrong"}'
+            return json.dumps(
+                {
+                    "sub": "account-123",
+                    "email": "alice@example.com",
+                    "nonce": "wrong",
+                    "iss": "https://auth.example.com",
+                    "aud": "explorer",
+                    "exp": int(time.time()) + 300,
+                }
+            ).encode()
 
     monkeypatch.setattr(
         "app.central_auth._open_token_request", lambda *_args: Response()
@@ -210,3 +225,108 @@ def read(self, _limit):
             code_verifier="pkce-verifier",
             expected_nonce="expected-nonce",
         )
+
+
+def _client() -> CentralAuthClient:
+    return CentralAuthClient(
+        issuer_url="https://auth.example.com",
+        public_url="https://explorer.example.com",
+        client_id="explorer",
+        client_secret="a-random-client-secret-with-32-bytes",
+    )
+
+
+def _fake_response(payload: dict):
+    class Response:
+        def __enter__(self):
+            return self
+
+        def __exit__(self, *_args):
+            return None
+
+        def read(self, _limit):
+            return json.dumps(payload).encode()
+
+    return Response()
+
+
+def _valid_payload() -> dict:
+    return {
+        "sub": "account-123",
+        "email": "alice@example.com",
+        "nonce": "expected-nonce",
+        "iss": "https://auth.example.com",
+        "aud": "explorer",
+        "exp": int(time.time()) + 300,
+    }
+
+
+@pytest.mark.parametrize(
+    "mutation",
+    [
+        {"aud": "lens"},
+        {"iss": "https://other-auth.example.com"},
+        {"exp": int(time.time()) - 120},
+        {"exp": "soon"},
+        {"exp": True},
+    ],
+)
+def test_code_exchange_rejects_wrong_audience_issuer_or_expired(
+    monkeypatch, mutation
+) -> None:
+    payload = _valid_payload() | mutation
+    monkeypatch.setattr(
+        "app.central_auth._open_token_request",
+        lambda *_args: _fake_response(payload),
+    )
+    with pytest.raises(CentralAuthError, match="invalid"):
+        _client().exchange(
+            code="code", code_verifier="v" * 43, expected_nonce="expected-nonce"
+        )
+
+
+def test_code_exchange_tolerates_small_clock_skew(monkeypatch) -> None:
+    payload = _valid_payload() | {"exp": int(time.time()) - 10}
+    monkeypatch.setattr(
+        "app.central_auth._open_token_request",
+        lambda *_args: _fake_response(payload),
+    )
+    principal = _client().exchange(
+        code="code", code_verifier="v" * 43, expected_nonce="expected-nonce"
+    )
+    assert principal.subject == "account-123"
+
+
+def test_non_ascii_nonce_from_issuer_is_rejected_not_crashed(monkeypatch) -> None:
+    payload = _valid_payload() | {"nonce": "expected-nonc\u00e9"}
+    monkeypatch.setattr(
+        "app.central_auth._open_token_request",
+        lambda *_args: _fake_response(payload),
+    )
+    with pytest.raises(CentralAuthError, match="invalid"):
+        _client().exchange(
+            code="code", code_verifier="v" * 43, expected_nonce="expected-nonce"
+        )
+
+
+def test_http_400_from_token_endpoint_is_a_rejected_code(monkeypatch) -> None:
+    def refuse(*_args):
+        raise urllib.error.HTTPError(
+            "https://auth.example.com/token", 400, "Bad Request", {}, None
+        )
+
+    monkeypatch.setattr("app.central_auth._open_token_request", refuse)
+    with pytest.raises(CodeExchangeRejectedError):
+        _client().exchange(code="code", code_verifier="v" * 43, expected_nonce="n")
+
+
+def test_http_401_from_token_endpoint_is_an_outage_not_a_retry(monkeypatch) -> None:
+    def refuse(*_args):
+        raise urllib.error.HTTPError(
+            "https://auth.example.com/token", 401, "Unauthorized", {}, None
+        )
+
+    monkeypatch.setattr("app.central_auth._open_token_request", refuse)
+    with pytest.raises(CentralAuthError) as excinfo:
+        _client().exchange(code="code", code_verifier="v" * 43, expected_nonce="n")
+    assert not isinstance(excinfo.value, CodeExchangeRejectedError)

tests/test_central_auth_endpoints.py

@@ -17,6 +17,7 @@
     CentralAuthClient,
     CentralAuthError,
     CentralAuthStore,
+    CodeExchangeRejectedError,
 )
 
 
@@ -191,6 +192,31 @@ def test_callback_fails_closed_when_code_exchange_fails(central_client) -> None:
     assert CENTRAL_AUTH_COOKIE_NAME not in client.cookies
 
 
+def test_rejected_code_is_a_retry_not_an_outage(central_client) -> None:
+    client, _store = central_client
+    FakeAuthClient.exchange_error = CodeExchangeRejectedError("code consumed")
+
+    response = complete_login(client)
+
+    assert response.status_code == 400
+    assert "Try again" in response.text
+    assert CENTRAL_AUTH_COOKIE_NAME not in client.cookies
+
+
+def test_non_ascii_state_is_rejected_not_crashed(central_client) -> None:
+    client, _store = central_client
+    begin_login(client)
+
+    response = client.get(
+        "/auth/callback",
+        params={"code": "irrelevant", "state": "\u00e9tat"},
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 400
+    assert FakeAuthClient.exchanged_codes == []
+
+
 def test_external_next_url_is_not_used_after_callback(central_client) -> None:
     client, _store = central_client