Skip to content

Commit 3ec4b6a

Browse files
LEANDERANTONYclaude
andcommitted
fix(low): assert Secure/SameSite + clear scope on auth cookies (L8)
Background: set_auth_cookies sets httponly+secure+samesite (config-driven), but the auth-endpoint tests only asserted HttpOnly. A config/refactor that dropped Secure or SameSite — or set samesite='none' without secure, which browsers reject — would have failed no test, silently weakening session-cookie security. Fix: no production change. Add two unit tests exercising the cookie helper directly with prod-shaped settings monkeypatched in (the dev defaults leave Secure off): one asserts set_auth_cookies emits HttpOnly + Secure + SameSite + the right Domain/Path on both cookies; one asserts clear_auth_cookies deletes with the SAME path/domain (otherwise the browser keeps the original cookie and the user stays signed in) and actually expires them. Test: new tests pass; full test_backend_auth.py suite green (7 tests). Fixes: L8 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 064532c commit 3ec4b6a

1 file changed

Lines changed: 74 additions & 0 deletions

File tree

tests/test_backend_auth.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,3 +223,77 @@ def test_workspace_handoff_exchange_sets_cookies_and_scrubs_tokens(monkeypatch):
223223
assert payload["session"] == {"authenticated": True}
224224
assert response.cookies["ja_access_token"] == "access-token"
225225
assert response.cookies["ja_refresh_token"] == "refresh-token"
226+
227+
228+
# ---------------------------------------------------------------------------
229+
# Auth-cookie security attributes (review L8)
230+
#
231+
# The endpoint tests above only assert HttpOnly, so a config/refactor that
232+
# dropped Secure / SameSite (or set samesite='none' without secure — which
233+
# browsers reject) would fail no test. These pin the full security envelope by
234+
# exercising the cookie helper directly with PROD-shaped settings monkeypatched
235+
# in, since the dev defaults leave Secure off.
236+
# ---------------------------------------------------------------------------
237+
238+
239+
def _prod_cookie_settings():
240+
from types import SimpleNamespace
241+
242+
return SimpleNamespace(
243+
auth_cookie_secure=True,
244+
auth_cookie_samesite="lax",
245+
auth_cookie_domain="app.example.com",
246+
)
247+
248+
249+
def _set_cookie_headers(response):
250+
return [
251+
value.decode()
252+
for (key, value) in response.raw_headers
253+
if key == b"set-cookie"
254+
]
255+
256+
257+
def test_set_auth_cookies_carries_secure_samesite_and_scope(monkeypatch):
258+
from fastapi import Response
259+
260+
from backend.services import auth_cookies
261+
262+
monkeypatch.setattr(
263+
auth_cookies, "get_backend_settings", _prod_cookie_settings
264+
)
265+
response = Response()
266+
auth_cookies.set_auth_cookies(response, "access-token", "refresh-token")
267+
268+
cookies = _set_cookie_headers(response)
269+
assert len(cookies) == 2
270+
for cookie in cookies:
271+
lowered = cookie.lower()
272+
assert "httponly" in lowered
273+
assert "secure" in lowered
274+
assert "samesite=lax" in lowered
275+
assert "domain=app.example.com" in lowered
276+
assert "path=/" in lowered
277+
278+
279+
def test_clear_auth_cookies_matches_path_and_domain(monkeypatch):
280+
from fastapi import Response
281+
282+
from backend.services import auth_cookies
283+
284+
monkeypatch.setattr(
285+
auth_cookies, "get_backend_settings", _prod_cookie_settings
286+
)
287+
response = Response()
288+
auth_cookies.clear_auth_cookies(response)
289+
290+
cookies = _set_cookie_headers(response)
291+
assert len(cookies) == 2
292+
for cookie in cookies:
293+
lowered = cookie.lower()
294+
# Same path/domain the cookie was set with — otherwise the browser
295+
# keeps the original and the user stays "signed in" indefinitely.
296+
assert "domain=app.example.com" in lowered
297+
assert "path=/" in lowered
298+
# An actual deletion: expired (Max-Age=0 and/or a past Expires).
299+
assert "max-age=0" in lowered or "expires=" in lowered

0 commit comments

Comments
 (0)