Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import logging

from fastapi import APIRouter, Cookie, Depends, HTTPException, Response
from fastapi import APIRouter, Cookie, Depends, HTTPException, Request, Response
from pydantic import BaseModel

from config import (
Expand All @@ -20,6 +20,7 @@
from identity_access_management_context.domain.exceptions import (
InvalidRefreshTokenException,
)
from shared_kernel.adapters.primary.cookie_revocation import revoke_cookie

logger = logging.getLogger(__name__)

Expand All @@ -37,6 +38,7 @@ class RefreshAccessTokenResponse(BaseModel):
summary="Refresh access token",
)
async def refresh_access_token(
request: Request,
response: Response,
refresh_token_cookie: str | None = Cookie(None, alias="refresh_token"),
usecase: RefreshAccessTokenUseCase = Depends(get_refresh_access_token_usecase),
Expand Down Expand Up @@ -96,9 +98,10 @@ async def refresh_access_token(
return RefreshAccessTokenResponse(message="Access token refreshed successfully")
except InvalidRefreshTokenException as e:
is_secure = get_cookie_secure_setting()
response.delete_cookie("access_token", secure=is_secure, samesite="strict")
response.delete_cookie("refresh_token", secure=is_secure, samesite="strict")
response.delete_cookie("logged_in", secure=is_secure, samesite="strict")
# The session is dead: revoke it in the browser too. access_token and
# refresh_token are httpOnly, so the server is the only actor that can.
for cookie_name in ("access_token", "refresh_token", "logged_in"):
revoke_cookie(request, response, cookie_name, secure=is_secure, samesite="strict")
raise HTTPException(status_code=400, detail=str(e)) from e
except Exception as e:
logger.exception("Unexpected error in refresh access token")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,17 @@
InvalidSsoCodeException,
SsoEncryptionUnavailableError,
)
from shared_kernel.adapters.primary.cookie_revocation import revoke_cookie

logger = logging.getLogger(__name__)

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


def _revoke_sso_state(request: Request, response: Response) -> None:
revoke_cookie(request, response, SSO_STATE_COOKIE, secure=get_cookie_secure_setting(), samesite="lax")


def _state_rejection_reason(state: str | None, expected_state: str | None) -> str | None:
"""Why the CSRF state check failed, or None when it passed."""
if not state:
Expand Down Expand Up @@ -87,9 +92,11 @@ async def sso_callback(
# The body stays generic so a forged state learns nothing. The reason goes
# to the log only, and never carries the state itself — it is a CSRF token.
logger.warning("Rejecting SSO callback: %s", rejection_reason)
response.delete_cookie(SSO_STATE_COOKIE)
_revoke_sso_state(request, response)
raise HTTPException(status_code=400, detail="Invalid SSO state")
response.delete_cookie(SSO_STATE_COOKIE)
# The state is single-use: revoke it now, so it stays revoked even when the
# code exchange below fails and the route raises.
_revoke_sso_state(request, response)

try:
command = SsoLoginCommand(code=code, redirect_uri=redirect_uri)
Expand Down
16 changes: 11 additions & 5 deletions server/src/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
SecurityHeadersMiddleware,
csrf_router,
)
from shared_kernel.adapters.primary.cookie_revocation import replay_cookie_revocations
from shared_kernel.adapters.primary.request_id_middleware import (
RequestIdFilter,
RequestIdMiddleware,
Expand Down Expand Up @@ -274,7 +275,9 @@ async def _run_migrations_in_background():
@app.exception_handler(Exception)
async def unhandled_exception_handler(request: Request, exc: Exception) -> JSONResponse:
logger.error("Unhandled exception on %s %s", request.method, request.url.path, exc_info=exc)
return JSONResponse(status_code=500, content={"detail": "Internal server error"})
return replay_cookie_revocations(
request, JSONResponse(status_code=500, content={"detail": "Internal server error"})
)


@app.exception_handler(HTTPException)
Expand All @@ -287,10 +290,13 @@ async def http_exception_handler(request: Request, exc: HTTPException) -> JSONRe
request.url.path,
exc.detail,
)
return JSONResponse(
status_code=exc.status_code,
content={"detail": exc.detail},
headers=exc.headers,
return replay_cookie_revocations(
request,
JSONResponse(
status_code=exc.status_code,
content={"detail": exc.detail},
headers=exc.headers,
),
)


Expand Down
36 changes: 36 additions & 0 deletions server/src/shared_kernel/adapters/primary/cookie_revocation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
from typing import Literal, TypeVar

from starlette.requests import Request
from starlette.responses import Response

SameSite = Literal["lax", "strict", "none"]
ResponseT = TypeVar("ResponseT", bound=Response)

_REVOCATIONS_SCOPE_KEY = "cookie_revocations"


def revoke_cookie(request: Request, response: Response, key: str, *, secure: bool, samesite: SameSite) -> None:
"""
Clear a cookie whether the endpoint returns or raises.

``response.delete_cookie`` alone only reaches the client on the success path:
raising ``HTTPException`` makes the global handler build a fresh JSONResponse,
which drops everything staged on the injected Response. Recording the
revocation on the request lets that handler replay it.

This matters most for httpOnly cookies — the server is the only actor able to
clear them, since no frontend code can touch them.
"""
response.delete_cookie(key, secure=secure, samesite=samesite)
revocations = getattr(request.state, _REVOCATIONS_SCOPE_KEY, None)
if revocations is None:
revocations = []
setattr(request.state, _REVOCATIONS_SCOPE_KEY, revocations)
revocations.append((key, secure, samesite))


def replay_cookie_revocations(request: Request, response: ResponseT) -> ResponseT:
"""Re-apply on ``response`` the revocations staged during the request."""
for key, secure, samesite in getattr(request.state, _REVOCATIONS_SCOPE_KEY, ()):
response.delete_cookie(key, secure=secure, samesite=samesite)
return response
34 changes: 34 additions & 0 deletions server/tests/e2e/test_complete_authentication_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,40 @@ async def test_complete_authentication_workflow(
assert "refresh token" in refresh_after_logout.json()["detail"].lower()
print("✅ Refresh-only logout clears server session and blocks token refresh")

# Step 6.4: a rejected refresh must also revoke the browser's cookies. Only the
# server can do this: access_token and refresh_token are httpOnly, so no frontend
# code is able to clear them. The route stages the deletions on its Response and
# then raises, so they have to survive the trip through the global HTTPException
# handler to reach the client at all.
print("\n🧹 Step 6.4: A rejected refresh must revoke the session cookies...")
stale_client = client_factory()
stale_login = stale_client.post(
"/api/auth/login",
json={"email": "admin@example.com", "password": "securepassword123"},
)
assert stale_login.status_code == 200
assert stale_client.cookies.get("logged_in") == "true"

# Rotate that session's refresh token from elsewhere, which invalidates the copy
# stale_client still holds. Every cookie in its jar is server-issued, so the
# revocations below are matched the way a browser would match them.
rotator = client_factory()
rotator.cookies.set("refresh_token", stale_client.cookies.get("refresh_token"))
assert rotator.post("/api/auth/refresh-token").status_code == 200

rejected_refresh = stale_client.post("/api/auth/refresh-token")
assert rejected_refresh.status_code == 400

revocations = " ".join(rejected_refresh.headers.get_list("set-cookie"))
for cookie_name in ("access_token", "refresh_token", "logged_in"):
assert f"{cookie_name}=" in revocations, (
f"the 400 must revoke {cookie_name} — only the server can clear an httpOnly cookie"
)
assert stale_client.cookies.get(cookie_name) is None, (
f"{cookie_name} should be gone from the jar after a rejected refresh"
)
print("✅ Rejected refresh revokes access_token, refresh_token and logged_in")

# =========================================================================
# PHASE 7: ACCOUNT LOCKOUT
# =========================================================================
Expand Down
13 changes: 13 additions & 0 deletions server/tests/e2e/test_sso_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,16 @@ def test_given_no_state_parameter_when_calling_callback_should_log_the_missing_p
assert response.status_code == 400
assert response.json()["detail"] == "Invalid SSO state"
assert "state query parameter missing" in caplog.text


def test_given_an_invalid_state_when_calling_callback_should_revoke_the_sso_state_cookie(e2e_client, configured_sso):
# The stale state must not survive a rejected callback: the route stages the
# deletion before raising, so it has to outlive the HTTPException.
e2e_client.get("/api/auth/sso/url")
assert e2e_client.cookies.get("sso_state")

response = e2e_client.get("/api/auth/sso/callback?code=anything&state=forged-state")

assert response.status_code == 400
assert "sso_state=" in " ".join(response.headers.get_list("set-cookie"))
assert e2e_client.cookies.get("sso_state") is None