From b19d327f03dd7f26cb1e942300f63728a49fd6fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o?= Date: Sun, 16 Aug 2026 23:56:36 +0200 Subject: [PATCH] fix(auth): derive the session cookie Secure flag from the request scheme (#6048) * fix(auth): derive the session cookie Secure flag from the request scheme SECURE_COOKIES only marked the login cookie Secure when it was explicitly set to true, so an HTTPS login on an install that never set it handed out a session cookie the browser is happy to send back in cleartext. Unset now derives the flag from the request: the connection scheme, which uvicorn's proxy-headers middleware rewrites for the proxies it trusts, or X-Forwarded-Proto for a terminator that is not on a trusted address. That is the same test core/middleware.py already applies before sending HSTS, so the two stop disagreeing about whether a request arrived over TLS. An explicit true still forces the flag on and an explicit false turns it off for an install still answering on both HTTP and HTTPS. Strictly more Secure flags than before and never fewer. Empty counts as unset, because docker-compose pinned SECURE_COOKIES=false for every container; the compose files now pass the variable through unset, the way FASTEMBED_CACHE_PATH already does. The helper and its decision order come from #3799, which was closed for being too large to review and whose six replacement PRs dropped this fix. Part of #3803. * docs(setup): flag the leftover SECURE_COOKIES=false on upgrades The old default was false, so an install set up before scheme derivation can still carry an explicit SECURE_COOKIES=false in its own .env. That value stays authoritative, so HTTPS logins keep getting a non-Secure session cookie even after the tracked compose defaults are updated by a pull. Say so where people look: the security notes and the variable's own comment in .env.example. * docs(setup): align TLS guidance with scheme-derived cookies --------- Co-authored-by: Alexandre Teixeira --- .env.example | 8 ++- SECURITY.md | 2 +- docker-compose.gpu-amd.yml | 2 +- docker-compose.gpu-nvidia.yml | 2 +- docker-compose.yml | 2 +- docs/setup.md | 29 ++++++++--- routes/auth_routes.py | 29 ++++++++++- tests/test_auth_event_loop.py | 7 ++- tests/test_auth_policy.py | 95 ++++++++++++++++++++++++++++++++++- 9 files changed, 158 insertions(+), 18 deletions(-) diff --git a/.env.example b/.env.example index 2c6c2065a..61d874d55 100644 --- a/.env.example +++ b/.env.example @@ -88,8 +88,12 @@ SEARXNG_INSTANCE=http://localhost:8080 # Keep false for Docker, LAN, reverse proxy, and any shared deployment. # LOCALHOST_BYPASS=false -# Mark session cookies Secure. Set true when Odysseus is served through HTTPS -# by a trusted reverse proxy or private access gateway. +# Mark session cookies Secure. Left unset, this follows the request scheme: +# an HTTPS login gets a Secure cookie, a plain-HTTP one does not. Set true to +# force it on, or false to force it off while you still serve plain HTTP. +# Upgrading: this used to default to false. Drop a leftover SECURE_COOKIES=false +# from your .env unless you still need that escape hatch — it keeps HTTPS logins +# on a non-Secure cookie. # SECURE_COOKIES=true # Optional: pre-seed the first admin password during setup. diff --git a/SECURITY.md b/SECURITY.md index 1fa5b0b3b..f3165c0b3 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -10,7 +10,7 @@ Security fixes are handled on the default branch until formal releases are cut. - Keep `AUTH_ENABLED=true` for any network-accessible deployment. - Keep `LOCALHOST_BYPASS=false` outside local development. -- Set `SECURE_COOKIES=true` when Odysseus is served through HTTPS by a trusted reverse proxy or private access gateway. +- Leave `SECURE_COOKIES` unset unless you need to override it: session cookies are marked `Secure` whenever the request arrives over HTTPS. Set `SECURE_COOKIES=true` to force it on (for a proxy Odysseus cannot see the scheme of), or `SECURE_COOKIES=false` to force it off while you still serve plain HTTP alongside HTTPS. - Use HTTPS when exposing the app beyond localhost. - Put the authenticated Odysseus web/API entrypoint behind a trusted reverse proxy or private access layer such as Cloudflare Access, Tailscale, or a VPN. - Keep ChromaDB, SearXNG, ntfy, Ollama, vLLM, llama.cpp, databases, and raw model/provider APIs internal-only. diff --git a/docker-compose.gpu-amd.yml b/docker-compose.gpu-amd.yml index 24f65e5cb..8d0cf1653 100644 --- a/docker-compose.gpu-amd.yml +++ b/docker-compose.gpu-amd.yml @@ -50,7 +50,7 @@ services: - ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin} - ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-} - ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1} - - SECURE_COOKIES=${SECURE_COOKIES:-false} + - SECURE_COOKIES=${SECURE_COOKIES:-} - EMBEDDING_URL=${EMBEDDING_URL:-} - EMBEDDING_MODEL=${EMBEDDING_MODEL:-} - EMBEDDING_API_KEY=${EMBEDDING_API_KEY:-} diff --git a/docker-compose.gpu-nvidia.yml b/docker-compose.gpu-nvidia.yml index fc66234db..69331ffb6 100644 --- a/docker-compose.gpu-nvidia.yml +++ b/docker-compose.gpu-nvidia.yml @@ -49,7 +49,7 @@ services: - ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin} - ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-} - ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1} - - SECURE_COOKIES=${SECURE_COOKIES:-false} + - SECURE_COOKIES=${SECURE_COOKIES:-} - EMBEDDING_URL=${EMBEDDING_URL:-} - EMBEDDING_MODEL=${EMBEDDING_MODEL:-} - EMBEDDING_API_KEY=${EMBEDDING_API_KEY:-} diff --git a/docker-compose.yml b/docker-compose.yml index 38cb654e5..708e5df82 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -38,7 +38,7 @@ services: - ODYSSEUS_ADMIN_USER=${ODYSSEUS_ADMIN_USER:-admin} - ODYSSEUS_ADMIN_PASSWORD=${ODYSSEUS_ADMIN_PASSWORD:-} - ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-http://localhost,http://127.0.0.1} - - SECURE_COOKIES=${SECURE_COOKIES:-false} + - SECURE_COOKIES=${SECURE_COOKIES:-} - EMBEDDING_URL=${EMBEDDING_URL:-} - EMBEDDING_MODEL=${EMBEDDING_MODEL:-} - EMBEDDING_API_KEY=${EMBEDDING_API_KEY:-} diff --git a/docs/setup.md b/docs/setup.md index 2f3b66bf1..5c4f1348f 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -475,7 +475,7 @@ Odysseus is a self-hosted workspace with powerful local tools: shell access, fil - Keep `AUTH_ENABLED=true` for any network-accessible deployment. - Keep `LOCALHOST_BYPASS=false` outside local development. -- Use `SECURE_COOKIES=true` when Odysseus is served through HTTPS by a trusted reverse proxy or private access gateway. +- Leave `SECURE_COOKIES` unset unless you need to override it: session cookies are marked `Secure` whenever the request arrives over HTTPS. Use `SECURE_COOKIES=true` to force it on for a proxy whose scheme Odysseus cannot see, or `SECURE_COOKIES=false` to force it off while you still serve plain HTTP alongside HTTPS. - Do not expose it directly to the public internet without HTTPS and a trusted reverse proxy or private access layer. - Keep `.env`, `data/`, `logs/`, databases, uploads, generated media, backups, auth/session files, API keys, and model/provider tokens out of Git and private shares. They are ignored by default. - Review `data/auth.json` after first boot: disable open signup unless you intentionally want it, make only your own account admin, and keep demo/test accounts non-admin. @@ -486,6 +486,14 @@ Odysseus is a self-hosted workspace with powerful local tools: shell access, fil - Keep ChromaDB, SearXNG, ntfy, Ollama, vLLM, llama.cpp, databases, and raw model/provider APIs internal-only. Expose only the authenticated Odysseus web/API entrypoint through your trusted proxy or private access layer. - Before publishing a fork, run `git status --short` and confirm no private files from `.env`, `data/`, `logs/`, uploads, backups, or local databases are staged. +> **Upgrading an existing install:** `SECURE_COOKIES` used to default to +> `false`, so an install set up before scheme derivation may still carry +> `SECURE_COOKIES=false` in its own `.env`. That explicit value stays +> authoritative, so HTTPS logins keep getting a non-`Secure` session cookie. +> Pulling this change updates the tracked Compose files, but nothing rewrites +> your `.env` — drop the line from it unless you deliberately serve plain HTTP +> alongside HTTPS and want the escape hatch. + ### Private or proxied deployments Odysseus serves plain HTTP on its app port. Docker Compose binds Odysseus and the bundled services to `127.0.0.1` by default, so a typical production/private setup is: @@ -494,7 +502,7 @@ Odysseus serves plain HTTP on its app port. Docker Compose binds Odysseus and th 3. Put the authenticated Odysseus web/API entrypoint behind that layer. 4. Keep raw service and model ports internal-only. -Cloudflare Access, Tailscale, Caddy, nginx, and Traefik can all fit this pattern; none are required by Odysseus. If your access layer reaches Odysseus on the same host, proxy to `http://127.0.0.1:7000` and keep `AUTH_ENABLED=true`, `LOCALHOST_BYPASS=false`, and `SECURE_COOKIES=true`. +Cloudflare Access, Tailscale, Caddy, nginx, and Traefik can all fit this pattern; none are required by Odysseus. If your access layer reaches Odysseus on the same host, proxy to `http://127.0.0.1:7000` and keep `AUTH_ENABLED=true` and `LOCALHOST_BYPASS=false`. Any proxy that forwards `X-Forwarded-Proto: https` gets `Secure` session cookies without configuration, so `SECURE_COOKIES` only needs setting when you want to override that — force it on for a proxy that forwards no scheme at all, or off while you still serve plain HTTP. `ALLOWED_ORIGINS` lists exact permitted origins for cross-origin browser/API clients; ordinary same-origin reverse-proxy access usually does not need a special CORS entry. #### Faster over the network: HTTP/2 @@ -582,9 +590,12 @@ Odysseus's own service is unchanged; the proxy runs alongside it. Under Docker, run the proxy as another container, or on the host pointing at the published port. -**4. Point Odysseus at the new origin** in `.env`, then restart it: +**4. Point Odysseus at the new origin** in `.env`, then restart it. + +A proxy that exposes the HTTPS request scheme to Odysseus needs no `SECURE_COOKIES` setting. Only force it on when the proxy cannot expose that scheme: ```bash +# only if the proxy cannot expose the external HTTPS scheme to Odysseus: SECURE_COOKIES=true # only if you use remote MCP servers with OAuth: OAUTH_REDIRECT_BASE_URL=https://odysseus.example.com @@ -619,10 +630,12 @@ enable it by right-clicking the column headers. Three things bite when moving an existing install behind TLS: -- Set `SECURE_COOKIES=true` **at the same time** you stop serving plain HTTP, - not before. The flag is applied to every login regardless of the scheme the - request arrived on, so while an HTTP entrypoint is still reachable the - browser will reject the `Secure` cookie there and login will appear to loop. +- Leave `SECURE_COOKIES` unset when Odysseus can see the external HTTPS scheme; + the cookie then follows the request automatically. If your proxy cannot expose + that scheme, set `SECURE_COOKIES=true` **at the same time** you stop serving + plain HTTP, not before. An explicit `true` applies to every login, so while an + HTTP entrypoint is still reachable the browser will reject the `Secure` cookie + there and login will appear to loop. - `OAUTH_REDIRECT_BASE_URL` defaults to `http://localhost:7000`. Unlike the Gmail redirect URI it cannot be derived from a request — it is registered with each MCP authorization server up front — so set it to the external @@ -675,7 +688,7 @@ Key settings: | `AUTH_ENABLED` | `true` | Enable/disable login | | `LOCALHOST_BYPASS` | `false` | Development-only auth bypass for loopback requests. Keep false for shared/network deployments. | | `ALLOWED_ORIGINS` | `http://localhost,http://127.0.0.1` | Comma-separated exact permitted origins for cross-origin browser/API clients. | -| `SECURE_COOKIES` | `false` | Set true when serving Odysseus through HTTPS at a trusted proxy or private access gateway. | +| `SECURE_COOKIES` | derived from the request scheme | Marks session cookies `Secure` on HTTPS requests. Set true to force it on, false to force it off. | | `DATABASE_URL` | `sqlite:///./data/app.db` | Database connection string | | `CHROMADB_HOST` | `localhost` | ChromaDB host for vector memory. Docker overrides this to `chromadb`. | | `CHROMADB_PORT` | `8100` | ChromaDB port for manual host runs. Docker overrides this to `8000`. | diff --git a/routes/auth_routes.py b/routes/auth_routes.py index 183ca50a9..a35d466c7 100644 --- a/routes/auth_routes.py +++ b/routes/auth_routes.py @@ -86,6 +86,33 @@ class SetOpenRegistrationRequest(BaseModel): SESSION_COOKIE = "odysseus_session" +def _secure_cookie(request: Request) -> bool: + """Decide the ``Secure`` attribute of the session cookie. + + ``SECURE_COOKIES`` stays authoritative when it holds an explicit value: + ``true`` always marks the cookie Secure (the documented knob for a TLS + proxy), ``false`` never does, which is the escape hatch for an install + that still answers on plain HTTP alongside HTTPS. Anything else — + unset, or the present-but-empty value docker-compose injects for a + variable the host has not defined — derives it from the request, so an + HTTPS login gets a Secure cookie without any configuration. + + Either the connection scheme or ``X-Forwarded-Proto`` saying https is + enough, which is the same test ``core/middleware.py`` applies before it + sends HSTS. Uvicorn's proxy-headers middleware already folds that header + into the scheme for the proxies it trusts, so reading it here only adds + the case of a terminator that is not on a trusted address; the cost is + that a client talking to the app directly can set the header and lock + its own session out over plain HTTP. + """ + configured = os.getenv("SECURE_COOKIES", "").strip().lower() + if configured in ("true", "false"): + return configured == "true" + # A chained proxy sends a list — the client-facing hop comes first. + forwarded_proto = request.headers.get("x-forwarded-proto", "").split(",")[0] + return request.url.scheme == "https" or forwarded_proto.strip().lower() == "https" + + def setup_auth_routes(auth_manager: AuthManager) -> APIRouter: router = APIRouter(prefix="/api/auth", tags=["auth"]) @@ -159,7 +186,7 @@ def setup_auth_routes(auth_manager: AuthManager) -> APIRouter: value=token, httponly=True, samesite="lax", - secure=os.getenv("SECURE_COOKIES", "false").lower() == "true", + secure=_secure_cookie(request), path="/", ) if body.remember: diff --git a/tests/test_auth_event_loop.py b/tests/test_auth_event_loop.py index 112e19d74..3ff118272 100644 --- a/tests/test_auth_event_loop.py +++ b/tests/test_auth_event_loop.py @@ -99,7 +99,12 @@ def test_login_offloads_bcrypt_bearing_calls(monkeypatch): login = _login_endpoint(auth) - request = SimpleNamespace(client=SimpleNamespace(host="203.0.113.7"), cookies={}) + request = SimpleNamespace( + client=SimpleNamespace(host="203.0.113.7"), + cookies={}, + url=SimpleNamespace(scheme="http"), + headers={}, + ) response = MagicMock() body = LoginRequest(username="alice", password="hunter2", remember=True) diff --git a/tests/test_auth_policy.py b/tests/test_auth_policy.py index 8fceeccc0..7331284a2 100644 --- a/tests/test_auth_policy.py +++ b/tests/test_auth_policy.py @@ -242,12 +242,26 @@ def _login_endpoint(auth_manager): raise AssertionError("login route not found") +def _login_request(scheme="http", forwarded_proto=None): + """Stand-in for fastapi.Request carrying the fields login reads: the + client host (rate limiter), and the URL scheme plus `X-Forwarded-Proto` + (cookie Secure flag).""" + headers = {} + if forwarded_proto is not None: + headers["x-forwarded-proto"] = forwarded_proto + return SimpleNamespace( + client=SimpleNamespace(host="127.0.0.1"), + url=SimpleNamespace(scheme=scheme), + headers=headers, + ) + + def test_remember_cookie_max_age_matches_token_ttl(tmp_path): auth_mod = _auth_module() mgr = _make_manager(tmp_path) mgr.create_user("alice", "alice-password", is_admin=False) endpoint, LoginRequest = _login_endpoint(mgr) - request = SimpleNamespace(client=SimpleNamespace(host="127.0.0.1")) + request = _login_request() response = _CapturingResponse() body = LoginRequest(username="alice", password="alice-password", remember=True) @@ -262,7 +276,7 @@ def test_no_remember_omits_cookie_max_age(tmp_path): mgr = _make_manager(tmp_path) mgr.create_user("bob", "bob-password", is_admin=False) endpoint, LoginRequest = _login_endpoint(mgr) - request = SimpleNamespace(client=SimpleNamespace(host="127.0.0.1")) + request = _login_request() response = _CapturingResponse() body = LoginRequest(username="bob", password="bob-password", remember=False) @@ -270,3 +284,80 @@ def test_no_remember_omits_cookie_max_age(tmp_path): # Without "remember", the cookie is a session cookie (no max_age). assert "max_age" not in response.cookie_kwargs + + +# ── Session cookie Secure flag ───────────────────────────────────────── + + +def _login_secure_flag(tmp_path, scheme, forwarded_proto=None): + """Log a user in over ``scheme`` and return the cookie's Secure flag.""" + mgr = _make_manager(tmp_path) + mgr.create_user("carol", "carol-password", is_admin=False) + endpoint, LoginRequest = _login_endpoint(mgr) + response = _CapturingResponse() + body = LoginRequest(username="carol", password="carol-password") + request = _login_request(scheme, forwarded_proto) + + asyncio.run(endpoint(body=body, request=request, response=response)) + + return response.cookie_kwargs["secure"] + + +def test_https_login_marks_cookie_secure_without_config(tmp_path, monkeypatch): + monkeypatch.delenv("SECURE_COOKIES", raising=False) + + # A session token handed out over HTTPS must not be allowed to travel + # back in cleartext just because nobody set an env var. + assert _login_secure_flag(tmp_path, "https") is True + + +def test_plain_http_login_leaves_cookie_insecure_without_config(tmp_path, monkeypatch): + monkeypatch.delenv("SECURE_COOKIES", raising=False) + + # Marking it Secure here would make the browser drop the cookie and + # break login on a plain-HTTP install. + assert _login_secure_flag(tmp_path, "http") is False + + +def test_forwarded_proto_https_marks_cookie_secure(tmp_path, monkeypatch): + monkeypatch.delenv("SECURE_COOKIES", raising=False) + + # A terminator that is not on an address uvicorn trusts leaves the + # connection scheme as http, so the header is the only signal there. + assert _login_secure_flag(tmp_path, "http", forwarded_proto="https") is True + # Chained proxies send a list; the client-facing hop is the first entry. + assert _login_secure_flag(tmp_path, "http", forwarded_proto="https, http") is True + + +def test_forwarded_proto_http_leaves_plain_http_login_insecure(tmp_path, monkeypatch): + monkeypatch.delenv("SECURE_COOKIES", raising=False) + + # The header is not a downgrade switch: either signal saying https is + # enough, the same test core/middleware.py applies before sending HSTS. + assert _login_secure_flag(tmp_path, "http", forwarded_proto="http") is False + assert _login_secure_flag(tmp_path, "https", forwarded_proto="http") is True + + +def test_empty_secure_cookies_still_derives_from_scheme(tmp_path, monkeypatch): + # docker-compose injects `SECURE_COOKIES=${SECURE_COOKIES:-}`, which sets + # the variable to "" when the host has not defined it. Empty means + # unconfigured, not "off". + monkeypatch.setenv("SECURE_COOKIES", "") + + assert _login_secure_flag(tmp_path, "https") is True + + +def test_secure_cookies_true_forces_secure_on_plain_http(tmp_path, monkeypatch): + # The documented knob keeps working for a proxy whose scheme the app + # cannot see, e.g. one that is not on a trusted loopback address. + monkeypatch.setenv("SECURE_COOKIES", "true") + + assert _login_secure_flag(tmp_path, "http") is True + + +def test_secure_cookies_false_forces_insecure_on_https(tmp_path, monkeypatch): + # The escape hatch for an install still answering on both HTTP and + # HTTPS: an explicit false wins over the request scheme. + monkeypatch.setenv("SECURE_COOKIES", "false") + + assert _login_secure_flag(tmp_path, "https") is False