diff --git a/backend/chainlit/auth/cookie.py b/backend/chainlit/auth/cookie.py index 1cb4cc5ace..7ff72f3abc 100644 --- a/backend/chainlit/auth/cookie.py +++ b/backend/chainlit/auth/cookie.py @@ -1,3 +1,4 @@ +import hmac import os from typing import Literal, Optional, cast @@ -24,7 +25,7 @@ ) _cookie_secure = _cookie_samesite == "none" if _cookie_root_path := os.environ.get("CHAINLIT_ROOT_PATH", None): - _cookie_path = os.environ.get(_cookie_root_path, "/") + _cookie_path = _cookie_root_path else: _cookie_path = os.environ.get("CHAINLIT_AUTH_COOKIE_PATH", "/") _state_cookie_lifetime = int( @@ -186,11 +187,15 @@ def set_oauth_state_cookie(response: Response, token: str): def validate_oauth_state_cookie(request: Request, state: str): - """Check the state from the oauth provider against the browser cookie.""" + """Check the state from the oauth provider against the browser cookie. + + Uses ``hmac.compare_digest`` for constant-time comparison to prevent + timing-based side-channel leakage of the expected state value. + """ oauth_state = request.cookies.get(_state_cookie_name) - if oauth_state != state: + if oauth_state is None or not hmac.compare_digest(oauth_state, state): raise Exception("oauth state does not correspond") diff --git a/backend/tests/auth/test_cookie.py b/backend/tests/auth/test_cookie.py index 5f5c3848a5..a9482305a7 100644 --- a/backend/tests/auth/test_cookie.py +++ b/backend/tests/auth/test_cookie.py @@ -133,6 +133,68 @@ def test_overwrite_shorter_token_unchunked(client): assert len(chunk_cookies) == 0, f"Found {len(chunk_cookies)} residual cookies" +def test_cookie_path_uses_chainlit_root_path_value(monkeypatch): + """CHAINLIT_ROOT_PATH must be used as the cookie path directly. + + Before the fix, the code read:: + + _cookie_path = os.environ.get(_cookie_root_path, "/") + + where ``_cookie_root_path`` was the *value* of ``CHAINLIT_ROOT_PATH``, + not a key. This caused ``_cookie_path`` to always be ``"/"`` (the + default) whenever ``CHAINLIT_ROOT_PATH`` was set, because the value + (e.g. ``"/app"``) is not a valid environment variable name. Cookie + deletion then used the wrong path, leaving stale cookies on the client. + """ + monkeypatch.setenv("CHAINLIT_ROOT_PATH", "/myapp") + monkeypatch.delenv("CHAINLIT_AUTH_COOKIE_PATH", raising=False) + importlib.reload(cookie_module) + assert cookie_module._cookie_path == "/myapp", ( + f"Expected _cookie_path to be '/myapp' but got '{cookie_module._cookie_path}'. " + "CHAINLIT_ROOT_PATH value should be used directly as the cookie path." + ) + + +def test_validate_oauth_state_cookie_rejects_missing_state(monkeypatch): + """validate_oauth_state_cookie must raise when the state cookie is absent.""" + from chainlit.auth.cookie import validate_oauth_state_cookie + from starlette.requests import Request as StarletteRequest + + scope = { + "type": "http", + "headers": [], + "query_string": b"", + "method": "GET", + "path": "/", + } + request = StarletteRequest(scope) + # Cookies dict is empty — oauth_state cookie is absent + with pytest.raises(Exception, match="oauth state does not correspond"): + validate_oauth_state_cookie(request, "expected_state") + + +def test_validate_oauth_state_cookie_accepts_correct_state(): + """validate_oauth_state_cookie must not raise when states match.""" + from chainlit.auth.cookie import validate_oauth_state_cookie + from starlette.requests import Request as StarletteRequest + + state_value = "secret_state_token" + + # Build a minimal request with the oauth_state cookie set + scope = { + "type": "http", + "headers": [ + (b"cookie", f"oauth_state={state_value}".encode()), + ], + "query_string": b"", + "method": "GET", + "path": "/", + } + request = StarletteRequest(scope) + # Should not raise + validate_oauth_state_cookie(request, state_value) + + def test_state_cookie_lifetime_default(monkeypatch): """Test that _state_cookie_lifetime defaults to 180 seconds (3 minutes).""" monkeypatch.delenv("CHAINLIT_STATE_COOKIE_LIFETIME", raising=False)