Skip to content

Commit c546c11

Browse files
vianneybacoupclaude
andcommitted
fix(iam): revoke the single-use sso_state cookie on error paths
Both `delete_cookie` calls in the SSO callback sat in front of a `raise`, so neither ever reached the client. The consequence is smaller than the refresh one but real: once the state check passes, the state is meant to be spent, and a failing code exchange left it replayable for the rest of its 10-minute window. Route them through `revoke_cookie` so the deletion survives the raise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b0e6759 commit c546c11

2 files changed

Lines changed: 22 additions & 2 deletions

File tree

server/src/identity_access_management_context/adapters/primary/fastapi/routes/sso/sso_callback_route.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,17 @@
2525
InvalidSsoCodeException,
2626
SsoEncryptionUnavailableError,
2727
)
28+
from shared_kernel.adapters.primary.cookie_revocation import revoke_cookie
2829

2930
logger = logging.getLogger(__name__)
3031

3132
router = APIRouter(prefix="/auth", tags=["Authentication"])
3233

3334

35+
def _revoke_sso_state(request: Request, response: Response) -> None:
36+
revoke_cookie(request, response, SSO_STATE_COOKIE, secure=get_cookie_secure_setting(), samesite="lax")
37+
38+
3439
def _state_rejection_reason(state: str | None, expected_state: str | None) -> str | None:
3540
"""Why the CSRF state check failed, or None when it passed."""
3641
if not state:
@@ -87,9 +92,11 @@ async def sso_callback(
8792
# The body stays generic so a forged state learns nothing. The reason goes
8893
# to the log only, and never carries the state itself — it is a CSRF token.
8994
logger.warning("Rejecting SSO callback: %s", rejection_reason)
90-
response.delete_cookie(SSO_STATE_COOKIE)
95+
_revoke_sso_state(request, response)
9196
raise HTTPException(status_code=400, detail="Invalid SSO state")
92-
response.delete_cookie(SSO_STATE_COOKIE)
97+
# The state is single-use: revoke it now, so it stays revoked even when the
98+
# code exchange below fails and the route raises.
99+
_revoke_sso_state(request, response)
93100

94101
try:
95102
command = SsoLoginCommand(code=code, redirect_uri=redirect_uri)

server/tests/e2e/test_sso_security.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,16 @@ def test_given_no_state_parameter_when_calling_callback_should_log_the_missing_p
130130
assert response.status_code == 400
131131
assert response.json()["detail"] == "Invalid SSO state"
132132
assert "state query parameter missing" in caplog.text
133+
134+
135+
def test_given_an_invalid_state_when_calling_callback_should_revoke_the_sso_state_cookie(e2e_client, configured_sso):
136+
# The stale state must not survive a rejected callback: the route stages the
137+
# deletion before raising, so it has to outlive the HTTPException.
138+
e2e_client.get("/api/auth/sso/url")
139+
assert e2e_client.cookies.get("sso_state")
140+
141+
response = e2e_client.get("/api/auth/sso/callback?code=anything&state=forged-state")
142+
143+
assert response.status_code == 400
144+
assert "sso_state=" in " ".join(response.headers.get_list("set-cookie"))
145+
assert e2e_client.cookies.get("sso_state") is None

0 commit comments

Comments
 (0)