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) <noreply@anthropic.com>
This commit is contained in:
parent
519e3659a1
commit
27763d19ea
2 changed files with 72 additions and 3 deletions
|
|
@ -67,7 +67,10 @@ def _build_server_config(settings: Settings) -> dict:
|
||||||
"expires_in": 3600,
|
"expires_in": 3600,
|
||||||
},
|
},
|
||||||
"refresh_token": {
|
"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,
|
"expires_in": 86400,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
|
|
||||||
|
|
@ -33,8 +33,8 @@ async def _setup_rp_and_user(app: FastAPI) -> None:
|
||||||
"redirect_uris": [(REDIRECT_URI, {})],
|
"redirect_uris": [(REDIRECT_URI, {})],
|
||||||
"response_types_supported": ["code"],
|
"response_types_supported": ["code"],
|
||||||
"token_endpoint_auth_method": "client_secret_basic",
|
"token_endpoint_auth_method": "client_secret_basic",
|
||||||
"scope": ["openid", "profile", "email"],
|
"scope": ["openid", "profile", "email", "offline_access"],
|
||||||
"allowed_scopes": ["openid", "profile", "email"],
|
"allowed_scopes": ["openid", "profile", "email", "offline_access"],
|
||||||
"client_salt": secrets.token_hex(8),
|
"client_salt": secrets.token_hex(8),
|
||||||
}
|
}
|
||||||
oidc_server.keyjar.add_symmetric(CLIENT_ID, CLIENT_SECRET)
|
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}"
|
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
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue