diff --git a/docs/guides/configuration/index.md b/docs/guides/configuration/index.md index 68ba84d6294..08bf65760fb 100644 --- a/docs/guides/configuration/index.md +++ b/docs/guides/configuration/index.md @@ -128,6 +128,7 @@ marimo supports the following environment variables for advanced configuration: | `MARIMO_SKIP_UPDATE_CHECK` | If set to "1", marimo will skip checking for updates when starting. | Not set | | `MARIMO_SQL_DEFAULT_LIMIT` | Default limit for SQL query results. If not set, no limit is applied. | Not set | | `MARIMO_SESSION_COOKIE_SECURE` | If set to `true`/`1`, marks the session cookie as `Secure` so browsers only send it over HTTPS. Enable when serving marimo behind TLS. | `false` | +| `MARIMO_SESSION_SECRET` | Secret used to sign the session cookie. Defaults to a random value generated per server process, so sessions are invalidated on restart. Set to a stable value (e.g. `openssl rand -hex 32`) to keep sessions across restarts or replicas. | Random per process | | `MARIMO_SERVER_TRANSPORT` | Experimental. The transport for streaming kernel messages to the browser: `websocket` or `sse`. Use `sse` when deploying behind proxies or services that do not support WebSockets. | `websocket` | ### Tips diff --git a/marimo/_config/settings.py b/marimo/_config/settings.py index fc70cd897fe..49a6c8b30f9 100644 --- a/marimo/_config/settings.py +++ b/marimo/_config/settings.py @@ -2,6 +2,7 @@ from __future__ import annotations import logging +import os from dataclasses import dataclass from marimo._utils.env import is_env_true @@ -22,6 +23,14 @@ class GlobalSettings: # Enable when serving marimo behind TLS / a TLS-terminating proxy. Default # "false" to preserve local (plain-HTTP) development. SESSION_COOKIE_SECURE: bool = is_env_true("MARIMO_SESSION_COOKIE_SECURE") + # Secret used to sign the session cookie and to hash the auth token stored + # in it. Defaults to a random value generated once per server process, so + # cookies are invalidated on restart. Set this to a stable value (e.g. + # `openssl rand -hex 32`) when cookies must survive restarts or be shared + # across replicas. + SESSION_SECRET: str | None = ( + os.environ.get("MARIMO_SESSION_SECRET") or None + ) # Disable authentication on the virtual file endpoint (`/@file/...`). # Useful in sandboxed/embedded deployments where virtual file URLs need # to be fetched in trusted contexts. Default "false", meaning auth is required. diff --git a/marimo/_server/api/auth.py b/marimo/_server/api/auth.py index 14c167c0e56..5dba8f26d06 100644 --- a/marimo/_server/api/auth.py +++ b/marimo/_server/api/auth.py @@ -2,6 +2,7 @@ from __future__ import annotations import base64 +import hashlib import hmac import secrets import typing @@ -16,6 +17,7 @@ from starlette.responses import JSONResponse from marimo import _loggers +from marimo._config.settings import GLOBAL_SETTINGS if TYPE_CHECKING: from starlette.authentication import AuthenticationError @@ -41,8 +43,11 @@ def validate_auth( # Check for session cookie cookie_session = CookieSession(conn.session) - # Validate the cookie - if hmac.compare_digest(cookie_session.get_access_token(), auth_token): + # Validate the cookie. The cookie stores a keyed hash of the token, + # never the token itself. + if hmac.compare_digest( + cookie_session.get_access_token(), hash_access_token(auth_token) + ): return True # Success # Check for access_token @@ -137,8 +142,26 @@ def on_auth_error( ) -# This is random/new for each server instance -RANDOM_SECRET = Secret(secrets.token_hex(32)) +# Random/new for each server process unless overridden via +# MARIMO_SESSION_SECRET. Used both to sign the session cookie and to hash the +# auth token stored inside it. +SESSION_SECRET = Secret( + GLOBAL_SETTINGS.SESSION_SECRET or secrets.token_hex(32) +) + + +def hash_access_token(token: str) -> str: + """Keyed hash of the auth token, safe to store in the session cookie. + + The session cookie is signed by starlette (tamper-proof) but not + encrypted, so its contents are readable by anyone holding the cookie. + Storing `HMAC(secret, token)` rather than the token means a leaked + cookie does not leak the token itself and cannot be replayed as a + bearer token or `?access_token=` query param. + """ + return hmac.new( + str(SESSION_SECRET).encode(), token.encode(), hashlib.sha256 + ).hexdigest() class CookieSession: @@ -150,6 +173,7 @@ def __init__(self, session_state: dict[str, Any]) -> None: self.session_state = session_state def get_access_token(self) -> str: + """Returns the hashed access token stored in the session, or "".""" access_token: str = self.session_state.get("access_token", "") return access_token @@ -158,7 +182,8 @@ def get_username(self) -> str: return username def set_access_token(self, token: str) -> None: - self.session_state["access_token"] = token + """Stores a keyed hash of `token`; the raw token never hits the cookie.""" + self.session_state["access_token"] = hash_access_token(token) def set_username(self, username: str) -> None: self.session_state["username"] = username diff --git a/marimo/_server/main.py b/marimo/_server/main.py index 6dafde1ddce..bc6029dfcb1 100644 --- a/marimo/_server/main.py +++ b/marimo/_server/main.py @@ -12,7 +12,7 @@ from marimo import _loggers from marimo._config.settings import GLOBAL_SETTINGS from marimo._server.api.auth import ( - RANDOM_SECRET, + SESSION_SECRET, CustomAuthenticationMiddleware, CustomSessionMiddleware, on_auth_error, @@ -71,7 +71,7 @@ def create_starlette_app( [ Middleware( CustomSessionMiddleware, - secret_key=RANDOM_SECRET, + secret_key=SESSION_SECRET, https_only=GLOBAL_SETTINGS.SESSION_COOKIE_SECURE, ), ] diff --git a/tests/_server/api/test_auth.py b/tests/_server/api/test_auth.py index 1ffce1de1bb..bc5c81639f1 100644 --- a/tests/_server/api/test_auth.py +++ b/tests/_server/api/test_auth.py @@ -1,6 +1,9 @@ from __future__ import annotations import base64 +import os +import subprocess +import sys from typing import Any import pytest @@ -11,8 +14,10 @@ from marimo._config.manager import MarimoConfigManager, UserConfigManager from marimo._server.api.auth import ( + CookieSession, CustomAuthenticationMiddleware, CustomSessionMiddleware, + hash_access_token, validate_auth, ) from marimo._server.api.deps import AppState @@ -131,8 +136,8 @@ async def test_validate_auth_with_valid_cookie(app: Starlette): conn = create_connection(app) # Run all middleware await app.build_middleware_stack()(conn.scope, mock_receive, mock_send) - conn.session["access_token"] = str( - AppState.from_app(app).session_manager.auth_token + conn.session["access_token"] = hash_access_token( + str(AppState.from_app(app).session_manager.auth_token) ) assert validate_auth(conn) is True @@ -147,6 +152,70 @@ async def test_validate_auth_with_bad_cookie(app: Starlette): assert validate_auth(conn) is False +async def test_validate_auth_rejects_raw_token_in_cookie(app: Starlette): + # A cookie carrying the raw token (e.g. from an older marimo version, or + # forged by someone who knows the token) must not be accepted; only the + # keyed hash is. + conn = create_connection(app) + await app.build_middleware_stack()(conn.scope, mock_receive, mock_send) + conn.session["access_token"] = str( + AppState.from_app(app).session_manager.auth_token + ) + + assert validate_auth(conn) is False + + +def test_cookie_session_never_stores_raw_token(): + session: dict[str, str] = {} + cookie_session = CookieSession(session) + cookie_session.set_access_token("super-secret") + + assert session["access_token"] != "super-secret" + assert "super-secret" not in session["access_token"] + assert session["access_token"] == hash_access_token("super-secret") + assert cookie_session.get_access_token() == hash_access_token( + "super-secret" + ) + + +def test_hash_access_token_is_keyed_and_deterministic(): + assert hash_access_token("a") == hash_access_token("a") + assert hash_access_token("a") != hash_access_token("b") + # 64 hex chars (sha256) + assert len(hash_access_token("a")) == 64 + + +def test_session_secret_env_override(): + # GlobalSettings reads env vars at import time, so check in a fresh + # interpreter. + code = ( + "from marimo._server.api.auth import SESSION_SECRET, hash_access_token;" + "print(str(SESSION_SECRET));" + "print(hash_access_token('tok'))" + ) + + def run(env: dict[str, str]) -> list[str]: + out = subprocess.check_output( + [sys.executable, "-c", code], + env={**os.environ, **env}, + text=True, + ) + return out.strip().splitlines() + + stable_a = run({"MARIMO_SESSION_SECRET": "stable-secret"}) + stable_b = run({"MARIMO_SESSION_SECRET": "stable-secret"}) + assert stable_a[0] == "stable-secret" + # Same secret => same cookie hash across processes + assert stable_a[1] == stable_b[1] + + random_a = run({"MARIMO_SESSION_SECRET": ""}) + random_b = run({"MARIMO_SESSION_SECRET": ""}) + # Empty/unset => random per process, so hashes differ + assert random_a[0] != "stable-secret" + assert random_a[0] != random_b[0] + assert random_a[1] != random_b[1] + + async def test_validate_auth_with_valid_access_token(app: Starlette): conn = create_connection(app) # Run all middleware