From 2d2f54729f1fe78e8975a08a3922dcb71924adc6 Mon Sep 17 00:00:00 2001 From: Michael Joshua Saul Date: Wed, 11 Feb 2026 10:44:27 -0700 Subject: [PATCH] Add OIDC authentication and multi-user support (#606) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #552 ## Summary Adds OIDC authentication and multi-user support to Shelfmark. Users can now be managed individually with per-user download settings, while maintaining full backwards compatibility with existing auth modes (no-auth, builtin, proxy, CWA). ### Authentication - **OIDC login** with PKCE, auto-discovery, group-based admin mapping - **Password fallback** when OIDC is enabled (prevents admin lockout) - **Auto-provisioning** of OIDC users (configurable on/off) - **Email-based linking** of pre-created users to OIDC accounts - **Lockout prevention** — requires a local admin before OIDC can be enabled ### User Management - **SQLite user database** (`users.db`) with admin CRUD API - **Users management tab** in settings UI (admin-only) - **Settings restricted to admins** in multi-user modes (builtin/OIDC) — non-admin users cannot access settings - Create, edit, and delete users with role assignment (admin/user) - Password management for builtin auth users - OIDC users shown with provider badge (password fields hidden) - Per-user configurable settings: - **Download destination** — custom folder path per user - **BookLore library & path** — dropdown select, each user's books go to their own library - **Email recipients** — per-user email delivery targets - **`{User}` template variable** — use in destination paths (e.g., `/books/{User}/`) - Settings override model: per-user values override globals, empty/unset falls back to global defaults ### Download Scoping - **Per-user download visibility** — non-admins only see their own downloads - **Username display** in downloads sidebar (shows who requested each download) - **WebSocket room-based filtering** — admins see all, users see only their own - **Download progress scoping** — progress events routed to correct user rooms ### BookLore Integration - **Dynamic dropdown selects** for library/path (replaces text inputs) - **Per-user library/path overrides** via user settings - **Options cache refresh** after Test Connection ### Security - SQL injection prevention (column whitelist on user updates) - Generic OIDC error messages (no internal detail leakage) - Admin self-deletion and last-local-admin deletion guards - OIDC role overwrite fix (only updates role when admin_group is configured) ## Migration **No migration script needed.** The `users.db` is created automatically on first startup. Existing builtin auth users are auto-migrated to the database on their first login. All other auth modes (no-auth, proxy, CWA) continue working unchanged. ## Test Plan - [x] All 519 tests passing, 0 failures - [ ] Test no-auth mode: settings accessible, downloads work without login - [ ] Test builtin auth: legacy credentials auto-migrate on login, new users can be created - [ ] Test OIDC auth: login flow, callback, auto-provisioning, group-based admin - [ ] Test CWA auth: unchanged behavior - [ ] Test proxy auth: unchanged behavior - [ ] Test per-user downloads: non-admin sees only own downloads - [ ] Test BookLore dropdowns: library/path selection, per-user overrides - [ ] Test Docker build: no Dockerfile changes needed --------- Co-authored-by: Claude Opus 4.6 --- .gitignore | 1 + readme.md | 28 +- shelfmark/api/websocket.py | 68 +- shelfmark/config/security.py | 143 ++++ shelfmark/config/settings.py | 10 +- shelfmark/config/users_settings.py | 23 + shelfmark/core/admin_routes.py | 246 ++++++ shelfmark/core/models.py | 4 + shelfmark/core/naming.py | 2 +- shelfmark/core/oidc_auth.py | 102 +++ shelfmark/core/oidc_routes.py | 272 +++++++ shelfmark/core/queue.py | 14 +- shelfmark/core/user_db.py | 189 +++++ shelfmark/download/orchestrator.py | 67 +- shelfmark/download/outputs/booklore.py | 21 +- shelfmark/download/postprocess/destination.py | 7 +- shelfmark/download/postprocess/transfer.py | 1 + shelfmark/main.py | 145 +++- src/frontend/package-lock.json | 9 - src/frontend/src/App.tsx | 2 + .../src/components/DownloadsSidebar.tsx | 6 + src/frontend/src/components/LoginForm.tsx | 22 + .../src/components/settings/SettingsModal.tsx | 81 +- .../components/settings/SettingsSidebar.tsx | 6 + .../src/components/settings/UsersPanel.tsx | 743 ++++++++++++++++++ .../settings/fields/TagListField.tsx | 14 +- src/frontend/src/hooks/useAuth.ts | 5 + src/frontend/src/hooks/useSettings.ts | 13 +- src/frontend/src/pages/LoginPage.tsx | 5 +- src/frontend/src/services/api.ts | 79 ++ src/frontend/src/types/index.ts | 2 + tests/README.md | 34 + tests/config/test_oidc_settings.py | 160 ++++ tests/core/test_admin_users_api.py | 705 +++++++++++++++++ tests/core/test_booklore_multiuser.py | 90 +++ tests/core/test_builtin_multiuser.py | 140 ++++ tests/core/test_oidc_auth.py | 258 ++++++ tests/core/test_oidc_integration.py | 206 +++++ tests/core/test_oidc_routes.py | 313 ++++++++ tests/core/test_per_user_downloads.py | 231 ++++++ tests/core/test_user_db.py | 195 +++++ tests/core/test_user_template_variable.py | 84 ++ tests/e2e/test_auth_flow.py | 2 +- 43 files changed, 4646 insertions(+), 102 deletions(-) create mode 100644 shelfmark/config/users_settings.py create mode 100644 shelfmark/core/admin_routes.py create mode 100644 shelfmark/core/oidc_auth.py create mode 100644 shelfmark/core/oidc_routes.py create mode 100644 shelfmark/core/user_db.py create mode 100644 src/frontend/src/components/settings/UsersPanel.tsx create mode 100644 tests/config/test_oidc_settings.py create mode 100644 tests/core/test_admin_users_api.py create mode 100644 tests/core/test_booklore_multiuser.py create mode 100644 tests/core/test_builtin_multiuser.py create mode 100644 tests/core/test_oidc_auth.py create mode 100644 tests/core/test_oidc_integration.py create mode 100644 tests/core/test_oidc_routes.py create mode 100644 tests/core/test_per_user_downloads.py create mode 100644 tests/core/test_user_db.py create mode 100644 tests/core/test_user_template_variable.py diff --git a/.gitignore b/.gitignore index f35bad08..441919c8 100644 --- a/.gitignore +++ b/.gitignore @@ -232,3 +232,4 @@ pyrightconfig.json AGENTS.md .claude/ .playwright-mcp/ +frontend-dist/ diff --git a/readme.md b/readme.md index 317228e3..fd93a2b9 100644 --- a/readme.md +++ b/readme.md @@ -151,15 +151,21 @@ If you need Cloudflare bypass with the Lite image, configure an external resolve ## 🔐 Authentication -Authentication is optional but recommended for shared or exposed instances. Three authentication methods are available in Settings: +Authentication is optional but recommended for shared or exposed instances. Four authentication methods are available in Settings: -**1. Single Username/Password** +**1. Built-in Username/Password** -**2. Proxy (Forward) Authentication** +Multi-user support with admin user management. The first user is always admin. Admins can create additional users, set per-user download destinations, and manage roles. + +**2. OpenID Connect (OIDC)** + +Integrate with any OIDC provider (Authentik, Keycloak, Pocket ID, etc.) for SSO. Supports auto-provisioning, group-based admin mapping, and per-user download settings. Configure your provider's discovery URL, client ID, and client secret in Settings. + +**3. Proxy (Forward) Authentication** Proxy auth trusts headers set by your reverse proxy (e.g. `X-Auth-User`). Ensure Shelfmark is not directly exposed, and configure your proxy to strip/overwrite these headers for all inbound requests. -**3. Calibre-Web Database** +**4. Calibre-Web Database** If you're running Calibre-Web, you can reuse its user database by mounting it: @@ -168,6 +174,15 @@ volumes: - /path/to/calibre-web/app.db:/auth/app.db:ro ``` +### Multi-User Features (Built-in & OIDC) + +- Admin user management panel in Settings +- Per-user download destination overrides +- Per-user BookLore library/path overrides +- Per-user email recipient overrides +- Download queue scoped per user (admins see all) +- `{User}` template variable for organizing downloads by user + ## Health Monitoring The application exposes a health endpoint at `/api/health` (no authentication required). Add a health check to your compose: @@ -215,13 +230,16 @@ The frontend dev server proxies to the backend on port 8084. ├─────────────────────────────────────────────────────────────┤ │ Flask Backend │ │ (REST API + WebSocket) │ +├─────────────────────────────────────────────────────────────┤ +│ Authentication │ +│ (Built-in / OIDC / Proxy / CWA / None) │ ├───────────────────┬─────────────────────┬───────────────────┤ │ Metadata Providers│ Download Queue │ Cloudflare │ │ │ & Orchestrator │ Bypass │ ├───────────────────┼─────────────────────┼───────────────────┤ │ • Hardcover │ • Task scheduling │ • Internal │ │ • Open Library │ • Progress tracking │ • External │ -│ │ • Retry logic │ (FlareSolverr) │ +│ │ • Per-user scoping │ (FlareSolverr) │ ├───────────────────┴─────────────────────┴───────────────────┤ │ Release Sources │ ├─────────────────────────────────────────────────────────────┤ diff --git a/shelfmark/api/websocket.py b/shelfmark/api/websocket.py index 32b3f43d..1823d83b 100644 --- a/shelfmark/api/websocket.py +++ b/shelfmark/api/websocket.py @@ -4,7 +4,7 @@ import logging import threading from typing import Optional, Dict, Any, Callable, List -from flask_socketio import SocketIO +from flask_socketio import SocketIO, join_room, leave_room logger = logging.getLogger(__name__) @@ -20,6 +20,9 @@ class WebSocketManager: self._on_first_connect_callbacks: List[Callable[[], None]] = [] self._on_all_disconnect_callbacks: List[Callable[[], None]] = [] self._needs_rewarm = False # Flag to trigger warmup callbacks on next connect + self._user_rooms: Dict[str, int] = {} # room_name -> ref count + self._rooms_lock = threading.Lock() + self._queue_status_fn: Optional[Callable] = None # Reference to queue_status() def init_app(self, app, socketio: SocketIO): """Initialize the WebSocket manager with Flask-SocketIO instance.""" @@ -100,19 +103,62 @@ class WebSocketManager: """Check if WebSocket is enabled and ready.""" return self._enabled and self.socketio is not None + def set_queue_status_fn(self, fn: Callable): + """Set the queue_status function reference for per-room filtering.""" + self._queue_status_fn = fn + + def join_user_room(self, sid: str, is_admin: bool, db_user_id: Optional[int] = None): + """Join the appropriate room based on user role.""" + if is_admin or db_user_id is None: + join_room("admins", sid=sid) + else: + room = f"user_{db_user_id}" + join_room(room, sid=sid) + with self._rooms_lock: + self._user_rooms[room] = self._user_rooms.get(room, 0) + 1 + + def leave_user_room(self, sid: str, is_admin: bool, db_user_id: Optional[int] = None): + """Leave the user's room on disconnect.""" + if is_admin or db_user_id is None: + leave_room("admins", sid=sid) + else: + room = f"user_{db_user_id}" + leave_room(room, sid=sid) + with self._rooms_lock: + count = self._user_rooms.get(room, 1) - 1 + if count <= 0: + self._user_rooms.pop(room, None) + else: + self._user_rooms[room] = count + def broadcast_status_update(self, status_data: Dict[str, Any]): - """Broadcast status update to all connected clients.""" + """Broadcast status update to all connected clients, filtered by user room.""" if not self.is_enabled(): return try: - # When calling socketio.emit() outside event handlers, it broadcasts by default - self.socketio.emit('status_update', status_data) - logger.debug(f"Broadcasted status update to all clients") + # Admins (and no-auth users) get full status + self.socketio.emit('status_update', status_data, to="admins") + + # Each user room gets filtered status + with self._rooms_lock: + active_rooms = list(self._user_rooms.keys()) + + if active_rooms and self._queue_status_fn: + for room in active_rooms: + try: + # Extract user_id from room name "user_123" + uid = int(room.split("_", 1)[1]) + filtered = self._queue_status_fn(user_id=uid) + self.socketio.emit('status_update', filtered, to=room) + except Exception as e: + logger.error(f"Failed to send status update for room {room}: {e}") + + logger.debug("Broadcasted status update to all rooms") except Exception as e: logger.error(f"Error broadcasting status update: {e}") - def broadcast_download_progress(self, book_id: str, progress: float, status: str): + def broadcast_download_progress(self, book_id: str, progress: float, status: str, user_id: Optional[int] = None): """Broadcast download progress update for a specific book.""" if not self.is_enabled(): return @@ -123,8 +169,14 @@ class WebSocketManager: 'progress': progress, 'status': status } - # When calling socketio.emit() outside event handlers, it broadcasts by default - self.socketio.emit('download_progress', data) + # Admins always see all progress + self.socketio.emit('download_progress', data, to="admins") + # If task belongs to a specific user, send to their room too + if user_id is not None: + room = f"user_{user_id}" + with self._rooms_lock: + if room in self._user_rooms: + self.socketio.emit('download_progress', data, to=room) logger.debug(f"Broadcasted progress for book {book_id}: {progress}%") except Exception as e: logger.error(f"Error broadcasting download progress: {e}") diff --git a/shelfmark/config/security.py b/shelfmark/config/security.py index 7b357359..e29c5990 100644 --- a/shelfmark/config/security.py +++ b/shelfmark/config/security.py @@ -14,6 +14,7 @@ from shelfmark.core.settings_registry import ( PasswordField, CheckboxField, ActionButton, + TagListField, ) logger = setup_logger(__name__) @@ -114,6 +115,29 @@ def _on_save_security(values: Dict[str, Any]) -> Dict[str, Any]: Returns: Dict with processed values to save and any validation errors. """ + # If switching to OIDC, ensure a local admin exists as fallback + if values.get("AUTH_METHOD") == "oidc": + import os + from shelfmark.core.user_db import UserDB + + db_path = os.path.join(os.environ.get("CONFIG_DIR", "/config"), "users.db") + udb = UserDB(db_path) + udb.initialize() + users = udb.list_users() + has_local_admin = any( + u.get("password_hash") and u.get("role") == "admin" + for u in users + ) + if not has_local_admin: + return { + "error": True, + "message": ( + "Create a local admin account first (Users tab) before enabling OIDC. " + "This ensures you can still log in with a password if SSO is unavailable." + ), + "values": values, + } + password = values.get("BUILTIN_PASSWORD", "") password_confirm = values.get("BUILTIN_PASSWORD_CONFIRM", "") @@ -157,6 +181,33 @@ def _on_save_security(values: Dict[str, Any]) -> Dict[str, Any]: return {"error": False, "values": values} +def _test_oidc_connection() -> Dict[str, Any]: + """Test OIDC connection by fetching the discovery document.""" + import requests + + try: + config = load_config_file("security") + discovery_url = config.get("OIDC_DISCOVERY_URL", "") + if not discovery_url: + return {"success": False, "message": "Discovery URL is not configured."} + + resp = requests.get(discovery_url, timeout=10) + resp.raise_for_status() + doc = resp.json() + + # Validate required fields + required = ["issuer", "authorization_endpoint", "token_endpoint"] + missing = [f for f in required if f not in doc] + if missing: + return {"success": False, "message": f"Discovery document missing fields: {', '.join(missing)}"} + + return {"success": True, "message": f"Connected to {doc['issuer']}"} + + except Exception as e: + logger.error(f"OIDC connection test failed: {e}") + return {"success": False, "message": f"Connection failed: {str(e)}"} + + @register_settings("security", "Security", icon="shield", order=5) def security_settings(): """Security and authentication settings.""" @@ -168,6 +219,7 @@ def security_settings(): {"label": "No Authentication", "value": "none"}, {"label": "Username/Password", "value": "builtin"}, {"label": "Proxy Authentication", "value": "proxy"}, + {"label": "OIDC (OpenID Connect)", "value": "oidc"}, ] if cwa_db_available: auth_method_options.append({"label": "Calibre-Web Database", "value": "cwa"}) @@ -281,6 +333,97 @@ def security_settings(): env_supported=False, show_when={"field": "AUTH_METHOD", "value": "cwa"}, ), + # === OIDC SETTINGS === + TextField( + key="OIDC_DISCOVERY_URL", + label="Discovery URL", + description=( + "OpenID Connect discovery endpoint URL." + " Usually ends with /.well-known/openid-configuration." + ), + placeholder="https://auth.example.com/.well-known/openid-configuration", + required=True, + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + TextField( + key="OIDC_CLIENT_ID", + label="Client ID", + description="OAuth2 client ID from your identity provider.", + placeholder="shelfmark", + required=True, + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + PasswordField( + key="OIDC_CLIENT_SECRET", + label="Client Secret", + description="OAuth2 client secret from your identity provider.", + required=True, + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + TagListField( + key="OIDC_SCOPES", + label="Scopes", + description="OAuth2 scopes to request from the identity provider. Managed automatically: includes essential scopes and the group claim when using admin group authorization.", + default=["openid", "email", "profile"], + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + TextField( + key="OIDC_GROUP_CLAIM", + label="Group Claim Name", + description=( + "The name of the claim in the ID token that contains user groups." + ), + placeholder="groups", + default="groups", + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + TextField( + key="OIDC_ADMIN_GROUP", + label="Admin Group Name", + description=( + "Users in this group will be given admin access (if enabled below). " + "Leave empty to use database roles only." + ), + placeholder="shelfmark-admins", + default="", + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + CheckboxField( + key="OIDC_USE_ADMIN_GROUP", + label="Use Admin Group for Authorization", + description=( + "When enabled, users in the Admin Group are granted admin access. " + "When disabled, admin access is determined solely by database roles." + ), + default=True, + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + CheckboxField( + key="OIDC_AUTO_PROVISION", + label="Auto-Provision Users", + description=( + "Automatically create a user account on first OIDC login." + " When disabled, users must be pre-created by an admin." + ), + default=True, + env_supported=False, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), + ActionButton( + key="test_oidc", + label="Test Connection", + description="Fetch the OIDC discovery document and validate configuration.", + style="primary", + callback=_test_oidc_connection, + show_when={"field": "AUTH_METHOD", "value": "oidc"}, + ), ] return fields diff --git a/shelfmark/config/settings.py b/shelfmark/config/settings.py index d50e9376..849d8c2e 100644 --- a/shelfmark/config/settings.py +++ b/shelfmark/config/settings.py @@ -799,7 +799,7 @@ def download_settings(): TextField( key="DESTINATION", label="Destination", - description="Directory where downloaded files are saved.", + description="Directory where downloaded files are saved. Use {User} for per-user folders (e.g. /books/{User}).", default="/books", required=True, env_var="INGEST_DIR", # Legacy env var name for backwards compatibility @@ -839,7 +839,7 @@ def download_settings(): TextField( key="TEMPLATE_RENAME", label="Naming Template", - description="Variables: {Author}, {Title}, {Year}. Universal adds: {Series}, {SeriesPosition}, {Subtitle}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty. Rename templates are filename-only (no '/' or '\\'); use Organize for folders.", + description="Variables: {Author}, {Title}, {Year}, {User}. Universal adds: {Series}, {SeriesPosition}, {Subtitle}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty. Rename templates are filename-only (no '/' or '\\'); use Organize for folders.", default="{Author} - {Title} ({Year})", placeholder="{Author} - {Title} ({Year})", show_when=[ @@ -851,7 +851,7 @@ def download_settings(): TextField( key="TEMPLATE_ORGANIZE", label="Path Template", - description="Use / to create folders. Variables: {Author}, {Title}, {Year}. Universal adds: {Series}, {SeriesPosition}, {Subtitle}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty.", + description="Use / to create folders. Variables: {Author}, {Title}, {Year}, {User}. Universal adds: {Series}, {SeriesPosition}, {Subtitle}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty.", default="{Author}/{Title} ({Year})", placeholder="{Author}/{Series/}{Title} ({Year})", show_when=[ @@ -1074,7 +1074,7 @@ def download_settings(): TextField( key="TEMPLATE_AUDIOBOOK_RENAME", label="Naming Template", - description="Variables: {Author}, {Title}, {Year}, {Series}, {SeriesPosition}, {Subtitle}, {PartNumber}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty. Rename templates are filename-only (no '/' or '\\'); use Organize for folders.", + description="Variables: {Author}, {Title}, {Year}, {User}, {Series}, {SeriesPosition}, {Subtitle}, {PartNumber}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty. Rename templates are filename-only (no '/' or '\\'); use Organize for folders.", default="{Author} - {Title}", placeholder="{Author} - {Title}{ - Part }{PartNumber}", show_when={"field": "FILE_ORGANIZATION_AUDIOBOOK", "value": "rename"}, @@ -1084,7 +1084,7 @@ def download_settings(): TextField( key="TEMPLATE_AUDIOBOOK_ORGANIZE", label="Path Template", - description="Use / to create folders. Variables: {Author}, {Title}, {Year}, {Series}, {SeriesPosition}, {Subtitle}, {PartNumber}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty.", + description="Use / to create folders. Variables: {Author}, {Title}, {Year}, {User}, {Series}, {SeriesPosition}, {Subtitle}, {PartNumber}. Use arbitrary prefix/suffix: {Vol. SeriesPosition - } outputs 'Vol. 2 - ' when set, nothing when empty.", default="{Author}/{Title}", placeholder="{Author}/{Series/}{Title}{ - Part }{PartNumber}", show_when={"field": "FILE_ORGANIZATION_AUDIOBOOK", "value": "organize"}, diff --git a/shelfmark/config/users_settings.py b/shelfmark/config/users_settings.py new file mode 100644 index 00000000..0710cc46 --- /dev/null +++ b/shelfmark/config/users_settings.py @@ -0,0 +1,23 @@ +"""Users settings tab registration. + +This registers a 'users' tab in the settings sidebar. +The actual user management is handled by a custom frontend component +that talks to /api/admin/users endpoints. +""" + +from shelfmark.core.settings_registry import ( + HeadingField, + register_settings, +) + + +@register_settings("users", "Users", icon="users", order=6) +def users_settings(): + """User management tab - rendered as a custom component on the frontend.""" + return [ + HeadingField( + key="users_heading", + title="User Accounts", + description="Manage user accounts for multi-user authentication.", + ), + ] diff --git a/shelfmark/core/admin_routes.py b/shelfmark/core/admin_routes.py new file mode 100644 index 00000000..fd3cc739 --- /dev/null +++ b/shelfmark/core/admin_routes.py @@ -0,0 +1,246 @@ +"""Admin user management API routes. + +Registers /api/admin/users CRUD endpoints for managing users. +All endpoints require admin session. +""" + +from functools import wraps + +from flask import Flask, jsonify, request, session +from werkzeug.security import generate_password_hash + +from shelfmark.config.booklore_settings import ( + get_booklore_library_options, + get_booklore_path_options, +) +from shelfmark.core.logger import setup_logger +from shelfmark.core.settings_registry import load_config_file +from shelfmark.core.user_db import UserDB + +logger = setup_logger(__name__) + +_DOWNLOAD_DEFAULTS = { + "BOOKS_OUTPUT_MODE": "folder", + "DESTINATION": "/books", + "BOOKLORE_LIBRARY_ID": "", + "BOOKLORE_PATH_ID": "", + "EMAIL_RECIPIENTS": [], +} + + +def _get_auth_mode(): + """Get current auth mode from config.""" + try: + config = load_config_file("security") + return config.get("AUTH_METHOD", "none") + except Exception: + return "none" + + +def _require_admin(f): + """Decorator to require admin session for admin routes. + + In no-auth mode, everyone has access (is_admin defaults True). + In auth-required modes, requires an authenticated session with admin role. + """ + @wraps(f) + def decorated(*args, **kwargs): + auth_mode = _get_auth_mode() + if auth_mode != "none": + if "user_id" not in session: + return jsonify({"error": "Authentication required"}), 401 + if not session.get("is_admin", False): + return jsonify({"error": "Admin access required"}), 403 + return f(*args, **kwargs) + return decorated + + +def _sanitize_user(user: dict) -> dict: + """Remove sensitive fields from user dict before returning to client.""" + user.pop("password_hash", None) + return user + + +def register_admin_routes(app: Flask, user_db: UserDB) -> None: + """Register admin user management routes on the Flask app.""" + + @app.route("/api/admin/users", methods=["GET"]) + @_require_admin + def admin_list_users(): + """List all users.""" + users = user_db.list_users() + return jsonify([_sanitize_user(u) for u in users]) + + @app.route("/api/admin/users", methods=["POST"]) + @_require_admin + def admin_create_user(): + """Create a new user with password authentication.""" + data = request.get_json() or {} + + username = (data.get("username") or "").strip() + password = data.get("password", "") + email = (data.get("email") or "").strip() or None + display_name = (data.get("display_name") or "").strip() or None + role = data.get("role", "user") + + if not username: + return jsonify({"error": "Username is required"}), 400 + if not password or len(password) < 4: + return jsonify({"error": "Password must be at least 4 characters"}), 400 + if role not in ("admin", "user"): + return jsonify({"error": "Role must be 'admin' or 'user'"}), 400 + + # First user is always admin + if not user_db.list_users(): + role = "admin" + + # Check if username already exists + if user_db.get_user(username=username): + return jsonify({"error": "Username already exists"}), 409 + + password_hash = generate_password_hash(password) + try: + user = user_db.create_user( + username=username, + password_hash=password_hash, + email=email, + display_name=display_name, + role=role, + ) + except ValueError: + return jsonify({"error": "Username already exists"}), 409 + logger.info(f"Admin created user: {username} (role={role})") + return jsonify(_sanitize_user(user)), 201 + + @app.route("/api/admin/users/", methods=["GET"]) + @_require_admin + def admin_get_user(user_id): + """Get a user by ID with their settings.""" + user = user_db.get_user(user_id=user_id) + if not user: + return jsonify({"error": "User not found"}), 404 + + result = _sanitize_user(user) + result["settings"] = user_db.get_user_settings(user_id) + return jsonify(result) + + @app.route("/api/admin/users/", methods=["PUT"]) + @_require_admin + def admin_update_user(user_id): + """Update user fields and/or settings.""" + user = user_db.get_user(user_id=user_id) + if not user: + return jsonify({"error": "User not found"}), 404 + + data = request.get_json() or {} + + # Handle optional password update + password = data.get("password", "") + if password: + if len(password) < 4: + return jsonify({"error": "Password must be at least 4 characters"}), 400 + user_db.update_user(user_id, password_hash=generate_password_hash(password)) + + # Update user fields + user_fields = {} + for field in ("role", "email", "display_name"): + if field in data: + user_fields[field] = data[field] + + if "role" in user_fields and user_fields["role"] not in ("admin", "user"): + return jsonify({"error": "Role must be 'admin' or 'user'"}), 400 + + # Prevent changing OIDC user role when group-based auth is enabled + if "role" in user_fields and user.get("oidc_subject") and user_fields["role"] != user.get("role"): + security_config = load_config_file("security") + use_admin_group = security_config.get("OIDC_USE_ADMIN_GROUP", True) + if use_admin_group: + admin_group = security_config.get("OIDC_ADMIN_GROUP", "") + msg = ( + f"Admin roles for OIDC users are managed by the '{admin_group}' group in your identity provider" + if admin_group + else "Disable 'Use Admin Group for Authorization' in security settings to manage roles manually" + ) + return jsonify({ + "error": "Cannot change role for OIDC user when group-based authorization is enabled", + "message": msg, + }), 400 + + # Prevent demoting the last admin + if "role" in user_fields and user_fields["role"] != "admin": + if user.get("role") == "admin": + other_admins = [ + u for u in user_db.list_users() + if u["role"] == "admin" and u["id"] != user_id + ] + if not other_admins: + return jsonify({"error": "Cannot remove admin role from the last admin account"}), 400 + + if user_fields: + user_db.update_user(user_id, **user_fields) + + # Update per-user settings + if "settings" in data and isinstance(data["settings"], dict): + user_db.set_user_settings(user_id, data["settings"]) + + updated = user_db.get_user(user_id=user_id) + result = _sanitize_user(updated) + result["settings"] = user_db.get_user_settings(user_id) + logger.info(f"Admin updated user {user_id}") + return jsonify(result) + + @app.route("/api/admin/download-defaults", methods=["GET"]) + @_require_admin + def admin_download_defaults(): + """Return global download settings relevant to per-user overrides.""" + config = load_config_file("downloads") + keys = [ + "BOOKS_OUTPUT_MODE", + "DESTINATION", + "BOOKLORE_LIBRARY_ID", + "BOOKLORE_PATH_ID", + "EMAIL_RECIPIENTS", + ] + defaults = {k: config.get(k, _DOWNLOAD_DEFAULTS.get(k)) for k in keys} + + # Include OIDC settings for UI warnings (e.g., when admin tries to set OIDC user role) + security_config = load_config_file("security") + defaults["OIDC_ADMIN_GROUP"] = security_config.get("OIDC_ADMIN_GROUP", "") + defaults["OIDC_USE_ADMIN_GROUP"] = security_config.get("OIDC_USE_ADMIN_GROUP", True) + defaults["OIDC_AUTO_PROVISION"] = security_config.get("OIDC_AUTO_PROVISION", True) + + return jsonify(defaults) + + @app.route("/api/admin/booklore-options", methods=["GET"]) + @_require_admin + def admin_booklore_options(): + """Return available BookLore library and path options.""" + return jsonify({ + "libraries": get_booklore_library_options(), + "paths": get_booklore_path_options(), + }) + + @app.route("/api/admin/users/", methods=["DELETE"]) + @_require_admin + def admin_delete_user(user_id): + """Delete a user.""" + # Prevent self-deletion + if session.get("db_user_id") == user_id: + return jsonify({"error": "Cannot delete your own account"}), 400 + + user = user_db.get_user(user_id=user_id) + if not user: + return jsonify({"error": "User not found"}), 404 + + # Prevent deleting the last local admin + if user.get("role") == "admin" and user.get("password_hash"): + local_admins = [ + u for u in user_db.list_users() + if u["role"] == "admin" and u.get("password_hash") and u["id"] != user_id + ] + if not local_admins: + return jsonify({"error": "Cannot delete the last local admin account"}), 400 + + user_db.delete_user(user_id) + logger.info(f"Admin deleted user {user_id}: {user['username']}") + return jsonify({"success": True}) diff --git a/shelfmark/core/models.py b/shelfmark/core/models.py index eea928f3..dd9f7d0a 100644 --- a/shelfmark/core/models.py +++ b/shelfmark/core/models.py @@ -94,6 +94,10 @@ class DownloadTask: output_mode: Optional[str] = None # e.g. "folder", "booklore", "email" output_args: Dict[str, Any] = field(default_factory=dict) # Per-output parameters (e.g. email recipient) + # User association (multi-user support) + user_id: Optional[int] = None # DB user ID who queued this download + username: Optional[str] = None # Username for {User} template variable + # Runtime state priority: int = 0 added_time: float = field(default_factory=time.time) diff --git a/shelfmark/core/naming.py b/shelfmark/core/naming.py index 883c1a50..4b037e89 100644 --- a/shelfmark/core/naming.py +++ b/shelfmark/core/naming.py @@ -12,7 +12,7 @@ logger = setup_logger(__name__) # Known variable tokens, sorted longest-first to avoid partial matches # e.g., "SeriesPosition" must match before "Series" -KNOWN_TOKENS = ['seriesposition', 'partnumber', 'subtitle', 'author', 'series', 'title', 'year'] +KNOWN_TOKENS = ['seriesposition', 'partnumber', 'subtitle', 'author', 'series', 'title', 'year', 'user'] # Match any {...} block for template parsing BRACE_PATTERN = re.compile(r'\{([^}]+)\}') diff --git a/shelfmark/core/oidc_auth.py b/shelfmark/core/oidc_auth.py new file mode 100644 index 00000000..2693ab96 --- /dev/null +++ b/shelfmark/core/oidc_auth.py @@ -0,0 +1,102 @@ +"""OIDC authentication helpers. + +Handles group claim parsing, user info extraction, and user provisioning. +Flask route handlers are registered separately in main.py. +""" + +from typing import Any, Dict, List, Optional + +from shelfmark.core.logger import setup_logger +from shelfmark.core.user_db import UserDB + +logger = setup_logger(__name__) + + +def parse_group_claims(id_token: Dict[str, Any], group_claim: str) -> List[str]: + """Extract group list from an ID token claim. + + Supports list, comma-separated string, or pipe-separated string. + Returns empty list if claim is missing. + """ + raw = id_token.get(group_claim) + if raw is None: + return [] + if isinstance(raw, list): + return [str(g).strip() for g in raw if str(g).strip()] + if isinstance(raw, str): + delimiter = "," if "," in raw else "|" + return [g.strip() for g in raw.split(delimiter) if g.strip()] + return [] + + +def extract_user_info(id_token: Dict[str, Any]) -> Dict[str, Any]: + """Extract user info from OIDC ID token claims. + + Returns a dict with keys: oidc_subject, username, email, display_name. + Falls back through preferred_username -> email -> sub for username. + """ + sub = id_token.get("sub", "") + email = id_token.get("email") + display_name = id_token.get("name") + username = id_token.get("preferred_username") or email or sub + + return { + "oidc_subject": sub, + "username": username, + "email": email, + "display_name": display_name, + } + + +def provision_oidc_user( + db: UserDB, + user_info: Dict[str, Any], + is_admin: Optional[bool] = None, +) -> Dict[str, Any]: + """Create or update a user from OIDC claims. + + If a user with the same oidc_subject exists, updates their info. + If the username is taken by a different user, appends a numeric suffix. + + Admin role is synced from IdP when group-based auth is enabled (is_admin is not None): + - is_admin=True → DB role = admin + - is_admin=False → DB role = user (downgrade if needed) + + When group-based auth is disabled (is_admin=None), preserve existing DB role. + The database is always the single source of truth for auth checks. + """ + oidc_subject = user_info["oidc_subject"] + + # Check if user already exists by OIDC subject + existing = db.get_user(oidc_subject=oidc_subject) + if existing: + updates: Dict[str, Any] = { + "email": user_info.get("email"), + "display_name": user_info.get("display_name"), + } + # Sync role from IdP when group-based auth is enabled + if is_admin is not None: + updates["role"] = "admin" if is_admin else "user" + db.update_user(existing["id"], **updates) + return db.get_user(user_id=existing["id"]) + + role = "admin" if is_admin else "user" + + # New user — resolve username conflicts + username = user_info["username"] or oidc_subject + if db.get_user(username=username): + # Username taken, append suffix + suffix = 1 + while db.get_user(username=f"{username}_{suffix}"): + suffix += 1 + username = f"{username}_{suffix}" + + user = db.create_user( + username=username, + email=user_info.get("email"), + display_name=user_info.get("display_name"), + oidc_subject=oidc_subject, + role=role, + ) + logger.info(f"Provisioned OIDC user: {username} (sub={oidc_subject})") + return user diff --git a/shelfmark/core/oidc_routes.py b/shelfmark/core/oidc_routes.py new file mode 100644 index 00000000..da5e7018 --- /dev/null +++ b/shelfmark/core/oidc_routes.py @@ -0,0 +1,272 @@ +"""OIDC Flask route handlers. + +Registers /api/auth/oidc/login and /api/auth/oidc/callback endpoints. +Separated from main.py to keep the OIDC logic self-contained. +""" + +import hashlib +import secrets +import base64 +from urllib.parse import urlencode + +import requests as http_requests +from flask import Flask, redirect, request, session, jsonify + +from shelfmark.core.logger import setup_logger +from shelfmark.core.oidc_auth import ( + extract_user_info, + parse_group_claims, + provision_oidc_user, +) +from shelfmark.core.settings_registry import load_config_file +from shelfmark.core.user_db import UserDB + +logger = setup_logger(__name__) + +# Cache discovery document in memory (refreshed on restart) +_discovery_cache = {} + + +def _fetch_discovery(discovery_url: str) -> dict: + """Fetch and cache the OIDC discovery document.""" + if discovery_url in _discovery_cache: + return _discovery_cache[discovery_url] + + resp = http_requests.get(discovery_url, timeout=10) + resp.raise_for_status() + doc = resp.json() + _discovery_cache[discovery_url] = doc + return doc + + +def _generate_pkce(): + """Generate PKCE code_verifier and code_challenge.""" + code_verifier = secrets.token_urlsafe(64) + digest = hashlib.sha256(code_verifier.encode("ascii")).digest() + code_challenge = base64.urlsafe_b64encode(digest).rstrip(b"=").decode("ascii") + return code_verifier, code_challenge + + +def _exchange_code( + token_endpoint: str, + code: str, + code_verifier: str, + client_id: str, + client_secret: str, + redirect_uri: str, + userinfo_endpoint: str | None = None, +) -> dict: + """Exchange authorization code for tokens and return ID token claims. + + If id_token is missing from the response, falls back to calling the userinfo endpoint. + """ + resp = http_requests.post( + token_endpoint, + data={ + "grant_type": "authorization_code", + "code": code, + "code_verifier": code_verifier, + "client_id": client_id, + "client_secret": client_secret, + "redirect_uri": redirect_uri, + }, + timeout=10, + ) + resp.raise_for_status() + token_data = resp.json() + + # Decode ID token (we trust the IdP since we just exchanged the code over TLS) + import json as json_mod + + id_token_raw = token_data.get("id_token", "") + if id_token_raw: + # Decode JWT payload without verification (already validated by TLS + code exchange) + payload = id_token_raw.split(".")[1] + # Add required Base64 padding (0-3 '=' characters) + payload += "=" * ((-len(payload)) % 4) + claims = json_mod.loads(base64.urlsafe_b64decode(payload)) + else: + # No ID token in response — try userinfo endpoint + access_token = token_data.get("access_token") + if userinfo_endpoint and access_token: + try: + logger.info("ID token not found in token response, fetching from userinfo endpoint") + userinfo_resp = http_requests.get( + userinfo_endpoint, + headers={"Authorization": f"Bearer {access_token}"}, + timeout=10, + ) + userinfo_resp.raise_for_status() + claims = userinfo_resp.json() + except http_requests.RequestException as e: + logger.error(f"Failed to fetch userinfo: {e}") + raise ValueError("OIDC authentication failed: missing id_token and userinfo endpoint unavailable") + else: + logger.error("OIDC token response missing both id_token and access_token") + raise ValueError("OIDC authentication failed: invalid token response") + + return claims + + +def register_oidc_routes(app: Flask, user_db: UserDB) -> None: + """Register OIDC authentication routes on the Flask app.""" + + @app.route("/api/auth/oidc/login", methods=["GET"]) + def oidc_login(): + """Initiate OIDC login flow. Redirects to IdP.""" + try: + config = load_config_file("security") + discovery_url = config.get("OIDC_DISCOVERY_URL", "") + client_id = config.get("OIDC_CLIENT_ID", "") + + # Build scopes from config (user-editable) with openid guaranteed + configured_scopes = config.get("OIDC_SCOPES", ["openid", "email", "profile"]) + scopes = list(dict.fromkeys(["openid"] + configured_scopes)) # dedupe, openid first + admin_group = config.get("OIDC_ADMIN_GROUP", "") + group_claim = config.get("OIDC_GROUP_CLAIM", "groups") + use_admin_group = config.get("OIDC_USE_ADMIN_GROUP", True) + + # Add group claim to scopes when using admin group authorization + if admin_group and use_admin_group and group_claim and group_claim not in scopes: + scopes.append(group_claim) + + if not discovery_url or not client_id: + return jsonify({"error": "OIDC not configured"}), 500 + + discovery = _fetch_discovery(discovery_url) + auth_endpoint = discovery["authorization_endpoint"] + + # Generate PKCE and state + code_verifier, code_challenge = _generate_pkce() + state = secrets.token_urlsafe(32) + + # Store in session for callback validation + session["oidc_state"] = state + session["oidc_code_verifier"] = code_verifier + + # Build callback URL + redirect_uri = request.url_root.rstrip("/") + "/api/auth/oidc/callback" + + params = { + "client_id": client_id, + "response_type": "code", + "scope": " ".join(scopes), + "redirect_uri": redirect_uri, + "state": state, + "code_challenge": code_challenge, + "code_challenge_method": "S256", + } + + return redirect(f"{auth_endpoint}?{urlencode(params)}") + + except Exception as e: + logger.error(f"OIDC login error: {e}") + return jsonify({"error": "OIDC login failed"}), 500 + + @app.route("/api/auth/oidc/callback", methods=["GET"]) + def oidc_callback(): + """Handle OIDC callback from IdP.""" + try: + code = request.args.get("code") + state = request.args.get("state") + error = request.args.get("error") + + if error: + logger.warning(f"OIDC callback error from IdP: {error}") + return jsonify({"error": "Authentication failed"}), 400 + + # Validate state + expected_state = session.get("oidc_state") + code_verifier = session.get("oidc_code_verifier") + + if not expected_state or not code_verifier: + return jsonify({"error": "Session expired. Please try logging in again."}), 400 + + if not state or state != expected_state: + return jsonify({"error": "Invalid state parameter"}), 400 + + if not code: + return jsonify({"error": "Missing authorization code"}), 400 + + # Load config + config = load_config_file("security") + discovery_url = config.get("OIDC_DISCOVERY_URL", "") + client_id = config.get("OIDC_CLIENT_ID", "") + client_secret = config.get("OIDC_CLIENT_SECRET", "") + group_claim = config.get("OIDC_GROUP_CLAIM", "groups") + admin_group = config.get("OIDC_ADMIN_GROUP", "") + use_admin_group = config.get("OIDC_USE_ADMIN_GROUP", True) + auto_provision = config.get("OIDC_AUTO_PROVISION", True) + + discovery = _fetch_discovery(discovery_url) + token_endpoint = discovery["token_endpoint"] + userinfo_endpoint = discovery.get("userinfo_endpoint") + redirect_uri = request.url_root.rstrip("/") + "/api/auth/oidc/callback" + + # Exchange code for tokens + claims = _exchange_code( + token_endpoint=token_endpoint, + code=code, + code_verifier=code_verifier, + client_id=client_id, + client_secret=client_secret, + redirect_uri=redirect_uri, + userinfo_endpoint=userinfo_endpoint, + ) + + # Extract user info and check groups + user_info = extract_user_info(claims) + groups = parse_group_claims(claims, group_claim) + # Determine admin status from group membership (if enabled) + is_admin = None + if admin_group and use_admin_group: + is_admin = admin_group in groups + + # Check if user exists by OIDC subject first + existing_user = user_db.get_user(oidc_subject=user_info["oidc_subject"]) + + # If no match by subject, try email linking (for pre-created users) + # Only link when the IdP has verified the email to prevent privilege escalation + email_verified = claims.get("email_verified", False) + if not existing_user and user_info.get("email") and email_verified: + matching_users = [ + u for u in user_db.list_users() + if u.get("email") and u["email"].lower() == user_info["email"].lower() + ] + if len(matching_users) == 1: + existing_user = matching_users[0] + # Link OIDC subject to existing user + user_db.update_user(existing_user["id"], oidc_subject=user_info["oidc_subject"]) + logger.info(f"Linked OIDC subject {user_info['oidc_subject']} to existing user {existing_user['username']}") + elif len(matching_users) > 1: + logger.warning(f"OIDC email linking skipped: multiple local accounts match email {user_info['email']}") + + if not existing_user and not auto_provision: + logger.warning(f"OIDC login rejected: auto-provision disabled for {user_info['username']}") + return jsonify({"error": "Account not found. Contact your administrator."}), 403 + + # Provision or update user (database role is synced from group if enabled) + user = provision_oidc_user(user_db, user_info, is_admin=is_admin) + + # Set session - database role is the single source of truth + session["user_id"] = user["username"] + session["is_admin"] = user.get("role") == "admin" + session["db_user_id"] = user["id"] + session.permanent = True + + # Clean up OIDC session data + session.pop("oidc_state", None) + session.pop("oidc_code_verifier", None) + + logger.info(f"OIDC login successful: {user['username']} (admin={is_admin})") + + # Redirect to frontend (respect subpath deployments) + return redirect(request.script_root or "/") + + except ValueError as e: + # Specific errors from _exchange_code (e.g., missing id_token and userinfo unavailable) + logger.error(f"OIDC callback error: {e}") + return jsonify({"error": str(e)}), 400 + except Exception as e: + logger.error(f"OIDC callback error: {e}") + return jsonify({"error": "Authentication failed"}), 500 diff --git a/shelfmark/core/queue.py b/shelfmark/core/queue.py index d378dc50..c7d26db3 100644 --- a/shelfmark/core/queue.py +++ b/shelfmark/core/queue.py @@ -107,14 +107,22 @@ class BookQueue: if task_id in self._task_data: self._task_data[task_id].status_message = message - def get_status(self) -> Dict[QueueStatus, Dict[str, DownloadTask]]: - """Get current queue status grouped by status.""" + def get_status(self, user_id: Optional[int] = None) -> Dict[QueueStatus, Dict[str, DownloadTask]]: + """Get current queue status grouped by status. + + Args: + user_id: If provided, only return tasks belonging to this user + (plus legacy tasks with no user_id). If None, return all. + """ self.refresh() with self._lock: result: Dict[QueueStatus, Dict[str, DownloadTask]] = {status: {} for status in QueueStatus} for task_id, status in self._status.items(): if task_id in self._task_data: - result[status][task_id] = self._task_data[task_id] + task = self._task_data[task_id] + if user_id is not None and task.user_id is not None and task.user_id != user_id: + continue + result[status][task_id] = task return result def get_queue_order(self) -> List[Dict[str, Any]]: diff --git a/shelfmark/core/user_db.py b/shelfmark/core/user_db.py new file mode 100644 index 00000000..a318a59d --- /dev/null +++ b/shelfmark/core/user_db.py @@ -0,0 +1,189 @@ +"""SQLite user database for multi-user support.""" + +import json +import sqlite3 +import threading +from typing import Any, Dict, List, Optional + +from shelfmark.core.logger import setup_logger + +logger = setup_logger(__name__) + +_CREATE_TABLES_SQL = """ +CREATE TABLE IF NOT EXISTS users ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + username TEXT UNIQUE NOT NULL, + email TEXT, + display_name TEXT, + password_hash TEXT, + oidc_subject TEXT UNIQUE, + role TEXT NOT NULL DEFAULT 'user', + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP +); + +CREATE TABLE IF NOT EXISTS user_settings ( + user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, + settings_json TEXT NOT NULL DEFAULT '{}' +); +""" + + +class UserDB: + """Thread-safe SQLite user database.""" + + def __init__(self, db_path: str): + self._db_path = db_path + self._lock = threading.Lock() + + def _connect(self) -> sqlite3.Connection: + conn = sqlite3.connect(self._db_path) + conn.row_factory = sqlite3.Row + conn.execute("PRAGMA foreign_keys = ON") + return conn + + def initialize(self) -> None: + """Create database and tables if they don't exist.""" + with self._lock: + conn = self._connect() + try: + conn.executescript(_CREATE_TABLES_SQL) + conn.execute("PRAGMA journal_mode=WAL") + conn.commit() + finally: + conn.close() + logger.info(f"User database initialized at {self._db_path}") + + def create_user( + self, + username: str, + email: Optional[str] = None, + display_name: Optional[str] = None, + password_hash: Optional[str] = None, + oidc_subject: Optional[str] = None, + role: str = "user", + ) -> Dict[str, Any]: + """Create a new user. Raises ValueError if username or oidc_subject already exists.""" + with self._lock: + conn = self._connect() + try: + cursor = conn.execute( + """INSERT INTO users (username, email, display_name, password_hash, oidc_subject, role) + VALUES (?, ?, ?, ?, ?, ?)""", + (username, email, display_name, password_hash, oidc_subject, role), + ) + conn.commit() + user_id = cursor.lastrowid + return self._get_user_by_id(conn, user_id) + except sqlite3.IntegrityError as e: + raise ValueError(f"User already exists: {e}") + finally: + conn.close() + + def get_user( + self, + user_id: Optional[int] = None, + username: Optional[str] = None, + oidc_subject: Optional[str] = None, + ) -> Optional[Dict[str, Any]]: + """Get a user by id, username, or oidc_subject. Returns None if not found.""" + conn = self._connect() + try: + if user_id is not None: + return self._get_user_by_id(conn, user_id) + elif username is not None: + row = conn.execute( + "SELECT * FROM users WHERE username = ?", (username,) + ).fetchone() + elif oidc_subject is not None: + row = conn.execute( + "SELECT * FROM users WHERE oidc_subject = ?", (oidc_subject,) + ).fetchone() + else: + return None + return dict(row) if row else None + finally: + conn.close() + + def _get_user_by_id(self, conn: sqlite3.Connection, user_id: int) -> Optional[Dict[str, Any]]: + row = conn.execute("SELECT * FROM users WHERE id = ?", (user_id,)).fetchone() + return dict(row) if row else None + + _ALLOWED_UPDATE_COLUMNS = {"email", "display_name", "password_hash", "oidc_subject", "role"} + + def update_user(self, user_id: int, **kwargs) -> None: + """Update user fields. Raises ValueError if user not found or invalid column.""" + if not kwargs: + return + for k in kwargs: + if k not in self._ALLOWED_UPDATE_COLUMNS: + raise ValueError(f"Invalid column: {k}") + with self._lock: + conn = self._connect() + try: + # Verify user exists + if not self._get_user_by_id(conn, user_id): + raise ValueError(f"User {user_id} not found") + sets = ", ".join(f"{k} = ?" for k in kwargs) + values = list(kwargs.values()) + [user_id] + conn.execute(f"UPDATE users SET {sets} WHERE id = ?", values) + conn.commit() + finally: + conn.close() + + def delete_user(self, user_id: int) -> None: + """Delete a user and their settings.""" + with self._lock: + conn = self._connect() + try: + conn.execute("DELETE FROM users WHERE id = ?", (user_id,)) + conn.commit() + finally: + conn.close() + + def list_users(self) -> List[Dict[str, Any]]: + """List all users.""" + conn = self._connect() + try: + rows = conn.execute("SELECT * FROM users ORDER BY id").fetchall() + return [dict(r) for r in rows] + finally: + conn.close() + + def get_user_settings(self, user_id: int) -> Dict[str, Any]: + """Get per-user settings. Returns empty dict if none set.""" + conn = self._connect() + try: + row = conn.execute( + "SELECT settings_json FROM user_settings WHERE user_id = ?", (user_id,) + ).fetchone() + if row: + return json.loads(row["settings_json"]) + return {} + finally: + conn.close() + + def set_user_settings(self, user_id: int, settings: Dict[str, Any]) -> None: + """Merge settings into user's existing settings.""" + with self._lock: + conn = self._connect() + try: + existing = {} + row = conn.execute( + "SELECT settings_json FROM user_settings WHERE user_id = ?", (user_id,) + ).fetchone() + if row: + existing = json.loads(row["settings_json"]) + + existing.update(settings) + # Remove keys set to None (meaning "clear this override") + existing = {k: v for k, v in existing.items() if v is not None} + settings_json = json.dumps(existing) + + conn.execute( + """INSERT INTO user_settings (user_id, settings_json) VALUES (?, ?) + ON CONFLICT(user_id) DO UPDATE SET settings_json = ?""", + (user_id, settings_json, settings_json), + ) + conn.commit() + finally: + conn.close() diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index fe1d99ea..d1ec3d1a 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -93,9 +93,14 @@ def _normalize_email_recipients(value: Any) -> List[Dict[str, str]]: return recipients -def _resolve_email_recipient(nickname: Optional[str]) -> Tuple[Optional[str], Optional[str], Optional[str]]: +def _resolve_email_recipient( + nickname: Optional[str], + user_recipients: Optional[List[Dict[str, str]]] = None, +) -> Tuple[Optional[str], Optional[str], Optional[str]]: """Resolve a configured email recipient nickname to an email address. + Checks user-specific recipients first, then global config. + Returns: (email_to, label, error_message) """ @@ -104,6 +109,13 @@ def _resolve_email_recipient(nickname: Optional[str]) -> Tuple[Optional[str], Op if not label: return None, None, None + # Check per-user recipients first + if user_recipients: + for entry in _normalize_email_recipients(user_recipients): + if entry["nickname"].strip().lower() == label.lower(): + return entry["email"], entry["nickname"], None + + # Fall back to global recipients recipients = _normalize_email_recipients(config.get("EMAIL_RECIPIENTS", [])) for entry in recipients: if entry["nickname"].strip().lower() == label.lower(): @@ -117,6 +129,9 @@ def queue_book( priority: int = 0, source: str = "direct_download", email_recipient: Optional[str] = None, + user_id: Optional[int] = None, + username: Optional[str] = None, + user_overrides: Optional[Dict[str, Any]] = None, ) -> Tuple[bool, Optional[str]]: """Add a book to the download queue. Returns (success, error_message).""" try: @@ -133,11 +148,17 @@ def queue_book( output_mode = "folder" if is_audiobook else books_output_mode output_args: Dict[str, Any] = {} + # Extract per-user email recipients for resolution (if any) + _user_email_recipients = (user_overrides or {}).get("email_recipients") if user_overrides else None + if output_mode == "email" and not is_audiobook: - if not _normalize_email_recipients(config.get("EMAIL_RECIPIENTS", [])): + all_recipients = _user_email_recipients or config.get("EMAIL_RECIPIENTS", []) + if not _normalize_email_recipients(all_recipients): return False, "No email recipients configured" - email_to, email_label, email_error = _resolve_email_recipient(email_recipient) + email_to, email_label, email_error = _resolve_email_recipient( + email_recipient, user_recipients=_user_email_recipients, + ) if email_error: return False, email_error if not email_to: @@ -145,6 +166,13 @@ def queue_book( output_args = {"to": email_to, "label": email_label} + # Merge per-user overrides into output_args (only known keys) + _ALLOWED_OVERRIDE_KEYS = {"destination", "booklore_library_id", "booklore_path_id"} + if user_overrides: + for k, v in user_overrides.items(): + if k in _ALLOWED_OVERRIDE_KEYS and v is not None and k not in output_args: + output_args[k] = v + # Create a source-agnostic download task task = DownloadTask( task_id=book_id, @@ -159,6 +187,8 @@ def queue_book( output_mode=output_mode, output_args=output_args, priority=priority, + user_id=user_id, + username=username, ) if not book_queue.add(task): @@ -186,6 +216,9 @@ def queue_release( release_data: dict, priority: int = 0, email_recipient: Optional[str] = None, + user_id: Optional[int] = None, + username: Optional[str] = None, + user_overrides: Optional[Dict[str, Any]] = None, ) -> Tuple[bool, Optional[str]]: """Add a release to the download queue. Returns (success, error_message).""" try: @@ -209,11 +242,17 @@ def queue_release( output_mode = "folder" if is_audiobook else books_output_mode output_args: Dict[str, Any] = {} + # Extract per-user email recipients for resolution (if any) + _user_email_recipients = (user_overrides or {}).get("email_recipients") if user_overrides else None + if output_mode == "email" and not is_audiobook: - if not _normalize_email_recipients(config.get("EMAIL_RECIPIENTS", [])): + all_recipients = _user_email_recipients or config.get("EMAIL_RECIPIENTS", []) + if not _normalize_email_recipients(all_recipients): return False, "No email recipients configured" - email_to, email_label, email_error = _resolve_email_recipient(email_recipient) + email_to, email_label, email_error = _resolve_email_recipient( + email_recipient, user_recipients=_user_email_recipients, + ) if email_error: return False, email_error if not email_to: @@ -221,6 +260,13 @@ def queue_release( output_args = {"to": email_to, "label": email_label} + # Merge per-user overrides into output_args (only known keys) + _ALLOWED_OVERRIDE_KEYS = {"destination", "booklore_library_id", "booklore_path_id"} + if user_overrides: + for k, v in user_overrides.items(): + if k in _ALLOWED_OVERRIDE_KEYS and v is not None and k not in output_args: + output_args[k] = v + # Create a source-agnostic download task from release data task = DownloadTask( task_id=release_data['source_id'], @@ -239,6 +285,8 @@ def queue_release( output_mode=output_mode, output_args=output_args, priority=priority, + user_id=user_id, + username=username, ) if not book_queue.add(task): @@ -267,9 +315,9 @@ def queue_release( logger.error_trace(error_msg) return False, error_msg -def queue_status() -> Dict[str, Dict[str, Any]]: +def queue_status(user_id: Optional[int] = None) -> Dict[str, Dict[str, Any]]: """Get current status of the download queue.""" - status = book_queue.get_status() + status = book_queue.get_status(user_id=user_id) for _, tasks in status.items(): for _, task in tasks.items(): if task.download_path and not run_blocking_io(os.path.exists, task.download_path): @@ -339,6 +387,7 @@ def _task_to_dict(task: DownloadTask) -> Dict[str, Any]: 'status': task.status, 'status_message': task.status_message, 'download_path': task.download_path, + 'username': task.username, } @@ -472,7 +521,9 @@ def update_download_progress(book_id: str, progress: float) -> None: _progress_last_broadcast[f"{book_id}_progress"] = progress if should_broadcast: - ws_manager.broadcast_download_progress(book_id, progress, 'downloading') + task = book_queue.get_task(book_id) + task_user_id = task.user_id if task else None + ws_manager.broadcast_download_progress(book_id, progress, 'downloading', user_id=task_user_id) def update_download_status(book_id: str, status: str, message: Optional[str] = None) -> None: """Update download status with optional message for UI display.""" diff --git a/shelfmark/download/outputs/booklore.py b/shelfmark/download/outputs/booklore.py index 4a639722..bb65fb2b 100644 --- a/shelfmark/download/outputs/booklore.py +++ b/shelfmark/download/outputs/booklore.py @@ -48,7 +48,10 @@ def _parse_int(value: Any, label: str) -> int: raise BookloreError(f"{label} must be a number") from exc -def build_booklore_config(values: Mapping[str, Any]) -> BookloreConfig: +def build_booklore_config( + values: Mapping[str, Any], + user_overrides: Optional[Dict[str, Any]] = None, +) -> BookloreConfig: base_url = str(values.get("BOOKLORE_HOST", "")).strip() username = str(values.get("BOOKLORE_USERNAME", "")).strip() password = values.get("BOOKLORE_PASSWORD", "") or "" @@ -60,8 +63,15 @@ def build_booklore_config(values: Mapping[str, Any]) -> BookloreConfig: if not password: raise BookloreError("Booklore password is required") - library_id = _parse_int(values.get("BOOKLORE_LIBRARY_ID"), "Booklore library ID") - path_id = _parse_int(values.get("BOOKLORE_PATH_ID"), "Booklore path ID") + # Per-user library/path overrides (auth stays global) + overrides = user_overrides or {} + _lib_override = overrides.get("booklore_library_id") + library_id_val = _lib_override if _lib_override is not None else values.get("BOOKLORE_LIBRARY_ID") + _path_override = overrides.get("booklore_path_id") + path_id_val = _path_override if _path_override is not None else values.get("BOOKLORE_PATH_ID") + + library_id = _parse_int(library_id_val, "Booklore library ID") + path_id = _parse_int(path_id_val, "Booklore path ID") return BookloreConfig( base_url=base_url.rstrip("/"), @@ -209,7 +219,10 @@ def _post_process_booklore( return None try: - booklore_config = build_booklore_config(_get_booklore_settings()) + booklore_config = build_booklore_config( + _get_booklore_settings(), + user_overrides=task.output_args if task.output_args else None, + ) except BookloreError as e: logger.warning("Task %s: Booklore configuration error: %s", task.task_id, e) status_callback("error", str(e)) diff --git a/shelfmark/download/postprocess/destination.py b/shelfmark/download/postprocess/destination.py index deae8b95..85b139e0 100644 --- a/shelfmark/download/postprocess/destination.py +++ b/shelfmark/download/postprocess/destination.py @@ -59,7 +59,12 @@ def validate_destination(destination: Path, status_callback) -> bool: def get_final_destination(task: DownloadTask) -> Path: - """Get final destination directory, with content-type routing support.""" + """Get final destination directory, with content-type routing and per-user override support.""" + + # Per-user destination override (set by admin in user settings) + user_dest = task.output_args.get("destination", "") + if user_dest: + return Path(user_dest) is_audiobook = check_audiobook(task.content_type) diff --git a/shelfmark/download/postprocess/transfer.py b/shelfmark/download/postprocess/transfer.py index 4db11869..571c692f 100644 --- a/shelfmark/download/postprocess/transfer.py +++ b/shelfmark/download/postprocess/transfer.py @@ -53,6 +53,7 @@ def build_metadata_dict(task: DownloadTask) -> dict: "Year": task.year, "Series": task.series_name, "SeriesPosition": task.series_position, + "User": task.username, } diff --git a/shelfmark/main.py b/shelfmark/main.py index e1c07929..e6ad8692 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -72,6 +72,7 @@ socketio = SocketIO( # Initialize WebSocket manager ws_manager.init_app(app, socketio) +ws_manager.set_queue_status_fn(backend.queue_status) logger.info(f"Flask-SocketIO initialized with async_mode='{async_mode}'") # Ensure all plugins are loaded before starting the download coordinator. @@ -88,6 +89,28 @@ except ImportError as e: from shelfmark.config.security import _migrate_security_settings _migrate_security_settings() +# Initialize user database and register multi-user routes +# If CONFIG_DIR doesn't exist or is read-only, multi-user features will be disabled +import os as _os +from shelfmark.core.user_db import UserDB +_user_db_path = _os.path.join(_os.environ.get("CONFIG_DIR", "/config"), "users.db") +user_db: UserDB | None = None +try: + user_db = UserDB(_user_db_path) + user_db.initialize() + import shelfmark.config.users_settings as _ # noqa: F401 - registers users tab + from shelfmark.core.oidc_routes import register_oidc_routes + from shelfmark.core.admin_routes import register_admin_routes + register_oidc_routes(app, user_db) + register_admin_routes(app, user_db) +except (sqlite3.OperationalError, OSError) as e: + logger.warning( + f"User database initialization failed: {e}. " + f"Multi-user authentication features will be disabled. " + f"Ensure CONFIG_DIR ({_os.environ.get('CONFIG_DIR', '/config')}) exists and is writable." + ) + user_db = None + # Start download coordinator backend.start() @@ -174,6 +197,8 @@ def get_auth_mode() -> str: return "builtin" if auth_mode == "proxy" and security_config.get("PROXY_AUTH_USER_HEADER"): return "proxy" + if auth_mode == "oidc" and security_config.get("OIDC_DISCOVERY_URL") and security_config.get("OIDC_CLIENT_ID"): + return "oidc" except Exception: pass @@ -346,17 +371,22 @@ def login_required(f): if 'user_id' not in session: return jsonify({"error": "Unauthorized"}), 401 - # Check admin access for settings endpoints (proxy and CWA modes) - if auth_mode in ("proxy", "cwa") and (request.path.startswith('/api/settings') or request.path.startswith('/api/onboarding')): + # Check admin access for settings endpoints (proxy, CWA, OIDC, and builtin modes) + if auth_mode in ("proxy", "cwa", "oidc", "builtin") and (request.path.startswith('/api/settings') or request.path.startswith('/api/onboarding')): from shelfmark.core.settings_registry import load_config_file try: security_config = load_config_file("security") - if auth_mode == "proxy": + if auth_mode == "builtin": + # Builtin multi-user: settings are always admin-only + restrict_to_admin = 'db_user_id' in session + elif auth_mode == "proxy": restrict_to_admin = security_config.get("PROXY_AUTH_RESTRICT_SETTINGS_TO_ADMIN", False) else: - restrict_to_admin = security_config.get("CWA_RESTRICT_SETTINGS_TO_ADMIN", False) + # For OIDC and CWA, settings are always admin-only + # The user's admin status comes from their group or database role + restrict_to_admin = True if restrict_to_admin and not session.get('is_admin', False): return jsonify({"error": "Admin access required"}), 403 @@ -563,7 +593,14 @@ def api_download() -> Union[Response, Tuple[Response, int]]: try: priority = int(request.args.get('priority', 0)) email_recipient = request.args.get('email_recipient') - success, error_msg = backend.queue_book(book_id, priority, email_recipient=email_recipient) + # Per-user download overrides + db_user_id = session.get('db_user_id') + _username = session.get('user_id') + _user_overrides = user_db.get_user_settings(db_user_id) if (user_db and db_user_id) else {} + success, error_msg = backend.queue_book( + book_id, priority, email_recipient=email_recipient, + user_id=db_user_id, username=_username, user_overrides=_user_overrides, + ) if success: return jsonify({"status": "queued", "priority": priority}) return jsonify({"error": error_msg or "Failed to queue book"}), 500 @@ -602,7 +639,14 @@ def api_download_release() -> Union[Response, Tuple[Response, int]]: priority = data.get('priority', 0) email_recipient = data.get('email_recipient') - success, error_msg = backend.queue_release(data, priority, email_recipient=email_recipient) + # Per-user download overrides + db_user_id = session.get('db_user_id') + _username = session.get('user_id') + _user_overrides = user_db.get_user_settings(db_user_id) if (user_db and db_user_id) else {} + success, error_msg = backend.queue_release( + data, priority, email_recipient=email_recipient, + user_id=db_user_id, username=_username, user_overrides=_user_overrides, + ) if success: return jsonify({"status": "queued", "priority": priority}) @@ -704,7 +748,11 @@ def api_status() -> Union[Response, Tuple[Response, int]]: flask.Response: JSON object with queue status. """ try: - status = backend.queue_status() + # Non-admin users only see their own downloads + user_id = None + if not session.get('is_admin', True): + user_id = session.get('db_user_id') + status = backend.queue_status(user_id=user_id) return jsonify(status) except Exception as e: logger.error_trace(f"Status error: {e}") @@ -1054,22 +1102,47 @@ def api_login() -> Union[Response, Tuple[Response, int]]: logger.info(f"Login successful for user '{username}' from IP {ip_address} (no auth configured)") return jsonify({"success": True}) - # Built-in authentication mode - if auth_mode == "builtin": + # Password authentication (builtin and OIDC modes) + # OIDC mode also allows password login as a fallback so admins don't get locked out + if auth_mode in ("builtin", "oidc"): + if user_db is None: + logger.error(f"User database not available for {auth_mode} auth") + return jsonify({"error": "Authentication service unavailable"}), 503 try: - security_config = load_config_file("security") - stored_username = security_config.get("BUILTIN_USERNAME", "") - stored_hash = security_config.get("BUILTIN_PASSWORD_HASH", "") + db_user = user_db.get_user(username=username) - # Check credentials - if username == stored_username and check_password_hash(stored_hash, password): + # If user not in DB, try legacy config credentials and auto-migrate + if not db_user: + security_config = load_config_file("security") + stored_username = security_config.get("BUILTIN_USERNAME", "") + stored_hash = security_config.get("BUILTIN_PASSWORD_HASH", "") + + if username == stored_username and stored_hash and check_password_hash(stored_hash, password): + # Auto-migrate: create admin user in DB from config + db_user = user_db.create_user( + username=stored_username, + password_hash=stored_hash, + role="admin", + ) + logger.info(f"Migrated builtin admin '{stored_username}' to users database") + else: + return _failed_login_response(username, ip_address) + + # Authenticate against DB user + if db_user: + if not db_user.get("password_hash") or not check_password_hash(db_user["password_hash"], password): + return _failed_login_response(username, ip_address) + + is_admin = db_user["role"] == "admin" session['user_id'] = username + session['db_user_id'] = db_user["id"] + session['is_admin'] = is_admin session.permanent = remember_me clear_failed_logins(username) - logger.info(f"Login successful for user '{username}' from IP {ip_address} (builtin auth, remember_me={remember_me})") + logger.info(f"Login successful for user '{username}' from IP {ip_address} ({auth_mode} auth, is_admin={is_admin}, remember_me={remember_me})") return jsonify({"success": True}) - else: - return _failed_login_response(username, ip_address) + + return _failed_login_response(username, ip_address) except Exception as e: logger.error_trace(f"Built-in auth error: {e}") @@ -1176,11 +1249,11 @@ def api_auth_check() -> Union[Response, Tuple[Response, int]]: is_authenticated = 'user_id' in session # Determine admin status for settings access - # - Built-in auth: single user is always admin + # - Built-in auth: check DB user role (legacy single-user is always admin) # - CWA auth: check RESTRICT_SETTINGS_TO_ADMIN setting # - Proxy auth: check PROXY_AUTH_RESTRICT_SETTINGS_TO_ADMIN setting if auth_mode == "builtin": - is_admin = True + is_admin = session.get('is_admin', True) elif auth_mode == "cwa": restrict_to_admin = security_config.get("CWA_RESTRICT_SETTINGS_TO_ADMIN", False) if restrict_to_admin: @@ -1191,6 +1264,10 @@ def api_auth_check() -> Union[Response, Tuple[Response, int]]: elif auth_mode == "proxy": restrict_to_admin = security_config.get("PROXY_AUTH_RESTRICT_SETTINGS_TO_ADMIN", False) is_admin = session.get('is_admin', not restrict_to_admin) + elif auth_mode == "oidc": + # OIDC admin status is determined by group membership during login + # and stored in session['is_admin'] - use it directly + is_admin = session.get('is_admin', False) else: is_admin = False @@ -1846,22 +1923,35 @@ def catch_all(path: str) -> Response: def handle_connect(): """Handle client connection.""" logger.info("WebSocket client connected") - + # Track the connection (triggers warmup callbacks on first connect) ws_manager.client_connected() - - # Send initial status to the newly connected client + + # Join appropriate room based on user session + is_admin = session.get('is_admin', True) + db_user_id = session.get('db_user_id') + ws_manager.join_user_room(request.sid, is_admin, db_user_id) + + # Send initial status to the newly connected client (filtered) try: - status = backend.queue_status() + user_id = None + if not is_admin: + user_id = db_user_id + status = backend.queue_status(user_id=user_id) emit('status_update', status) except Exception as e: logger.error(f"Error sending initial status: {e}") @socketio.on('disconnect') def handle_disconnect(): - """Handle client disconnection.""" + """Handle client disconnection.""" logger.info("WebSocket client disconnected") - + + # Leave room + is_admin = session.get('is_admin', True) + db_user_id = session.get('db_user_id') + ws_manager.leave_user_room(request.sid, is_admin, db_user_id) + # Track the disconnection ws_manager.client_disconnected() @@ -1869,7 +1959,10 @@ def handle_disconnect(): def handle_status_request(): """Handle manual status request from client.""" try: - status = backend.queue_status() + user_id = None + if not session.get('is_admin', True): + user_id = session.get('db_user_id') + status = backend.queue_status(user_id=user_id) emit('status_update', status) except Exception as e: logger.error(f"Error handling status request: {e}") diff --git a/src/frontend/package-lock.json b/src/frontend/package-lock.json index 2841f89c..ec7ccc51 100644 --- a/src/frontend/package-lock.json +++ b/src/frontend/package-lock.json @@ -69,7 +69,6 @@ "integrity": "sha512-e7jT4DxYvIDLk1ZHmU/m/mB19rex9sv0c2ftBtjSBv+kVM/902eh0fINUzD7UwLLNR+jU585GxUJ8/EBfAM5fw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.27.1", "@babel/generator": "^7.28.5", @@ -1217,7 +1216,6 @@ "integrity": "sha512-qzQZRBqkFsYyaSWXuEHc2WR9c0a0CXwiE5FWUvn7ZM+vdy1uZLfCunD38UzhuB7YN/J11ndbDBcTmOdxJo9Q7A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -1235,7 +1233,6 @@ "integrity": "sha512-RFA/bURkcKzx/X9oumPG9Vp3D3JUgus/d0b67KB0t5S/raciymilkOa66olh78MUI92QLbEJevO7rvqU/kjwKA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/prop-types": "*", "csstype": "^3.0.2" @@ -1437,7 +1434,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.19", "caniuse-lite": "^1.0.30001751", @@ -2001,7 +1997,6 @@ "integrity": "sha512-/imKNG4EbWNrVjoNC/1H5/9GFy+tqjGBHCaSsN+P2RnPqjsLmv6UD3Ej+Kj8nBWaRAwyk7kK5ZUc+OEatnTR3A==", "dev": true, "license": "MIT", - "peer": true, "bin": { "jiti": "bin/jiti.js" } @@ -2322,7 +2317,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -2492,7 +2486,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-18.3.1.tgz", "integrity": "sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==", "license": "MIT", - "peer": true, "dependencies": { "loose-envify": "^1.1.0" }, @@ -2505,7 +2498,6 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.3.1.tgz", "integrity": "sha512-5m4nQKp+rZRb09LNH59GM4BxTh9251/ylbKIbpe7TpGxfJ+9kv6BLkLBXIjjspbgbnIBNqlI23tRnTWT0snUIw==", "license": "MIT", - "peer": true, "dependencies": { "loose-envify": "^1.1.0", "scheduler": "^0.23.2" @@ -3090,7 +3082,6 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index 90d3cff1..32eba11b 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -79,6 +79,7 @@ function App() { authRequired, authChecked, isAdmin, + authMode, loginError, isLoggingIn, setIsAuthenticated, @@ -854,6 +855,7 @@ function App() { onLogin={handleLogin} error={loginError} isLoading={isLoggingIn} + authMode={authMode} /> ) } diff --git a/src/frontend/src/components/DownloadsSidebar.tsx b/src/frontend/src/components/DownloadsSidebar.tsx index f87abab8..8b1a8289 100644 --- a/src/frontend/src/components/DownloadsSidebar.tsx +++ b/src/frontend/src/components/DownloadsSidebar.tsx @@ -241,6 +241,12 @@ export const DownloadsSidebar = ({ {book.source_display_name} )} + {book.username && ( + <> + • + {book.username} + + )} {/* Status Badge */} diff --git a/src/frontend/src/components/LoginForm.tsx b/src/frontend/src/components/LoginForm.tsx index 0cc57f0e..6b3761ec 100644 --- a/src/frontend/src/components/LoginForm.tsx +++ b/src/frontend/src/components/LoginForm.tsx @@ -7,6 +7,7 @@ interface LoginFormProps { error?: string | null; isLoading?: boolean; autoFocus?: boolean; + authMode?: string; } const EyeIcon = () => ( @@ -53,6 +54,7 @@ export const LoginForm = ({ error = null, isLoading = false, autoFocus = true, + authMode, }: LoginFormProps) => { const [username, setUsername] = useState(''); const [password, setPassword] = useState(''); @@ -225,6 +227,26 @@ export const LoginForm = ({ )} + + {authMode === 'oidc' && ( + <> +
+
+ or +
+
+ + Sign in with OIDC + + + )}
); }; diff --git a/src/frontend/src/components/settings/SettingsModal.tsx b/src/frontend/src/components/settings/SettingsModal.tsx index fd985bbf..070e8e92 100644 --- a/src/frontend/src/components/settings/SettingsModal.tsx +++ b/src/frontend/src/components/settings/SettingsModal.tsx @@ -4,6 +4,7 @@ import { useSearchMode } from '../../contexts/SearchModeContext'; import { SettingsHeader } from './SettingsHeader'; import { SettingsSidebar } from './SettingsSidebar'; import { SettingsContent } from './SettingsContent'; +import { UsersPanel } from './UsersPanel'; interface SettingsModalProps { isOpen: boolean; @@ -149,11 +150,37 @@ export const SettingsModal = ({ isOpen, onClose, onShowToast, onSettingsSaved }: // Memoize the field change handler to prevent creating new functions on every render const handleFieldChange = useCallback( (key: string, value: unknown) => { - if (selectedTab) { - updateValue(selectedTab, key, value); + if (!selectedTab) return; + updateValue(selectedTab, key, value); + + // Auto-manage OIDC scopes when admin group settings change + if (selectedTab === 'security') { + const tabValues = values[selectedTab] || {}; + const currentScopes = (tabValues['OIDC_SCOPES'] as string[]) || []; + + if (key === 'OIDC_USE_ADMIN_GROUP') { + const groupClaim = (tabValues['OIDC_GROUP_CLAIM'] as string) || 'groups'; + if (value === true && !currentScopes.includes(groupClaim)) { + updateValue(selectedTab, 'OIDC_SCOPES', [...currentScopes, groupClaim]); + } else if (value === false && currentScopes.includes(groupClaim)) { + updateValue(selectedTab, 'OIDC_SCOPES', currentScopes.filter(s => s !== groupClaim)); + } + } + + if (key === 'OIDC_GROUP_CLAIM' && typeof value === 'string') { + const useAdminGroup = tabValues['OIDC_USE_ADMIN_GROUP'] as boolean; + if (useAdminGroup) { + const oldClaim = (tabValues['OIDC_GROUP_CLAIM'] as string) || 'groups'; + const newScopes = currentScopes.filter(s => s !== oldClaim); + if (value && !newScopes.includes(value)) { + newScopes.push(value); + } + updateValue(selectedTab, 'OIDC_SCOPES', newScopes); + } + } } }, - [selectedTab, updateValue] + [selectedTab, updateValue, values] ); // Memoize hasChanges to avoid expensive JSON.stringify comparisons on every render @@ -280,16 +307,20 @@ export const SettingsModal = ({ isOpen, onClose, onShowToast, onSettingsSaved }: onClose={handleClose} /> {currentTab && ( - + selectedTab === 'users' ? ( + + ) : ( + + ) )} )} @@ -331,16 +362,20 @@ export const SettingsModal = ({ isOpen, onClose, onShowToast, onSettingsSaved }: /> {currentTab ? ( - + selectedTab === 'users' ? ( + + ) : ( + + ) ) : (
Select a category to configure diff --git a/src/frontend/src/components/settings/SettingsSidebar.tsx b/src/frontend/src/components/settings/SettingsSidebar.tsx index 38a24606..0c49b568 100644 --- a/src/frontend/src/components/settings/SettingsSidebar.tsx +++ b/src/frontend/src/components/settings/SettingsSidebar.tsx @@ -62,6 +62,12 @@ const getIcon = (iconName?: string) => { ); + case 'users': + return ( + + + + ); case 'beaker': case 'wrench': return ( diff --git a/src/frontend/src/components/settings/UsersPanel.tsx b/src/frontend/src/components/settings/UsersPanel.tsx new file mode 100644 index 00000000..086aa689 --- /dev/null +++ b/src/frontend/src/components/settings/UsersPanel.tsx @@ -0,0 +1,743 @@ +import { useCallback, useEffect, useState } from 'react'; +import { + AdminUser, + BookloreOption, + DownloadDefaults, + getAdminUsers, + getAdminUser, + getBookloreOptions, + getDownloadDefaults, + createAdminUser, + updateAdminUser, + deleteAdminUser, +} from '../../services/api'; + +interface UsersPanelProps { + onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void; +} + +const inputClasses = + 'w-full px-3 py-2 rounded-lg border border-[var(--border-muted)] bg-[var(--bg-soft)] text-sm focus:outline-none focus:ring-2 focus:ring-sky-500/50 focus:border-sky-500 transition-colors'; + +const disabledInputClasses = + 'w-full px-3 py-2 rounded-lg border border-[var(--border-muted)] bg-[var(--bg-soft)] text-sm opacity-50 cursor-not-allowed'; + +interface PerUserSettings { + destination?: string; + booklore_library_id?: string; + booklore_path_id?: string; + email_recipients?: Array<{ nickname: string; email: string }>; +} + +export const UsersPanel = ({ onShowToast }: UsersPanelProps) => { + const [users, setUsers] = useState([]); + const [loading, setLoading] = useState(true); + const [loadError, setLoadError] = useState(null); + const [editingUser, setEditingUser] = useState(null); + const [confirmDelete, setConfirmDelete] = useState(null); + const [showCreateForm, setShowCreateForm] = useState(false); + const [createForm, setCreateForm] = useState({ username: '', email: '', password: '', display_name: '', role: 'user' }); + const [creating, setCreating] = useState(false); + + // Edit view state + const [editPassword, setEditPassword] = useState(''); + const [editPasswordConfirm, setEditPasswordConfirm] = useState(''); + const [downloadDefaults, setDownloadDefaults] = useState(null); + const [userSettings, setUserSettings] = useState({}); + const [overrides, setOverrides] = useState>({}); + const [bookloreLibraries, setBookloreLibraries] = useState([]); + const [booklorePaths, setBooklorePaths] = useState([]); + + const fetchUsers = useCallback(async () => { + try { + setLoading(true); + setLoadError(null); + const data = await getAdminUsers(); + setUsers(data); + } catch (err) { + const msg = err instanceof Error ? err.message : 'Failed to load users'; + setLoadError(msg); + onShowToast?.(msg, 'error'); + } finally { + setLoading(false); + } + }, [onShowToast]); + + useEffect(() => { + fetchUsers(); + }, [fetchUsers]); + + const startEditing = useCallback(async (user: AdminUser) => { + setEditingUser({ ...user }); + setEditPassword(''); + setEditPasswordConfirm(''); + + // Fetch full user data (with settings) and download defaults in parallel + try { + const [fullUser, defaults] = await Promise.all([ + getAdminUser(user.id), + getDownloadDefaults(), + ]); + setDownloadDefaults(defaults); + const settings = (fullUser.settings || {}) as PerUserSettings; + setUserSettings(settings); + + // Fetch BookLore options if in booklore mode + if (defaults.BOOKS_OUTPUT_MODE === 'booklore') { + try { + const blOptions = await getBookloreOptions(); + setBookloreLibraries(blOptions.libraries || []); + setBooklorePaths(blOptions.paths || []); + } catch { + setBookloreLibraries([]); + setBooklorePaths([]); + } + } + + // Set override toggles based on which settings exist + setOverrides({ + destination: !!settings.destination, + booklore_library_id: !!settings.booklore_library_id, + booklore_path_id: !!settings.booklore_path_id, + email_recipients: !!settings.email_recipients?.length, + }); + } catch { + setDownloadDefaults(null); + setUserSettings({}); + setOverrides({}); + } + }, []); + + const handleDelete = async (userId: number) => { + try { + await deleteAdminUser(userId); + setConfirmDelete(null); + onShowToast?.('User deleted', 'success'); + fetchUsers(); + } catch { + onShowToast?.('Failed to delete user', 'error'); + } + }; + + const handleSaveEdit = async () => { + if (!editingUser) return; + + // Validate password if provided + if (editPassword) { + if (editPassword.length < 4) { + onShowToast?.('Password must be at least 4 characters', 'error'); + return; + } + if (editPassword !== editPasswordConfirm) { + onShowToast?.('Passwords do not match', 'error'); + return; + } + } + + // Build settings payload: include overridden values, null out cleared overrides + const settingsPayload: Record = {}; + if (overrides.destination) { + settingsPayload.destination = userSettings.destination || ''; + } else { + settingsPayload.destination = null; + } + if (overrides.booklore_library_id) { + settingsPayload.booklore_library_id = userSettings.booklore_library_id || ''; + } else { + settingsPayload.booklore_library_id = null; + } + if (overrides.booklore_path_id) { + settingsPayload.booklore_path_id = userSettings.booklore_path_id || ''; + } else { + settingsPayload.booklore_path_id = null; + } + if (overrides.email_recipients) { + settingsPayload.email_recipients = userSettings.email_recipients || []; + } else { + settingsPayload.email_recipients = null; + } + + // Skip sending role when it's managed by OIDC group auth + const roleManaged = !!editingUser.oidc_subject && downloadDefaults?.OIDC_USE_ADMIN_GROUP === true; + + try { + await updateAdminUser(editingUser.id, { + email: editingUser.email, + display_name: editingUser.display_name, + ...(!roleManaged ? { role: editingUser.role } : {}), + ...(editPassword ? { password: editPassword } : {}), + ...(Object.keys(settingsPayload).length ? { settings: settingsPayload } : {}), + }); + setEditingUser(null); + onShowToast?.('User updated', 'success'); + fetchUsers(); + } catch { + onShowToast?.('Failed to update user', 'error'); + } + }; + + const handleCreate = async () => { + if (!createForm.username || !createForm.password) { + onShowToast?.('Username and password are required', 'error'); + return; + } + if (createForm.password.length < 4) { + onShowToast?.('Password must be at least 4 characters', 'error'); + return; + } + setCreating(true); + try { + const data = await createAdminUser(createForm as { username: string; password: string; email?: string; display_name?: string; role?: string }); + setShowCreateForm(false); + setCreateForm({ username: '', email: '', password: '', display_name: '', role: 'user' }); + onShowToast?.(`User ${data.username} created`, 'success'); + fetchUsers(); + } catch (err) { + onShowToast?.((err as Error).message || 'Failed to create user', 'error'); + } finally { + setCreating(false); + } + }; + + const toggleOverride = (key: string, enabled: boolean) => { + setOverrides((prev) => ({ ...prev, [key]: enabled })); + if (!enabled) { + setUserSettings((prev) => { + const next = { ...prev }; + (next as Record)[key] = null; + return next; + }); + } + }; + + if (loading) { + return ( +
+ Loading users... +
+ ); + } + + if (loadError) { + return ( +
+

{loadError}

+ +
+ ); + } + + // Edit view + if (editingUser) { + const outputMode = downloadDefaults?.BOOKS_OUTPUT_MODE || 'folder'; + + return ( +
+
+ +

Edit {editingUser.username}

+
+ +
+ {editingUser.oidc_subject && ( +
+
+ + + + This user authenticates via SSO. Password is managed by the identity provider. +
+ {downloadDefaults?.OIDC_USE_ADMIN_GROUP === true && ( +
+ + + + {downloadDefaults?.OIDC_ADMIN_GROUP + ? `Admin role is managed by the ${downloadDefaults.OIDC_ADMIN_GROUP} group in your identity provider.` + : 'Admin group authorization is enabled but no group name is configured.'} +
+ )} +
+ )} + +
+ + setEditingUser({ ...editingUser, display_name: e.target.value || null })} + className={inputClasses} + placeholder="Display name" + /> +
+ +
+ + setEditingUser({ ...editingUser, email: e.target.value || null })} + className={inputClasses} + placeholder="user@example.com" + /> +
+ + {/* Hide role dropdown for OIDC users when admin group auth is on (like password) */} + {!(!!editingUser.oidc_subject && downloadDefaults?.OIDC_USE_ADMIN_GROUP === true) && ( +
+ + +
+ )} + + {/* Password section */} + {!editingUser.oidc_subject && ( + <> +
+

Change Password

+
+
+ + setEditPassword(e.target.value)} + className={inputClasses} + placeholder="Leave empty to keep current" + /> +
+ {editPassword && ( +
+ + setEditPasswordConfirm(e.target.value)} + className={inputClasses} + placeholder="Confirm new password" + /> +
+ )} + + )} + + {/* Per-user download settings overrides */} + {downloadDefaults && ( + <> +
+

Download Settings Overrides

+

Override global defaults for this user.

+
+ + {/* Destination override (shown for folder mode) */} + {(outputMode === 'folder' || outputMode === 'booklore') && ( + toggleOverride('destination', v)} + globalValue={downloadDefaults.DESTINATION || '/books'} + > + setUserSettings((s) => ({ ...s, destination: e.target.value }))} + className={overrides.destination ? inputClasses : disabledInputClasses} + disabled={!overrides.destination} + placeholder={downloadDefaults.DESTINATION || '/books'} + /> + + )} + + {/* BookLore overrides */} + {outputMode === 'booklore' && ( + <> + toggleOverride('booklore_library_id', v)} + globalValue={ + bookloreLibraries.find((l) => l.value === downloadDefaults.BOOKLORE_LIBRARY_ID)?.label + || downloadDefaults.BOOKLORE_LIBRARY_ID + || 'Not set' + } + > + + + toggleOverride('booklore_path_id', v)} + globalValue={ + booklorePaths.find((p) => p.value === downloadDefaults.BOOKLORE_PATH_ID)?.label + || downloadDefaults.BOOKLORE_PATH_ID + || 'Not set' + } + > + + + + )} + + {/* Email recipients override */} + {outputMode === 'email' && ( + toggleOverride('email_recipients', v)} + globalValue={ + downloadDefaults.EMAIL_RECIPIENTS?.length + ? downloadDefaults.EMAIL_RECIPIENTS.map((r) => r.nickname || r.email).join(', ') + : 'None configured' + } + > + {overrides.email_recipients && ( + setUserSettings((s) => ({ ...s, email_recipients: r }))} + /> + )} + + )} + + )} + +
+ + +
+
+
+ ); + } + + // List view + return ( +
+
+

+ Users are created automatically via OIDC login, or manually below. +

+ +
+ + {showCreateForm && ( +
+ {users.length === 0 && ( +

+ This will be the first account and will be created as admin. +

+ )} +
+
+ + setCreateForm({ ...createForm, username: e.target.value })} + className={inputClasses} + placeholder="username" + /> +
+
+ + setCreateForm({ ...createForm, display_name: e.target.value })} + className={inputClasses} + placeholder="Display Name" + /> +
+
+ + setCreateForm({ ...createForm, email: e.target.value })} + className={inputClasses} + placeholder="user@example.com" + /> +
+
+ + setCreateForm({ ...createForm, password: e.target.value })} + className={inputClasses} + placeholder="Min 4 characters" + /> +
+
+
+ + +
+
+ )} + + {users.length === 0 ? ( +
+

No users yet.

+

+ Create a local admin account before enabling OIDC to avoid getting locked out. +

+
+ ) : ( +
+ {users.map((user) => ( +
+
+
+ {user.username.charAt(0).toUpperCase()} +
+
+
+ + {user.display_name || user.username} + + {user.display_name && ( + @{user.username} + )} + + {user.oidc_subject ? 'OIDC' : 'Password'} + +
+
+ {user.email || 'No email'} +
+
+
+ +
+ + {user.role} + + + + + {confirmDelete === user.id ? ( +
+ + +
+ ) : ( + + )} +
+
+ ))} +
+ )} +
+ ); +}; + +// --------------------------------------------------------------------------- +// Sub-components +// --------------------------------------------------------------------------- + +interface OverrideFieldProps { + label: string; + enabled: boolean; + onToggle: (enabled: boolean) => void; + globalValue: string; + children: React.ReactNode; +} + +const OverrideField = ({ label, enabled, onToggle, globalValue, children }: OverrideFieldProps) => ( +
+
+ + +
+ {!enabled && ( +

Using global: {globalValue}

+ )} + {children} +
+); + +interface EmailRecipientsEditorProps { + recipients: Array<{ nickname: string; email: string }>; + onChange: (recipients: Array<{ nickname: string; email: string }>) => void; +} + +const EmailRecipientsEditor = ({ recipients, onChange }: EmailRecipientsEditorProps) => { + const addRecipient = () => { + onChange([...recipients, { nickname: '', email: '' }]); + }; + + const removeRecipient = (index: number) => { + onChange(recipients.filter((_, i) => i !== index)); + }; + + const updateRecipient = (index: number, field: 'nickname' | 'email', value: string) => { + const updated = [...recipients]; + updated[index] = { ...updated[index], [field]: value }; + onChange(updated); + }; + + return ( +
+ {recipients.map((r, i) => ( +
+ updateRecipient(i, 'nickname', e.target.value)} + className={inputClasses} + placeholder="Nickname" + /> + updateRecipient(i, 'email', e.target.value)} + className={inputClasses} + placeholder="email@example.com" + /> + +
+ ))} + +
+ ); +}; diff --git a/src/frontend/src/components/settings/fields/TagListField.tsx b/src/frontend/src/components/settings/fields/TagListField.tsx index 3943c4fe..ed494474 100644 --- a/src/frontend/src/components/settings/fields/TagListField.tsx +++ b/src/frontend/src/components/settings/fields/TagListField.tsx @@ -6,6 +6,7 @@ interface TagListFieldProps { value: string[]; onChange: (value: string[]) => void; disabled?: boolean; + requiredTags?: string[]; // Tags that cannot be removed } function normalizeTag(raw: string): string { @@ -24,15 +25,18 @@ function normalizeTag(raw: string): string { if (s.toLowerCase() === 'auto') return ''; // Basic URL normalization to keep UX friendly; backend also normalizes on save. - if (!s.includes('://') && !s.startsWith('/')) { + // Only add https:// if it looks like a domain (contains a dot) and has no protocol. + // This avoids adding prefixes to non-URL values like OIDC scopes (openid, email, etc.) + if (!s.includes('://') && !s.startsWith('/') && s.includes('.')) { s = `https://${s}`; } s = s.replace(/\/+$/, ''); return s.trim(); } -export const TagListField = ({ field, value, onChange, disabled }: TagListFieldProps) => { +export const TagListField = ({ field, value, onChange, disabled, requiredTags }: TagListFieldProps) => { const isDisabled = disabled ?? false; + const required = requiredTags ?? []; const inputRef = useRef(null); const [draft, setDraft] = useState(''); @@ -59,8 +63,10 @@ export const TagListField = ({ field, value, onChange, disabled }: TagListFieldP } }; + const isRequired = (tag: string) => required.includes(tag); + const removeAt = (idx: number) => { - if (isDisabled) return; + if (isDisabled || isRequired(tags[idx])) return; onChange(tags.filter((_, i) => i !== idx)); }; @@ -91,7 +97,7 @@ export const TagListField = ({ field, value, onChange, disabled }: TagListFieldP title={tag} > {tag} - {!isDisabled && ( + {!isDisabled && !isRequired(tag) && (
diff --git a/src/frontend/src/services/api.ts b/src/frontend/src/services/api.ts index ba014798..2b9d19ee 100644 --- a/src/frontend/src/services/api.ts +++ b/src/frontend/src/services/api.ts @@ -379,3 +379,82 @@ export const getReleases = async ( // Let the backend control timeouts for release searches (can be long-running). return fetchJSON(`${API_BASE}/releases?${params.toString()}`, {}, null); }; + +// Admin user management API + +export interface AdminUser { + id: number; + username: string; + email: string | null; + display_name: string | null; + role: string; + oidc_subject: string | null; + created_at: string; + settings?: Record; +} + +export const getAdminUsers = async (): Promise => { + return fetchJSON(`${API_BASE}/admin/users`); +}; + +export const getAdminUser = async (userId: number): Promise => { + return fetchJSON(`${API_BASE}/admin/users/${userId}`); +}; + +export const createAdminUser = async ( + data: { username: string; password: string; email?: string; display_name?: string; role?: string } +): Promise => { + return fetchJSON(`${API_BASE}/admin/users`, { + method: 'POST', + body: JSON.stringify(data), + }); +}; + +export const updateAdminUser = async ( + userId: number, + data: Partial> & { + password?: string; + settings?: Record; + } +): Promise => { + return fetchJSON(`${API_BASE}/admin/users/${userId}`, { + method: 'PUT', + body: JSON.stringify(data), + }); +}; + +export const deleteAdminUser = async (userId: number): Promise<{ success: boolean }> => { + return fetchJSON<{ success: boolean }>(`${API_BASE}/admin/users/${userId}`, { + method: 'DELETE', + }); +}; + +export interface DownloadDefaults { + BOOKS_OUTPUT_MODE: string; + DESTINATION: string; + BOOKLORE_LIBRARY_ID: string; + BOOKLORE_PATH_ID: string; + EMAIL_RECIPIENTS: Array<{ nickname: string; email: string }>; + OIDC_ADMIN_GROUP: string; + OIDC_USE_ADMIN_GROUP: boolean; + OIDC_AUTO_PROVISION: boolean; +} + +export const getDownloadDefaults = async (): Promise => { + return fetchJSON(`${API_BASE}/admin/download-defaults`); +}; + +export interface BookloreOption { + value: string; + label: string; + childOf?: string; +} + +export interface BookloreOptions { + libraries: BookloreOption[]; + paths: BookloreOption[]; +} + +export const getBookloreOptions = async (): Promise => { + return fetchJSON(`${API_BASE}/admin/booklore-options`); +}; diff --git a/src/frontend/src/types/index.ts b/src/frontend/src/types/index.ts index a94cc080..b7acdfcc 100644 --- a/src/frontend/src/types/index.ts +++ b/src/frontend/src/types/index.ts @@ -48,6 +48,7 @@ export interface Book { subtitle?: string; search_title?: string; search_author?: string; + username?: string; } // Status response types @@ -191,6 +192,7 @@ export interface AuthResponse { success?: boolean; authenticated?: boolean; auth_required?: boolean; + auth_mode?: string; is_admin?: boolean; error?: string; logout_url?: string; diff --git a/tests/README.md b/tests/README.md index ee56f4ac..15998712 100644 --- a/tests/README.md +++ b/tests/README.md @@ -19,6 +19,35 @@ docker exec test-cwabd python3 -m pytest tests/ -v -m "not integration" ``` tests/ +├── config/ # Settings & configuration tests +│ ├── test_docker_volumes.py # Docker volume mapping +│ ├── test_environment.py # Environment variable handling +│ ├── test_mirror_settings_live_apply.py # Mirror settings live reload +│ ├── test_mirror_settings_options.py # Mirror settings options +│ ├── test_security.py # Security settings +│ └── test_oidc_settings.py # OIDC settings fields & show_when conditions +│ +├── core/ # Core application logic tests +│ ├── test_admin_users_api.py # Admin user CRUD API endpoints +│ ├── test_booklore_multiuser.py # BookLore per-user override merging +│ ├── test_builtin_multiuser.py # Builtin auth multi-user migration +│ ├── test_download_processing.py # Download file processing +│ ├── test_hardlink.py # Hardlink/copy operations +│ ├── test_library_processing.py # Library file processing +│ ├── test_manual_query.py # Manual search query handling +│ ├── test_mirrors_config.py # Mirror configuration +│ ├── test_naming.py # File naming templates +│ ├── test_oidc_auth.py # OIDC auth helpers (group claims, user provisioning) +│ ├── test_oidc_integration.py # OIDC integration into auth system (logic mirror) +│ ├── test_oidc_routes.py # OIDC Flask route handlers +│ ├── test_part_number_extraction.py # Part number extraction +│ ├── test_per_user_downloads.py # Per-user download queue filtering +│ ├── test_permission_handling.py # File permission handling +│ ├── test_processing_integration.py # Processing integration +│ ├── test_search_plan.py # Search plan logic +│ ├── test_user_db.py # UserDB CRUD operations +│ └── test_user_template_variable.py # {User} template variable in naming +│ ├── e2e/ # End-to-end API tests │ ├── conftest.py # Fixtures (APIClient, DownloadTracker) │ ├── test_api.py # Core API endpoint tests @@ -59,6 +88,11 @@ docker exec test-cwabd python3 -m pytest tests/prowlarr/ -v -m "not integration" - Protocol detection (torrent vs usenet) - Release cache operations - Handler download flow logic +- User database (CRUD, settings, OIDC subject linking) +- OIDC authentication (group claims, user provisioning, route handlers) +- Admin user management API (create, update, delete, password, per-user settings) +- Multi-user download queue filtering and per-user overrides +- Settings configuration (OIDC fields, show_when conditions) ### E2E Tests Test the full application through its HTTP API. Require the app to be running. diff --git a/tests/config/test_oidc_settings.py b/tests/config/test_oidc_settings.py new file mode 100644 index 00000000..05d8fec2 --- /dev/null +++ b/tests/config/test_oidc_settings.py @@ -0,0 +1,160 @@ +""" +Tests for OIDC settings fields in security configuration. + +Tests that OIDC fields are registered correctly with proper +show_when conditions, defaults, and field types. +""" + +from shelfmark.core.settings_registry import ( + TextField, + PasswordField, + CheckboxField, + TagListField, +) + + +def _reload_security_module(): + """Reload security module to pick up patched values.""" + import importlib + import shelfmark.config.security + importlib.reload(shelfmark.config.security) + return shelfmark.config.security.security_settings() + + +def _get_field(fields, key): + """Find a field by key.""" + return next((f for f in fields if f.key == key), None) + + +class TestOIDCAuthMethodOption: + """Tests that OIDC appears as an auth method option.""" + + def test_oidc_option_available(self): + fields = _reload_security_module() + auth_field = _get_field(fields, "AUTH_METHOD") + option_values = [opt["value"] for opt in auth_field.options] + assert "oidc" in option_values + + def test_oidc_option_label(self): + fields = _reload_security_module() + auth_field = _get_field(fields, "AUTH_METHOD") + oidc_option = next(o for o in auth_field.options if o["value"] == "oidc") + assert "OIDC" in oidc_option["label"] + + +class TestOIDCFieldsPresent: + """Tests that all OIDC configuration fields are registered.""" + + def test_discovery_url_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_DISCOVERY_URL") + assert field is not None + assert isinstance(field, TextField) + + def test_client_id_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_CLIENT_ID") + assert field is not None + assert isinstance(field, TextField) + + def test_client_secret_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_CLIENT_SECRET") + assert field is not None + assert isinstance(field, PasswordField) + + def test_scopes_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_SCOPES") + assert field is not None + assert isinstance(field, TagListField) + + def test_scopes_default_includes_essentials(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_SCOPES") + assert "openid" in field.default + assert "email" in field.default + assert "profile" in field.default + + def test_use_admin_group_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_USE_ADMIN_GROUP") + assert field is not None + assert isinstance(field, CheckboxField) + assert field.default is True + + def test_group_claim_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_GROUP_CLAIM") + assert field is not None + assert field.default == "groups" + + def test_admin_group_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_ADMIN_GROUP") + assert field is not None + + def test_auto_provision_field_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "OIDC_AUTO_PROVISION") + assert field is not None + assert isinstance(field, CheckboxField) + assert field.default is True + + def test_test_connection_button_exists(self): + fields = _reload_security_module() + field = _get_field(fields, "test_oidc") + assert field is not None + + +class TestOIDCFieldShowWhen: + """Tests that OIDC fields are conditionally shown.""" + + def test_oidc_fields_show_when_oidc_selected(self): + fields = _reload_security_module() + oidc_keys = [ + "OIDC_DISCOVERY_URL", + "OIDC_CLIENT_ID", + "OIDC_CLIENT_SECRET", + "OIDC_SCOPES", + "OIDC_USE_ADMIN_GROUP", + "OIDC_AUTO_PROVISION", + "OIDC_GROUP_CLAIM", + "OIDC_ADMIN_GROUP", + ] + for key in oidc_keys: + field = _get_field(fields, key) + assert field is not None, f"Field {key} not found" + show_when = field.show_when + # show_when can be a dict or list of dicts + if isinstance(show_when, list): + conditions = show_when + else: + conditions = [show_when] + # At least one condition should reference AUTH_METHOD=oidc + has_oidc_condition = any( + c.get("field") == "AUTH_METHOD" and c.get("value") == "oidc" + for c in conditions + ) + assert has_oidc_condition, f"Field {key} missing AUTH_METHOD=oidc show_when" + + +class TestOIDCFieldsEnvSupport: + """Tests that OIDC fields are UI-only (no env var support).""" + + def test_oidc_fields_not_env_supported(self): + fields = _reload_security_module() + oidc_keys = [ + "OIDC_DISCOVERY_URL", + "OIDC_CLIENT_ID", + "OIDC_CLIENT_SECRET", + "OIDC_SCOPES", + "OIDC_USE_ADMIN_GROUP", + "OIDC_GROUP_CLAIM", + "OIDC_ADMIN_GROUP", + "OIDC_AUTO_PROVISION", + ] + for key in oidc_keys: + field = _get_field(fields, key) + assert field is not None, f"Field {key} not found" + assert field.env_supported is False, f"Field {key} should not support env vars" diff --git a/tests/core/test_admin_users_api.py b/tests/core/test_admin_users_api.py new file mode 100644 index 00000000..252db01f --- /dev/null +++ b/tests/core/test_admin_users_api.py @@ -0,0 +1,705 @@ +""" +Tests for admin user management API routes. + +Tests CRUD endpoints for managing users from the admin panel. +""" + +import os +import tempfile + +from unittest.mock import patch + +import pytest +from flask import Flask + +from shelfmark.core.user_db import UserDB + + +@pytest.fixture +def db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "shelfmark.db") + + +@pytest.fixture +def user_db(db_path): + db = UserDB(db_path) + db.initialize() + return db + + +@pytest.fixture +def app(user_db): + from shelfmark.core.admin_routes import register_admin_routes + + test_app = Flask(__name__) + test_app.config["SECRET_KEY"] = "test-secret" + test_app.config["TESTING"] = True + + register_admin_routes(test_app, user_db) + return test_app + + +@pytest.fixture +def admin_client(app): + client = app.test_client() + with client.session_transaction() as sess: + sess["user_id"] = "admin" + sess["is_admin"] = True + return client + + +@pytest.fixture +def regular_client(app): + """Non-admin client with auth mode set to builtin (auth-required).""" + client = app.test_client() + with client.session_transaction() as sess: + sess["user_id"] = "user" + sess["is_admin"] = False + with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + yield client + + +@pytest.fixture +def no_session_client(app): + """Client with no session at all (unauthenticated, no-auth mode).""" + return app.test_client() + + +@pytest.fixture +def no_session_auth_client(app): + """Client with no session but auth mode enabled (should be rejected).""" + client = app.test_client() + with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + yield client + + +# --------------------------------------------------------------------------- +# GET /api/admin/users +# --------------------------------------------------------------------------- + + +class TestAdminUsersListEndpoint: + """Tests for GET /api/admin/users.""" + + def test_list_users_empty(self, admin_client): + resp = admin_client.get("/api/admin/users") + assert resp.status_code == 200 + assert resp.json == [] + + def test_list_users_returns_all(self, admin_client, user_db): + user_db.create_user(username="alice", email="alice@example.com") + user_db.create_user(username="bob", email="bob@example.com") + + resp = admin_client.get("/api/admin/users") + assert resp.status_code == 200 + assert len(resp.json) == 2 + usernames = [u["username"] for u in resp.json] + assert "alice" in usernames + assert "bob" in usernames + + def test_list_users_excludes_password_hash(self, admin_client, user_db): + user_db.create_user(username="alice", password_hash="secret_hash") + + resp = admin_client.get("/api/admin/users") + users = resp.json + assert "password_hash" not in users[0] + + def test_list_users_requires_admin(self, regular_client): + resp = regular_client.get("/api/admin/users") + assert resp.status_code == 403 + + def test_list_users_no_session_allows_access_in_no_auth(self, no_session_client): + """No session + no-auth mode = admin access allowed.""" + resp = no_session_client.get("/api/admin/users") + assert resp.status_code == 200 + + def test_list_users_no_session_rejected_when_auth_enabled(self, no_session_auth_client): + """No session + auth enabled = 401.""" + resp = no_session_auth_client.get("/api/admin/users") + assert resp.status_code == 401 + + +# --------------------------------------------------------------------------- +# POST /api/admin/users +# --------------------------------------------------------------------------- + + +class TestAdminUserCreateEndpoint: + """Tests for POST /api/admin/users.""" + + def test_create_user(self, admin_client, user_db): + # Seed an existing user so alice doesn't get auto-promoted to admin + user_db.create_user(username="seed_admin", role="admin") + + resp = admin_client.post( + "/api/admin/users", + json={"username": "alice", "password": "pass1234"}, + ) + assert resp.status_code == 201 + assert resp.json["username"] == "alice" + assert resp.json["role"] == "user" + assert "password_hash" not in resp.json + + def test_create_user_with_all_fields(self, admin_client): + resp = admin_client.post( + "/api/admin/users", + json={ + "username": "alice", + "password": "pass1234", + "email": "alice@example.com", + "display_name": "Alice W", + "role": "admin", + }, + ) + assert resp.status_code == 201 + data = resp.json + assert data["username"] == "alice" + assert data["email"] == "alice@example.com" + assert data["display_name"] == "Alice W" + assert data["role"] == "admin" + + def test_create_user_password_is_hashed(self, admin_client, user_db): + admin_client.post( + "/api/admin/users", + json={"username": "alice", "password": "pass1234"}, + ) + user = user_db.get_user(username="alice") + assert user["password_hash"] is not None + assert user["password_hash"] != "pass1234" + assert user["password_hash"].startswith("scrypt:") or user["password_hash"].startswith("pbkdf2:") + + def test_create_user_requires_admin(self, regular_client): + resp = regular_client.post( + "/api/admin/users", + json={"username": "alice", "password": "pass1234"}, + ) + assert resp.status_code == 403 + + def test_create_user_missing_username(self, admin_client): + resp = admin_client.post( + "/api/admin/users", + json={"password": "pass1234"}, + ) + assert resp.status_code == 400 + assert "Username" in resp.json["error"] + + def test_create_user_empty_username(self, admin_client): + resp = admin_client.post( + "/api/admin/users", + json={"username": " ", "password": "pass1234"}, + ) + assert resp.status_code == 400 + + def test_create_user_missing_password(self, admin_client): + resp = admin_client.post( + "/api/admin/users", + json={"username": "alice"}, + ) + assert resp.status_code == 400 + assert "Password" in resp.json["error"] + + def test_create_user_short_password(self, admin_client): + resp = admin_client.post( + "/api/admin/users", + json={"username": "alice", "password": "abc"}, + ) + assert resp.status_code == 400 + assert "4 characters" in resp.json["error"] + + def test_create_user_invalid_role(self, admin_client): + resp = admin_client.post( + "/api/admin/users", + json={"username": "alice", "password": "pass1234", "role": "superadmin"}, + ) + assert resp.status_code == 400 + assert "Role" in resp.json["error"] + + def test_create_user_duplicate_username(self, admin_client, user_db): + user_db.create_user(username="alice") + + resp = admin_client.post( + "/api/admin/users", + json={"username": "alice", "password": "pass1234"}, + ) + assert resp.status_code == 409 + assert "already exists" in resp.json["error"] + + def test_first_user_is_always_admin(self, admin_client, user_db): + """First user created should be promoted to admin even if role=user.""" + assert len(user_db.list_users()) == 0 + + resp = admin_client.post( + "/api/admin/users", + json={"username": "alice", "password": "pass1234", "role": "user"}, + ) + assert resp.status_code == 201 + assert resp.json["role"] == "admin" + + def test_second_user_keeps_requested_role(self, admin_client, user_db): + """After the first user, role should be respected.""" + user_db.create_user(username="admin_user", role="admin") + + resp = admin_client.post( + "/api/admin/users", + json={"username": "bob", "password": "pass1234", "role": "user"}, + ) + assert resp.status_code == 201 + assert resp.json["role"] == "user" + + def test_create_user_trims_whitespace(self, admin_client): + resp = admin_client.post( + "/api/admin/users", + json={ + "username": " alice ", + "password": "pass1234", + "email": " alice@example.com ", + "display_name": " Alice ", + }, + ) + assert resp.status_code == 201 + assert resp.json["username"] == "alice" + assert resp.json["email"] == "alice@example.com" + assert resp.json["display_name"] == "Alice" + + def test_create_user_default_role_is_user(self, admin_client, user_db): + """When role is omitted and DB already has users, default to 'user'.""" + user_db.create_user(username="existing", role="admin") + + resp = admin_client.post( + "/api/admin/users", + json={"username": "bob", "password": "pass1234"}, + ) + assert resp.status_code == 201 + assert resp.json["role"] == "user" + + +# --------------------------------------------------------------------------- +# GET /api/admin/users/ +# --------------------------------------------------------------------------- + + +class TestAdminUserGetEndpoint: + """Tests for GET /api/admin/users/.""" + + def test_get_user(self, admin_client, user_db): + user = user_db.create_user(username="alice", email="alice@example.com") + + resp = admin_client.get(f"/api/admin/users/{user['id']}") + assert resp.status_code == 200 + assert resp.json["username"] == "alice" + assert resp.json["email"] == "alice@example.com" + + def test_get_user_includes_settings(self, admin_client, user_db): + user = user_db.create_user(username="alice") + user_db.set_user_settings(user["id"], {"booklore_library_id": 5}) + + resp = admin_client.get(f"/api/admin/users/{user['id']}") + assert resp.json["settings"]["booklore_library_id"] == 5 + + def test_get_user_empty_settings(self, admin_client, user_db): + user = user_db.create_user(username="alice") + + resp = admin_client.get(f"/api/admin/users/{user['id']}") + assert resp.json["settings"] == {} + + def test_get_user_excludes_password_hash(self, admin_client, user_db): + user = user_db.create_user(username="alice", password_hash="secret_hash") + + resp = admin_client.get(f"/api/admin/users/{user['id']}") + assert "password_hash" not in resp.json + + def test_get_nonexistent_user(self, admin_client): + resp = admin_client.get("/api/admin/users/9999") + assert resp.status_code == 404 + + def test_get_user_requires_admin(self, regular_client, user_db): + user = user_db.create_user(username="alice") + resp = regular_client.get(f"/api/admin/users/{user['id']}") + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# PUT /api/admin/users/ +# --------------------------------------------------------------------------- + + +class TestAdminUserUpdateEndpoint: + """Tests for PUT /api/admin/users/.""" + + def test_update_user_role(self, admin_client, user_db): + user = user_db.create_user(username="alice", role="user") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"role": "admin"}, + ) + assert resp.status_code == 200 + updated = user_db.get_user(user_id=user["id"]) + assert updated["role"] == "admin" + + def test_update_user_email(self, admin_client, user_db): + user = user_db.create_user(username="alice") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"email": "alice@new.com"}, + ) + assert resp.status_code == 200 + assert resp.json["email"] == "alice@new.com" + + def test_update_user_display_name(self, admin_client, user_db): + user = user_db.create_user(username="alice") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"display_name": "Alice Wonderland"}, + ) + assert resp.status_code == 200 + assert resp.json["display_name"] == "Alice Wonderland" + + def test_update_multiple_fields(self, admin_client, user_db): + user = user_db.create_user(username="alice", role="user") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"role": "admin", "email": "alice@admin.com", "display_name": "Admin Alice"}, + ) + assert resp.status_code == 200 + assert resp.json["role"] == "admin" + assert resp.json["email"] == "alice@admin.com" + assert resp.json["display_name"] == "Admin Alice" + + def test_update_user_settings(self, admin_client, user_db): + user = user_db.create_user(username="alice") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"settings": {"booklore_library_id": 3}}, + ) + assert resp.status_code == 200 + settings = user_db.get_user_settings(user["id"]) + assert settings["booklore_library_id"] == 3 + + def test_update_settings_merges(self, admin_client, user_db): + user = user_db.create_user(username="alice") + user_db.set_user_settings(user["id"], {"existing_key": "keep"}) + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"settings": {"new_key": "added"}}, + ) + assert resp.status_code == 200 + assert resp.json["settings"]["existing_key"] == "keep" + assert resp.json["settings"]["new_key"] == "added" + + def test_update_response_includes_settings(self, admin_client, user_db): + user = user_db.create_user(username="alice") + user_db.set_user_settings(user["id"], {"theme": "dark"}) + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"role": "admin"}, + ) + assert resp.status_code == 200 + assert "settings" in resp.json + assert resp.json["settings"]["theme"] == "dark" + + def test_update_response_excludes_password_hash(self, admin_client, user_db): + user = user_db.create_user(username="alice", password_hash="secret") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"role": "admin"}, + ) + assert "password_hash" not in resp.json + + def test_update_nonexistent_user(self, admin_client): + resp = admin_client.put( + "/api/admin/users/9999", + json={"role": "admin"}, + ) + assert resp.status_code == 404 + + def test_update_requires_admin(self, regular_client, user_db): + user = user_db.create_user(username="alice", role="user") + resp = regular_client.put( + f"/api/admin/users/{user['id']}", + json={"role": "admin"}, + ) + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# PUT /api/admin/users/ — password update +# --------------------------------------------------------------------------- + + +class TestAdminUserPasswordUpdate: + """Tests for password update via PUT /api/admin/users/.""" + + def test_update_password(self, admin_client, user_db): + """Setting a new password should hash and store it.""" + user = user_db.create_user(username="alice", password_hash="old_hash") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"password": "newpass99"}, + ) + assert resp.status_code == 200 + + updated = user_db.get_user(user_id=user["id"]) + assert updated["password_hash"] != "old_hash" + assert updated["password_hash"].startswith("scrypt:") or updated["password_hash"].startswith("pbkdf2:") + + def test_update_password_too_short(self, admin_client, user_db): + """Password shorter than 4 characters should be rejected.""" + user = user_db.create_user(username="alice", password_hash="old_hash") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"password": "ab"}, + ) + assert resp.status_code == 400 + assert "4 characters" in resp.json["error"] + + def test_update_password_empty_string_ignored(self, admin_client, user_db): + """Empty password string should not change existing hash.""" + user = user_db.create_user(username="alice", password_hash="original_hash") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"password": ""}, + ) + assert resp.status_code == 200 + + updated = user_db.get_user(user_id=user["id"]) + assert updated["password_hash"] == "original_hash" + + def test_update_password_with_other_fields(self, admin_client, user_db): + """Password update should work alongside other field updates.""" + user = user_db.create_user(username="alice", role="user", password_hash="old") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"password": "newpass99", "role": "admin"}, + ) + assert resp.status_code == 200 + assert resp.json["role"] == "admin" + + updated = user_db.get_user(user_id=user["id"]) + assert updated["password_hash"] != "old" + + def test_update_password_hash_not_in_response(self, admin_client, user_db): + """Response should never contain password_hash.""" + user = user_db.create_user(username="alice", password_hash="old") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"password": "newpass99"}, + ) + assert resp.status_code == 200 + assert "password_hash" not in resp.json + assert "password" not in resp.json + + +# --------------------------------------------------------------------------- +# GET /api/admin/download-defaults +# --------------------------------------------------------------------------- + + +class TestAdminDownloadDefaults: + """Tests for GET /api/admin/download-defaults.""" + + @pytest.fixture(autouse=True) + def setup_config(self, tmp_path, monkeypatch): + """Create a temporary downloads config file.""" + import json + from pathlib import Path + + config_dir = str(tmp_path) + monkeypatch.setenv("CONFIG_DIR", config_dir) + monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", Path(config_dir)) + plugins_dir = tmp_path / "plugins" + plugins_dir.mkdir() + config = { + "BOOKS_OUTPUT_MODE": "folder", + "DESTINATION": "/books", + "BOOKLORE_LIBRARY_ID": "2", + "BOOKLORE_PATH_ID": "5", + "EMAIL_RECIPIENTS": [{"nickname": "kindle", "email": "me@kindle.com"}], + } + (plugins_dir / "downloads.json").write_text(json.dumps(config)) + + def test_returns_download_defaults(self, admin_client): + resp = admin_client.get("/api/admin/download-defaults") + assert resp.status_code == 200 + data = resp.json + assert data["BOOKS_OUTPUT_MODE"] == "folder" + assert data["DESTINATION"] == "/books" + assert data["BOOKLORE_LIBRARY_ID"] == "2" + assert data["BOOKLORE_PATH_ID"] == "5" + assert data["EMAIL_RECIPIENTS"] == [{"nickname": "kindle", "email": "me@kindle.com"}] + + def test_returns_defaults_when_no_config(self, admin_client, tmp_path): + """If no downloads config file exists, return sensible defaults.""" + + config_path = tmp_path / "plugins" / "downloads.json" + if config_path.exists(): + os.remove(config_path) + + resp = admin_client.get("/api/admin/download-defaults") + assert resp.status_code == 200 + data = resp.json + assert "BOOKS_OUTPUT_MODE" in data + assert "DESTINATION" in data + + def test_requires_admin(self, regular_client): + resp = regular_client.get("/api/admin/download-defaults") + assert resp.status_code == 403 + + +class TestAdminBookloreOptions: + """Tests for GET /api/admin/booklore-options.""" + + def test_returns_library_and_path_options(self, admin_client, monkeypatch): + mock_libraries = [{"value": "1", "label": "My Library"}] + mock_paths = [{"value": "10", "label": "My Library: /books", "childOf": "1"}] + monkeypatch.setattr( + "shelfmark.core.admin_routes.get_booklore_library_options", + lambda: mock_libraries, + ) + monkeypatch.setattr( + "shelfmark.core.admin_routes.get_booklore_path_options", + lambda: mock_paths, + ) + resp = admin_client.get("/api/admin/booklore-options") + assert resp.status_code == 200 + data = resp.json + assert data["libraries"] == mock_libraries + assert data["paths"] == mock_paths + + def test_returns_empty_when_not_configured(self, admin_client, monkeypatch): + monkeypatch.setattr( + "shelfmark.core.admin_routes.get_booklore_library_options", + lambda: [], + ) + monkeypatch.setattr( + "shelfmark.core.admin_routes.get_booklore_path_options", + lambda: [], + ) + resp = admin_client.get("/api/admin/booklore-options") + assert resp.status_code == 200 + data = resp.json + assert data["libraries"] == [] + assert data["paths"] == [] + + def test_requires_admin(self, regular_client): + resp = regular_client.get("/api/admin/booklore-options") + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# DELETE /api/admin/users/ +# --------------------------------------------------------------------------- + + +class TestAdminUserDeleteEndpoint: + """Tests for DELETE /api/admin/users/.""" + + def test_delete_user(self, admin_client, user_db): + user = user_db.create_user(username="alice") + + resp = admin_client.delete(f"/api/admin/users/{user['id']}") + assert resp.status_code == 200 + assert resp.json["success"] is True + assert user_db.get_user(user_id=user["id"]) is None + + def test_delete_nonexistent_user(self, admin_client): + resp = admin_client.delete("/api/admin/users/9999") + assert resp.status_code == 404 + + def test_delete_requires_admin(self, regular_client, user_db): + user = user_db.create_user(username="alice") + resp = regular_client.delete(f"/api/admin/users/{user['id']}") + assert resp.status_code == 403 + + def test_delete_user_removes_from_list(self, admin_client, user_db): + user = user_db.create_user(username="alice") + user_db.create_user(username="bob") + + admin_client.delete(f"/api/admin/users/{user['id']}") + + resp = admin_client.get("/api/admin/users") + assert len(resp.json) == 1 + assert resp.json[0]["username"] == "bob" + + +# --------------------------------------------------------------------------- +# OIDC lockout prevention (security on_save handler) +# --------------------------------------------------------------------------- + + +class TestOIDCLockoutPrevention: + """Tests for _on_save_security blocking OIDC without a local admin.""" + + @pytest.fixture(autouse=True) + def setup_config_dir(self, db_path, tmp_path, monkeypatch): + """Point CONFIG_DIR to a temp dir so _on_save_security can find users.db.""" + config_dir = str(tmp_path) + monkeypatch.setenv("CONFIG_DIR", config_dir) + # Create user_db at the path _on_save_security will look for + self._user_db = UserDB(os.path.join(config_dir, "users.db")) + self._user_db.initialize() + + def _call_on_save(self, values): + from shelfmark.config.security import _on_save_security + return _on_save_security(values) + + def test_oidc_blocked_without_local_admin(self): + """OIDC should be blocked when no local password admin exists.""" + result = self._call_on_save({"AUTH_METHOD": "oidc"}) + assert result["error"] is True + assert "local admin" in result["message"].lower() + + def test_oidc_blocked_with_oidc_only_admin(self): + """OIDC admin without password should not count as local admin.""" + self._user_db.create_user( + username="sso_admin", + oidc_subject="sub123", + role="admin", + ) + result = self._call_on_save({"AUTH_METHOD": "oidc"}) + assert result["error"] is True + + def test_oidc_blocked_with_local_non_admin(self): + """A local password user who is not admin should not unblock OIDC.""" + self._user_db.create_user( + username="regular", + password_hash="hashed_pw", + role="user", + ) + result = self._call_on_save({"AUTH_METHOD": "oidc"}) + assert result["error"] is True + + def test_oidc_allowed_with_local_admin(self): + """OIDC should be allowed when a local password admin exists.""" + self._user_db.create_user( + username="admin_user", + password_hash="hashed_pw", + role="admin", + ) + result = self._call_on_save({"AUTH_METHOD": "oidc"}) + assert result["error"] is False + + def test_non_oidc_methods_not_blocked(self): + """Other auth methods should not trigger the OIDC check.""" + for method in ("none", "builtin", "proxy", "cwa"): + result = self._call_on_save({"AUTH_METHOD": method}) + assert result["error"] is False, f"AUTH_METHOD={method} should not be blocked" + + def test_oidc_check_preserves_values(self): + """When OIDC is blocked, the original values should be returned.""" + values = {"AUTH_METHOD": "oidc", "OIDC_CLIENT_ID": "myapp"} + result = self._call_on_save(values) + assert result["values"]["OIDC_CLIENT_ID"] == "myapp" diff --git a/tests/core/test_booklore_multiuser.py b/tests/core/test_booklore_multiuser.py new file mode 100644 index 00000000..50517a54 --- /dev/null +++ b/tests/core/test_booklore_multiuser.py @@ -0,0 +1,90 @@ +"""Tests for per-user BookLore library/path support.""" + +from shelfmark.download.outputs.booklore import build_booklore_config + + +class TestBuildBookloreConfigWithOverrides: + """build_booklore_config should accept per-user library/path overrides.""" + + BASE_SETTINGS = { + "BOOKLORE_HOST": "http://booklore:6060", + "BOOKLORE_USERNAME": "admin", + "BOOKLORE_PASSWORD": "secret", + "BOOKLORE_LIBRARY_ID": 1, + "BOOKLORE_PATH_ID": 10, + } + + def test_global_config_no_overrides(self): + config = build_booklore_config(self.BASE_SETTINGS) + assert config.library_id == 1 + assert config.path_id == 10 + + def test_override_library_and_path(self): + overrides = {"booklore_library_id": 2, "booklore_path_id": 20} + config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides) + assert config.library_id == 2 + assert config.path_id == 20 + + def test_override_library_only(self): + overrides = {"booklore_library_id": 3} + config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides) + assert config.library_id == 3 + assert config.path_id == 10 # falls back to global + + def test_override_path_only(self): + overrides = {"booklore_path_id": 30} + config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides) + assert config.library_id == 1 # falls back to global + assert config.path_id == 30 + + def test_empty_overrides_uses_global(self): + config = build_booklore_config(self.BASE_SETTINGS, user_overrides={}) + assert config.library_id == 1 + assert config.path_id == 10 + + def test_none_overrides_uses_global(self): + config = build_booklore_config(self.BASE_SETTINGS, user_overrides=None) + assert config.library_id == 1 + assert config.path_id == 10 + + def test_auth_fields_not_overridable(self): + """Auth stays global - user overrides should not affect host/user/pass.""" + overrides = { + "booklore_library_id": 5, + "BOOKLORE_HOST": "http://evil:6060", + "BOOKLORE_USERNAME": "hacker", + } + config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides) + assert config.base_url == "http://booklore:6060" + assert config.username == "admin" + assert config.library_id == 5 + + +class TestOutputArgsForBooklore: + """Download tasks should carry per-user booklore settings in output_args.""" + + def test_output_args_with_booklore_settings(self): + from shelfmark.core.models import DownloadTask + + task = DownloadTask( + task_id="test-1", + source="direct_download", + title="Book1", + output_mode="booklore", + output_args={"booklore_library_id": 2, "booklore_path_id": 20}, + user_id=1, + ) + assert task.output_args["booklore_library_id"] == 2 + assert task.output_args["booklore_path_id"] == 20 + + def test_output_args_empty_for_global_booklore(self): + from shelfmark.core.models import DownloadTask + + task = DownloadTask( + task_id="test-2", + source="direct_download", + title="Book1", + output_mode="booklore", + output_args={}, + ) + assert task.output_args == {} diff --git a/tests/core/test_builtin_multiuser.py b/tests/core/test_builtin_multiuser.py new file mode 100644 index 00000000..041b82dc --- /dev/null +++ b/tests/core/test_builtin_multiuser.py @@ -0,0 +1,140 @@ +"""Tests for multi-user builtin authentication via users table.""" + +import os +import tempfile + +import pytest +from werkzeug.security import generate_password_hash + +from shelfmark.core.user_db import UserDB + + +@pytest.fixture +def db(): + with tempfile.TemporaryDirectory() as tmpdir: + db_path = os.path.join(tmpdir, "users.db") + user_db = UserDB(db_path) + user_db.initialize() + yield user_db + + +class TestBuiltinMultiUserLogin: + """Builtin auth should support multiple users via users table.""" + + def test_create_builtin_user_with_password(self, db): + password_hash = generate_password_hash("secret123") + user = db.create_user( + username="alice", + password_hash=password_hash, + role="user", + ) + assert user["username"] == "alice" + assert user["role"] == "user" + + def test_create_admin_and_regular_user(self, db): + db.create_user(username="admin", password_hash=generate_password_hash("admin123"), role="admin") + db.create_user(username="user1", password_hash=generate_password_hash("user123"), role="user") + users = db.list_users() + assert len(users) == 2 + roles = {u["username"]: u["role"] for u in users} + assert roles["admin"] == "admin" + assert roles["user1"] == "user" + + def test_authenticate_builtin_user(self, db): + from werkzeug.security import check_password_hash + + password = "mypassword" + password_hash = generate_password_hash(password) + db.create_user(username="bob", password_hash=password_hash, role="user") + user = db.get_user(username="bob") + assert user is not None + assert check_password_hash(user["password_hash"], password) + + def test_authenticate_wrong_password(self, db): + from werkzeug.security import check_password_hash + + password_hash = generate_password_hash("correct") + db.create_user(username="carol", password_hash=password_hash, role="user") + user = db.get_user(username="carol") + assert not check_password_hash(user["password_hash"], "wrong") + + def test_user_not_found(self, db): + user = db.get_user(username="nonexistent") + assert user is None + + +class TestMigrateBuiltinConfig: + """When migrating from single-user config to multi-user DB, + the existing admin credentials should be auto-imported.""" + + def test_migrate_existing_admin(self, db): + """Simulate migrating BUILTIN_USERNAME/BUILTIN_PASSWORD_HASH to users table.""" + existing_username = "myadmin" + existing_hash = generate_password_hash("oldpassword") + + # No users yet + assert len(db.list_users()) == 0 + + # Migration: create admin user from config values + user = db.create_user( + username=existing_username, + password_hash=existing_hash, + role="admin", + ) + assert user["username"] == "myadmin" + assert user["role"] == "admin" + assert user["password_hash"] == existing_hash + + def test_skip_migration_if_users_exist(self, db): + """Don't re-migrate if users already exist in DB.""" + db.create_user(username="existing_admin", password_hash=generate_password_hash("pw"), role="admin") + # Should have 1 user already, migration should be skipped + assert len(db.list_users()) == 1 + + +class TestBuiltinLoginLogic: + """Test the login logic that mirrors what main.py will do for builtin multi-user.""" + + def _builtin_login(self, db, username, password): + """Mirror the multi-user builtin login logic.""" + from werkzeug.security import check_password_hash + + user = db.get_user(username=username) + if not user: + return None + if not user.get("password_hash"): + return None + if not check_password_hash(user["password_hash"], password): + return None + return { + "user_id": username, + "db_user_id": user["id"], + "is_admin": user["role"] == "admin", + } + + def test_login_admin(self, db): + db.create_user(username="admin", password_hash=generate_password_hash("admin123"), role="admin") + result = self._builtin_login(db, "admin", "admin123") + assert result is not None + assert result["is_admin"] is True + assert result["user_id"] == "admin" + + def test_login_regular_user(self, db): + db.create_user(username="user1", password_hash=generate_password_hash("pass1"), role="user") + result = self._builtin_login(db, "user1", "pass1") + assert result is not None + assert result["is_admin"] is False + + def test_login_wrong_password(self, db): + db.create_user(username="user1", password_hash=generate_password_hash("correct"), role="user") + result = self._builtin_login(db, "user1", "wrong") + assert result is None + + def test_login_nonexistent_user(self, db): + result = self._builtin_login(db, "nobody", "pass") + assert result is None + + def test_login_sets_db_user_id(self, db): + user = db.create_user(username="dave", password_hash=generate_password_hash("pw"), role="user") + result = self._builtin_login(db, "dave", "pw") + assert result["db_user_id"] == user["id"] diff --git a/tests/core/test_oidc_auth.py b/tests/core/test_oidc_auth.py new file mode 100644 index 00000000..1eab800a --- /dev/null +++ b/tests/core/test_oidc_auth.py @@ -0,0 +1,258 @@ +""" +Tests for OIDC authentication flow. + +Tests the OIDCAuth helper: login URL generation, callback handling, +user provisioning, and group claim parsing. +""" + + +import os +import tempfile +import pytest + + +MOCK_DISCOVERY = { + "issuer": "https://auth.example.com", + "authorization_endpoint": "https://auth.example.com/authorize", + "token_endpoint": "https://auth.example.com/token", + "userinfo_endpoint": "https://auth.example.com/userinfo", + "jwks_uri": "https://auth.example.com/.well-known/jwks.json", +} + +MOCK_OIDC_CONFIG = { + "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration", + "OIDC_CLIENT_ID": "shelfmark", + "OIDC_CLIENT_SECRET": "secret123", + "OIDC_SCOPES": ["openid", "email", "profile", "groups"], + "OIDC_GROUP_CLAIM": "groups", + "OIDC_ADMIN_GROUP": "shelfmark-admins", + "OIDC_AUTO_PROVISION": True, + "OIDC_USE_ADMIN_GROUP": True, +} + + +@pytest.fixture +def db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "shelfmark.db") + + +@pytest.fixture +def user_db(db_path): + from shelfmark.core.user_db import UserDB + db = UserDB(db_path) + db.initialize() + return db + + +class TestParseGroupClaims: + """Tests for parsing group claims from ID tokens.""" + + def test_parse_groups_list(self): + from shelfmark.core.oidc_auth import parse_group_claims + id_token = {"groups": ["admins", "users", "shelfmark-admins"]} + groups = parse_group_claims(id_token, "groups") + assert "shelfmark-admins" in groups + assert "admins" in groups + + def test_parse_groups_comma_separated_string(self): + from shelfmark.core.oidc_auth import parse_group_claims + id_token = {"groups": "admins, users, shelfmark-admins"} + groups = parse_group_claims(id_token, "groups") + assert "shelfmark-admins" in groups + + def test_parse_groups_pipe_separated_string(self): + from shelfmark.core.oidc_auth import parse_group_claims + id_token = {"groups": "admins|users|shelfmark-admins"} + groups = parse_group_claims(id_token, "groups") + assert "shelfmark-admins" in groups + + def test_parse_groups_missing_claim(self): + from shelfmark.core.oidc_auth import parse_group_claims + id_token = {"email": "user@example.com"} + groups = parse_group_claims(id_token, "groups") + assert groups == [] + + def test_parse_groups_empty(self): + from shelfmark.core.oidc_auth import parse_group_claims + id_token = {"groups": []} + groups = parse_group_claims(id_token, "groups") + assert groups == [] + + +class TestCheckAdminFromGroups: + """Tests for determining admin status from group claims. + + Admin check is now inline: `admin_group in groups` when use_admin_group is True. + These tests verify the logic that was previously in is_admin_from_groups(). + """ + + def test_admin_when_group_matches(self): + groups = ["users", "shelfmark-admins"] + admin_group = "shelfmark-admins" + assert admin_group in groups + + def test_not_admin_when_group_missing(self): + groups = ["users", "editors"] + admin_group = "shelfmark-admins" + assert admin_group not in groups + + def test_not_admin_when_no_groups(self): + assert "shelfmark-admins" not in [] + + def test_not_admin_when_admin_group_empty(self): + groups = ["users", "admins"] + # When admin_group is empty, use_admin_group check is skipped (is_admin stays None) + admin_group = "" + use_admin_group = True + is_admin = None + if admin_group and use_admin_group: + is_admin = admin_group in groups + assert is_admin is None + + +class TestExtractUserInfo: + """Tests for extracting user info from OIDC claims.""" + + def test_extract_standard_claims(self): + from shelfmark.core.oidc_auth import extract_user_info + id_token = { + "sub": "user-123", + "email": "john@example.com", + "name": "John Doe", + "preferred_username": "john", + } + info = extract_user_info(id_token) + assert info["oidc_subject"] == "user-123" + assert info["email"] == "john@example.com" + assert info["display_name"] == "John Doe" + assert info["username"] == "john" + + def test_extract_falls_back_to_email_for_username(self): + from shelfmark.core.oidc_auth import extract_user_info + id_token = { + "sub": "user-123", + "email": "john@example.com", + "name": "John Doe", + } + info = extract_user_info(id_token) + assert info["username"] == "john@example.com" + + def test_extract_falls_back_to_sub_for_username(self): + from shelfmark.core.oidc_auth import extract_user_info + id_token = { + "sub": "user-123", + } + info = extract_user_info(id_token) + assert info["username"] == "user-123" + + def test_extract_handles_missing_optional_fields(self): + from shelfmark.core.oidc_auth import extract_user_info + id_token = {"sub": "user-123"} + info = extract_user_info(id_token) + assert info["oidc_subject"] == "user-123" + assert info["email"] is None + assert info["display_name"] is None + + +class TestProvisionOIDCUser: + """Tests for creating/updating users from OIDC claims.""" + + def test_provision_creates_new_user(self, user_db): + from shelfmark.core.oidc_auth import provision_oidc_user + user_info = { + "oidc_subject": "sub-123", + "username": "john", + "email": "john@example.com", + "display_name": "John Doe", + } + user = provision_oidc_user(user_db, user_info, is_admin=False) + assert user["username"] == "john" + assert user["oidc_subject"] == "sub-123" + assert user["role"] == "user" + + def test_provision_creates_admin_user(self, user_db): + from shelfmark.core.oidc_auth import provision_oidc_user + user_info = { + "oidc_subject": "sub-123", + "username": "john", + "email": "john@example.com", + "display_name": "John Doe", + } + user = provision_oidc_user(user_db, user_info, is_admin=True) + assert user["role"] == "admin" + + def test_provision_returns_existing_user(self, user_db): + from shelfmark.core.oidc_auth import provision_oidc_user + user_info = { + "oidc_subject": "sub-123", + "username": "john", + "email": "john@example.com", + "display_name": "John Doe", + } + user1 = provision_oidc_user(user_db, user_info, is_admin=False) + user2 = provision_oidc_user(user_db, user_info, is_admin=False) + assert user1["id"] == user2["id"] + + def test_provision_updates_existing_user_info(self, user_db): + from shelfmark.core.oidc_auth import provision_oidc_user + user_info = { + "oidc_subject": "sub-123", + "username": "john", + "email": "john@example.com", + "display_name": "John Doe", + } + provision_oidc_user(user_db, user_info, is_admin=False) + + user_info["email"] = "newemail@example.com" + user_info["display_name"] = "John D." + user = provision_oidc_user(user_db, user_info, is_admin=False) + assert user["email"] == "newemail@example.com" + assert user["display_name"] == "John D." + + def test_provision_updates_admin_role(self, user_db): + from shelfmark.core.oidc_auth import provision_oidc_user + user_info = { + "oidc_subject": "sub-123", + "username": "john", + "email": "john@example.com", + "display_name": "John Doe", + } + user = provision_oidc_user(user_db, user_info, is_admin=False) + assert user["role"] == "user" + + user = provision_oidc_user(user_db, user_info, is_admin=True) + assert user["role"] == "admin" + + def test_provision_preserves_role_when_group_auth_disabled(self, user_db): + """When is_admin=None (group auth disabled), DB role should be preserved.""" + from shelfmark.core.oidc_auth import provision_oidc_user + user_info = { + "oidc_subject": "sub-123", + "username": "john", + "email": "john@example.com", + "display_name": "John Doe", + } + # Create as admin via group auth + user = provision_oidc_user(user_db, user_info, is_admin=True) + assert user["role"] == "admin" + + # Login again with group auth disabled (is_admin=None) — should preserve admin + user = provision_oidc_user(user_db, user_info, is_admin=None) + assert user["role"] == "admin" + + def test_provision_handles_duplicate_username(self, user_db): + """If OIDC subject is new but username exists, append suffix.""" + from shelfmark.core.oidc_auth import provision_oidc_user + # Create a local user with the same username + user_db.create_user(username="john", password_hash="hash") + + user_info = { + "oidc_subject": "sub-456", + "username": "john", + "email": "john@example.com", + "display_name": "John Doe", + } + user = provision_oidc_user(user_db, user_info, is_admin=False) + assert user["username"] != "john" # Should have a suffix + assert user["oidc_subject"] == "sub-456" diff --git a/tests/core/test_oidc_integration.py b/tests/core/test_oidc_integration.py new file mode 100644 index 00000000..38f83447 --- /dev/null +++ b/tests/core/test_oidc_integration.py @@ -0,0 +1,206 @@ +""" +Tests for OIDC integration into existing auth system. + +Tests get_auth_mode() logic with OIDC and login_required admin +restriction logic. Since main.py has heavy dependencies, we test +the logic directly rather than importing from main. +""" + + +class TestGetAuthModeOIDCLogic: + """Tests that get_auth_mode logic handles OIDC correctly. + + Mirrors the logic in main.py:get_auth_mode() to verify OIDC + support without importing the full app. + """ + + def _get_auth_mode(self, config): + """Replicate get_auth_mode logic with OIDC support.""" + auth_mode = config.get("AUTH_METHOD", "none") + if auth_mode == "oidc": + if config.get("OIDC_DISCOVERY_URL") and config.get("OIDC_CLIENT_ID"): + return "oidc" + return "none" + if auth_mode == "builtin": + if config.get("BUILTIN_USERNAME") and config.get("BUILTIN_PASSWORD_HASH"): + return "builtin" + return "none" + if auth_mode == "proxy": + if config.get("PROXY_AUTH_USER_HEADER"): + return "proxy" + return "none" + return "none" + + def test_returns_oidc_when_fully_configured(self): + config = { + "AUTH_METHOD": "oidc", + "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration", + "OIDC_CLIENT_ID": "shelfmark", + } + assert self._get_auth_mode(config) == "oidc" + + def test_returns_none_when_oidc_missing_client_id(self): + config = { + "AUTH_METHOD": "oidc", + "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration", + } + assert self._get_auth_mode(config) == "none" + + def test_returns_none_when_oidc_missing_discovery_url(self): + config = { + "AUTH_METHOD": "oidc", + "OIDC_CLIENT_ID": "shelfmark", + } + assert self._get_auth_mode(config) == "none" + + def test_returns_none_when_oidc_empty_strings(self): + config = { + "AUTH_METHOD": "oidc", + "OIDC_DISCOVERY_URL": "", + "OIDC_CLIENT_ID": "", + } + assert self._get_auth_mode(config) == "none" + + def test_builtin_still_works(self): + config = { + "AUTH_METHOD": "builtin", + "BUILTIN_USERNAME": "admin", + "BUILTIN_PASSWORD_HASH": "hash", + } + assert self._get_auth_mode(config) == "builtin" + + def test_proxy_still_works(self): + config = { + "AUTH_METHOD": "proxy", + "PROXY_AUTH_USER_HEADER": "X-Auth-User", + } + assert self._get_auth_mode(config) == "proxy" + + +class TestLoginRequiredOIDCLogic: + """Tests the OIDC admin restriction logic. + + Mirrors the admin check in main.py:login_required() to verify + OIDC support without importing the full app. + """ + + def _check_admin_access(self, auth_mode, config, session, path): + """Replicate login_required admin check logic with OIDC.""" + if auth_mode == "none": + return True # Allowed + + if "user_id" not in session: + return 401 # Unauthorized + + settings_path = path.startswith("/api/settings") or path.startswith("/api/onboarding") + + if auth_mode in ("proxy", "cwa", "oidc") and settings_path: + if auth_mode == "proxy": + restrict = config.get("PROXY_AUTH_RESTRICT_SETTINGS_TO_ADMIN", False) + elif auth_mode == "cwa": + restrict = config.get("CWA_RESTRICT_SETTINGS_TO_ADMIN", False) + elif auth_mode == "oidc": + restrict = config.get("OIDC_RESTRICT_SETTINGS_TO_ADMIN", False) + else: + restrict = False + + if restrict and not session.get("is_admin", False): + return 403 # Forbidden + + return True # Allowed + + def test_oidc_unauthenticated_returns_401(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + result = self._check_admin_access("oidc", config, {}, "/api/settings/test") + assert result == 401 + + def test_oidc_non_admin_blocked_from_settings(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + session = {"user_id": "user", "is_admin": False} + result = self._check_admin_access("oidc", config, session, "/api/settings/test") + assert result == 403 + + def test_oidc_admin_can_access_settings(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + session = {"user_id": "admin", "is_admin": True} + result = self._check_admin_access("oidc", config, session, "/api/settings/test") + assert result is True + + def test_oidc_non_admin_can_access_non_settings(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + session = {"user_id": "user", "is_admin": False} + result = self._check_admin_access("oidc", config, session, "/api/search") + assert result is True + + def test_oidc_no_restrict_allows_non_admin_settings(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": False} + session = {"user_id": "user", "is_admin": False} + result = self._check_admin_access("oidc", config, session, "/api/settings/test") + assert result is True + + def test_oidc_non_admin_blocked_from_onboarding(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + session = {"user_id": "user", "is_admin": False} + result = self._check_admin_access("oidc", config, session, "/api/onboarding") + assert result == 403 + + +class TestAuthCheckOIDCLogic: + """Tests the /api/auth/check response logic for OIDC mode.""" + + def _build_auth_check_response(self, auth_mode, config, session): + """Replicate auth check logic with OIDC.""" + if auth_mode == "none": + return {"authenticated": True, "auth_required": False, "auth_mode": "none", "is_admin": True} + + is_authenticated = "user_id" in session + + if auth_mode == "builtin": + is_admin = True + elif auth_mode == "cwa": + restrict = config.get("CWA_RESTRICT_SETTINGS_TO_ADMIN", False) + is_admin = session.get("is_admin", False) if restrict else True + elif auth_mode == "proxy": + restrict = config.get("PROXY_AUTH_RESTRICT_SETTINGS_TO_ADMIN", False) + is_admin = session.get("is_admin", not restrict) + elif auth_mode == "oidc": + restrict = config.get("OIDC_RESTRICT_SETTINGS_TO_ADMIN", False) + is_admin = session.get("is_admin", False) if restrict else True + else: + is_admin = False + + return { + "authenticated": is_authenticated, + "auth_required": True, + "auth_mode": auth_mode, + "is_admin": is_admin if is_authenticated else False, + "username": session.get("user_id") if is_authenticated else None, + } + + def test_oidc_authenticated_admin(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + session = {"user_id": "admin", "is_admin": True} + result = self._build_auth_check_response("oidc", config, session) + assert result["authenticated"] is True + assert result["auth_mode"] == "oidc" + assert result["is_admin"] is True + assert result["username"] == "admin" + + def test_oidc_authenticated_non_admin(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + session = {"user_id": "user", "is_admin": False} + result = self._build_auth_check_response("oidc", config, session) + assert result["is_admin"] is False + + def test_oidc_no_restrict_all_are_admin(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": False} + session = {"user_id": "user", "is_admin": False} + result = self._build_auth_check_response("oidc", config, session) + assert result["is_admin"] is True + + def test_oidc_unauthenticated(self): + config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True} + result = self._build_auth_check_response("oidc", config, {}) + assert result["authenticated"] is False + assert result["is_admin"] is False + assert result["auth_required"] is True diff --git a/tests/core/test_oidc_routes.py b/tests/core/test_oidc_routes.py new file mode 100644 index 00000000..063d7f2a --- /dev/null +++ b/tests/core/test_oidc_routes.py @@ -0,0 +1,313 @@ +""" +Tests for OIDC Flask route handlers. + +Tests the /api/auth/oidc/login and /api/auth/oidc/callback endpoints +using a minimal Flask test app (not the full shelfmark app). +""" + +import os +import tempfile +from unittest.mock import patch +from urllib.parse import parse_qs, urlparse + +import pytest +from flask import Flask + +from shelfmark.core.user_db import UserDB + + +@pytest.fixture +def db_path(): + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "shelfmark.db") + + +@pytest.fixture +def user_db(db_path): + db = UserDB(db_path) + db.initialize() + return db + + +MOCK_OIDC_CONFIG = { + "AUTH_METHOD": "oidc", + "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration", + "OIDC_CLIENT_ID": "shelfmark", + "OIDC_CLIENT_SECRET": "secret123", + "OIDC_SCOPES": ["openid", "email", "profile", "groups"], + "OIDC_GROUP_CLAIM": "groups", + "OIDC_ADMIN_GROUP": "shelfmark-admins", + "OIDC_AUTO_PROVISION": True, + "OIDC_RESTRICT_SETTINGS_TO_ADMIN": True, +} + +MOCK_DISCOVERY = { + "issuer": "https://auth.example.com", + "authorization_endpoint": "https://auth.example.com/authorize", + "token_endpoint": "https://auth.example.com/token", + "userinfo_endpoint": "https://auth.example.com/userinfo", + "jwks_uri": "https://auth.example.com/.well-known/jwks.json", +} + + +@pytest.fixture +def app(user_db, db_path): + """Create a minimal Flask test app with OIDC routes.""" + from shelfmark.core.oidc_routes import register_oidc_routes + + test_app = Flask(__name__) + test_app.config["SECRET_KEY"] = "test-secret" + test_app.config["TESTING"] = True + + register_oidc_routes(test_app, user_db) + + return test_app + + +@pytest.fixture +def client(app): + return app.test_client() + + +class TestOIDCLoginEndpoint: + """Tests for GET /api/auth/oidc/login.""" + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + def test_login_redirects_to_idp(self, mock_discovery, mock_config, client): + resp = client.get("/api/auth/oidc/login") + assert resp.status_code == 302 + location = resp.headers["Location"] + assert location.startswith("https://auth.example.com/authorize") + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + def test_login_includes_required_params(self, mock_discovery, mock_config, client): + resp = client.get("/api/auth/oidc/login") + location = resp.headers["Location"] + parsed = urlparse(location) + params = parse_qs(parsed.query) + + assert params["client_id"] == ["shelfmark"] + assert params["response_type"] == ["code"] + assert "state" in params + assert "code_challenge" in params + assert params["code_challenge_method"] == ["S256"] + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + def test_login_includes_scopes(self, mock_discovery, mock_config, client): + resp = client.get("/api/auth/oidc/login") + location = resp.headers["Location"] + parsed = urlparse(location) + params = parse_qs(parsed.query) + + scope = params["scope"][0] + assert "openid" in scope + assert "email" in scope + assert "profile" in scope + assert "groups" in scope + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + def test_login_stores_state_in_session(self, mock_discovery, mock_config, client): + with client.session_transaction() as sess: + assert "oidc_state" not in sess + + client.get("/api/auth/oidc/login") + + with client.session_transaction() as sess: + assert "oidc_state" in sess + assert "oidc_code_verifier" in sess + + +class TestOIDCCallbackEndpoint: + """Tests for GET /api/auth/oidc/callback.""" + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + def test_callback_rejects_missing_state(self, mock_config, client): + resp = client.get("/api/auth/oidc/callback?code=abc123") + assert resp.status_code == 400 + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + def test_callback_rejects_mismatched_state(self, mock_config, client): + with client.session_transaction() as sess: + sess["oidc_state"] = "correct-state" + sess["oidc_code_verifier"] = "verifier" + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=wrong-state") + assert resp.status_code == 400 + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + @patch("shelfmark.core.oidc_routes._exchange_code") + def test_callback_creates_session(self, mock_exchange, mock_discovery, mock_config, client, user_db): + mock_exchange.return_value = { + "sub": "user-123", + "email": "john@example.com", + "name": "John Doe", + "preferred_username": "john", + "groups": ["users"], + } + + with client.session_transaction() as sess: + sess["oidc_state"] = "test-state" + sess["oidc_code_verifier"] = "test-verifier" + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + assert resp.status_code == 302 # Redirect to frontend + + with client.session_transaction() as sess: + assert sess["user_id"] == "john" + assert "oidc_state" not in sess # Cleaned up + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + @patch("shelfmark.core.oidc_routes._exchange_code") + def test_callback_sets_admin_from_groups(self, mock_exchange, mock_discovery, mock_config, client, user_db): + mock_exchange.return_value = { + "sub": "admin-123", + "email": "admin@example.com", + "preferred_username": "admin", + "groups": ["users", "shelfmark-admins"], + } + + with client.session_transaction() as sess: + sess["oidc_state"] = "test-state" + sess["oidc_code_verifier"] = "test-verifier" + + client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + + with client.session_transaction() as sess: + assert sess["is_admin"] is True + + @patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG) + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + @patch("shelfmark.core.oidc_routes._exchange_code") + def test_callback_provisions_user_in_db(self, mock_exchange, mock_discovery, mock_config, client, user_db): + mock_exchange.return_value = { + "sub": "user-789", + "email": "new@example.com", + "name": "New User", + "preferred_username": "newuser", + "groups": [], + } + + with client.session_transaction() as sess: + sess["oidc_state"] = "test-state" + sess["oidc_code_verifier"] = "test-verifier" + + client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + + user = user_db.get_user(oidc_subject="user-789") + assert user is not None + assert user["username"] == "newuser" + assert user["email"] == "new@example.com" + + @patch("shelfmark.core.oidc_routes.load_config_file") + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + @patch("shelfmark.core.oidc_routes._exchange_code") + def test_callback_rejects_when_auto_provision_disabled(self, mock_exchange, mock_discovery, mock_config, client, user_db): + config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False} + mock_config.return_value = config + + mock_exchange.return_value = { + "sub": "unknown-user", + "email": "unknown@example.com", + "preferred_username": "unknown", + "groups": [], + } + + with client.session_transaction() as sess: + sess["oidc_state"] = "test-state" + sess["oidc_code_verifier"] = "test-verifier" + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + assert resp.status_code == 403 + + @patch("shelfmark.core.oidc_routes.load_config_file") + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + @patch("shelfmark.core.oidc_routes._exchange_code") + def test_callback_allows_pre_created_user_by_email_when_no_provision( + self, mock_exchange, mock_discovery, mock_config, client, user_db + ): + """Pre-created user (by email) should log in even when auto-provision is off.""" + config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False} + mock_config.return_value = config + + # Admin pre-creates a user with this email (no oidc_subject yet) + user_db.create_user(username="alice", email="alice@example.com", password_hash="hash") + + mock_exchange.return_value = { + "sub": "oidc-alice-sub", + "email": "alice@example.com", + "preferred_username": "alice_oidc", + "groups": [], + } + + with client.session_transaction() as sess: + sess["oidc_state"] = "test-state" + sess["oidc_code_verifier"] = "test-verifier" + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + assert resp.status_code == 302 # Success, redirects to frontend + + with client.session_transaction() as sess: + assert sess["user_id"] == "alice" + assert sess.get("db_user_id") is not None + + @patch("shelfmark.core.oidc_routes.load_config_file") + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + @patch("shelfmark.core.oidc_routes._exchange_code") + def test_callback_links_oidc_subject_to_pre_created_user( + self, mock_exchange, mock_discovery, mock_config, client, user_db + ): + """When a pre-created user logs in via OIDC, their oidc_subject should be linked.""" + config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False} + mock_config.return_value = config + + user = user_db.create_user(username="bob", email="bob@example.com", password_hash="hash") + + mock_exchange.return_value = { + "sub": "oidc-bob-sub", + "email": "bob@example.com", + "preferred_username": "bob_oidc", + "groups": [], + } + + with client.session_transaction() as sess: + sess["oidc_state"] = "test-state" + sess["oidc_code_verifier"] = "test-verifier" + + client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + + # The OIDC subject should now be linked to the existing user + updated_user = user_db.get_user(user_id=user["id"]) + assert updated_user["oidc_subject"] == "oidc-bob-sub" + + @patch("shelfmark.core.oidc_routes.load_config_file") + @patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY) + @patch("shelfmark.core.oidc_routes._exchange_code") + def test_callback_rejects_unknown_email_when_no_provision( + self, mock_exchange, mock_discovery, mock_config, client, user_db + ): + """When auto-provision is off and no user matches by email, reject login.""" + config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False} + mock_config.return_value = config + + # Pre-create a user with a different email + user_db.create_user(username="charlie", email="charlie@example.com", password_hash="hash") + + mock_exchange.return_value = { + "sub": "oidc-unknown-sub", + "email": "stranger@example.com", + "preferred_username": "stranger", + "groups": [], + } + + with client.session_transaction() as sess: + sess["oidc_state"] = "test-state" + sess["oidc_code_verifier"] = "test-verifier" + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + assert resp.status_code == 403 diff --git a/tests/core/test_per_user_downloads.py b/tests/core/test_per_user_downloads.py new file mode 100644 index 00000000..38b64c3d --- /dev/null +++ b/tests/core/test_per_user_downloads.py @@ -0,0 +1,231 @@ +""" +Tests for per-user download scoping. + +Tests that DownloadTask has a user_id field and that the queue +can be filtered by user. +""" + +from shelfmark.core.models import DownloadTask +from shelfmark.core.queue import BookQueue + + +class TestDownloadTaskUserId: + """Tests that DownloadTask supports user_id.""" + + def test_download_task_has_user_id_field(self): + task = DownloadTask( + task_id="test-123", + source="direct_download", + title="Test Book", + user_id=42, + ) + assert task.user_id == 42 + + def test_download_task_user_id_defaults_to_none(self): + task = DownloadTask( + task_id="test-123", + source="direct_download", + title="Test Book", + ) + assert task.user_id is None + + def test_download_task_preserves_user_id_in_queue(self): + q = BookQueue() + task = DownloadTask( + task_id="test-123", + source="direct_download", + title="Test Book", + user_id=42, + ) + q.add(task) + retrieved = q.get_task("test-123") + assert retrieved.user_id == 42 + + +class TestQueueFilterByUser: + """Tests for filtering queue status by user.""" + + def _make_task(self, task_id, user_id=None): + return DownloadTask( + task_id=task_id, + source="direct_download", + title=f"Book {task_id}", + user_id=user_id, + ) + + def test_get_status_returns_all_when_no_filter(self): + q = BookQueue() + q.add(self._make_task("book-1", user_id=1)) + q.add(self._make_task("book-2", user_id=2)) + q.add(self._make_task("book-3", user_id=1)) + + status = q.get_status() + all_tasks = {} + for tasks_by_status in status.values(): + all_tasks.update(tasks_by_status) + assert len(all_tasks) == 3 + + def test_get_status_for_user_filters(self): + q = BookQueue() + q.add(self._make_task("book-1", user_id=1)) + q.add(self._make_task("book-2", user_id=2)) + q.add(self._make_task("book-3", user_id=1)) + + status = q.get_status(user_id=1) + all_tasks = {} + for tasks_by_status in status.values(): + all_tasks.update(tasks_by_status) + assert len(all_tasks) == 2 + assert "book-1" in all_tasks + assert "book-3" in all_tasks + assert "book-2" not in all_tasks + + def test_get_status_for_user_returns_empty_when_none(self): + q = BookQueue() + q.add(self._make_task("book-1", user_id=1)) + + status = q.get_status(user_id=999) + all_tasks = {} + for tasks_by_status in status.values(): + all_tasks.update(tasks_by_status) + assert len(all_tasks) == 0 + + def test_get_status_no_user_id_filter_includes_legacy_tasks(self): + """Tasks without user_id (legacy) are visible to everyone.""" + q = BookQueue() + q.add(self._make_task("book-1", user_id=None)) + q.add(self._make_task("book-2", user_id=1)) + + # No filter - see all + status = q.get_status() + all_tasks = {} + for tasks_by_status in status.values(): + all_tasks.update(tasks_by_status) + assert len(all_tasks) == 2 + + def test_get_status_user_filter_includes_legacy_tasks(self): + """Tasks without user_id are visible to any user (backward compat).""" + q = BookQueue() + q.add(self._make_task("book-1", user_id=None)) + q.add(self._make_task("book-2", user_id=1)) + + status = q.get_status(user_id=1) + all_tasks = {} + for tasks_by_status in status.values(): + all_tasks.update(tasks_by_status) + # User 1 sees their own + legacy (no user_id) + assert len(all_tasks) == 2 + + +# --------------------------------------------------------------------------- +# Per-user destination override in get_final_destination +# --------------------------------------------------------------------------- + + +class TestPerUserDestination: + """get_final_destination should respect per-user destination override in output_args.""" + + def test_uses_per_user_destination(self, monkeypatch): + """When output_args has a destination, it should be used instead of global.""" + from pathlib import Path + + task = DownloadTask( + task_id="book1", + source="direct_download", + title="Test Book", + output_args={"destination": "/user-books/alice"}, + ) + + monkeypatch.setattr( + "shelfmark.download.postprocess.destination.get_destination", + lambda is_audiobook=False: Path("/global/books"), + ) + monkeypatch.setattr( + "shelfmark.download.postprocess.destination.get_aa_content_type_dir", + lambda ct: None, + ) + + from shelfmark.download.postprocess.destination import get_final_destination + + result = get_final_destination(task) + assert result == Path("/user-books/alice") + + def test_falls_back_to_global_without_override(self, monkeypatch): + """When no per-user destination, should use global destination.""" + from pathlib import Path + + task = DownloadTask( + task_id="book1", + source="direct_download", + title="Test Book", + output_args={}, + ) + + monkeypatch.setattr( + "shelfmark.download.postprocess.destination.get_destination", + lambda is_audiobook=False: Path("/global/books"), + ) + monkeypatch.setattr( + "shelfmark.download.postprocess.destination.get_aa_content_type_dir", + lambda ct: None, + ) + + from shelfmark.download.postprocess.destination import get_final_destination + + result = get_final_destination(task) + assert result == Path("/global/books") + + def test_per_user_destination_empty_string_falls_back_to_global(self, monkeypatch): + """Empty string destination should fall back to global.""" + from pathlib import Path + + task = DownloadTask( + task_id="book1", + source="direct_download", + title="Test Book", + output_args={"destination": ""}, + ) + + monkeypatch.setattr( + "shelfmark.download.postprocess.destination.get_destination", + lambda is_audiobook=False: Path("/global/books"), + ) + monkeypatch.setattr( + "shelfmark.download.postprocess.destination.get_aa_content_type_dir", + lambda ct: None, + ) + + from shelfmark.download.postprocess.destination import get_final_destination + + result = get_final_destination(task) + assert result == Path("/global/books") + + +class TestTaskToDictUsername: + """Tests that _task_to_dict includes username for frontend display.""" + + def test_task_to_dict_includes_username(self): + """Username should be included in serialized task dict.""" + from shelfmark.download.orchestrator import _task_to_dict + + task = DownloadTask( + task_id="book1", + source="direct_download", + title="Test Book", + user_id=5, + username="alice", + ) + result = _task_to_dict(task) + assert result["username"] == "alice" + + def test_task_to_dict_username_none_when_no_auth(self): + """Username should be None when no user is set (no-auth mode).""" + from shelfmark.download.orchestrator import _task_to_dict + + task = DownloadTask( + task_id="book1", + source="direct_download", + title="Test Book", + ) + result = _task_to_dict(task) + assert result["username"] is None diff --git a/tests/core/test_user_db.py b/tests/core/test_user_db.py new file mode 100644 index 00000000..23092f3b --- /dev/null +++ b/tests/core/test_user_db.py @@ -0,0 +1,195 @@ +""" +Tests for SQLite user database. + +Tests CRUD operations on users and user_settings tables. +""" + +import os +import sqlite3 +import tempfile + +import pytest + + +@pytest.fixture +def db_path(): + """Create a temporary database path.""" + with tempfile.TemporaryDirectory() as tmpdir: + yield os.path.join(tmpdir, "shelfmark.db") + + +@pytest.fixture +def user_db(db_path): + """Create a UserDB instance with a temporary database.""" + from shelfmark.core.user_db import UserDB + + db = UserDB(db_path) + db.initialize() + return db + + +class TestUserDBInitialization: + """Tests for database creation and schema setup.""" + + def test_initialize_creates_database_file(self, db_path): + from shelfmark.core.user_db import UserDB + + db = UserDB(db_path) + db.initialize() + assert os.path.exists(db_path) + + def test_initialize_creates_users_table(self, user_db, db_path): + conn = sqlite3.connect(db_path) + cursor = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='users'" + ) + assert cursor.fetchone() is not None + conn.close() + + def test_initialize_creates_user_settings_table(self, user_db, db_path): + conn = sqlite3.connect(db_path) + cursor = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='user_settings'" + ) + assert cursor.fetchone() is not None + conn.close() + + def test_initialize_enables_wal_mode(self, user_db, db_path): + conn = sqlite3.connect(db_path) + cursor = conn.execute("PRAGMA journal_mode") + mode = cursor.fetchone()[0] + assert mode == "wal" + conn.close() + + def test_initialize_is_idempotent(self, db_path): + from shelfmark.core.user_db import UserDB + + db = UserDB(db_path) + db.initialize() + db.initialize() # Should not raise + assert os.path.exists(db_path) + + +class TestUserCRUD: + """Tests for user create, read, update, delete operations.""" + + def test_create_user(self, user_db): + user = user_db.create_user( + username="john", + email="john@example.com", + display_name="John Doe", + ) + assert user["id"] is not None + assert user["username"] == "john" + assert user["email"] == "john@example.com" + assert user["display_name"] == "John Doe" + assert user["role"] == "user" + + def test_create_user_with_password(self, user_db): + user = user_db.create_user( + username="admin", + password_hash="hashed_pw", + role="admin", + ) + assert user["role"] == "admin" + assert user["password_hash"] == "hashed_pw" + + def test_create_user_with_oidc_subject(self, user_db): + user = user_db.create_user( + username="oidcuser", + oidc_subject="sub-12345", + email="oidc@example.com", + ) + assert user["oidc_subject"] == "sub-12345" + + def test_create_duplicate_username_fails(self, user_db): + user_db.create_user(username="john") + with pytest.raises(ValueError, match="already exists"): + user_db.create_user(username="john") + + def test_create_duplicate_oidc_subject_fails(self, user_db): + user_db.create_user(username="user1", oidc_subject="sub-123") + with pytest.raises(ValueError, match="already exists"): + user_db.create_user(username="user2", oidc_subject="sub-123") + + def test_get_user_by_id(self, user_db): + created = user_db.create_user(username="john") + fetched = user_db.get_user(user_id=created["id"]) + assert fetched["username"] == "john" + + def test_get_user_by_username(self, user_db): + user_db.create_user(username="john", email="john@example.com") + fetched = user_db.get_user(username="john") + assert fetched["email"] == "john@example.com" + + def test_get_user_by_oidc_subject(self, user_db): + user_db.create_user(username="john", oidc_subject="sub-123") + fetched = user_db.get_user(oidc_subject="sub-123") + assert fetched["username"] == "john" + + def test_get_nonexistent_user_returns_none(self, user_db): + assert user_db.get_user(username="nobody") is None + + def test_update_user(self, user_db): + user = user_db.create_user(username="john", role="user") + user_db.update_user(user["id"], role="admin", email="new@example.com") + updated = user_db.get_user(user_id=user["id"]) + assert updated["role"] == "admin" + assert updated["email"] == "new@example.com" + + def test_update_nonexistent_user_raises(self, user_db): + with pytest.raises(ValueError, match="not found"): + user_db.update_user(9999, role="admin") + + def test_delete_user(self, user_db): + user = user_db.create_user(username="john") + user_db.delete_user(user["id"]) + assert user_db.get_user(user_id=user["id"]) is None + + def test_delete_user_cascades_settings(self, user_db): + user = user_db.create_user(username="john") + user_db.set_user_settings(user["id"], {"booklore_library_id": 1}) + user_db.delete_user(user["id"]) + assert user_db.get_user_settings(user["id"]) == {} + + def test_list_users(self, user_db): + user_db.create_user(username="alice") + user_db.create_user(username="bob") + user_db.create_user(username="charlie") + users = user_db.list_users() + assert len(users) == 3 + usernames = [u["username"] for u in users] + assert "alice" in usernames + assert "bob" in usernames + assert "charlie" in usernames + + +class TestUserSettings: + """Tests for per-user settings.""" + + def test_set_and_get_user_settings(self, user_db): + user = user_db.create_user(username="john") + settings = {"booklore_library_id": 5, "booklore_path_id": 2} + user_db.set_user_settings(user["id"], settings) + fetched = user_db.get_user_settings(user["id"]) + assert fetched["booklore_library_id"] == 5 + assert fetched["booklore_path_id"] == 2 + + def test_get_settings_for_user_without_settings(self, user_db): + user = user_db.create_user(username="john") + assert user_db.get_user_settings(user["id"]) == {} + + def test_update_user_settings_merges(self, user_db): + user = user_db.create_user(username="john") + user_db.set_user_settings(user["id"], {"key1": "val1"}) + user_db.set_user_settings(user["id"], {"key2": "val2"}) + settings = user_db.get_user_settings(user["id"]) + assert settings["key1"] == "val1" + assert settings["key2"] == "val2" + + def test_update_user_settings_overwrites_existing_key(self, user_db): + user = user_db.create_user(username="john") + user_db.set_user_settings(user["id"], {"key1": "old"}) + user_db.set_user_settings(user["id"], {"key1": "new"}) + settings = user_db.get_user_settings(user["id"]) + assert settings["key1"] == "new" diff --git a/tests/core/test_user_template_variable.py b/tests/core/test_user_template_variable.py new file mode 100644 index 00000000..c4268612 --- /dev/null +++ b/tests/core/test_user_template_variable.py @@ -0,0 +1,84 @@ +"""Tests for {User} template variable in folder destination paths.""" + +from shelfmark.core.naming import KNOWN_TOKENS, parse_naming_template + + +class TestUserInKnownTokens: + """User should be a recognized template token.""" + + def test_user_in_known_tokens(self): + assert "user" in KNOWN_TOKENS + + def test_user_token_parsed(self): + result = parse_naming_template("{User}", {"User": "alice"}) + assert result == "alice" + + def test_user_token_case_insensitive(self): + result = parse_naming_template("{user}", {"User": "alice"}) + assert result == "alice" + + +class TestUserTemplateSubstitution: + """User variable should work in organize templates with path separators.""" + + def test_user_in_organize_template(self): + metadata = {"Author": "Author1", "Title": "Book1", "Year": "2024", "User": "alice"} + result = parse_naming_template("{User}/{Author}/{Title} ({Year})", metadata) + assert result == "alice/Author1/Book1 (2024)" + + def test_user_empty_when_not_set(self): + metadata = {"Author": "Author1", "Title": "Book1", "User": None} + result = parse_naming_template("{User}/{Author}/{Title}", metadata) + # Empty user should be cleaned up, no leading slash + assert result == "Author1/Book1" + + def test_user_with_prefix_suffix(self): + metadata = {"Author": "Author1", "Title": "Book1", "User": "bob"} + result = parse_naming_template("{User}/books/{Author}/{Title}", metadata) + assert result == "bob/books/Author1/Book1" + + def test_user_sanitized(self): + metadata = {"User": "user:with*special", "Title": "Book1"} + result = parse_naming_template("{User}/{Title}", metadata) + # Special chars should be replaced with underscores + assert ":" not in result + assert "*" not in result + + def test_user_missing_from_metadata(self): + metadata = {"Author": "Author1", "Title": "Book1"} + result = parse_naming_template("{User}/{Author}/{Title}", metadata) + assert result == "Author1/Book1" + + +class TestBuildMetadataWithUser: + """build_metadata_dict should include User when task has user_id.""" + + def test_build_metadata_includes_user(self): + from shelfmark.core.models import DownloadTask + from shelfmark.download.postprocess.transfer import build_metadata_dict + + task = DownloadTask( + task_id="test-1", + source="direct_download", + title="Book1", + author="Author1", + user_id=1, + username="alice", + ) + metadata = build_metadata_dict(task) + assert metadata["User"] == "alice" + + def test_build_metadata_user_none_when_no_user_id(self): + from shelfmark.core.models import DownloadTask + from shelfmark.download.postprocess.transfer import build_metadata_dict + + task = DownloadTask( + task_id="test-2", + source="direct_download", + title="Book1", + author="Author1", + user_id=None, + username=None, + ) + metadata = build_metadata_dict(task) + assert metadata.get("User") is None diff --git a/tests/e2e/test_auth_flow.py b/tests/e2e/test_auth_flow.py index 7f96c913..41ffd419 100644 --- a/tests/e2e/test_auth_flow.py +++ b/tests/e2e/test_auth_flow.py @@ -33,7 +33,7 @@ class TestAuthenticationFlow: data = resp.json() assert "auth_mode" in data # Should be one of the valid auth modes - assert data["auth_mode"] in ["none", "builtin", "cwa", "proxy"] + assert data["auth_mode"] in ["none", "builtin", "cwa", "proxy", "oidc"] def test_auth_check_includes_admin_status(self, api_client: APIClient): """Test that auth check includes admin status."""