diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 06fcc68..1f8b64b 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -12,6 +12,8 @@ services: - SYS_PTRACE environment: DEBUG: true + # HIDE_LOCAL_AUTH: true + OIDC_AUTO_REDIRECT: true volumes: - ./.local/config:/config - ./.local/books:/books diff --git a/docs/oidc.md b/docs/oidc.md index 169622b..ec5ecfd 100644 --- a/docs/oidc.md +++ b/docs/oidc.md @@ -32,6 +32,17 @@ Configure in **Settings → Security → Authentication Method → OIDC**. Use **Test Connection** to verify discovery and client configuration before attempting login. +## Environment Variables + +These optional environment variables control login page behavior when OIDC is enabled. + +| Variable | Description | Default | +|----------|-------------|---------| +| `HIDE_LOCAL_AUTH` | Hide the username/password login option, so only the OIDC button is shown | `false` | +| `OIDC_AUTO_REDIRECT` | Automatically redirect to the OIDC provider instead of showing the login page | `false` | + +If both are enabled, users are redirected straight to the OIDC provider. On failure they return to the login page with an error message but no password fallback. + ## Troubleshooting - **Issuer validation failed** — The issuer in the token doesn't match the discovery document. Check your provider's external URL / issuer configuration. diff --git a/shelfmark/config/env.py b/shelfmark/config/env.py index 3fb6495..6d488c2 100644 --- a/shelfmark/config/env.py +++ b/shelfmark/config/env.py @@ -115,6 +115,8 @@ FLASK_PORT = int(os.getenv("FLASK_PORT", "8084")) SESSION_COOKIE_SECURE_ENV = os.getenv("SESSION_COOKIE_SECURE", "false") SESSION_COOKIE_NAME = "shelfmark_session" CWA_DB_PATH = _resolve_cwa_db_path() +HIDE_LOCAL_AUTH = string_to_bool(os.getenv("HIDE_LOCAL_AUTH", "false")) +OIDC_AUTO_REDIRECT = string_to_bool(os.getenv("OIDC_AUTO_REDIRECT", "false")) # ============================================================================= diff --git a/shelfmark/config/security.py b/shelfmark/config/security.py index b211d49..de81607 100644 --- a/shelfmark/config/security.py +++ b/shelfmark/config/security.py @@ -18,6 +18,7 @@ from shelfmark.core.settings_registry import ( CheckboxField, ActionButton, TagListField, + CustomComponentField, ) from shelfmark.core.user_db import sync_builtin_admin_user @@ -59,9 +60,10 @@ def _on_save_security(values: Dict[str, Any]) -> Dict[str, Any]: return on_save_security(values) -def _test_oidc_connection() -> Dict[str, Any]: +def _test_oidc_connection(current_values: Dict[str, Any] = None) -> Dict[str, Any]: return test_oidc_connection( load_security_config=lambda: load_config_file("security"), + current_values=current_values or {}, logger=logger, ) @@ -95,12 +97,18 @@ def security_settings(): default="none", env_supported=False, ), + CustomComponentField( + key="oidc_admin_requirement", + component="oidc_admin_hint", + label="A local admin account is required before OIDC can be enabled.", + show_when=_auth_condition("oidc"), + ), ActionButton( key="open_users_tab", label="Go to Users", description="Configure local users and admin access in the Users tab.", style="primary", - show_when=_auth_condition("builtin"), + show_when={"field": "AUTH_METHOD", "value": ["builtin", "oidc"]}, ), _auth_ui_field( TextField, @@ -140,6 +148,16 @@ def security_settings(): ), ] + fields.append( + CustomComponentField( + key="oidc_callback_url", + component="settings_label", + label="Callback URL", + description="{origin}/api/auth/oidc/callback", + show_when=_auth_condition("oidc"), + ) + ) + oidc_specs = [ ( TextField, @@ -239,6 +257,16 @@ def security_settings(): show_when=_auth_condition("oidc"), ) ) + fields.append( + CustomComponentField( + key="oidc_env_info", + component="oidc_env_info", + label="Environment-Only Options", + description="These options can only be set via environment variables because changing them through the UI could lock you out of the application.", + wrap_in_field_wrapper=True, + show_when=_auth_condition("oidc"), + ) + ) return fields diff --git a/shelfmark/config/security_handlers.py b/shelfmark/config/security_handlers.py index a340d13..0ade312 100644 --- a/shelfmark/config/security_handlers.py +++ b/shelfmark/config/security_handlers.py @@ -7,7 +7,7 @@ from shelfmark.core.utils import normalize_http_url from shelfmark.core.user_db import UserDB -_OIDC_LOCKOUT_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." +_OIDC_LOCKOUT_MESSAGE = "A local admin account with a password is required before enabling OIDC. Use the 'Go to Users' button above to create one. This ensures you can still sign in if your identity provider is unavailable." def _has_local_password_admin() -> bool: @@ -47,13 +47,15 @@ def on_save_security( def test_oidc_connection( *, load_security_config: Callable[[], dict[str, Any]], + current_values: dict[str, Any] | None = None, logger: Any, ) -> dict[str, Any]: """Fetch and validate the configured OIDC discovery document.""" import requests try: - discovery_url = load_security_config().get("OIDC_DISCOVERY_URL", "") + # Prefer the current (unsaved) form value over the saved config + discovery_url = (current_values or {}).get("OIDC_DISCOVERY_URL") or load_security_config().get("OIDC_DISCOVERY_URL", "") if not discovery_url: return {"success": False, "message": "Discovery URL is not configured."} diff --git a/shelfmark/core/oidc_routes.py b/shelfmark/core/oidc_routes.py index 5ee5569..4dc2740 100644 --- a/shelfmark/core/oidc_routes.py +++ b/shelfmark/core/oidc_routes.py @@ -5,6 +5,7 @@ Business logic remains in oidc_auth.py. """ from typing import Any +from urllib.parse import quote from authlib.jose.errors import InvalidClaimError from authlib.integrations.flask_client import OAuth @@ -56,6 +57,13 @@ def _has_username_or_email(claims: dict[str, Any]) -> bool: return False +def _login_error_url(message: str) -> str: + """Build a login URL (with script_root) that includes an OIDC error message.""" + script_root = request.script_root.rstrip("/") + login_url = f"{script_root}/login" if script_root else "/login" + return f"{login_url}?oidc_error={quote(message)}" + + def _get_oidc_client() -> tuple[Any, dict[str, Any]]: """Register and return an OIDC client from the current security config.""" config = load_config_file("security") @@ -126,7 +134,7 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None: error = request.args.get("error") if error: logger.warning(f"OIDC callback error from IdP: {error}") - return jsonify({"error": "Authentication failed"}), 400 + return redirect(_login_error_url("Authentication failed")) client, config = _get_oidc_client() try: @@ -150,19 +158,13 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None: provider_issuer or "", ) if claim_name == "iss": - return ( - jsonify( - { - "error": ( - "OIDC issuer validation failed. Verify your discovery URL and IdP issuer/" - "external URL configuration." - ) - } - ), - 400, + msg = ( + "OIDC issuer validation failed. Verify your discovery URL and IdP issuer/" + "external URL configuration." ) + return redirect(_login_error_url(msg)) - return jsonify({"error": f"OIDC token claim validation failed: {claim_name}"}), 400 + return redirect(_login_error_url(f"OIDC token claim validation failed: {claim_name}")) claims = _normalize_claims(token.get("userinfo")) # If userinfo is missing or claims are too sparse, request it explicitly. @@ -178,7 +180,9 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None: claims = {**claims, **fetched_claims} if not claims: - raise ValueError("OIDC authentication failed: missing user claims") + msg = "OIDC authentication failed: missing user claims" + logger.error(msg) + return redirect(_login_error_url(msg)) group_claim = config.get("OIDC_GROUP_CLAIM", "groups") admin_group = config.get("OIDC_ADMIN_GROUP", "") @@ -204,7 +208,7 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None: logger.warning( f"OIDC login rejected: auto-provision disabled for {user_info['username']}" ) - return jsonify({"error": "Account not found. Contact your administrator."}), 403 + return redirect(_login_error_url("Account not found. Contact your administrator.")) session["user_id"] = user["username"] session["is_admin"] = user.get("role") == "admin" @@ -216,7 +220,7 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None: except ValueError as e: logger.error(f"OIDC callback error: {e}") - return jsonify({"error": str(e)}), 400 + return redirect(_login_error_url(str(e))) except Exception as e: logger.error(f"OIDC callback error: {e}") - return jsonify({"error": "Authentication failed"}), 500 + return redirect(_login_error_url("Authentication failed")) diff --git a/shelfmark/core/user_db.py b/shelfmark/core/user_db.py index a59a1e0..9c0b0ae 100644 --- a/shelfmark/core/user_db.py +++ b/shelfmark/core/user_db.py @@ -380,6 +380,7 @@ class UserDB: with self._lock: conn = self._connect() try: + conn.execute("UPDATE download_requests SET reviewed_by = NULL WHERE reviewed_by = ?", (user_id,)) conn.execute("DELETE FROM users WHERE id = ?", (user_id,)) conn.commit() finally: diff --git a/shelfmark/download/clients/qbittorrent.py b/shelfmark/download/clients/qbittorrent.py index d7fb474..05a5fa6 100644 --- a/shelfmark/download/clients/qbittorrent.py +++ b/shelfmark/download/clients/qbittorrent.py @@ -1,7 +1,6 @@ """qBittorrent download client for Prowlarr integration.""" import time -from pathlib import Path from types import SimpleNamespace from typing import Optional, Tuple @@ -503,37 +502,14 @@ class QBittorrentClient(DownloadClient): Centralizes the logic shared by `get_status()` and `get_download_path()`: - accept `content_path` only when it's not equal to `save_path` - - when the torrent is complete and both `content_path` and `save_path` are present, - prefer a path rooted at `save_path` to avoid races where qBittorrent briefly reports - a temp/incomplete `content_path` and then moves the payload - otherwise derive via properties+files - finally fall back to `save_path + name` """ - torrent_progress = getattr(torrent, "progress", 0.0) - try: - progress = float(torrent_progress) - except (TypeError, ValueError): - progress = 0.0 - # Prefer content_path, but treat content_path == save_path as invalid. content_path = getattr(torrent, "content_path", "") save_path = getattr(torrent, "save_path", "") if content_path and (not save_path or str(content_path) != str(save_path)): - # When using a temp/incomplete directory, qBittorrent can briefly keep reporting - # `content_path` under that temp path right at completion, then move the files - # into `save_path`. Returning the temp path can race with that move. - if save_path and progress >= 1.0: - # Use the basename of content_path under save_path (works for single-file - # torrents and multi-file torrents where content_path is a top-level dir). - try: - content_basename = str(Path(str(content_path)).name) - except Exception: - content_basename = "" - rooted = self._build_path(str(save_path), content_basename) - if rooted: - return rooted - return str(content_path) download_id = getattr(torrent, "hash", "") diff --git a/shelfmark/main.py b/shelfmark/main.py index 2fedfad..b051da8 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -21,8 +21,9 @@ from shelfmark.download import orchestrator as backend from shelfmark.release_sources.direct_download import SearchUnavailable from shelfmark.config.settings import _SUPPORTED_BOOK_LANGUAGE from shelfmark.config.env import ( - BUILD_VERSION, CONFIG_DIR, CWA_DB_PATH, DEBUG, FLASK_HOST, FLASK_PORT, - RELEASE_VERSION, _is_config_dir_writable, + BUILD_VERSION, CONFIG_DIR, CWA_DB_PATH, DEBUG, HIDE_LOCAL_AUTH, + FLASK_HOST, FLASK_PORT, OIDC_AUTO_REDIRECT, RELEASE_VERSION, + _is_config_dir_writable, ) from shelfmark.core.config import config as app_config from shelfmark.core.logger import setup_logger @@ -1596,6 +1597,9 @@ def api_login() -> Union[Response, Tuple[Response, int]]: if auth_mode == "proxy": return jsonify({"error": "Proxy authentication is enabled"}), 401 + if auth_mode == "oidc" and HIDE_LOCAL_AUTH: + return jsonify({"error": "Local authentication is disabled"}), 403 + username = data.get('username', '').strip() password = data.get('password', '') remember_me = data.get('remember_me', False) @@ -1794,11 +1798,15 @@ def api_auth_check() -> Union[Response, Tuple[Response, int]]: if logout_url: response_data["logout_url"] = logout_url - # Add custom OIDC button label if configured + # Add custom OIDC button label and SSO enforcement flags if configured if auth_mode == "oidc": oidc_button_label = security_config.get("OIDC_BUTTON_LABEL", "") if oidc_button_label: response_data["oidc_button_label"] = oidc_button_label + if HIDE_LOCAL_AUTH: + response_data["hide_local_auth"] = True + if OIDC_AUTO_REDIRECT: + response_data["oidc_auto_redirect"] = True return jsonify(response_data) except Exception as e: diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index adee3e5..9328dd7 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -150,9 +150,12 @@ function App() { username, displayName, oidcButtonLabel, + hideLocalAuth, + oidcAutoRedirect, loginError, isLoggingIn, setIsAuthenticated, + refreshAuth, handleLogin, handleLogout, } = useAuth({ @@ -1409,6 +1412,7 @@ function App() { onClose={() => setSettingsOpen(false)} onShowToast={showToast} onSettingsSaved={handleSettingsSaved} + onRefreshAuth={refreshAuth} /> ) } diff --git a/src/frontend/src/components/LoginForm.tsx b/src/frontend/src/components/LoginForm.tsx index 5aba512..994f74e 100644 --- a/src/frontend/src/components/LoginForm.tsx +++ b/src/frontend/src/components/LoginForm.tsx @@ -1,4 +1,5 @@ import { FormEvent, KeyboardEvent, useEffect, useRef, useState } from 'react'; +import { useSearchParams } from 'react-router-dom'; import { LoginCredentials } from '../types'; import { withBasePath } from '../utils/basePath'; @@ -9,6 +10,8 @@ interface LoginFormProps { autoFocus?: boolean; authMode?: string; oidcButtonLabel?: string | null; + hideLocalAuth?: boolean; + oidcAutoRedirect?: boolean; } const EyeIcon = () => ( @@ -219,9 +222,13 @@ export const LoginForm = ({ autoFocus = true, authMode, oidcButtonLabel, + hideLocalAuth = false, + oidcAutoRedirect = false, }: LoginFormProps) => { const isOidc = authMode === 'oidc'; const [showPasswordLogin, setShowPasswordLogin] = useState(false); + const [searchParams] = useSearchParams(); + const oidcError = searchParams.get('oidc_error'); // Auto-expand password form if there's an error (likely from a password attempt) useEffect(() => { @@ -230,6 +237,13 @@ export const LoginForm = ({ } }, [error, isOidc]); + // Auto-redirect to OIDC provider when enabled and no errors present + useEffect(() => { + if (oidcAutoRedirect && isOidc && !error && !oidcError) { + window.location.href = withBasePath('/api/auth/oidc/login'); + } + }, [oidcAutoRedirect, isOidc, error, oidcError]); + const handleSubmit = (e: FormEvent) => { e.preventDefault(); const formData = new FormData(e.currentTarget); @@ -245,11 +259,13 @@ export const LoginForm = ({ } }; + const displayError = oidcError || error; + return (
- {error && ( + {displayError && (
- {error} + {displayError}
)} @@ -262,22 +278,26 @@ export const LoginForm = ({ {oidcButtonLabel || 'Sign in with OIDC'} -
-
- -
-
+ {!hideLocalAuth && ( + <> +
+
+ +
+
- {showPasswordLogin && ( -
- -
+ {showPasswordLogin && ( +
+ +
+ )} + )} ) : ( diff --git a/src/frontend/src/components/settings/SettingsContent.tsx b/src/frontend/src/components/settings/SettingsContent.tsx index 489f39b..ddbb3f1 100644 --- a/src/frontend/src/components/settings/SettingsContent.tsx +++ b/src/frontend/src/components/settings/SettingsContent.tsx @@ -53,6 +53,7 @@ interface SettingsContentProps { authMode?: string; onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void; onRefreshOverrideSummary?: () => void; + onRefreshAuth?: () => Promise; }; } @@ -407,6 +408,7 @@ export const SettingsContent = ({ authMode: customFieldContext?.authMode, onShowToast: customFieldContext?.onShowToast, onRefreshOverrideSummary: customFieldContext?.onRefreshOverrideSummary, + onRefreshAuth: customFieldContext?.onRefreshAuth, }) : renderField( field, diff --git a/src/frontend/src/components/settings/SettingsModal.tsx b/src/frontend/src/components/settings/SettingsModal.tsx index e0ce345..660fc43 100644 --- a/src/frontend/src/components/settings/SettingsModal.tsx +++ b/src/frontend/src/components/settings/SettingsModal.tsx @@ -13,9 +13,10 @@ interface SettingsModalProps { onClose: () => void; onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void; onSettingsSaved?: () => void; + onRefreshAuth?: () => Promise; } -export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettingsSaved }: SettingsModalProps) => { +export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettingsSaved, onRefreshAuth }: SettingsModalProps) => { const { tabs, groups, @@ -301,6 +302,7 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin authMode: usersAuthMode, onShowToast, onRefreshOverrideSummary: handleRefreshCurrentTabOverrideSummary, + onRefreshAuth, }} /> )) diff --git a/src/frontend/src/components/settings/customFields/OidcAdminHint.tsx b/src/frontend/src/components/settings/customFields/OidcAdminHint.tsx new file mode 100644 index 0000000..2d33abc --- /dev/null +++ b/src/frontend/src/components/settings/customFields/OidcAdminHint.tsx @@ -0,0 +1,31 @@ +import { useEffect, useState } from 'react'; +import { getAdminUsers } from '../../../services/api'; +import { CustomSettingsFieldRendererProps } from './types'; + +export const OidcAdminHint = ({ field }: CustomSettingsFieldRendererProps) => { + const [needsAdmin, setNeedsAdmin] = useState(null); + + useEffect(() => { + let cancelled = false; + getAdminUsers() + .then((users) => { + if (!cancelled) { + setNeedsAdmin(!users.some(u => u.role === 'admin' && u.auth_source === 'builtin')); + } + }) + .catch(() => { + if (!cancelled) { + setNeedsAdmin(true); + } + }); + return () => { cancelled = true; }; + }, []); + + if (!needsAdmin) return null; + + return ( +
+ {field.label} +
+ ); +}; diff --git a/src/frontend/src/components/settings/customFields/OidcEnvInfo.tsx b/src/frontend/src/components/settings/customFields/OidcEnvInfo.tsx new file mode 100644 index 0000000..1512eb1 --- /dev/null +++ b/src/frontend/src/components/settings/customFields/OidcEnvInfo.tsx @@ -0,0 +1,26 @@ +import { CustomSettingsFieldRendererProps } from './types'; + +export const OidcEnvInfo = (_props: CustomSettingsFieldRendererProps) => { + return ( +
+
+ docker-compose.yml +
+ 
+        
+          environment:{'\n'}
+          {'  '}- HIDE_LOCAL_AUTH=true
+          {'    '}# Hide the local login form{'\n'}
+          {'  '}- OIDC_AUTO_REDIRECT=true
+          {'  '}# Skip login page, redirect straight to OIDC
+        
+
+
+ ); +}; diff --git a/src/frontend/src/components/settings/customFields/SettingsLabel.tsx b/src/frontend/src/components/settings/customFields/SettingsLabel.tsx new file mode 100644 index 0000000..5fc8f3e --- /dev/null +++ b/src/frontend/src/components/settings/customFields/SettingsLabel.tsx @@ -0,0 +1,15 @@ +import { CustomSettingsFieldRendererProps } from './types'; + +const interpolate = (text: string): string => + text.replace(/\{origin\}/g, window.location.origin); + +export const SettingsLabel = ({ field }: CustomSettingsFieldRendererProps) => { + return ( +
+ {field.label && {field.label} } + {field.description && ( + {interpolate(field.description)} + )} +
+ ); +}; diff --git a/src/frontend/src/components/settings/customFields/UsersManagementField.tsx b/src/frontend/src/components/settings/customFields/UsersManagementField.tsx index e1384bc..7ec73be 100644 --- a/src/frontend/src/components/settings/customFields/UsersManagementField.tsx +++ b/src/frontend/src/components/settings/customFields/UsersManagementField.tsx @@ -21,6 +21,7 @@ export const UsersManagementField = ({ authMode, onShowToast, onRefreshOverrideSummary, + onRefreshAuth, }: CustomSettingsFieldRendererProps) => { const { route, openCreate, openEdit, openEditOverrides, backToList } = useUsersPanelState(); const activeEditRequestIdRef = useRef(0); @@ -111,6 +112,7 @@ export const UsersManagementField = ({ }; const canCreateLocalUsers = canCreateLocalUsersForAuthMode(authMode || 'none'); + const needsLocalAdmin = !users.some(u => u.role === 'admin' && u.auth_source === 'builtin'); const handleBackToList = () => { onUiStateChange('routeKind', 'list'); @@ -129,6 +131,7 @@ export const UsersManagementField = ({ const ok = await createUser(); if (ok) { onRefreshOverrideSummary?.(); + onRefreshAuth?.(); backToList(); } }; @@ -213,9 +216,10 @@ export const UsersManagementField = ({ const ok = await deleteUser(userId); if (ok) { onRefreshOverrideSummary?.(); + onRefreshAuth?.(); } return ok; - }, [deleteUser, onRefreshOverrideSummary]); + }, [deleteUser, onRefreshAuth, onRefreshOverrideSummary]); useEffect(() => { if (route.kind !== 'edit-overrides') { @@ -263,7 +267,13 @@ export const UsersManagementField = ({ loadingUsers={loading} loadError={loadError} onRetryLoadUsers={() => void fetchUsers({ force: true })} - onCreate={openCreate} + onCreate={() => { + if (needsLocalAdmin) { + setCreateForm({ ...createForm, role: 'admin' }); + } + openCreate(); + }} + needsLocalAdmin={needsLocalAdmin} showCreateForm={route.kind === 'create'} createForm={createForm} onCreateFormChange={setCreateForm} diff --git a/src/frontend/src/components/settings/customFields/index.tsx b/src/frontend/src/components/settings/customFields/index.tsx index 8c6f3d7..8d5c8c1 100644 --- a/src/frontend/src/components/settings/customFields/index.tsx +++ b/src/frontend/src/components/settings/customFields/index.tsx @@ -1,5 +1,8 @@ import { ComponentType, ReactNode } from 'react'; +import { OidcAdminHint } from './OidcAdminHint'; +import { OidcEnvInfo } from './OidcEnvInfo'; import { RequestPolicyGridField } from './RequestPolicyGridField'; +import { SettingsLabel } from './SettingsLabel'; import { UsersManagementField } from './UsersManagementField'; import { CustomSettingsFieldLayout, @@ -39,6 +42,15 @@ const CUSTOM_FIELD_DEFINITIONS: Record = { request_policy_grid: { renderer: RequestPolicyGridField, }, + settings_label: { + renderer: SettingsLabel, + }, + oidc_admin_hint: { + renderer: OidcAdminHint, + }, + oidc_env_info: { + renderer: OidcEnvInfo, + }, }; export const renderCustomSettingsField = ( diff --git a/src/frontend/src/components/settings/customFields/types.ts b/src/frontend/src/components/settings/customFields/types.ts index 9351212..9c7bfa2 100644 --- a/src/frontend/src/components/settings/customFields/types.ts +++ b/src/frontend/src/components/settings/customFields/types.ts @@ -13,6 +13,7 @@ export interface CustomSettingsFieldRendererProps { authMode?: string; onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void; onRefreshOverrideSummary?: () => void; + onRefreshAuth?: () => Promise; } export interface CustomSettingsFieldLayout { diff --git a/src/frontend/src/components/settings/users/UserCard.tsx b/src/frontend/src/components/settings/users/UserCard.tsx index 8fb884b..5c2c8d0 100644 --- a/src/frontend/src/components/settings/users/UserCard.tsx +++ b/src/frontend/src/components/settings/users/UserCard.tsx @@ -336,6 +336,7 @@ interface UserCreateCardProps { onChange: (form: CreateUserFormState) => void; creating: boolean; isFirstUser: boolean; + needsLocalAdmin?: boolean; onSubmit: () => void; onCancel: () => void; } @@ -345,6 +346,7 @@ export const UserCreateCard = ({ onChange, creating, isFirstUser, + needsLocalAdmin = false, onSubmit, onCancel, }: UserCreateCardProps) => { @@ -368,6 +370,11 @@ export const UserCreateCard = ({ This will be the first account and will be created as admin.

)} + {needsLocalAdmin && !isFirstUser && ( +

+ An admin account is required before OIDC can be enabled. +

+ )}
{renderTextField(usernameField, form.username, (value) => onChange({ ...form, username: value }))} diff --git a/src/frontend/src/components/settings/users/UserListView.tsx b/src/frontend/src/components/settings/users/UserListView.tsx index 4ab7aea..e08d7f9 100644 --- a/src/frontend/src/components/settings/users/UserListView.tsx +++ b/src/frontend/src/components/settings/users/UserListView.tsx @@ -12,6 +12,7 @@ interface UserListViewProps { loadingUsers: boolean; loadError: string | null; onRetryLoadUsers: () => void; + needsLocalAdmin: boolean; onCreate: () => void; showCreateForm: boolean; createForm: CreateUserFormState; @@ -46,6 +47,7 @@ export const UserListView = ({ loadingUsers, loadError, onRetryLoadUsers, + needsLocalAdmin, onCreate, showCreateForm, createForm, @@ -225,6 +227,7 @@ export const UserListView = ({ onChange={onCreateFormChange} creating={creating} isFirstUser={isFirstUser} + needsLocalAdmin={needsLocalAdmin} onSubmit={onCreateSubmit} onCancel={onCancelCreate} /> diff --git a/src/frontend/src/hooks/useAuth.ts b/src/frontend/src/hooks/useAuth.ts index e3b9a6e..a8fa685 100644 --- a/src/frontend/src/hooks/useAuth.ts +++ b/src/frontend/src/hooks/useAuth.ts @@ -18,9 +18,12 @@ interface UseAuthReturn { username: string | null; displayName: string | null; oidcButtonLabel: string | null; + hideLocalAuth: boolean; + oidcAutoRedirect: boolean; loginError: string | null; isLoggingIn: boolean; setIsAuthenticated: (value: boolean) => void; + refreshAuth: () => Promise; handleLogin: (credentials: LoginCredentials) => Promise; handleLogout: () => Promise; } @@ -38,6 +41,8 @@ export function useAuth(options: UseAuthOptions = {}): UseAuthReturn { const [username, setUsername] = useState(null); const [displayName, setDisplayName] = useState(null); const [oidcButtonLabel, setOidcButtonLabel] = useState(null); + const [hideLocalAuth, setHideLocalAuth] = useState(false); + const [oidcAutoRedirect, setOidcAutoRedirect] = useState(false); const [loginError, setLoginError] = useState(null); const [isLoggingIn, setIsLoggingIn] = useState(false); @@ -49,6 +54,8 @@ export function useAuth(options: UseAuthOptions = {}): UseAuthReturn { setUsername(response.username || null); setDisplayName(response.display_name || null); setOidcButtonLabel(response.oidc_button_label || null); + setHideLocalAuth(response.hide_local_auth || false); + setOidcAutoRedirect(response.oidc_auto_redirect || false); }, []); const refreshSocketSession = useCallback(() => { @@ -107,6 +114,14 @@ export function useAuth(options: UseAuthOptions = {}): UseAuthReturn { }; }, [applyAuthResponse]); + const refreshAuth = useCallback(async () => { + try { + applyAuthResponse(await checkAuth()); + } catch (error) { + console.error('Auth refresh failed:', error); + } + }, [applyAuthResponse]); + const handleLogin = useCallback(async (credentials: LoginCredentials) => { setIsLoggingIn(true); setLoginError(null); @@ -145,6 +160,8 @@ export function useAuth(options: UseAuthOptions = {}): UseAuthReturn { setUsername(null); setDisplayName(null); setOidcButtonLabel(null); + setHideLocalAuth(false); + setOidcAutoRedirect(false); onLogoutSuccess?.(); navigate('/login', { replace: true }); } catch (error) { @@ -162,9 +179,12 @@ export function useAuth(options: UseAuthOptions = {}): UseAuthReturn { username, displayName, oidcButtonLabel, + hideLocalAuth, + oidcAutoRedirect, loginError, isLoggingIn, setIsAuthenticated, + refreshAuth, handleLogin, handleLogout, }; diff --git a/src/frontend/src/pages/LoginPage.tsx b/src/frontend/src/pages/LoginPage.tsx index a51ffa8..ecf2748 100644 --- a/src/frontend/src/pages/LoginPage.tsx +++ b/src/frontend/src/pages/LoginPage.tsx @@ -8,9 +8,11 @@ interface LoginPageProps { isLoading: boolean; authMode?: string; oidcButtonLabel?: string | null; + hideLocalAuth?: boolean; + oidcAutoRedirect?: boolean; } -export const LoginPage = ({ onLogin, error, isLoading, authMode, oidcButtonLabel }: LoginPageProps) => { +export const LoginPage = ({ onLogin, error, isLoading, authMode, oidcButtonLabel, hideLocalAuth, oidcAutoRedirect }: LoginPageProps) => { const logoUrl = withBasePath('/logo.png'); return ( @@ -30,7 +32,7 @@ export const LoginPage = ({ onLogin, error, isLoading, authMode, oidcButtonLabel
Logo
- +
diff --git a/src/frontend/src/types/index.ts b/src/frontend/src/types/index.ts index d97436d..13ca006 100644 --- a/src/frontend/src/types/index.ts +++ b/src/frontend/src/types/index.ts @@ -261,6 +261,8 @@ export interface AuthResponse { error?: string; logout_url?: string; oidc_button_label?: string; + hide_local_auth?: boolean; + oidc_auto_redirect?: boolean; } // Type guard to check if a book is from a metadata provider diff --git a/tests/core/test_oidc_routes.py b/tests/core/test_oidc_routes.py index 0c4e326..c01a4f5 100644 --- a/tests/core/test_oidc_routes.py +++ b/tests/core/test_oidc_routes.py @@ -3,6 +3,7 @@ import os import tempfile from unittest.mock import Mock, patch +from urllib.parse import parse_qs, urlparse import pytest from authlib.jose.errors import InvalidClaimError @@ -11,6 +12,15 @@ from flask import Flask, redirect from shelfmark.core.user_db import UserDB +def _get_oidc_error(resp) -> str | None: + """Extract the oidc_error query param from a redirect response.""" + assert resp.status_code == 302 + parsed = urlparse(resp.headers["Location"]) + params = parse_qs(parsed.query) + errors = params.get("oidc_error", []) + return errors[0] if errors else None + + @pytest.fixture def db_path(): with tempfile.TemporaryDirectory() as tmpdir: @@ -142,6 +152,7 @@ class TestOIDCCallbackEndpoint: resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") assert resp.status_code == 302 + fake_client.userinfo.assert_not_called() with client.session_transaction() as sess: assert sess["user_id"] == "john" @@ -181,18 +192,54 @@ class TestOIDCCallbackEndpoint: assert resp.status_code == 302 @patch("shelfmark.core.oidc_routes._get_oidc_client") - def test_callback_returns_400_when_claims_missing(self, mock_get_client, client): + def test_callback_fetches_userinfo_when_token_claims_are_sparse(self, mock_get_client, client): + fake_client = Mock() + token = {"userinfo": {"sub": "sparse-sub"}} + fake_client.authorize_access_token.return_value = token + fake_client.userinfo.return_value = { + "sub": "sparse-sub", + "email": "sparse@example.com", + "preferred_username": "sparse-user", + "groups": [], + } + mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG) + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + + assert resp.status_code == 302 + fake_client.userinfo.assert_called_once_with(token=token) + with client.session_transaction() as sess: + assert sess["user_id"] == "sparse-user" + + @patch("shelfmark.core.oidc_routes._get_oidc_client") + def test_callback_uses_sparse_claims_when_userinfo_fetch_fails(self, mock_get_client, client): + fake_client = Mock() + token = {"userinfo": {"sub": "fallback-sub"}} + fake_client.authorize_access_token.return_value = token + fake_client.userinfo.side_effect = RuntimeError("userinfo failed") + mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG) + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + + assert resp.status_code == 302 + fake_client.userinfo.assert_called_once_with(token=token) + with client.session_transaction() as sess: + assert sess["user_id"] == "fallback-sub" + + @patch("shelfmark.core.oidc_routes._get_oidc_client") + def test_callback_redirects_with_error_when_claims_missing(self, mock_get_client, client): fake_client = Mock() fake_client.authorize_access_token.return_value = {} fake_client.userinfo.side_effect = RuntimeError("userinfo failed") mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG) resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") - assert resp.status_code == 400 - assert "missing user claims" in resp.get_json()["error"] + error = _get_oidc_error(resp) + assert error is not None + assert "missing user claims" in error @patch("shelfmark.core.oidc_routes._get_oidc_client") - def test_callback_returns_400_with_issuer_guidance_on_invalid_issuer_claim( + def test_callback_redirects_with_issuer_guidance_on_invalid_issuer_claim( self, mock_get_client, client ): fake_client = Mock() @@ -201,11 +248,12 @@ class TestOIDCCallbackEndpoint: mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG) resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") - assert resp.status_code == 400 - assert "issuer validation failed" in resp.get_json()["error"] + error = _get_oidc_error(resp) + assert error is not None + assert "issuer validation failed" in error @patch("shelfmark.core.oidc_routes._get_oidc_client") - def test_callback_rejects_when_auto_provision_disabled(self, mock_get_client, client): + def test_callback_redirects_when_auto_provision_disabled(self, mock_get_client, client): config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False} fake_client = Mock() fake_client.authorize_access_token.return_value = { @@ -219,7 +267,9 @@ class TestOIDCCallbackEndpoint: mock_get_client.return_value = (fake_client, config) resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") - assert resp.status_code == 403 + error = _get_oidc_error(resp) + assert error is not None + assert "Account not found" in error @patch("shelfmark.core.oidc_routes._get_oidc_client") def test_callback_allows_pre_created_user_by_verified_email_when_no_provision( @@ -267,7 +317,45 @@ class TestOIDCCallbackEndpoint: mock_get_client.return_value = (fake_client, config) resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") - assert resp.status_code == 403 + error = _get_oidc_error(resp) + assert error is not None + assert "Account not found" in error updated_user = user_db.get_user(user_id=user["id"]) assert updated_user["oidc_subject"] is None + + @patch("shelfmark.core.oidc_routes._get_oidc_client") + def test_callback_redirects_on_idp_error(self, mock_get_client, client): + mock_get_client.return_value = (Mock(), MOCK_OIDC_CONFIG) + + resp = client.get("/api/auth/oidc/callback?error=access_denied") + error = _get_oidc_error(resp) + assert error is not None + assert "Authentication failed" in error + + @patch("shelfmark.core.oidc_routes._get_oidc_client") + def test_callback_error_redirect_honors_script_root(self, mock_get_client, client): + mock_get_client.return_value = (Mock(), MOCK_OIDC_CONFIG) + + resp = client.get( + "/api/auth/oidc/callback?error=access_denied", + environ_overrides={"SCRIPT_NAME": "/shelfmark"}, + ) + + assert resp.status_code == 302 + parsed = urlparse(resp.headers["Location"]) + assert parsed.path == "/shelfmark/login" + error = _get_oidc_error(resp) + assert error is not None + assert "Authentication failed" in error + + @patch("shelfmark.core.oidc_routes._get_oidc_client") + def test_callback_redirects_on_generic_exception(self, mock_get_client, client): + fake_client = Mock() + fake_client.authorize_access_token.side_effect = RuntimeError("unexpected") + mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG) + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + error = _get_oidc_error(resp) + assert error is not None + assert "Authentication failed" in error diff --git a/tests/prowlarr/test_qbittorrent_client.py b/tests/prowlarr/test_qbittorrent_client.py index 0185128..8ae22e2 100644 --- a/tests/prowlarr/test_qbittorrent_client.py +++ b/tests/prowlarr/test_qbittorrent_client.py @@ -244,8 +244,8 @@ class TestQBittorrentClientGetStatus: assert status.complete is True assert status.file_path == "/downloads/completed.epub" - def test_get_status_complete_roots_content_path_at_save_path(self, monkeypatch): - """Prefer a save_path-rooted path when qBittorrent reports a temp/incomplete content_path.""" + def test_get_status_complete_returns_content_path(self, monkeypatch): + """Completed torrents return content_path as-is.""" config_values = { "QBITTORRENT_URL": "http://localhost:8080", "QBITTORRENT_USERNAME": "admin", @@ -261,12 +261,11 @@ class TestQBittorrentClientGetStatus: hash_val="abc123", progress=1.0, state="uploading", - content_path="/media/incomplete/book.m4b", + content_path="/downloads/shelfmark/Ground State - Craig Alanson/Ground State - Craig Alanson.epub", ) mock_client_instance = MagicMock() - # Include save_path in the info payload to simulate a temp/incomplete directory config. - info_payload = mock_torrent.to_dict() | {"save_path": "/media"} + info_payload = mock_torrent.to_dict() | {"save_path": "/downloads/shelfmark"} mock_client_instance._session.get.return_value = create_mock_session_response([info_payload], status_code=200) mock_client_class = MagicMock(return_value=mock_client_instance) @@ -279,7 +278,7 @@ class TestQBittorrentClientGetStatus: status = client.get_status("abc123") assert status.complete is True - assert status.file_path == "/media/book.m4b" + assert status.file_path == "/downloads/shelfmark/Ground State - Craig Alanson/Ground State - Craig Alanson.epub" def test_get_status_complete_derives_when_content_path_equals_save_path(self, monkeypatch): """Keep get_status() and get_download_path() consistent.""" @@ -723,8 +722,8 @@ class TestQBittorrentClientGetDownloadPath: assert path == "/downloads/some/book.epub" - def test_get_download_path_roots_content_path_at_save_path_when_complete(self, monkeypatch): - """Mirror get_status(): completed torrents should return the save_path-rooted path.""" + def test_get_download_path_returns_content_path_when_complete(self, monkeypatch): + """Completed torrents return content_path as-is, preserving subdirectories.""" config_values = { "QBITTORRENT_URL": "http://localhost:8080", "QBITTORRENT_USERNAME": "admin", @@ -740,11 +739,11 @@ class TestQBittorrentClientGetDownloadPath: hash_val="abc123", progress=1.0, state="uploading", - content_path="/media/incomplete/book.m4b", + content_path="/downloads/shelfmark/BookFolder/book.epub", ) mock_client_instance = MagicMock() - info_payload = mock_torrent.to_dict() | {"save_path": "/media"} + info_payload = mock_torrent.to_dict() | {"save_path": "/downloads/shelfmark"} mock_client_instance._session.get.return_value = create_mock_session_response([info_payload], status_code=200) mock_client_class = MagicMock(return_value=mock_client_instance) @@ -756,7 +755,7 @@ class TestQBittorrentClientGetDownloadPath: client = qb_module.QBittorrentClient() path = client.get_download_path("abc123") - assert path == "/media/book.m4b" + assert path == "/downloads/shelfmark/BookFolder/book.epub" def test_get_download_path_does_not_accept_content_path_equal_save_path(self, monkeypatch): """content_path == save_path indicates a path error."""