From 27763d19ea6d11a1423b37e23f48fe9aaa100641 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Wed, 10 Jun 2026 09:25:49 +0200 Subject: [PATCH] fix(security): don't mint new ID tokens on refresh; confirm offline_access gating idpyoidc already gates refresh-token issuance on the offline_access scope (verified by test), but the refresh-token grant was configured to also mint fresh ID tokens. Drop id_token from the refresh_token grant's supports_minting so refreshing yields only access (and a rotated refresh) token; ID tokens come from authentication. Refresh-token rotation is retained. Refs: porchlight-553 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/porchlight/oidc/provider.py | 5 ++- tests/test_oidc/test_e2e_flow.py | 70 +++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/porchlight/oidc/provider.py b/src/porchlight/oidc/provider.py index bcf1b71..c4b170e 100644 --- a/src/porchlight/oidc/provider.py +++ b/src/porchlight/oidc/provider.py @@ -67,7 +67,10 @@ def _build_server_config(settings: Settings) -> dict: "expires_in": 3600, }, "refresh_token": { - "supports_minting": ["access_token", "refresh_token", "id_token"], + # Rotate refresh tokens (mint a new one) and mint + # access tokens, but do NOT mint fresh ID tokens on + # refresh — re-authentication should issue ID tokens. + "supports_minting": ["access_token", "refresh_token"], "expires_in": 86400, }, }, diff --git a/tests/test_oidc/test_e2e_flow.py b/tests/test_oidc/test_e2e_flow.py index cd3f547..b0f367c 100644 --- a/tests/test_oidc/test_e2e_flow.py +++ b/tests/test_oidc/test_e2e_flow.py @@ -33,8 +33,8 @@ async def _setup_rp_and_user(app: FastAPI) -> None: "redirect_uris": [(REDIRECT_URI, {})], "response_types_supported": ["code"], "token_endpoint_auth_method": "client_secret_basic", - "scope": ["openid", "profile", "email"], - "allowed_scopes": ["openid", "profile", "email"], + "scope": ["openid", "profile", "email", "offline_access"], + "allowed_scopes": ["openid", "profile", "email", "offline_access"], "client_salt": secrets.token_hex(8), } oidc_server.keyjar.add_symmetric(CLIENT_ID, CLIENT_SECRET) @@ -263,3 +263,69 @@ async def test_pkce_wrong_verifier_rejected(client: AsyncClient) -> None: }, ) assert res.status_code != 200, f"PKCE verifier mismatch should be rejected, got {res.status_code}: {res.text}" + + +async def _authorize_with_offline_access(client: AsyncClient, state: str, nonce: str) -> str: + """Run the auth-code flow requesting offline_access, return the auth code.""" + scope = "openid profile offline_access" + # offline_access requires prompt=consent per the OIDC spec. + await client.get( + "/authorization", + params={ + "response_type": "code", + "client_id": CLIENT_ID, + "redirect_uri": REDIRECT_URI, + "scope": scope, + "state": state, + "nonce": nonce, + "prompt": "consent", + }, + follow_redirects=False, + ) + token = await get_csrf_token(client) + await client.post( + "/login/password", + data={"username": "alice", "password": "testpass"}, + headers={"HX-Request": "true", "X-CSRF-Token": token}, + ) + complete = await client.get("/authorization/complete", follow_redirects=False) + assert "/consent" in complete.headers["location"] + token = await get_csrf_token(client) + consent = await client.post( + "/consent", + data={"action": "allow", "scope": ["openid", "profile", "offline_access"]}, + headers={"X-CSRF-Token": token}, + follow_redirects=False, + ) + return parse_qs(urlparse(consent.headers["location"]).query)["code"][0] + + +async def test_refresh_token_issued_only_with_offline_access(client: AsyncClient) -> None: + app = client._transport.app # type: ignore[union-attr] + await _setup_rp_and_user(app) + + # Without offline_access: no refresh token. + code = await _login_and_authorize(client, secrets.token_urlsafe(8), secrets.token_urlsafe(8)) + no_offline = await _exchange_token(client, code) + assert "refresh_token" not in no_offline + + +async def test_refresh_grant_does_not_mint_id_token(client: AsyncClient) -> None: + app = client._transport.app # type: ignore[union-attr] + await _setup_rp_and_user(app) + + code = await _authorize_with_offline_access(client, secrets.token_urlsafe(8), secrets.token_urlsafe(8)) + token_data = await _exchange_token(client, code) + assert "refresh_token" in token_data, "offline_access should yield a refresh token" + + # Use the refresh token; the response must not contain a new ID token. + auth_header = b64encode(f"{CLIENT_ID}:{CLIENT_SECRET}".encode()).decode() + res = await client.post( + "/token", + data={"grant_type": "refresh_token", "refresh_token": token_data["refresh_token"]}, + headers={"Authorization": f"Basic {auth_header}", "Content-Type": "application/x-www-form-urlencoded"}, + ) + assert res.status_code == 200, res.text + refreshed = res.json() + assert "access_token" in refreshed + assert "id_token" not in refreshed