diff --git a/.env.example b/.env.example index 15fa051..eb5eadd 100644 --- a/.env.example +++ b/.env.example @@ -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 diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py index 102ac52..2c18784 100644 --- a/backend/app/api/auth.py +++ b/backend/app/api/auth.py @@ -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() diff --git a/backend/app/api/routes_auth.py b/backend/app/api/routes_auth.py index c7c9dbe..9b99f75 100644 --- a/backend/app/api/routes_auth.py +++ b/backend/app/api/routes_auth.py @@ -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, ) diff --git a/backend/tests/test_security_controls.py b/backend/tests/test_security_controls.py index 86c2161..09c4ede 100644 --- a/backend/tests/test_security_controls.py +++ b/backend/tests/test_security_controls.py @@ -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.""" diff --git a/doc/operations-and-configuration.md b/doc/operations-and-configuration.md index 8dfe4ed..88ede1b 100644 --- a/doc/operations-and-configuration.md +++ b/doc/operations-and-configuration.md @@ -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"]` | diff --git a/docker-compose.yml b/docker-compose.yml index 1a43ec9..2606748 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -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}"