diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 861d65e..07f3461 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -5,8 +5,8 @@ How `recoll-modern` is built today. Update in the same commit as any structural ## Overview The Python service scaffold, SQLite schema, collection and membership repositories, document -discovery/identity modules, durable job worker, and engine adapter are built. The HTTP API remains -intended structure. Recoll behaviour below was verified against a real container. +discovery/identity modules, durable job worker, engine adapter, and authentication service are built. +The HTTP API remains intended structure. Recoll behaviour below was verified against a real container. The goal: a self-hosted service that wraps Recoll's indexing and full-text search behind (a) a modern web UI and (b) an MCP server, running in Docker, with several independent collections such as @@ -29,6 +29,7 @@ web UI and (b) an MCP server, running in Docker, with several independent collec | `src/recoll_modern/indexing/map_export.py` | Writes a durable atomic map from canonical paths to opaque collection IDs | | `helper/` | C reader run by Recoll's `metadatacmds` for each indexed file | | `src/recoll_modern/engine/` | Shared Protocol, deterministic fake, and isolated Recoll Python/indexer adapter | +| `src/recoll_modern/domain/auth.py`, `storage/auth.py` | Argon2 password checks, seeded administrator, hashed server-side sessions and CSRF verification | | `tests/` | Automated service tests | Discovery produces content hashes and root-relative paths without writing to the document volume. @@ -89,8 +90,8 @@ is direct membership union non-excluded rule membership, so direct additions rem `jobs` and `job_file_errors` make work and partial failures durable. At startup, the worker requeues well-formed interrupted jobs, clears errors from the prior attempt, and fails unreadable requests. -`users` and hashed-token `sessions` -provide authentication. `schema_migrations` is forward-only. The exact columns and constraints are the +`users` and hashed-token `sessions` provide authentication; the session lifetime is configurable and +defaults to 12 hours. `schema_migrations` is forward-only. The exact columns and constraints are the contract in `docs/specs/001-search-engine-api.md`. ### HTTP boundary diff --git a/docs/TODO.md b/docs/TODO.md index e1127e8..33fe2fe 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -15,11 +15,12 @@ Where the workflow stands (`AGENTS.md` -> Workflow: brainstorm -> spec -> plan - selectable search fields, subpath hosting and the packaging seam for deferred desktop spec 005. - Plan `docs/plans/001-search-engine-api.md`: **approved 2026-09-18.** Its listed Python runtime and development dependencies are approved. Execution follows the plan task by task. -- Plan Tasks 1–3 are complete and independently reviewed. Tasks 4–8 add discovery, content-hash - identity, additive membership, durable jobs, the atomic membership map, and the engine boundary. - Their checkboxes are current in the plan. The next task is authentication. +- Plan Tasks 1–3 are complete and independently reviewed. Tasks 4–9 add discovery, content-hash + identity, additive membership, durable jobs, the atomic membership map, the engine boundary, and + session authentication. Their checkboxes are current in the plan. The next task is the HTTP shell. -The next action is Task 9, "Implement users, sessions, seeding, and CSRF." Do not repeat Tasks 1–8. +The next action is Task 10, "Build the FastAPI shell, middleware, and core resources." Do not repeat +Tasks 1–9. Things a resuming agent will otherwise get wrong: diff --git a/docs/plans/001-search-engine-api.md b/docs/plans/001-search-engine-api.md index 51ca0a7..0a97522 100644 --- a/docs/plans/001-search-engine-api.md +++ b/docs/plans/001-search-engine-api.md @@ -665,7 +665,7 @@ git commit -m "feat: add Recoll engine boundary" - Produces: `AuthService.login/authenticate/logout/seed_initial_admin` - Produces: `SessionCredentials(cookie_token, csrf_token, expires_at)` -- [ ] **Step 1: Write failing auth tests** +- [x] **Step 1: Write failing auth tests** ```python def test_passwords_are_argon2_hashes(auth: AuthService) -> None: @@ -687,15 +687,15 @@ def test_first_boot_without_credentials_refuses_startup(empty_auth: AuthService) Also test disabled/expired sessions, generic invalid credentials, case-folded usernames, CSRF mismatch, and cleanup. -- [ ] **Step 2: Run tests and observe failure** +- [x] **Step 2: Run tests and observe failure** Run: `uv run pytest tests/domain/test_auth.py tests/storage/test_auth_repository.py -v` -- [ ] **Step 3: Implement auth services** +- [x] **Step 3: Implement auth services** Use `argon2.PasswordHasher` defaults, `secrets.token_urlsafe(32)` for independent cookie/CSRF tokens, and SHA-256 digests for stored lookup values. Session lifetime is configurable, defaults to 12 hours, and sliding refresh updates `last_seen_at` without extending beyond the configured policy. Use `hmac.compare_digest` for CSRF hashes. Seed `RECOLL_MODERN_ADMIN_USERNAME` and `RECOLL_MODERN_ADMIN_PASSWORD` only when no user exists; erase local password references after hashing and never log them. -- [ ] **Step 4: Verify and commit** +- [x] **Step 4: Verify and commit** Run: `uv run pytest tests/domain/test_auth.py tests/storage/test_auth_repository.py -v && uv run ruff check . && uv run mypy src tests` diff --git a/src/recoll_modern/config.py b/src/recoll_modern/config.py index b26018b..167075b 100644 --- a/src/recoll_modern/config.py +++ b/src/recoll_modern/config.py @@ -20,6 +20,7 @@ class Settings: trusted_proxies: tuple[ipaddress.IPv4Network | ipaddress.IPv6Network, ...] inline_text_limit: int inline_text_limit_max: int + session_lifetime_hours: int @property def database_path(self) -> Path: @@ -43,6 +44,10 @@ class Settings: if not 0 < text_default <= text_max: raise ValueError("inline text limit must be positive and no greater than its maximum") + session_hours = int(environ.get("RECOLL_MODERN_SESSION_HOURS", "12")) + if session_hours <= 0: + raise ValueError("session lifetime must be positive") + proxy_text = environ.get("RECOLL_MODERN_TRUSTED_PROXIES", "") proxies = tuple( ipaddress.ip_network(value.strip(), strict=False) @@ -64,4 +69,5 @@ class Settings: trusted_proxies=proxies, inline_text_limit=text_default, inline_text_limit_max=text_max, + session_lifetime_hours=session_hours, ) diff --git a/src/recoll_modern/domain/auth.py b/src/recoll_modern/domain/auth.py new file mode 100644 index 0000000..d726e19 --- /dev/null +++ b/src/recoll_modern/domain/auth.py @@ -0,0 +1,147 @@ +"""Password, session, and CSRF behavior without HTTP dependencies.""" + +import hashlib +import hmac +import secrets +from collections.abc import Callable +from dataclasses import dataclass +from datetime import UTC, datetime, timedelta +from typing import Protocol +from unicodedata import normalize + +from argon2 import PasswordHasher +from argon2.exceptions import InvalidHashError, VerifyMismatchError + +from recoll_modern.domain.errors import AppError + + +@dataclass(frozen=True, slots=True) +class AuthUser: + id: str + username: str + username_key: str + password_hash: str + disabled_at: datetime | None + + +@dataclass(frozen=True, slots=True) +class SessionRecord: + token_hash: str + csrf_hash: str + user_id: str + expires_at: datetime + + +@dataclass(frozen=True, slots=True) +class SessionCredentials: + cookie_token: str + csrf_token: str + expires_at: datetime + + +@dataclass(frozen=True, slots=True) +class AuthenticatedUser: + id: str + username: str + + +class AuthRepository(Protocol): + def user_count(self) -> int: ... + + def create_user(self, username: str, password_hash: str) -> AuthUser: ... + + def get_user_by_name(self, username: str) -> AuthUser | None: ... + + def get_user_by_id(self, user_id: str) -> AuthUser | None: ... + + def create_session( + self, cookie_token: str, csrf_token: str, user_id: str, + expires_at: datetime, now: datetime, + ) -> None: ... + + def get_session(self, cookie_token: str) -> SessionRecord | None: ... + + def delete_session(self, cookie_token: str) -> None: ... + + def touch_session(self, cookie_token: str, now: datetime) -> None: ... + + +def normalize_username(username: str) -> tuple[str, str]: + display = normalize("NFC", username.strip()) + if not display: + raise ValueError("username cannot be empty") + return display, display.casefold() + + +class AuthService: + def __init__( + self, + repository: AuthRepository, + *, + lifetime: timedelta = timedelta(hours=12), + now: Callable[[], datetime] = lambda: datetime.now(UTC), + ) -> None: + if lifetime <= timedelta(0): + raise ValueError("session lifetime must be positive") + self._repository = repository + self._lifetime = lifetime + self._now = now + self._hasher = PasswordHasher() + + def seed_initial_admin(self, username: str | None, password: str | None) -> None: + if self._repository.user_count() > 0: + return + if not username or not password: + raise AppError( + "validation_failed", "initial administrator credentials are required", 422 + ) + digest = self._hasher.hash(password) + password = None + self._repository.create_user(username, digest) + + def login(self, username: str, password: str) -> SessionCredentials: + user = self._repository.get_user_by_name(username) + if user is None or user.disabled_at is not None: + raise _invalid_credentials() + try: + valid = self._hasher.verify(user.password_hash, password) + except (VerifyMismatchError, InvalidHashError): + raise _invalid_credentials() from None + if not valid: + raise _invalid_credentials() + now = self._now() + cookie_token = secrets.token_urlsafe(32) + csrf_token = secrets.token_urlsafe(32) + expires_at = now + self._lifetime + self._repository.create_session(cookie_token, csrf_token, user.id, expires_at, now) + return SessionCredentials(cookie_token, csrf_token, expires_at) + + def authenticate( + self, + cookie_token: str | None, + *, + csrf_token: str | None = None, + unsafe: bool = False, + ) -> AuthenticatedUser: + session = self._repository.get_session(cookie_token) if cookie_token else None + now = self._now() + if session is None or session.expires_at <= now: + raise AppError("unauthenticated", "Authentication is required", 401) + user = self._repository.get_user_by_id(session.user_id) + if user is None or user.disabled_at is not None: + raise AppError("unauthenticated", "Authentication is required", 401) + if unsafe: + supplied_hash = hashlib.sha256((csrf_token or "").encode()).hexdigest() + if not hmac.compare_digest(supplied_hash, session.csrf_hash): + raise AppError("csrf_failed", "CSRF token is invalid", 403) + assert cookie_token is not None + self._repository.touch_session(cookie_token, now) + return AuthenticatedUser(user.id, user.username) + + def logout(self, cookie_token: str | None) -> None: + if cookie_token: + self._repository.delete_session(cookie_token) + + +def _invalid_credentials() -> AppError: + return AppError("invalid_credentials", "Invalid username or password", 401) diff --git a/src/recoll_modern/domain/errors.py b/src/recoll_modern/domain/errors.py index dc43854..75a5fda 100644 --- a/src/recoll_modern/domain/errors.py +++ b/src/recoll_modern/domain/errors.py @@ -12,3 +12,6 @@ class AppError(Exception): message: str status: int details: Mapping[str, object] = field(default_factory=dict) + + def __str__(self) -> str: + return self.message diff --git a/src/recoll_modern/storage/auth.py b/src/recoll_modern/storage/auth.py new file mode 100644 index 0000000..76933af --- /dev/null +++ b/src/recoll_modern/storage/auth.py @@ -0,0 +1,136 @@ +"""SQLite users and hashed session-token persistence.""" + +import hashlib +import secrets +import sqlite3 +from datetime import UTC, datetime + +from recoll_modern.domain.auth import AuthUser, SessionRecord, normalize_username +from recoll_modern.storage.database import Database + + +def _hash_token(token: str) -> str: + return hashlib.sha256(token.encode()).hexdigest() + + +class SQLiteAuthRepository: + def __init__(self, database: Database) -> None: + self._database = database + + def user_count(self) -> int: + with self._database.connect() as connection: + return int(connection.execute("SELECT COUNT(*) FROM users").fetchone()[0]) + + def create_user(self, username: str, password_hash: str) -> AuthUser: + display, key = normalize_username(username) + now = datetime.now(UTC).isoformat() + while True: + user_id = f"u{secrets.token_hex(16)}" + try: + with self._database.transaction() as connection: + connection.execute( + "INSERT INTO users " + "(id, username, username_key, password_hash, created_at) " + "VALUES (?, ?, ?, ?, ?)", + (user_id, display, key, password_hash, now), + ) + except sqlite3.IntegrityError as error: + if "users.id" in str(error): + continue + raise + return AuthUser(user_id, display, key, password_hash, None) + + def get_user_by_name(self, username: str) -> AuthUser | None: + try: + _, key = normalize_username(username) + except ValueError: + return None + with self._database.connect() as connection: + row = connection.execute( + "SELECT id, username, username_key, password_hash, disabled_at " + "FROM users WHERE username_key = ?", + (key,), + ).fetchone() + return _user(row) if row is not None else None + + def get_user_by_id(self, user_id: str) -> AuthUser | None: + with self._database.connect() as connection: + row = connection.execute( + "SELECT id, username, username_key, password_hash, disabled_at " + "FROM users WHERE id = ?", + (user_id,), + ).fetchone() + return _user(row) if row is not None else None + + def disable_user(self, user_id: str) -> None: + with self._database.transaction() as connection: + connection.execute( + "UPDATE users SET disabled_at = ? WHERE id = ?", + (datetime.now(UTC).isoformat(), user_id), + ) + + def create_session( + self, + cookie_token: str, + csrf_token: str, + user_id: str, + expires_at: datetime, + now: datetime, + ) -> None: + with self._database.transaction() as connection: + connection.execute( + "INSERT INTO sessions " + "(token_hash, csrf_hash, user_id, expires_at, created_at, last_seen_at) " + "VALUES (?, ?, ?, ?, ?, ?)", + ( + _hash_token(cookie_token), _hash_token(csrf_token), user_id, + expires_at.isoformat(), now.isoformat(), now.isoformat(), + ), + ) + + def get_session(self, cookie_token: str) -> SessionRecord | None: + with self._database.connect() as connection: + row = connection.execute( + "SELECT token_hash, csrf_hash, user_id, expires_at " + "FROM sessions WHERE token_hash = ?", + (_hash_token(cookie_token),), + ).fetchone() + if row is None: + return None + return SessionRecord( + str(row["token_hash"]), str(row["csrf_hash"]), str(row["user_id"]), + datetime.fromisoformat(str(row["expires_at"])), + ) + + def delete_session(self, cookie_token: str) -> None: + with self._database.transaction() as connection: + connection.execute( + "DELETE FROM sessions WHERE token_hash = ?", (_hash_token(cookie_token),) + ) + + def touch_session(self, cookie_token: str, now: datetime) -> None: + with self._database.transaction() as connection: + connection.execute( + "UPDATE sessions SET last_seen_at = ? WHERE token_hash = ?", + (now.isoformat(), _hash_token(cookie_token)), + ) + + def cleanup_expired(self, now: datetime) -> int: + with self._database.transaction() as connection: + cursor = connection.execute( + "DELETE FROM sessions WHERE expires_at <= ?", (now.isoformat(),) + ) + return cursor.rowcount + + +def _user(row: sqlite3.Row) -> AuthUser: + return AuthUser( + id=str(row["id"]), + username=str(row["username"]), + username_key=str(row["username_key"]), + password_hash=str(row["password_hash"]), + disabled_at=( + datetime.fromisoformat(str(row["disabled_at"])) + if row["disabled_at"] is not None else None + ), + ) diff --git a/tests/domain/test_auth.py b/tests/domain/test_auth.py new file mode 100644 index 0000000..e034ad8 --- /dev/null +++ b/tests/domain/test_auth.py @@ -0,0 +1,84 @@ +from datetime import UTC, datetime, timedelta +from pathlib import Path + +import pytest + +from recoll_modern.domain.auth import AuthService +from recoll_modern.domain.errors import AppError +from recoll_modern.storage.auth import SQLiteAuthRepository +from recoll_modern.storage.database import Database + +PASSWORD = "correct horse battery staple" + + +def _auth(tmp_path: Path, now: datetime | None = None) -> tuple[AuthService, SQLiteAuthRepository]: + database = Database(tmp_path / "state.sqlite3") + database.migrate() + repository = SQLiteAuthRepository(database) + def clock() -> datetime: + return now or datetime.now(UTC) + + return AuthService(repository, now=clock), repository + + +def test_passwords_are_argon2_hashes(tmp_path: Path) -> None: + auth, repository = _auth(tmp_path) + auth.seed_initial_admin("admin", PASSWORD) + stored = repository.get_user_by_name("admin") + assert stored is not None + assert stored.password_hash.startswith("$argon2id$") + assert "correct horse" not in stored.password_hash + + +def test_first_boot_without_credentials_refuses_startup(tmp_path: Path) -> None: + auth, _ = _auth(tmp_path) + with pytest.raises(AppError, match="initial administrator"): + auth.seed_initial_admin(None, None) + + +def test_seed_is_idempotent_when_user_exists(tmp_path: Path) -> None: + auth, repository = _auth(tmp_path) + auth.seed_initial_admin("admin", PASSWORD) + auth.seed_initial_admin(None, None) + assert repository.user_count() == 1 + + +def test_login_is_case_folded_and_invalid_credentials_are_generic(tmp_path: Path) -> None: + auth, _ = _auth(tmp_path) + auth.seed_initial_admin("Straße", PASSWORD) + credentials = auth.login("STRASSE", PASSWORD) + assert auth.authenticate(credentials.cookie_token).username == "Straße" + with pytest.raises(AppError) as caught: + auth.login("unknown", "wrong") + assert caught.value.code == "invalid_credentials" + + +def test_csrf_mismatch_and_logout(tmp_path: Path) -> None: + auth, _ = _auth(tmp_path) + auth.seed_initial_admin("admin", PASSWORD) + credentials = auth.login("admin", PASSWORD) + with pytest.raises(AppError) as caught: + auth.authenticate(credentials.cookie_token, csrf_token="wrong", unsafe=True) + assert caught.value.code == "csrf_failed" + assert auth.authenticate( + credentials.cookie_token, csrf_token=credentials.csrf_token, unsafe=True + ).username == "admin" + auth.logout(credentials.cookie_token) + with pytest.raises(AppError) as caught: + auth.authenticate(credentials.cookie_token) + assert caught.value.code == "unauthenticated" + + +def test_expired_session_is_refused(tmp_path: Path) -> None: + now = datetime(2026, 1, 1, tzinfo=UTC) + database = Database(tmp_path / "state.sqlite3") + database.migrate() + repository = SQLiteAuthRepository(database) + clock = [now] + auth = AuthService(repository, now=lambda: clock[0], lifetime=timedelta(hours=1)) + auth.seed_initial_admin("admin", PASSWORD) + credentials = auth.login("admin", PASSWORD) + clock[0] = now + timedelta(hours=2) + with pytest.raises(AppError) as caught: + auth.authenticate(credentials.cookie_token) + assert caught.value.code == "unauthenticated" diff --git a/tests/storage/test_auth_repository.py b/tests/storage/test_auth_repository.py new file mode 100644 index 0000000..2c4a5bb --- /dev/null +++ b/tests/storage/test_auth_repository.py @@ -0,0 +1,53 @@ +import hashlib +from pathlib import Path + +import pytest + +from recoll_modern.domain.auth import AuthService +from recoll_modern.domain.errors import AppError +from recoll_modern.storage.auth import SQLiteAuthRepository +from recoll_modern.storage.database import Database + + +def test_database_stores_only_token_hashes(tmp_path: Path) -> None: + database = Database(tmp_path / "state.sqlite3") + database.migrate() + repository = SQLiteAuthRepository(database) + auth = AuthService(repository) + auth.seed_initial_admin("admin", "correct horse battery staple") + credentials = auth.login("admin", "correct horse battery staple") + assert repository.get_session(credentials.cookie_token) is not None + with database.connect() as connection: + row = connection.execute("SELECT token_hash, csrf_hash FROM sessions").fetchone() + assert row is not None + assert row["token_hash"] == hashlib.sha256(credentials.cookie_token.encode()).hexdigest() + assert row["csrf_hash"] == hashlib.sha256(credentials.csrf_token.encode()).hexdigest() + database_bytes = database.path.read_bytes() + assert credentials.cookie_token.encode() not in database_bytes + assert credentials.csrf_token.encode() not in database_bytes + + +def test_disabled_user_session_is_rejected(tmp_path: Path) -> None: + database = Database(tmp_path / "state.sqlite3") + database.migrate() + repository = SQLiteAuthRepository(database) + auth = AuthService(repository) + auth.seed_initial_admin("admin", "correct horse battery staple") + credentials = auth.login("admin", "correct horse battery staple") + user = repository.get_user_by_name("admin") + assert user is not None + repository.disable_user(user.id) + with pytest.raises(AppError) as caught: + auth.authenticate(credentials.cookie_token) + assert caught.value.code == "unauthenticated" + + +def test_cleanup_removes_expired_sessions(tmp_path: Path) -> None: + database = Database(tmp_path / "state.sqlite3") + database.migrate() + repository = SQLiteAuthRepository(database) + auth = AuthService(repository) + auth.seed_initial_admin("admin", "correct horse battery staple") + credentials = auth.login("admin", "correct horse battery staple") + assert repository.cleanup_expired(credentials.expires_at) == 1 + assert repository.get_session(credentials.cookie_token) is None diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index dbc2868..d8aacc3 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -24,3 +24,10 @@ def test_settings_are_rooted_in_configured_state(tmp_path: Path) -> None: def test_root_path_rejects_trailing_slash() -> None: with pytest.raises(ValueError, match="mount prefix"): Settings.from_env({"RECOLL_MODERN_ROOT_PATH": "/recoll/"}) + + +def test_session_lifetime_is_configurable() -> None: + settings = Settings.from_env({"RECOLL_MODERN_SESSION_HOURS": "6"}) + assert settings.session_lifetime_hours == 6 + with pytest.raises(ValueError, match="session lifetime"): + Settings.from_env({"RECOLL_MODERN_SESSION_HOURS": "0"})