From 7d8406a9c632dce603ec054c27ae80fb4b812b90 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 2 Jun 2026 16:22:28 +1000 Subject: [PATCH 1/3] feat(dashboard-auth): rotate dashboard sessions via refresh token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard auth-code grant now issues a 24h rotating refresh token (server side: NousResearch/nous-account-service#293). This wires up the Hermes client half so an expired access token is transparently refreshed instead of bouncing the user to /login every 15 minutes. plugins/dashboard_auth/nous: - refresh_session() now POSTs grant_type=refresh_token to Portal's token endpoint and returns a Session carrying the ROTATED refresh token (was an unconditional RefreshExpiredError under the old "no RT in V1" contract). The RT is sent in BOTH the request body (Portal's schema requires it there) and the X-Refresh-Token header (log redaction) — verified against the #293 preview deploy: header-only is rejected as invalid_request, body is accepted. - A 400 from Portal (expired / revoked / reuse-detected) maps to RefreshExpiredError so the middleware forces a clean re-login; network errors map to ProviderError; empty RT fast-fails without a network call. - complete_login now captures the initial refresh token Portal returns (forward-tolerant: empty string if a deploy omits it). - Extracted the shared token-response handling into _token_response_to_session, parameterised on the 400 exception type so the auth-code path raises InvalidCodeError and the refresh path raises RefreshExpiredError. - revoke_session stays a best-effort no-op: Portal exposes no public token-endpoint revocation grant (revocation is the authenticated /sessions UI, keyed by sessionId+userId), so logout is cookie-clearing and the 24h session expires on its own. Documented for a future revoke grant. hermes_cli/dashboard_auth/middleware: - On an expired/invalid access token the gate now attempts refresh via the session's RT BEFORE forcing re-login. On success it serves the request and re-sets the rotated cookies on the response (mandatory: Portal rotates the RT every refresh and reuse-detects, so a stale RT cookie would revoke the whole session on the next refresh). On RefreshExpiredError (or no RT) it falls through to clear-and-relogin. - ProviderError during refresh (Portal unreachable) forces a clean re-login rather than 500-ing the request. - Uses the existing REFRESH_SUCCESS / REFRESH_FAILURE audit events. Validation: - 176 dashboard-auth unit/integration tests pass. - Live E2E against the #293 preview deploy: refresh_session(bad rt) -> RefreshExpiredError through the real token endpoint; live JWKS fetch + RS256 verification rejects a forged token; empty-RT fast-fail. The successful happy-path rotation is covered by unit tests (a live run needs an interactive browser OAuth round trip + registered agent:* client). Depends on: NousResearch/nous-account-service#293 (server-side RT issuance). --- hermes_cli/dashboard_auth/middleware.py | 101 +++++++++++++- plugins/dashboard_auth/nous/__init__.py | 128 +++++++++++++++--- .../dashboard_auth/test_nous_provider.py | 113 ++++++++++++++-- 3 files changed, 305 insertions(+), 37 deletions(-) diff --git a/hermes_cli/dashboard_auth/middleware.py b/hermes_cli/dashboard_auth/middleware.py index 05330bc15466..828ce9e59a44 100644 --- a/hermes_cli/dashboard_auth/middleware.py +++ b/hermes_cli/dashboard_auth/middleware.py @@ -24,7 +24,7 @@ from hermes_cli.dashboard_auth import list_providers from hermes_cli.dashboard_auth.audit import AuditEvent, audit_log -from hermes_cli.dashboard_auth.base import ProviderError +from hermes_cli.dashboard_auth.base import ProviderError, RefreshExpiredError from hermes_cli.dashboard_auth.cookies import read_session_cookies from hermes_cli.dashboard_auth.public_paths import PUBLIC_API_PATHS @@ -215,14 +215,51 @@ async def gated_auth_middleware( break if session is None: + # Access token is expired/invalid. Before forcing re-login, try to + # rotate it using the refresh token (if the session cookie carries + # one). On success we re-set the rotated cookies on the response and + # serve the request transparently; on RefreshExpiredError (RT dead / + # revoked / reuse-detected) we fall through to clear-and-relogin. + refreshed = _attempt_refresh(request, refresh_token=_rt) + if refreshed is not None: + new_session, refreshing_provider = refreshed + request.state.session = new_session + response = await call_next(request) + # Persist the ROTATED tokens. Portal rotates the refresh token on + # every refresh and runs reuse-detection, so writing the new RT + # back is mandatory: a stale RT cookie would replay a rotated + # token on the next refresh and (outside Portal's grace) revoke + # the whole session. Bind cookie Secure/Path to the request shape. + from hermes_cli.dashboard_auth.cookies import ( + detect_https, + set_session_cookies, + ) + from hermes_cli.dashboard_auth.prefix import prefix_from_request + + set_session_cookies( + response, + access_token=new_session.access_token, + refresh_token=new_session.refresh_token, + access_token_expires_in=_expires_in_seconds(new_session), + use_https=detect_https(request), + prefix=prefix_from_request(request), + ) + audit_log( + AuditEvent.REFRESH_SUCCESS, + provider=refreshing_provider, + user_id=new_session.user_id, + ip=_client_ip(request), + ) + return response + audit_log( AuditEvent.SESSION_VERIFY_FAILURE, reason="no_provider_recognises", ip=_client_ip(request), ) response = _unauth_response(request, reason="invalid_or_expired_session") - # Clear the dead cookie so the browser doesn't keep sending it. - # Contract v1: no refresh token to retry with, so the only correct + # Clear the dead cookies so the browser doesn't keep sending them. + # Refresh already failed (or there was no RT), so the only correct # next step is full re-auth via /login. Importing locally avoids a # cycle with cookies → middleware at module load. Pass the active # prefix so the deletion's Path matches the set-Path (otherwise @@ -234,3 +271,61 @@ async def gated_auth_middleware( request.state.session = session return await call_next(request) + + +def _expires_in_seconds(session) -> int: + """Seconds until the access token's ``exp``, floored at 60. + + Mirrors the auth-route's ``max(60, exp - now)`` so the access-token + cookie's Max-Age tracks the token lifetime even on a slightly skewed + clock. ``time`` imported locally to keep the module's import surface + minimal. + """ + import time + + return max(60, int(session.expires_at) - int(time.time())) + + +def _attempt_refresh(request: Request, *, refresh_token): + """Try to rotate an expired session via the refresh token. + + Returns ``(new_session, provider_name)`` on success, or ``None`` if + there's no RT or every provider's ``refresh_session`` failed with + ``RefreshExpiredError`` (dead/revoked/reuse-detected RT → force re-login). + + A ``ProviderError`` (Portal unreachable) is NOT swallowed into a re-login + here — re-raising would 500 the request; instead we log and return None so + the caller forces a clean re-login, which is the safer UX than a hard + error on a transient network blip during the narrow refresh window. + """ + if not refresh_token: + return None + for provider in list_providers(): + try: + new_session = provider.refresh_session(refresh_token=refresh_token) + except RefreshExpiredError: + # This provider owns the RT but it's dead — stop trying others + # (an RT belongs to exactly one provider) and force re-login. + audit_log( + AuditEvent.REFRESH_FAILURE, + provider=provider.name, + reason="refresh_expired", + ip=_client_ip(request), + ) + return None + except ProviderError as e: + _log.warning( + "dashboard-auth: provider %r unreachable during refresh: %s", + provider.name, e, + ) + audit_log( + AuditEvent.REFRESH_FAILURE, + provider=provider.name, + reason="provider_unreachable", + ip=_client_ip(request), + ) + return None + if new_session is not None: + return new_session, provider.name + return None + diff --git a/plugins/dashboard_auth/nous/__init__.py b/plugins/dashboard_auth/nous/__init__.py index c9d4b744cf05..f252136430d1 100644 --- a/plugins/dashboard_auth/nous/__init__.py +++ b/plugins/dashboard_auth/nous/__init__.py @@ -36,8 +36,13 @@ - scope is ``agent_dashboard:access`` only (no OIDC scopes). - tokens are RS256 JWTs verified against ``/.well-known/jwks.json``; JWKS is cached for 5 minutes. - - V1 has NO refresh tokens — ``refresh_session`` always raises - ``RefreshExpiredError`` so the middleware redirects to ``/auth/login``. + - the dashboard auth-code grant issues a 24h rotating refresh token + (Portal NAS PR #293). ``refresh_session`` posts ``grant_type=refresh_token`` + to rotate the access token; ``complete_login`` and ``refresh_session`` + both populate ``Session.refresh_token`` with the (rotating) value the + middleware persists back to the HttpOnly cookie. On a dead/expired/ + reuse-detected refresh token Portal returns 400 → ``RefreshExpiredError`` + → middleware redirects to ``/auth/login``. - audience claim is the bare ``client_id`` (no ``hermes-cli:`` prefix). - tolerant ``oauth_contract_version`` check: missing → warn + proceed; present and ``!= 1`` → refuse. @@ -49,11 +54,11 @@ "state": …}`` and the route serializes those into the ``hermes_session_pkce`` cookie. -Forward compatibility: if a future Portal contract starts issuing refresh -tokens, ``complete_login`` already captures the value forward-compatibly -(populates ``Session.refresh_token``). Wiring the RT cookie back into the -middleware's near-expiry refresh path lives in the host application, not -here. +Refresh-token rotation: Portal rotates the refresh token on every +successful refresh and runs reuse-detection (replaying a rotated token +outside Portal's 60s grace revokes the whole session). The host +middleware therefore MUST persist the rotated ``Session.refresh_token`` +back to the cookie on every refresh. Skip reasons: The plugin exposes a module-level ``LAST_SKIP_REASON`` that the gate's @@ -229,12 +234,91 @@ def complete_login( except httpx.RequestError as exc: raise ProviderError(f"Portal token endpoint unreachable: {exc}") from exc + # The dashboard auth-code grant now issues a rotating refresh token + # (24h session, reuse-detected) — Portal NAS PR #293. A 400 here means + # the code/PKCE/redirect_uri failed, surfaced as InvalidCodeError. + return self._token_response_to_session( + response, bad_request_exc=InvalidCodeError + ) + + def refresh_session(self, *, refresh_token: str) -> Session: + """Rotate the access token using the refresh token. + + Posts ``grant_type=refresh_token`` to Portal's token endpoint. The + refresh token is sent in the ``X-Refresh-Token`` header (not the body) + so it never lands in Portal's request-body access logs — mirroring the + device-flow CLI convention; Portal reconciles header vs. body and + rejects conflicts. + + Portal rotates the refresh token on every successful refresh, so the + returned ``Session.refresh_token`` is a NEW value the caller MUST + persist (replacing the old cookie). Failing to persist it means the + next refresh replays a rotated token and — outside Portal's 60s grace + — trips reuse-detection and revokes the whole session. + + Raises ``RefreshExpiredError`` on a 400 (expired / revoked / reuse- + detected), so the middleware clears cookies and forces re-login. + Raises ``ProviderError`` if Portal is unreachable. + """ + if not refresh_token: + # No RT to present — treat as a dead session so middleware + # forces a clean re-login rather than emitting a malformed POST. + raise RefreshExpiredError("no refresh token present in session") + + try: + response = httpx.post( + self._token_url, + # The refresh token goes in BOTH the body and the + # ``X-Refresh-Token`` header. Portal's token endpoint requires + # ``refresh_token`` in the body (its request schema rejects a + # header-only request as ``invalid_request``), and additionally + # reconciles the header against the body — sending both lets + # Portal keep the value out of body-access-logs while still + # satisfying the schema. (Verified against the NAS #293 preview + # deploy: header-only → 400 invalid_request; body → accepted.) + data={ + "grant_type": "refresh_token", + "client_id": self._client_id, + "refresh_token": refresh_token, + }, + headers={ + "Accept": "application/json", + "X-Refresh-Token": refresh_token, + }, + timeout=_TOKEN_ENDPOINT_TIMEOUT_SEC, + ) + except httpx.RequestError as exc: + raise ProviderError( + f"Portal token endpoint unreachable: {exc}" + ) from exc + + # A 400 on refresh means the RT is expired / revoked / reuse-detected; + # surface as RefreshExpiredError so middleware forces re-login. + return self._token_response_to_session( + response, bad_request_exc=RefreshExpiredError + ) + + def _token_response_to_session( + self, + response: httpx.Response, + *, + bad_request_exc: type[Exception], + ) -> Session: + """Translate a Portal ``/api/oauth/token`` response into a Session. + + Shared by ``complete_login`` (auth-code grant) and ``refresh_session`` + (refresh grant). ``bad_request_exc`` is the exception type raised on a + 400 — ``InvalidCodeError`` for the auth-code path, ``RefreshExpiredError`` + for the refresh path — so the middleware's distinct handling + (400-on-callback vs. force-relogin) is preserved. + """ if response.status_code == 400: - # Contract: invalid_code, invalid_grant, redirect_uri_mismatch all + # Contract: invalid_code / invalid_grant / redirect_uri_mismatch + # (auth-code) and expired / revoked / reuse-detected (refresh) all # surface as 400 with an OAuth-shaped JSON error envelope. body = self._parse_json_body(response) error_code = body.get("error", "invalid_request") - raise InvalidCodeError(f"Portal rejected code: {error_code}") + raise bad_request_exc(f"Portal rejected token request: {error_code}") if response.status_code != 200: raise ProviderError( f"Portal token endpoint returned {response.status_code}: " @@ -251,21 +335,14 @@ def complete_login( raise ProviderError(f"unexpected token_type={token_type!r}") claims = self._verify_jwt(access_token) - # Contract V1: no refresh token expected. If a future Portal ever - # adds one, capture it forward-compatibly. + # The dashboard grant issues a rotating refresh token; capture it so + # the caller can persist it. Empty string if Portal omitted it (the + # session then behaves as access-token-only until expiry). refresh_token = payload.get("refresh_token") or "" if not isinstance(refresh_token, str): refresh_token = "" return self._session_from_claims(access_token, refresh_token, claims) - def refresh_session(self, *, refresh_token: str) -> Session: - # Contract V1 has no refresh tokens — always force re-auth. If a - # future Portal contract starts issuing them, this method needs to - # be re-implemented; until then it's an unconditional refusal. - raise RefreshExpiredError( - "Nous Portal does not issue refresh tokens in OAuth contract v1; " - "user must re-authenticate via /auth/login." - ) def verify_session(self, *, access_token: str) -> Optional[Session]: # Contract: returns None on expiry/invalidity (middleware then @@ -284,9 +361,16 @@ def verify_session(self, *, access_token: str) -> Optional[Session]: return self._session_from_claims(access_token, "", claims) def revoke_session(self, *, refresh_token: str) -> None: - # Contract V1: no refresh tokens to revoke, and no Portal revocation - # endpoint documented for dashboard tokens. Logout is purely - # client-side cookie clearing; this is a best-effort no-op. + # Portal exposes no public refresh-token revocation grant on its token + # endpoint (revocation is driven from the authenticated /sessions UI, + # keyed by sessionId + userId, not by the RT value). So logout is + # client-side cookie clearing; the server-side refresh session simply + # expires within its 24h TTL. Best-effort no-op, must not raise. + # + # If Portal later adds a token-endpoint revoke grant (e.g. + # grant_type=... + X-Refresh-Token), implement it here so logout + # invalidates the RT server-side immediately rather than waiting out + # the TTL. _ = refresh_token return None diff --git a/tests/plugins/dashboard_auth/test_nous_provider.py b/tests/plugins/dashboard_auth/test_nous_provider.py index f6fc6fca42c7..04e7185647fc 100644 --- a/tests/plugins/dashboard_auth/test_nous_provider.py +++ b/tests/plugins/dashboard_auth/test_nous_provider.py @@ -542,7 +542,12 @@ def _mock_post(self, status_code: int, body: Any, *, ctype: str = "application/j def test_happy_path_returns_session(self, provider, rsa_keypair): access_token = _mint_token(rsa_keypair) mock_resp = self._mock_post( - 200, {"access_token": access_token, "token_type": "Bearer"} + 200, + { + "access_token": access_token, + "token_type": "Bearer", + "refresh_token": "rt_initial_value", + }, ) with patch("plugins.dashboard_auth.nous.httpx.post", return_value=mock_resp): session = provider.complete_login( @@ -555,11 +560,29 @@ def test_happy_path_returns_session(self, provider, rsa_keypair): assert session.user_id == "usr_abc" assert session.provider == "nous" assert session.access_token == access_token - assert session.refresh_token == "" # contract V1 + # The dashboard auth-code grant now issues a refresh token (NAS #293); + # complete_login must surface it so the middleware persists it. + assert session.refresh_token == "rt_initial_value" assert session.org_id == "org_xyz" assert session.email == "" assert session.display_name == "" + def test_happy_path_tolerates_missing_refresh_token(self, provider, rsa_keypair): + # If Portal omits refresh_token (older deploy), the session is still + # valid as access-token-only; refresh_token defaults to "". + access_token = _mint_token(rsa_keypair) + mock_resp = self._mock_post( + 200, {"access_token": access_token, "token_type": "Bearer"} + ) + with patch("plugins.dashboard_auth.nous.httpx.post", return_value=mock_resp): + session = provider.complete_login( + code="abc", + state="state-val", + code_verifier="vfy", + redirect_uri="https://hermes.fly.dev/auth/callback", + ) + assert session.refresh_token == "" + def test_400_raises_invalid_code(self, provider): mock_resp = self._mock_post(400, {"error": "invalid_grant"}) with patch("plugins.dashboard_auth.nous.httpx.post", return_value=mock_resp): @@ -730,24 +753,90 @@ def test_jwks_unreachable_raises_provider_error(self, provider, rsa_keypair): # --------------------------------------------------------------------------- -# refresh_session + revoke_session (V1 contract: trivial) +# refresh_session + revoke_session # --------------------------------------------------------------------------- class TestRefreshAndRevoke: @pytest.fixture - def provider(self): - return nous_plugin.NousDashboardAuthProvider( - client_id="agent:inst1", portal_url="https://portal.example.com" + def provider(self, rsa_keypair): + p = nous_plugin.NousDashboardAuthProvider( + client_id="agent:inst123", portal_url="https://portal.example.com" + ) + _patched_jwks(p, rsa_keypair) + return p + + def _mock_post(self, status_code, body, *, ctype="application/json"): + resp = MagicMock(spec=httpx.Response) + resp.status_code = status_code + if isinstance(body, dict): + resp.text = json.dumps(body) + resp.json = MagicMock(return_value=body) + else: + resp.text = body + resp.json = MagicMock(side_effect=ValueError("not json")) + resp.headers = {"content-type": ctype} + return resp + + def test_refresh_happy_path_returns_rotated_session(self, provider, rsa_keypair): + # Portal returns a fresh access token AND a rotated refresh token. + access_token = _mint_token(rsa_keypair) + mock_resp = self._mock_post( + 200, + { + "access_token": access_token, + "token_type": "Bearer", + "refresh_token": "rt_rotated_value", + }, ) + with patch( + "plugins.dashboard_auth.nous.httpx.post", return_value=mock_resp + ) as mock_post: + session = provider.refresh_session(refresh_token="rt_old_value") + + assert isinstance(session, Session) + assert session.access_token == access_token + # The ROTATED refresh token must be surfaced so the middleware can + # persist it back to the cookie. + assert session.refresh_token == "rt_rotated_value" + assert session.provider == "nous" - def test_refresh_always_raises(self, provider): - with pytest.raises(RefreshExpiredError): - provider.refresh_session(refresh_token="anything") + # Posts grant_type=refresh_token with the RT in BOTH the body (Portal's + # schema requires it there) and the X-Refresh-Token header (log + # redaction). Verified against the live preview deploy. + _, kwargs = mock_post.call_args + assert kwargs["data"]["grant_type"] == "refresh_token" + assert kwargs["data"]["client_id"] == "agent:inst123" + assert kwargs["data"]["refresh_token"] == "rt_old_value" + assert kwargs["headers"]["X-Refresh-Token"] == "rt_old_value" + + def test_refresh_400_raises_refresh_expired(self, provider): + # Expired / revoked / reuse-detected RT → Portal 400 → force re-login. + mock_resp = self._mock_post(400, {"error": "invalid_grant"}) + with patch("plugins.dashboard_auth.nous.httpx.post", return_value=mock_resp): + with pytest.raises(RefreshExpiredError, match="invalid_grant"): + provider.refresh_session(refresh_token="rt_dead") - def test_refresh_raises_even_with_empty_token(self, provider): - with pytest.raises(RefreshExpiredError): - provider.refresh_session(refresh_token="") + def test_refresh_empty_token_raises_refresh_expired_without_network(self, provider): + # No RT present — fail fast as a dead session, never hit the network. + with patch("plugins.dashboard_auth.nous.httpx.post") as mock_post: + with pytest.raises(RefreshExpiredError): + provider.refresh_session(refresh_token="") + mock_post.assert_not_called() + + def test_refresh_network_error_raises_provider_error(self, provider): + with patch( + "plugins.dashboard_auth.nous.httpx.post", + side_effect=httpx.RequestError("boom"), + ): + with pytest.raises(ProviderError, match="unreachable"): + provider.refresh_session(refresh_token="rt_x") + + def test_refresh_500_raises_provider_error(self, provider): + mock_resp = self._mock_post(500, "oops", ctype="text/plain") + with patch("plugins.dashboard_auth.nous.httpx.post", return_value=mock_resp): + with pytest.raises(ProviderError): + provider.refresh_session(refresh_token="rt_x") def test_revoke_is_noop(self, provider): # Must not raise; returns None implicitly. From d4b07452f7e34b871cd04799bfde1e594f2837ff Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 2 Jun 2026 16:36:21 +1000 Subject: [PATCH 2/3] fix(dashboard-auth): use Portal's x-nous-refresh-token header name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refresh-token header must match Portal's REFRESH_TOKEN_HEADER exactly ("x-nous-refresh-token"); the initial cut used "X-Refresh-Token", which Portal silently ignores (harmless since the RT is also in the body, which is what the schema requires — but the header redaction was a no-op). Confirmed against the NAS token route + re-validated live against the #293 preview deploy. --- plugins/dashboard_auth/nous/__init__.py | 19 +++++++++++-------- .../dashboard_auth/test_nous_provider.py | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/plugins/dashboard_auth/nous/__init__.py b/plugins/dashboard_auth/nous/__init__.py index f252136430d1..b36ddbb0238d 100644 --- a/plugins/dashboard_auth/nous/__init__.py +++ b/plugins/dashboard_auth/nous/__init__.py @@ -269,13 +269,16 @@ def refresh_session(self, *, refresh_token: str) -> Session: response = httpx.post( self._token_url, # The refresh token goes in BOTH the body and the - # ``X-Refresh-Token`` header. Portal's token endpoint requires - # ``refresh_token`` in the body (its request schema rejects a - # header-only request as ``invalid_request``), and additionally - # reconciles the header against the body — sending both lets - # Portal keep the value out of body-access-logs while still - # satisfying the schema. (Verified against the NAS #293 preview - # deploy: header-only → 400 invalid_request; body → accepted.) + # ``x-nous-refresh-token`` header. Portal's token endpoint + # requires ``refresh_token`` in the body (its request schema + # rejects a header-only request as ``invalid_request``), and + # additionally reconciles the header against the body — sending + # both lets Portal keep the value out of body-access-logs while + # still satisfying the schema. The header name must match + # Portal's ``REFRESH_TOKEN_HEADER`` exactly (``x-nous-refresh- + # token``); any other name is silently ignored. (Verified + # against the NAS #293 preview deploy: header-only → 400 + # invalid_request; body → accepted.) data={ "grant_type": "refresh_token", "client_id": self._client_id, @@ -283,7 +286,7 @@ def refresh_session(self, *, refresh_token: str) -> Session: }, headers={ "Accept": "application/json", - "X-Refresh-Token": refresh_token, + "x-nous-refresh-token": refresh_token, }, timeout=_TOKEN_ENDPOINT_TIMEOUT_SEC, ) diff --git a/tests/plugins/dashboard_auth/test_nous_provider.py b/tests/plugins/dashboard_auth/test_nous_provider.py index 04e7185647fc..114120bded4b 100644 --- a/tests/plugins/dashboard_auth/test_nous_provider.py +++ b/tests/plugins/dashboard_auth/test_nous_provider.py @@ -808,7 +808,7 @@ def test_refresh_happy_path_returns_rotated_session(self, provider, rsa_keypair) assert kwargs["data"]["grant_type"] == "refresh_token" assert kwargs["data"]["client_id"] == "agent:inst123" assert kwargs["data"]["refresh_token"] == "rt_old_value" - assert kwargs["headers"]["X-Refresh-Token"] == "rt_old_value" + assert kwargs["headers"]["x-nous-refresh-token"] == "rt_old_value" def test_refresh_400_raises_refresh_expired(self, provider): # Expired / revoked / reuse-detected RT → Portal 400 → force re-login. From 5e3f2302261229d889b823b0fb96f58ec6c1c570 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 2 Jun 2026 20:50:46 +1000 Subject: [PATCH 3/3] fix(dashboard-auth): refresh session when access-token cookie has been evicted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gated middleware bounced users to /login the instant the access-token cookie was absent, without ever consulting the refresh token: at, _rt = read_session_cookies(request) if not at: return _unauth_response(...) # bailed here This made transparent refresh effectively dead for the common case. The access-token cookie is set with Max-Age = access_token_expires_in (~15 min), so a real browser EVICTS hermes_session_at the moment the token lapses while hermes_session_rt persists (30-day Max-Age). From that point the browser sends only the refresh-token cookie — and the old guard rejected it before _attempt_refresh could run. The _attempt_refresh path only fired for a present-but-invalid access token, which never happens in a browser. Fix: only hard-bounce when NEITHER cookie is present. A request carrying just the refresh token now skips verification (no AT to verify) and flows into the existing refresh path, which rotates both cookies and serves the request transparently. A dead/expired RT still raises RefreshExpiredError and falls through to clear-and-relogin. This failure mode escaped the original tests + manual refresh button because both kept the access-token cookie present; only a real browser evicting the cookie at Max-Age exposes it. Added 3 regression tests covering: AT-evicted + RT-present (transparent refresh), no-cookies (still bounces), and RT-only with a dead RT (clean 401, no 500). --- hermes_cli/dashboard_auth/middleware.py | 55 +++++++----- .../test_dashboard_auth_401_reauth.py | 89 +++++++++++++++++++ 2 files changed, 123 insertions(+), 21 deletions(-) diff --git a/hermes_cli/dashboard_auth/middleware.py b/hermes_cli/dashboard_auth/middleware.py index 828ce9e59a44..8c6216e9ed6a 100644 --- a/hermes_cli/dashboard_auth/middleware.py +++ b/hermes_cli/dashboard_auth/middleware.py @@ -185,34 +185,47 @@ async def gated_auth_middleware( return await call_next(request) at, _rt = read_session_cookies(request) - if not at: + if not at and not _rt: + # Neither token present — no session at all. Nothing to verify or + # refresh; force login. return _unauth_response(request, reason="no_cookie") # Try every registered provider's verify_session in turn. Providers # MUST return None for tokens they don't recognise (not raise). This # lets multiple providers stack — the first one that recognises a # token wins. + # + # When the access-token cookie is absent but a refresh-token cookie is + # present, skip verification and go straight to the refresh path below. + # This is the COMMON expiry case, not an edge case: the access-token + # cookie is set with ``Max-Age = access_token_expires_in`` (~15 min), so + # the browser EVICTS it the moment the token lapses, while the + # refresh-token cookie lives for 30 days. From that point the browser + # sends only ``hermes_session_rt``. If we bailed on ``not at`` here we'd + # bounce the user to /login on every expiry despite holding a perfectly + # good refresh token — defeating the whole transparent-refresh feature. session = None - for provider in list_providers(): - try: - session = provider.verify_session(access_token=at) - except ProviderError as e: - _log.warning( - "dashboard-auth: provider %r unreachable during verify: %s", - provider.name, e, - ) - audit_log( - AuditEvent.SESSION_VERIFY_FAILURE, - provider=provider.name, - reason="provider_unreachable", - ip=_client_ip(request), - ) - return JSONResponse( - {"detail": f"Auth provider {provider.name!r} unreachable"}, - status_code=503, - ) - if session is not None: - break + if at: + for provider in list_providers(): + try: + session = provider.verify_session(access_token=at) + except ProviderError as e: + _log.warning( + "dashboard-auth: provider %r unreachable during verify: %s", + provider.name, e, + ) + audit_log( + AuditEvent.SESSION_VERIFY_FAILURE, + provider=provider.name, + reason="provider_unreachable", + ip=_client_ip(request), + ) + return JSONResponse( + {"detail": f"Auth provider {provider.name!r} unreachable"}, + status_code=503, + ) + if session is not None: + break if session is None: # Access token is expired/invalid. Before forcing re-login, try to diff --git a/tests/hermes_cli/test_dashboard_auth_401_reauth.py b/tests/hermes_cli/test_dashboard_auth_401_reauth.py index e4b1a044035d..121931b53c04 100644 --- a/tests/hermes_cli/test_dashboard_auth_401_reauth.py +++ b/tests/hermes_cli/test_dashboard_auth_401_reauth.py @@ -192,6 +192,95 @@ def test_login_url_drops_next_for_analytics_path(self, gated_app): assert "next=" not in body["login_url"] +class TestTransparentRefreshOnAccessTokenEviction: + """Regression: an expired access token whose cookie the browser has + ALREADY EVICTED must still transparently refresh via the RT cookie — + not bounce to /login. + + This is the common-path expiry bug, not an edge case. The access-token + cookie is set with ``Max-Age = access_token_expires_in`` (~15 min), so + the browser deletes ``hermes_session_at`` the instant the token lapses, + while ``hermes_session_rt`` lives for 30 days. From that moment the + browser sends ONLY the refresh-token cookie. The original gate bailed at + ``if not at: return _unauth_response(...)`` — bouncing the user to + /login on every single expiry despite holding a perfectly good refresh + token, defeating the entire transparent-refresh feature. The fix lets a + request carrying only the RT flow into the refresh path. + + Discrimination: under the pre-fix code, scenario 1 (AT cookie absent, + RT present) returned 401/302 to login with NO rotated cookies and NO + REFRESH_SUCCESS — the refresh code never ran. With the fix it returns + 200 and rotates both cookies. + """ + + def _build_rt_only_app(self): + """Gate over the real app with a Stub provider whose RT is live + (default_ttl>0 so refresh succeeds). Mint a valid signed RT + directly (the stub's refresh_session only checks the RT's + signature + exp), then send ONLY that RT cookie. + """ + import time as _t + from tests.hermes_cli.conftest_dashboard_auth import _sign + + clear_providers() + provider = StubAuthProvider(default_ttl=900) + register_provider(provider) + valid_rt = _sign( + {"sub": "stub-user-1", "kind": "refresh", "exp": int(_t.time()) + 30 * 86400} + ) + return provider, valid_rt + + def test_at_evicted_rt_present_refreshes_transparently(self, gated_app): + provider, valid_rt = self._build_rt_only_app() + # Browser sends ONLY the RT cookie — the AT cookie has aged out. + gated_app.cookies.clear() + gated_app.cookies.set(SESSION_RT_COOKIE, valid_rt) + + r = gated_app.get("/api/sessions", follow_redirects=False) + # Transparent refresh — request served, NOT bounced. + assert r.status_code == 200, ( + f"expected 200 (transparent refresh) got {r.status_code} " + f"— the AT-evicted/RT-present case bounced to login" + ) + # Both cookies rotated onto the response. + set_cookies = r.headers.get_list("set-cookie") + assert any( + c.startswith(SESSION_AT_COOKIE) or f"-{SESSION_AT_COOKIE}" in c + for c in set_cookies + ), f"no rotated AT cookie in {set_cookies!r}" + assert any( + c.startswith(SESSION_RT_COOKIE) or f"-{SESSION_RT_COOKIE}" in c + for c in set_cookies + ), f"no rotated RT cookie in {set_cookies!r}" + + def test_no_cookies_at_all_still_bounces(self, gated_app): + """Guard the fix didn't over-reach: a request with NEITHER cookie + must still 401 to login (nothing to verify or refresh).""" + self._build_rt_only_app() + gated_app.cookies.clear() + r = gated_app.get("/api/sessions") + assert r.status_code == 401 + assert r.json()["error"] == "unauthenticated" + + def test_dead_rt_only_bounces_to_login(self, gated_app): + """An RT-only request whose RT is dead/expired must bounce (the + refresh raises RefreshExpiredError → clear + relogin), not 500.""" + clear_providers() + # default_ttl=0 → the stub treats the minted RT as born-expired, + # so refresh_session raises RefreshExpiredError. + provider = StubAuthProvider(default_ttl=0) + register_provider(provider) + gated_app.cookies.clear() + # A syntactically-real but expired RT (signed with exp<=now). + import time as _t + from tests.hermes_cli.conftest_dashboard_auth import _sign + dead_rt = _sign({"sub": "u", "kind": "refresh", "exp": int(_t.time()) - 1}) + gated_app.cookies.set(SESSION_RT_COOKIE, dead_rt) + r = gated_app.get("/api/sessions") + assert r.status_code == 401 + assert r.json()["error"] == "session_expired" + + class TestHtmlRedirectNext: def test_deep_html_path_redirects_with_next(self, gated_app): r = gated_app.get("/sessions", follow_redirects=False)