starred/shelfmark
1
0
Fork 0
mirror of https://github.com/calibrain/shelfmark.git synced 2026-10-05 11:41:12 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Patch: OIDC polish (#636)

Browse Source 
- Added two env vars for OIDC login: 
- HIDE_LOCAL_AUTH - Remove the "password" option on login page when OIDC
enabled
  - OIDC_AUTO_REDIRECT - Immediately launch OIDC provider page
- Improved UX for initial OIDC setup, including creating a local admin
user
- Added callback URL label to OIDC setup page
- Fix Qbittorrent save path bug
This commit is contained in:
Alex
2026-02-21 11:51:11 +00:00
committed by GitHub
parent fdd46852f2
commit 014fc38b48
26 changed files with 369 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docker-compose.dev.yml
+2
View File
@@ -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
docs/oidc.md
+11
View File
@@ -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.
shelfmark/config/env.py
+2
View File
@@ -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"))
# =============================================================================
shelfmark/config/security.py
+30 -2
View File
@@ -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
shelfmark/config/security_handlers.py
+4 -2
View File
@@ -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."}
shelfmark/core/oidc_routes.py
+20 -16
View File
@@ -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 "<unknown>",
)
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"))
shelfmark/core/user_db.py
+1
View File
@@ -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:
shelfmark/download/clients/qbittorrent.py
-24
View File
@@ -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", "")
shelfmark/main.py
+11 -3
View File
@@ -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:
src/frontend/src/App.tsx
+6
View File
@@ -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}
/>
<SelfSettingsModal
@@ -1497,6 +1501,8 @@ function App() {
isLoading={isLoggingIn}
authMode={authMode}
oidcButtonLabel={oidcButtonLabel}
hideLocalAuth={hideLocalAuth}
oidcAutoRedirect={oidcAutoRedirect}
/>
)
}
src/frontend/src/components/LoginForm.tsx
+37 -17
View File
@@ -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<HTMLFormElement>) => {
e.preventDefault();
const formData = new FormData(e.currentTarget);
@@ -245,11 +259,13 @@ export const LoginForm = ({
}
};
const displayError = oidcError || error;
return (
<div>
{error && (
{displayError && (
<div className="mb-4 p-3 rounded-lg text-sm bg-red-600 text-white">
{error}
{displayError}
</div>
)}
@@ -262,22 +278,26 @@ export const LoginForm = ({
{oidcButtonLabel || 'Sign in with OIDC'}
</a>
<div className="flex items-center mt-5 mb-2">
<div className="flex-1 border-t" style={{ borderColor: 'var(--border-color)' }} />
<button
type="button"
onClick={() => setShowPasswordLogin((prev) => !prev)}
className="px-3 text-sm opacity-60 hover:opacity-100 transition-opacity"
>
{showPasswordLogin ? 'Hide' : 'Use password'}
</button>
<div className="flex-1 border-t" style={{ borderColor: 'var(--border-color)' }} />
</div>
{!hideLocalAuth && (
<>
<div className="flex items-center mt-5 mb-2">
<div className="flex-1 border-t" style={{ borderColor: 'var(--border-color)' }} />
<button
type="button"
onClick={() => setShowPasswordLogin((prev) => !prev)}
className="px-3 text-sm opacity-60 hover:opacity-100 transition-opacity"
>
{showPasswordLogin ? 'Hide' : 'Use password'}
</button>
<div className="flex-1 border-t" style={{ borderColor: 'var(--border-color)' }} />
</div>
{showPasswordLogin && (
<div className="pt-2">
<PasswordLoginForm onSubmit={handleSubmit} isLoading={isLoading} autoFocus={true} />
</div>
{showPasswordLogin && (
<div className="pt-2">
<PasswordLoginForm onSubmit={handleSubmit} isLoading={isLoading} autoFocus={true} />
</div>
)}
</>
)}
</>
) : (
src/frontend/src/components/settings/SettingsContent.tsx
+2
View File
@@ -53,6 +53,7 @@ interface SettingsContentProps {
authMode?: string;
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
onRefreshOverrideSummary?: () => void;
onRefreshAuth?: () => Promise<void>;
};
}
@@ -407,6 +408,7 @@ export const SettingsContent = ({
authMode: customFieldContext?.authMode,
onShowToast: customFieldContext?.onShowToast,
onRefreshOverrideSummary: customFieldContext?.onRefreshOverrideSummary,
onRefreshAuth: customFieldContext?.onRefreshAuth,
})
: renderField(
field,
src/frontend/src/components/settings/SettingsModal.tsx
+3 -1
View File
@@ -13,9 +13,10 @@ interface SettingsModalProps {
onClose: () => void;
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
onSettingsSaved?: () => void;
onRefreshAuth?: () => Promise<void>;
}
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,
}}
/>
))
src/frontend/src/components/settings/customFields/OidcAdminHint.tsx
+31
View File
@@ -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<boolean | null>(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 (
<div className="text-sm px-3 py-2 rounded-lg bg-amber-500/15 text-amber-700 dark:text-amber-300">
{field.label}
</div>
);
};
src/frontend/src/components/settings/customFields/OidcEnvInfo.tsx
+26
View File
@@ -0,0 +1,26 @@
import { CustomSettingsFieldRendererProps } from './types';
export const OidcEnvInfo = (_props: CustomSettingsFieldRendererProps) => {
return (
<div className="rounded-lg overflow-hidden border border-[var(--border-muted)]">
<div
className="px-3 py-1.5 text-xs font-medium opacity-60 border-b border-[var(--border-muted)]"
style={{ background: 'var(--bg-soft)' }}
>
docker-compose.yml
</div>
<pre
className="px-3 py-3 text-xs overflow-x-auto"
style={{ background: 'var(--bg-soft)' }}
>
<code>
<span className="opacity-60">environment:</span>{'\n'}
{' '}- <span className="text-blue-400">HIDE_LOCAL_AUTH</span>=<span className="text-green-400">true</span>
{' '}<span className="opacity-40"># Hide the local login form</span>{'\n'}
{' '}- <span className="text-blue-400">OIDC_AUTO_REDIRECT</span>=<span className="text-green-400">true</span>
{' '}<span className="opacity-40"># Skip login page, redirect straight to OIDC</span>
</code>
</pre>
</div>
);
};
src/frontend/src/components/settings/customFields/SettingsLabel.tsx
+15
View File
@@ -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 (
<div className="text-sm px-3 py-2 rounded-lg bg-sky-500/20">
{field.label && <span className="opacity-60">{field.label} </span>}
{field.description && (
<code className="font-mono text-xs">{interpolate(field.description)}</code>
)}
</div>
);
};
src/frontend/src/components/settings/customFields/UsersManagementField.tsx
+12 -2
View File
@@ -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}
src/frontend/src/components/settings/customFields/index.tsx
+12
View File
@@ -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<string, CustomFieldDefinition> = {
request_policy_grid: {
renderer: RequestPolicyGridField,
},
settings_label: {
renderer: SettingsLabel,
},
oidc_admin_hint: {
renderer: OidcAdminHint,
},
oidc_env_info: {
renderer: OidcEnvInfo,
},
};
export const renderCustomSettingsField = (
src/frontend/src/components/settings/customFields/types.ts
+1
View File
@@ -13,6 +13,7 @@ export interface CustomSettingsFieldRendererProps {
authMode?: string;
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
onRefreshOverrideSummary?: () => void;
onRefreshAuth?: () => Promise<void>;
}
export interface CustomSettingsFieldLayout {
src/frontend/src/components/settings/users/UserCard.tsx
+7
View File
@@ -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.
</p>
)}
{needsLocalAdmin && !isFirstUser && (
<p className="text-xs text-zinc-500">
An admin account is required before OIDC can be enabled.
</p>
)}
<div className="grid grid-cols-1 sm:grid-cols-2 gap-4">
{renderTextField(usernameField, form.username, (value) => onChange({ ...form, username: value }))}
src/frontend/src/components/settings/users/UserListView.tsx
+3
View File
@@ -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}
/>
src/frontend/src/hooks/useAuth.ts
+20
View File
@@ -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<void>;
handleLogin: (credentials: LoginCredentials) => Promise<void>;
handleLogout: () => Promise<void>;
}
@@ -38,6 +41,8 @@ export function useAuth(options: UseAuthOptions = {}): UseAuthReturn {
const [username, setUsername] = useState<string | null>(null);
const [displayName, setDisplayName] = useState<string | null>(null);
const [oidcButtonLabel, setOidcButtonLabel] = useState<string | null>(null);
const [hideLocalAuth, setHideLocalAuth] = useState<boolean>(false);
const [oidcAutoRedirect, setOidcAutoRedirect] = useState<boolean>(false);
const [loginError, setLoginError] = useState<string | null>(null);
const [isLoggingIn, setIsLoggingIn] = useState<boolean>(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,
};
src/frontend/src/pages/LoginPage.tsx
+4 -2
View File
@@ -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
<div className="text-center mb-5">
<img src={logoUrl} alt="Logo" className="mx-auto w-12 h-12" />
</div>
<LoginForm onSubmit={onLogin} error={error} isLoading={isLoading} authMode={authMode} oidcButtonLabel={oidcButtonLabel} />
<LoginForm onSubmit={onLogin} error={error} isLoading={isLoading} authMode={authMode} oidcButtonLabel={oidcButtonLabel} hideLocalAuth={hideLocalAuth} oidcAutoRedirect={oidcAutoRedirect} />
</div>
</div>
</div>
src/frontend/src/types/index.ts
+2
View File
@@ -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
tests/core/test_oidc_routes.py
+97 -9
View File
@@ -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
tests/prowlarr/test_qbittorrent_client.py
+10 -11
View File
@@ -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."""