Fix cookie not accepted in safari
This commit is contained in:
@@ -27,6 +27,8 @@ AUTH_LOGIN_FAILURE_WINDOW_SECONDS=900
|
||||
AUTH_LOGIN_LOCKOUT_BASE_SECONDS=30
|
||||
AUTH_LOGIN_LOCKOUT_MAX_SECONDS=900
|
||||
# Optional cookie controls for split frontend/api hosts:
|
||||
# Leave AUTH_COOKIE_DOMAIN empty unless you explicitly need a parent-domain CSRF cookie mirror.
|
||||
# Host-only auth cookies are issued automatically for the API host.
|
||||
# AUTH_COOKIE_DOMAIN=docs.lan
|
||||
# AUTH_COOKIE_SAMESITE=auto
|
||||
|
||||
|
||||
@@ -107,12 +107,16 @@ def get_request_auth_context(
|
||||
|
||||
token = credentials.credentials.strip() if credentials is not None and credentials.credentials else ""
|
||||
using_cookie_session = False
|
||||
session_candidates: list[str] = []
|
||||
|
||||
if not token:
|
||||
token = (session_cookie or "").strip()
|
||||
using_cookie_session = True
|
||||
if not token:
|
||||
_raise_unauthorized()
|
||||
session_candidates = [candidate for candidate in _extract_cookie_values(request, SESSION_COOKIE_NAME) if candidate]
|
||||
normalized_session_cookie = (session_cookie or "").strip()
|
||||
if normalized_session_cookie and normalized_session_cookie not in session_candidates:
|
||||
session_candidates.append(normalized_session_cookie)
|
||||
if not session_candidates:
|
||||
_raise_unauthorized()
|
||||
|
||||
if _requires_csrf_validation(request.method) and using_cookie_session:
|
||||
normalized_csrf_header = (csrf_header or "").strip()
|
||||
@@ -127,7 +131,15 @@ def get_request_auth_context(
|
||||
):
|
||||
_raise_csrf_rejected()
|
||||
|
||||
resolved_session = resolve_auth_session(session, token=token)
|
||||
resolved_session = None
|
||||
if token:
|
||||
resolved_session = resolve_auth_session(session, token=token)
|
||||
else:
|
||||
for candidate in session_candidates:
|
||||
resolved_session = resolve_auth_session(session, token=candidate)
|
||||
if resolved_session is not None and resolved_session.user is not None:
|
||||
break
|
||||
|
||||
if resolved_session is None or resolved_session.user is None:
|
||||
_raise_unauthorized()
|
||||
|
||||
|
||||
@@ -90,12 +90,49 @@ def _resolve_cookie_domain() -> str | None:
|
||||
return configured_domain
|
||||
|
||||
|
||||
def _resolve_cookie_samesite(secure_cookie: bool) -> str:
|
||||
"""Returns cookie SameSite mode with secure-aware defaults for browser compatibility."""
|
||||
def _resolve_cookie_domains() -> tuple[str | None, ...]:
|
||||
"""Returns cookie domain variants with a host-only cookie first for browser compatibility."""
|
||||
|
||||
configured_domain = _resolve_cookie_domain()
|
||||
if configured_domain is None:
|
||||
return (None,)
|
||||
return (None, configured_domain)
|
||||
|
||||
|
||||
def _request_matches_cookie_domain(request: Request) -> bool:
|
||||
"""Returns whether request and origin hosts both sit under the configured cookie domain."""
|
||||
|
||||
configured_domain = _resolve_cookie_domain()
|
||||
if configured_domain is None:
|
||||
return False
|
||||
|
||||
origin_header = request.headers.get("origin", "").strip()
|
||||
origin_host = urlparse(origin_header).hostname.strip().lower() if origin_header else ""
|
||||
if not origin_host:
|
||||
return False
|
||||
|
||||
request_url = getattr(request, "url", None)
|
||||
request_host = str(getattr(request_url, "hostname", "")).strip().lower() if request_url is not None else ""
|
||||
if not request_host:
|
||||
parsed_public_base_url = urlparse(get_settings().public_base_url.strip())
|
||||
request_host = parsed_public_base_url.hostname.strip().lower() if parsed_public_base_url.hostname else ""
|
||||
if not request_host:
|
||||
return False
|
||||
|
||||
def _matches(candidate: str) -> bool:
|
||||
return candidate == configured_domain or candidate.endswith(f".{configured_domain}")
|
||||
|
||||
return _matches(origin_host) and _matches(request_host)
|
||||
|
||||
|
||||
def _resolve_cookie_samesite(request: Request, secure_cookie: bool) -> str:
|
||||
"""Returns cookie SameSite mode with same-site subdomain compatibility defaults."""
|
||||
|
||||
configured_mode = get_settings().auth_cookie_samesite.strip().lower()
|
||||
if configured_mode in {"strict", "lax", "none"}:
|
||||
if configured_mode in {"strict", "lax"}:
|
||||
return configured_mode
|
||||
if configured_mode == "none":
|
||||
return "lax" if _request_matches_cookie_domain(request) else "none"
|
||||
return "none" if secure_cookie else "lax"
|
||||
|
||||
|
||||
@@ -107,30 +144,39 @@ def _session_cookie_ttl_seconds(expires_at: datetime) -> int:
|
||||
return max(1, ttl)
|
||||
|
||||
|
||||
def _set_session_cookie(response: Response, session_token: str, *, expires_at: datetime, secure: bool) -> None:
|
||||
def _set_session_cookie(
|
||||
response: Response,
|
||||
session_token: str,
|
||||
*,
|
||||
request: Request,
|
||||
expires_at: datetime,
|
||||
secure: bool,
|
||||
) -> None:
|
||||
"""Stores the issued session token in a browser HttpOnly auth cookie."""
|
||||
|
||||
if response is None or not hasattr(response, "set_cookie"):
|
||||
return
|
||||
expires_seconds = _session_cookie_ttl_seconds(expires_at)
|
||||
cookie_domain = _resolve_cookie_domain()
|
||||
same_site_mode = _resolve_cookie_samesite(secure)
|
||||
response.set_cookie(
|
||||
SESSION_COOKIE_NAME,
|
||||
value=session_token,
|
||||
max_age=expires_seconds,
|
||||
httponly=True,
|
||||
secure=secure,
|
||||
samesite=same_site_mode,
|
||||
path="/",
|
||||
domain=cookie_domain,
|
||||
)
|
||||
same_site_mode = _resolve_cookie_samesite(request, secure)
|
||||
for cookie_domain in _resolve_cookie_domains():
|
||||
cookie_kwargs = {
|
||||
"value": session_token,
|
||||
"max_age": expires_seconds,
|
||||
"httponly": True,
|
||||
"secure": secure,
|
||||
"samesite": same_site_mode,
|
||||
"path": "/",
|
||||
}
|
||||
if cookie_domain is not None:
|
||||
cookie_kwargs["domain"] = cookie_domain
|
||||
response.set_cookie(SESSION_COOKIE_NAME, **cookie_kwargs)
|
||||
|
||||
|
||||
def _set_csrf_cookie(
|
||||
response: Response,
|
||||
csrf_token: str,
|
||||
*,
|
||||
request: Request,
|
||||
expires_at: datetime,
|
||||
secure: bool,
|
||||
) -> None:
|
||||
@@ -138,18 +184,19 @@ def _set_csrf_cookie(
|
||||
|
||||
if response is None or not hasattr(response, "set_cookie"):
|
||||
return
|
||||
cookie_domain = _resolve_cookie_domain()
|
||||
same_site_mode = _resolve_cookie_samesite(secure)
|
||||
response.set_cookie(
|
||||
CSRF_COOKIE_NAME,
|
||||
value=csrf_token,
|
||||
max_age=_session_cookie_ttl_seconds(expires_at),
|
||||
httponly=False,
|
||||
secure=secure,
|
||||
samesite=same_site_mode,
|
||||
path="/",
|
||||
domain=cookie_domain,
|
||||
)
|
||||
same_site_mode = _resolve_cookie_samesite(request, secure)
|
||||
for cookie_domain in _resolve_cookie_domains():
|
||||
cookie_kwargs = {
|
||||
"value": csrf_token,
|
||||
"max_age": _session_cookie_ttl_seconds(expires_at),
|
||||
"httponly": False,
|
||||
"secure": secure,
|
||||
"samesite": same_site_mode,
|
||||
"path": "/",
|
||||
}
|
||||
if cookie_domain is not None:
|
||||
cookie_kwargs["domain"] = cookie_domain
|
||||
response.set_cookie(CSRF_COOKIE_NAME, **cookie_kwargs)
|
||||
|
||||
|
||||
def _clear_session_cookies(response: Response) -> None:
|
||||
@@ -157,9 +204,12 @@ def _clear_session_cookies(response: Response) -> None:
|
||||
|
||||
if response is None or not hasattr(response, "delete_cookie"):
|
||||
return
|
||||
cookie_domain = _resolve_cookie_domain()
|
||||
response.delete_cookie(SESSION_COOKIE_NAME, path="/", domain=cookie_domain)
|
||||
response.delete_cookie(CSRF_COOKIE_NAME, path="/", domain=cookie_domain)
|
||||
for cookie_domain in _resolve_cookie_domains():
|
||||
delete_kwargs = {"path": "/"}
|
||||
if cookie_domain is not None:
|
||||
delete_kwargs["domain"] = cookie_domain
|
||||
response.delete_cookie(SESSION_COOKIE_NAME, **delete_kwargs)
|
||||
response.delete_cookie(CSRF_COOKIE_NAME, **delete_kwargs)
|
||||
|
||||
|
||||
@router.post("/login", response_model=AuthLoginResponse)
|
||||
@@ -241,12 +291,14 @@ def login(
|
||||
_set_session_cookie(
|
||||
response,
|
||||
issued_session.token,
|
||||
request=request,
|
||||
expires_at=issued_session.expires_at,
|
||||
secure=secure_cookie,
|
||||
)
|
||||
_set_csrf_cookie(
|
||||
response,
|
||||
csrf_token,
|
||||
request=request,
|
||||
expires_at=issued_session.expires_at,
|
||||
secure=secure_cookie,
|
||||
)
|
||||
|
||||
@@ -478,6 +478,39 @@ class AuthDependencyTests(unittest.TestCase):
|
||||
self.assertEqual(raised.exception.status_code, 403)
|
||||
self.assertEqual(raised.exception.detail, "Invalid CSRF token")
|
||||
|
||||
def test_cookie_auth_accepts_matching_session_among_duplicate_cookie_values(self) -> None:
|
||||
"""Cookie auth accepts the first valid session token among duplicate cookie values."""
|
||||
|
||||
request = SimpleNamespace(
|
||||
method="GET",
|
||||
headers={"cookie": "dcm_session=stale-token; dcm_session=fresh-token"},
|
||||
)
|
||||
resolved_session = SimpleNamespace(
|
||||
id=uuid.uuid4(),
|
||||
expires_at=datetime.now(UTC),
|
||||
user=SimpleNamespace(
|
||||
id=uuid.uuid4(),
|
||||
username="admin",
|
||||
role=UserRole.ADMIN,
|
||||
),
|
||||
)
|
||||
with patch.object(
|
||||
auth_dependency_module,
|
||||
"resolve_auth_session",
|
||||
side_effect=[None, resolved_session],
|
||||
) as resolve_mock:
|
||||
context = auth_dependency_module.get_request_auth_context(
|
||||
request=request,
|
||||
credentials=None,
|
||||
csrf_header=None,
|
||||
csrf_cookie=None,
|
||||
session_cookie="stale-token",
|
||||
session=SimpleNamespace(),
|
||||
)
|
||||
self.assertEqual(context.username, "admin")
|
||||
self.assertEqual(context.role, UserRole.ADMIN)
|
||||
self.assertEqual(resolve_mock.call_count, 2)
|
||||
|
||||
|
||||
class DocumentCatalogVisibilityTests(unittest.TestCase):
|
||||
"""Verifies predefined tag and path discovery visibility by caller role."""
|
||||
@@ -842,22 +875,44 @@ class AuthLoginRouteThrottleTests(unittest.TestCase):
|
||||
|
||||
self.commit_count += 1
|
||||
|
||||
@staticmethod
|
||||
def _response_stub() -> SimpleNamespace:
|
||||
class _ResponseStub:
|
||||
"""Captures response cookie calls for direct route invocation tests."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.set_cookie_calls: list[tuple[tuple[object, ...], dict[str, object]]] = []
|
||||
self.delete_cookie_calls: list[tuple[tuple[object, ...], dict[str, object]]] = []
|
||||
|
||||
def set_cookie(self, *args: object, **kwargs: object) -> None:
|
||||
"""Records one set-cookie call."""
|
||||
|
||||
self.set_cookie_calls.append((args, kwargs))
|
||||
|
||||
def delete_cookie(self, *args: object, **kwargs: object) -> None:
|
||||
"""Records one delete-cookie call."""
|
||||
|
||||
self.delete_cookie_calls.append((args, kwargs))
|
||||
|
||||
@classmethod
|
||||
def _response_stub(cls) -> "AuthLoginRouteThrottleTests._ResponseStub":
|
||||
"""Builds a minimal response object for direct route invocation."""
|
||||
|
||||
return SimpleNamespace(
|
||||
set_cookie=lambda *_args, **_kwargs: None,
|
||||
delete_cookie=lambda *_args, **_kwargs: None,
|
||||
)
|
||||
return cls._ResponseStub()
|
||||
|
||||
@staticmethod
|
||||
def _request_stub(ip_address: str = "203.0.113.2", user_agent: str = "unit-test") -> SimpleNamespace:
|
||||
def _request_stub(
|
||||
ip_address: str = "203.0.113.2",
|
||||
user_agent: str = "unit-test",
|
||||
origin: str | None = None,
|
||||
) -> SimpleNamespace:
|
||||
"""Builds request-like object containing client host and user-agent header fields."""
|
||||
|
||||
headers = {"user-agent": user_agent}
|
||||
if origin:
|
||||
headers["origin"] = origin
|
||||
return SimpleNamespace(
|
||||
client=SimpleNamespace(host=ip_address),
|
||||
headers={"user-agent": user_agent},
|
||||
headers=headers,
|
||||
url=SimpleNamespace(hostname="api.docs.lan"),
|
||||
)
|
||||
|
||||
def test_login_rejects_when_precheck_reports_active_throttle(self) -> None:
|
||||
@@ -970,6 +1025,57 @@ class AuthLoginRouteThrottleTests(unittest.TestCase):
|
||||
self.assertEqual(raised.exception.detail, auth_routes_module.LOGIN_RATE_LIMITER_UNAVAILABLE_DETAIL)
|
||||
self.assertEqual(session.commit_count, 0)
|
||||
|
||||
def test_login_sets_host_only_and_parent_domain_cookie_variants(self) -> None:
|
||||
"""Successful login sets a host-only cookie and an optional parent-domain mirror."""
|
||||
|
||||
payload = auth_routes_module.AuthLoginRequest(username="admin", password="correct-password")
|
||||
session = self._SessionStub()
|
||||
response_stub = self._response_stub()
|
||||
fake_user = SimpleNamespace(
|
||||
id=uuid.uuid4(),
|
||||
username="admin",
|
||||
role=UserRole.ADMIN,
|
||||
)
|
||||
fake_session = SimpleNamespace(
|
||||
token="session-token",
|
||||
expires_at=datetime.now(UTC),
|
||||
)
|
||||
fake_settings = SimpleNamespace(
|
||||
auth_cookie_domain="docs.lan",
|
||||
auth_cookie_samesite="none",
|
||||
public_base_url="https://api.docs.lan",
|
||||
)
|
||||
with (
|
||||
patch.object(
|
||||
auth_routes_module,
|
||||
"check_login_throttle",
|
||||
return_value=auth_login_throttle_module.LoginThrottleStatus(
|
||||
is_throttled=False,
|
||||
retry_after_seconds=0,
|
||||
),
|
||||
),
|
||||
patch.object(auth_routes_module, "authenticate_user", return_value=fake_user),
|
||||
patch.object(auth_routes_module, "clear_login_throttle"),
|
||||
patch.object(auth_routes_module, "issue_user_session", return_value=fake_session),
|
||||
patch.object(auth_routes_module, "get_settings", return_value=fake_settings),
|
||||
patch.object(auth_routes_module.secrets, "token_urlsafe", return_value="csrf-token"),
|
||||
):
|
||||
auth_routes_module.login(
|
||||
payload=payload,
|
||||
request=self._request_stub(origin="https://docs.lan"),
|
||||
response=response_stub,
|
||||
session=session,
|
||||
)
|
||||
|
||||
session_cookie_calls = [call for call in response_stub.set_cookie_calls if call[0][0] == auth_routes_module.SESSION_COOKIE_NAME]
|
||||
csrf_cookie_calls = [call for call in response_stub.set_cookie_calls if call[0][0] == auth_routes_module.CSRF_COOKIE_NAME]
|
||||
self.assertEqual(len(session_cookie_calls), 2)
|
||||
self.assertEqual(len(csrf_cookie_calls), 2)
|
||||
self.assertFalse(any("domain" in kwargs and kwargs["domain"] is None for _args, kwargs in session_cookie_calls))
|
||||
self.assertIn("domain", session_cookie_calls[1][1])
|
||||
self.assertEqual(session_cookie_calls[1][1]["domain"], "docs.lan")
|
||||
self.assertEqual(session_cookie_calls[0][1]["samesite"], "lax")
|
||||
|
||||
|
||||
class ProviderBaseUrlValidationTests(unittest.TestCase):
|
||||
"""Verifies allowlist, scheme, and private-network SSRF protections."""
|
||||
|
||||
@@ -98,8 +98,8 @@ Use `.env.example` as baseline. The table below documents user-managed settings
|
||||
| `AUTH_LOGIN_FAILURE_WINDOW_SECONDS` | default `900` | tune to identity-protection policy and support requirements |
|
||||
| `AUTH_LOGIN_LOCKOUT_BASE_SECONDS` | default `30` | tune to identity-protection policy and support requirements |
|
||||
| `AUTH_LOGIN_LOCKOUT_MAX_SECONDS` | default `900` | tune to identity-protection policy and support requirements |
|
||||
| `AUTH_COOKIE_DOMAIN` | empty (host-only cookies) | parent frontend/API domain for split hosts, for example `docs.lan` |
|
||||
| `AUTH_COOKIE_SAMESITE` | `auto` | `none` for cross-origin frontend/API deployments, `lax` or `strict` for same-origin |
|
||||
| `AUTH_COOKIE_DOMAIN` | empty (recommended; API always issues a host-only auth cookie) | optional parent domain only when you explicitly need a mirrored domain cookie, for example `docs.lan` |
|
||||
| `AUTH_COOKIE_SAMESITE` | `auto` | `none` only for truly cross-site frontend/API deployments; keep `auto` for same-site subdomains such as `docs.lan` and `api.docs.lan` |
|
||||
| `PROVIDER_BASE_URL_ALLOW_HTTP` | `true` only when intentionally testing local HTTP provider endpoints | `false` |
|
||||
| `PROVIDER_BASE_URL_ALLOW_PRIVATE_NETWORK` | `true` only for trusted local development targets | `false` |
|
||||
| `PROVIDER_BASE_URL_ALLOWLIST` | allow needed test hosts | explicit production allowlist, for example `["api.openai.com"]` |
|
||||
|
||||
@@ -45,7 +45,7 @@ services:
|
||||
- internal
|
||||
|
||||
typesense:
|
||||
image: typesense/typesense:30.1
|
||||
image: typesense/typesense:30.2.rc6
|
||||
command:
|
||||
- "--data-dir=/data"
|
||||
- "--api-key=${TYPESENSE_API_KEY:?TYPESENSE_API_KEY must be set}"
|
||||
|
||||
Reference in New Issue
Block a user