From a7e68e2eab0a82c0a5388f0544a47572dcc829dd Mon Sep 17 00:00:00 2001 From: Alex Blank Date: Mon, 27 Jul 2026 20:48:03 +0200 Subject: [PATCH] fix(auth): protect setup and expire sessions --- backend/src/backup_tool/api/app.py | 36 ++++++++++++++++--- backend/src/backup_tool/config.py | 2 ++ backend/src/backup_tool/security/auth.py | 29 ++++++++++++--- tests/security/test_session_security.py | 46 ++++++++++++++++++++---- 4 files changed, 97 insertions(+), 16 deletions(-) diff --git a/backend/src/backup_tool/api/app.py b/backend/src/backup_tool/api/app.py index 2cfbb25..5bf471b 100644 --- a/backend/src/backup_tool/api/app.py +++ b/backend/src/backup_tool/api/app.py @@ -2,7 +2,7 @@ import asyncio import base64 import hashlib from collections.abc import AsyncIterator -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Annotated, Any, cast @@ -57,6 +57,7 @@ def problem_response(request: Request, status: int, code: str, detail: str) -> J class SetupInput(BaseModel): username: str = Field(min_length=1, max_length=255) password: str = Field(min_length=12, max_length=1024) + bootstrap_secret: str | None = Field(default=None, max_length=1024) class LoginInput(BaseModel): @@ -127,6 +128,7 @@ def create_app(settings: Settings) -> FastAPI: app.state.sessions = async_sessionmaker(app.state.engine, expire_on_commit=False) app.state.cipher = EnvelopeCipher.from_file(settings.master_key_file) app.state.setup_lock = asyncio.Lock() + app.state.revoked_sessions = set() @app.middleware("http") async def request_id_middleware(request: Request, call_next: Any) -> Response: @@ -167,7 +169,7 @@ def create_app(settings: Settings) -> FastAPI: return user, set(token.scopes), False encoded = request.cookies.get("backup_tool_session") data = verify_session(encoded, settings.master_key_file) if encoded else None - if data is None: + if data is None or data["sid"] in app.state.revoked_sessions: raise Problem(401, "authentication_required", "Authentication is required.") user = await db.get(User, data["sub"]) if user is None or user.state != "active": @@ -213,16 +215,29 @@ def create_app(settings: Settings) -> FastAPI: def set_session(response: Response, user_id: str) -> None: csrf = new_csrf_token() + ttl = settings.session_ttl_seconds response.set_cookie( "backup_tool_session", - sign_session(user_id, csrf, settings.master_key_file), + sign_session( + user_id, + csrf, + settings.master_key_file, + expires_at=datetime.now(UTC) + timedelta(seconds=ttl), + ), httponly=True, secure=True, samesite="strict", path="/", + max_age=ttl, ) response.set_cookie( - "backup_tool_csrf", csrf, httponly=False, secure=True, samesite="strict", path="/" + "backup_tool_csrf", + csrf, + httponly=False, + secure=True, + samesite="strict", + path="/", + max_age=ttl, ) @app.get("/livez") @@ -247,6 +262,11 @@ def create_app(settings: Settings) -> FastAPI: db: Annotated[AsyncSession, Depends(session)], ) -> dict[str, str]: async with app.state.setup_lock: + if ( + settings.bootstrap_secret is not None + and input_.bootstrap_secret != settings.bootstrap_secret + ): + raise Problem(403, "bootstrap_required", "Bootstrap credentials are required.") if await db.scalar(select(User.id).limit(1)) is not None: raise Problem(409, "setup_complete", "Initial administrator already exists.") user = User(username=input_.username, password_hash=hash_password(input_.password)) @@ -286,8 +306,14 @@ def create_app(settings: Settings) -> FastAPI: @app.post("/api/v2/auth/logout", status_code=204) async def logout( - response: Response, _: Annotated[tuple[User, set[str], bool], Depends(require)] + request: Request, + response: Response, + _: Annotated[tuple[User, set[str], bool], Depends(require)], ) -> None: + encoded = request.cookies.get("backup_tool_session") + data = verify_session(encoded, settings.master_key_file) if encoded else None + if data is not None: + app.state.revoked_sessions.add(data["sid"]) response.delete_cookie("backup_tool_session", path="/") response.delete_cookie("backup_tool_csrf", path="/") diff --git a/backend/src/backup_tool/config.py b/backend/src/backup_tool/config.py index 8418cc5..1072502 100644 --- a/backend/src/backup_tool/config.py +++ b/backend/src/backup_tool/config.py @@ -25,6 +25,8 @@ class Settings(BaseSettings): restore_roots: tuple[Path, ...] master_key_file: Path public_base_url: str = "http://127.0.0.1:8000" + bootstrap_secret: str | None = Field(default=None, min_length=16) + session_ttl_seconds: int = Field(default=28_800, ge=60, le=2_592_000) cors_origins: tuple[str, ...] = () worker_concurrency: Literal[1] = 1 min_free_bytes: int = Field(default=1_073_741_824, ge=0) diff --git a/backend/src/backup_tool/security/auth.py b/backend/src/backup_tool/security/auth.py index 1e2f245..ba68265 100644 --- a/backend/src/backup_tool/security/auth.py +++ b/backend/src/backup_tool/security/auth.py @@ -5,7 +5,7 @@ import hashlib import hmac import json import secrets -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Any @@ -42,9 +42,24 @@ def _session_key(master_key_file: Path) -> bytes: return hashlib.sha256(b"backup-tool-session\0" + master_key_file.read_bytes()).digest() -def sign_session(user_id: str, csrf: str, master_key_file: Path) -> str: +def sign_session( + user_id: str, + csrf: str, + master_key_file: Path, + *, + expires_at: datetime | None = None, + session_id: str | None = None, +) -> str: + issued_at = datetime.now(UTC) + expires_at = expires_at or issued_at.replace(microsecond=0) + timedelta(hours=8) payload = json.dumps( - {"sub": user_id, "csrf": csrf, "iat": datetime.now(UTC).isoformat()}, + { + "sub": user_id, + "csrf": csrf, + "iat": issued_at.isoformat(), + "exp": expires_at.astimezone(UTC).isoformat(), + "sid": session_id or secrets.token_urlsafe(24), + }, separators=(",", ":"), sort_keys=True, ).encode() @@ -65,7 +80,13 @@ def verify_session(value: str, master_key_file: Path) -> dict[str, Any] | None: return None padded = encoded + "=" * (-len(encoded) % 4) decoded = json.loads(base64.urlsafe_b64decode(padded)) - if not isinstance(decoded, dict) or not isinstance(decoded.get("sub"), str): + if ( + not isinstance(decoded, dict) + or not isinstance(decoded.get("sub"), str) + or not isinstance(decoded.get("sid"), str) + or not isinstance(decoded.get("exp"), str) + or datetime.fromisoformat(decoded["exp"]).astimezone(UTC) <= datetime.now(UTC) + ): return None return decoded except (ValueError, json.JSONDecodeError, UnicodeDecodeError): diff --git a/tests/security/test_session_security.py b/tests/security/test_session_security.py index e55df0f..151e801 100644 --- a/tests/security/test_session_security.py +++ b/tests/security/test_session_security.py @@ -10,10 +10,25 @@ PASSWORD = "correct horse battery staple" @pytest.mark.asyncio async def test_remote_setup_requires_bootstrap_secret(tmp_path) -> None: - conftest = importlib.import_module("conftest") + config = importlib.import_module("backup_tool.config") app_module = importlib.import_module("backup_tool.api.app") - settings = conftest.make_settings(tmp_path).model_copy( - update={"public_base_url": "https://backup.example.test", "bootstrap_secret": "bootstrap"} + key = tmp_path / "master.key" + key.write_bytes(b"m2-test-master-key-material-32-bytes-minimum") + key.chmod(0o600) + roots = [] + for name in ("data", "repositories", "sources", "restores"): + path = tmp_path / name + path.mkdir() + roots.append(path) + settings = config.Settings( + data_dir=roots[0], + database_url=f"sqlite+aiosqlite:///{roots[0] / 'metadata.db'}", + repository_roots=(roots[1],), + local_source_roots=(roots[2],), + restore_roots=(roots[3],), + master_key_file=key, + public_base_url="https://backup.example.test", + bootstrap_secret="bootstrap-secret-123", ) cli = importlib.import_module("backup_tool.cli") from alembic import command @@ -21,10 +36,21 @@ async def test_remote_setup_requires_bootstrap_secret(tmp_path) -> None: command.upgrade(cli.build_alembic_config(settings), "head") app = app_module.create_app(settings) - async with AsyncClient(transport=ASGITransport(app=app), base_url="https://backup.example.test") as client: - denied = await client.post("/api/v2/setup", json={"username": "admin", "password": PASSWORD}) + async with AsyncClient( + transport=ASGITransport(app=app), base_url="https://backup.example.test" + ) as client: + denied = await client.post( + "/api/v2/setup", json={"username": "admin", "password": PASSWORD} + ) assert denied.status_code == 403 - accepted = await client.post("/api/v2/setup", json={"username": "admin", "password": PASSWORD, "bootstrap_secret": "bootstrap"}) + accepted = await client.post( + "/api/v2/setup", + json={ + "username": "admin", + "password": PASSWORD, + "bootstrap_secret": "bootstrap-secret-123", + }, + ) assert accepted.status_code == 201 await app.state.engine.dispose() @@ -33,5 +59,11 @@ def test_expired_session_is_rejected(tmp_path) -> None: auth = importlib.import_module("backup_tool.security.auth") key = tmp_path / "master.key" key.write_bytes(b"x" * 32) - expired = auth.sign_session("user", "csrf", key, expires_at=datetime.now(UTC) - timedelta(seconds=1), session_id="session") + expired = auth.sign_session( + "user", + "csrf", + key, + expires_at=datetime.now(UTC) - timedelta(seconds=1), + session_id="session", + ) assert auth.verify_session(expired, key) is None