Patch: Further multi-user fixes (#613)

This commit is contained in:
Alex
2026-02-12 17:47:10 +00:00
committed by GitHub
parent a7064939ce
commit af9d9ec8db
30 changed files with 833 additions and 325 deletions
+5 -12
View File
@@ -636,14 +636,8 @@ def _on_save_downloads(values: dict[str, Any]) -> dict[str, Any]:
# Preferred model: single recipient for global default and per-user override.
raw_recipient = str(effective.get("EMAIL_RECIPIENT", "") or "").strip()
if not raw_recipient:
return {
"error": True,
"message": "Email recipient is required (Downloads -> Books -> Email Recipient).",
"values": values,
}
if not _is_plain_email_address(raw_recipient):
# Optional global fallback: validate only when a default recipient is provided.
if raw_recipient and not _is_plain_email_address(raw_recipient):
return {
"error": True,
"message": "Email recipient must be a valid plain email address.",
@@ -909,8 +903,8 @@ def download_settings():
),
TextField(
key="EMAIL_RECIPIENT",
label="Email Recipient",
description="Email address that should receive downloaded books when Output Mode is Email.",
label="Default Email Recipient",
description="Optional fallback email address when no per-user email recipient override is configured.",
placeholder="reader@example.com",
user_overridable=True,
show_when={"field": "BOOKS_OUTPUT_MODE", "value": "email"},
@@ -1017,8 +1011,7 @@ def download_settings():
TextField(
key="DESTINATION_AUDIOBOOK",
label="Destination",
description="Leave empty to use Books destination. Use {User} for per-user folders (e.g. /audiobooks/{User}).",
placeholder="/audiobooks",
description="Directory where downloaded audiobook files are saved. Leave empty to use the Books destination.",
user_overridable=True,
universal_only=True,
),
+2 -1
View File
@@ -25,7 +25,8 @@ def users_settings():
label="Restrict Settings and Onboarding to Admins",
description=(
"When enabled, only admin users can access Settings and Onboarding. "
"When disabled, any authenticated user can access them."
"When disabled, any authenticated user can access them. "
"Security and Users are always admin-only."
),
default=True,
env_supported=False,
+9 -19
View File
@@ -328,15 +328,9 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None:
"message": "Display name is managed by your identity provider.",
}), 400
# Prevent demoting the last admin
if role_changed 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
# Allow demoting the last admin account.
# Auth mode resolution automatically falls back to "none" when no
# local password admin remains.
# Avoid unnecessary writes for no-op field updates.
for field in ("role", "email", "display_name"):
@@ -401,7 +395,8 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None:
message = (
f"Synced {summary['total']} CWA users "
f"({summary['created']} created, {summary['updated']} updated)."
f"({summary['created']} created, {summary['updated']} updated, "
f"{summary.get('deleted', 0)} deleted)."
)
logger.info(message)
return jsonify({
@@ -429,20 +424,15 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None:
user.get("auth_source"),
user.get("oidc_subject"),
)
if auth_source in {AUTH_SOURCE_PROXY, AUTH_SOURCE_CWA} and auth_source == auth_mode:
if auth_source == AUTH_SOURCE_CWA and auth_source == auth_mode:
return jsonify({
"error": f"Cannot delete active {auth_source.upper()} users",
"message": f"{auth_source.upper()} users are automatically re-provisioned on login.",
}), 400
# 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
# Allow deleting the last local admin account.
# Auth mode resolution automatically falls back to "none" when no
# local password admin remains.
user_db.delete_user(user_id)
logger.info(f"Admin deleted user {user_id}: {user['username']}")
+25
View File
@@ -14,6 +14,7 @@ AUTH_SOURCES = (
AUTH_SOURCE_CWA,
)
AUTH_SOURCE_SET = frozenset(AUTH_SOURCES)
_ALWAYS_ADMIN_SETTINGS_TABS = frozenset({"security", "users"})
def has_local_password_admin(user_db: Any | None = None) -> bool:
@@ -82,6 +83,18 @@ def is_settings_or_onboarding_path(path: str) -> bool:
return path.startswith("/api/settings") or path.startswith("/api/onboarding")
def get_settings_tab_from_path(path: str) -> str | None:
"""Extract tab name from /api/settings/<tab>[...] paths."""
if not path.startswith("/api/settings/"):
return None
suffix = path[len("/api/settings/"):]
if not suffix:
return None
return suffix.split("/", 1)[0] or None
def should_restrict_settings_to_admin(
users_config: Mapping[str, Any],
) -> bool:
@@ -89,6 +102,18 @@ def should_restrict_settings_to_admin(
return bool(users_config.get("RESTRICT_SETTINGS_TO_ADMIN", True))
def requires_admin_for_settings_access(
path: str,
users_config: Mapping[str, Any],
) -> bool:
"""Return whether this settings/onboarding request requires admin privileges."""
tab_name = get_settings_tab_from_path(path)
if tab_name in _ALWAYS_ADMIN_SETTINGS_TABS:
return True
return should_restrict_settings_to_admin(users_config)
def get_auth_check_admin_status(
_auth_mode: str,
users_config: Mapping[str, Any],
-1
View File
@@ -48,7 +48,6 @@ class Config:
_instance: Optional['Config'] = None
_lock = Lock()
def __new__(cls) -> 'Config':
if cls._instance is None:
with cls._lock:
+20 -1
View File
@@ -4,6 +4,7 @@ from __future__ import annotations
from typing import Any, Iterable
from shelfmark.core.auth_modes import AUTH_SOURCE_CWA, normalize_auth_source
from shelfmark.core.external_user_linking import upsert_external_user
from shelfmark.core.user_db import UserDB
@@ -50,26 +51,44 @@ def sync_cwa_users_from_rows(
"""Sync CWA users from raw `(name, role_flags, email)` rows."""
created = 0
updated = 0
active_cwa_user_ids: set[int] = set()
for username, role_flags, email in rows:
normalized_username = str(username or "").strip()
if not normalized_username:
continue
role = "admin" if (int(role_flags or 0) & 1) == 1 else "user"
_, action = upsert_cwa_user(
user, action = upsert_cwa_user(
user_db,
cwa_username=normalized_username,
cwa_email=_normalize_email(email),
role=role,
context="cwa_manual_sync",
)
active_cwa_user_ids.add(int(user["id"]))
if action == "created":
created += 1
else:
updated += 1
deleted = 0
for existing_user in user_db.list_users():
if normalize_auth_source(
existing_user.get("auth_source"),
existing_user.get("oidc_subject"),
) != AUTH_SOURCE_CWA:
continue
existing_id = int(existing_user.get("id") or 0)
if existing_id in active_cwa_user_ids:
continue
user_db.delete_user(existing_id)
deleted += 1
return {
"created": created,
"updated": updated,
"deleted": deleted,
"total": created + updated,
}
+5 -9
View File
@@ -95,7 +95,7 @@ def _resolve_email_destination(
return configured_recipient, None
return None, "Configured email recipient is invalid"
return None, "Email recipient is required"
return None, None
def queue_book(
@@ -126,10 +126,8 @@ def queue_book(
email_to, email_error = _resolve_email_destination(user_id=user_id)
if email_error:
return False, email_error
if not email_to:
return False, "Email recipient is required"
output_args = {"to": email_to}
if email_to:
output_args = {"to": email_to}
# Create a source-agnostic download task
task = DownloadTask(
@@ -204,10 +202,8 @@ def queue_release(
email_to, email_error = _resolve_email_destination(user_id=user_id)
if email_error:
return False, email_error
if not email_to:
return False, "Email recipient is required"
output_args = {"to": email_to}
if email_to:
output_args = {"to": email_to}
# Create a source-agnostic download task from release data
task = DownloadTask(
+4 -1
View File
@@ -296,7 +296,10 @@ def _post_process_email(
recipient = str(output_args.get("to", "") or "").strip()
label = str(output_args.get("label", "") or "").strip() or recipient
if not recipient:
status_callback("error", "No email recipient selected")
status_callback(
"error",
"No email recipient configured. Set a per-user email recipient or a default in Downloads -> Books.",
)
return None
status_callback("resolving", "Preparing email")
+5 -4
View File
@@ -32,7 +32,7 @@ from shelfmark.core.auth_modes import (
get_auth_check_admin_status,
has_local_password_admin,
is_settings_or_onboarding_path,
should_restrict_settings_to_admin,
requires_admin_for_settings_access,
)
from shelfmark.core.cwa_user_sync import upsert_cwa_user
from shelfmark.core.external_user_linking import upsert_external_user
@@ -403,9 +403,10 @@ def login_required(f):
try:
users_config = load_config_file("users")
restrict_to_admin = should_restrict_settings_to_admin(users_config)
if restrict_to_admin and not session.get('is_admin', False):
if (
requires_admin_for_settings_access(request.path, users_config)
and not session.get('is_admin', False)
):
return jsonify({"error": "Admin access required"}), 403
except Exception as e:
@@ -17,7 +17,7 @@ import {
ShowWhenCondition,
TableFieldConfig,
} from '../../types/settings';
import { FieldWrapper } from './shared';
import { FieldWrapper, SettingsSaveBar } from './shared';
import {
TextField,
PasswordField,
@@ -351,14 +351,7 @@ export const SettingsContent = ({
</div>
{/* Save button - only visible when there are changes */}
{hasChanges && (
<div
className="flex-shrink-0 px-6 py-4 border-t border-[var(--border-muted)] bg-[var(--bg)] animate-slide-up"
style={{ paddingBottom: 'calc(1rem + env(safe-area-inset-bottom))' }}
>
{saveButton}
</div>
)}
{hasChanges && <SettingsSaveBar onSave={onSave} isSaving={isSaving} />}
</div>
);
};
@@ -1,7 +1,7 @@
import { useEffect, useState, useCallback, useRef, useMemo } from 'react';
import { useSettings } from '../../hooks/useSettings';
import { useSearchMode } from '../../contexts/SearchModeContext';
import { getAdminSettingsOverridesSummary } from '../../services/api';
import { getAdminSettingsOverridesSummary, getSettingsTab } from '../../services/api';
import { SettingsHeader } from './SettingsHeader';
import { SettingsSidebar } from './SettingsSidebar';
import { SettingsContent } from './SettingsContent';
@@ -37,7 +37,7 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
const [isMobile, setIsMobile] = useState(false);
const [showMobileDetail, setShowMobileDetail] = useState(false);
const [isClosing, setIsClosing] = useState(false);
const [usersSubpageState, setUsersSubpageState] = useState<{ title: string; onBack: () => void } | null>(null);
const [securityAccessError, setSecurityAccessError] = useState<string | null>(null);
const [tabOverrideSummaries, setTabOverrideSummaries] = useState<
Record<string, Record<string, { count: number; users: Array<{ userId: number; username: string; value: unknown }> }>>
>({});
@@ -97,16 +97,36 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
if (isOpen) {
setShowMobileDetail(false);
setIsClosing(false);
setUsersSubpageState(null);
setTabOverrideSummaries({});
}
}, [isOpen]);
useEffect(() => {
if (selectedTab !== 'users') {
setUsersSubpageState(null);
if (!isOpen || selectedTab !== 'security') {
setSecurityAccessError(null);
return;
}
}, [selectedTab]);
let cancelled = false;
getSettingsTab("security")
.then(() => {
if (cancelled) return;
setSecurityAccessError(null);
})
.catch((err) => {
if (cancelled) return;
const message = err instanceof Error ? err.message : 'Failed to load security settings';
if (message.toLowerCase().includes('admin access required')) {
setSecurityAccessError(message);
return;
}
setSecurityAccessError(null);
});
return () => {
cancelled = true;
};
}, [isOpen, selectedTab]);
useEffect(() => {
if (!isOpen || !selectedTab || selectedTab === 'users') {
@@ -243,7 +263,6 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
const currentTab = tabs.find((t) => t.name === selectedTab);
const currentTabDisplayName = currentTab?.displayName || 'Settings';
const usersHeaderTitle = usersSubpageState ? `Settings / ${usersSubpageState.title}` : null;
const selectedAuthMethod = values.security?.AUTH_METHOD;
const usersAuthMode = typeof selectedAuthMethod === 'string' ? selectedAuthMethod : authMode;
const currentTabContent = currentTab
@@ -258,8 +277,11 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
isSaving={isSaving}
hasChanges={currentTabHasChanges}
onShowToast={onShowToast}
onSubpageStateChange={setUsersSubpageState}
/>
) : (selectedTab === 'security' && securityAccessError) ? (
<div className="flex-1 flex flex-col items-center justify-center p-8 gap-3">
<p className="text-sm opacity-60">{securityAccessError}</p>
</div>
) : (
<SettingsContent
tab={currentTab}
@@ -381,9 +403,9 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
// Detail view
<>
<SettingsHeader
title={selectedTab === 'users' && usersHeaderTitle ? usersHeaderTitle : currentTabDisplayName}
title={currentTabDisplayName}
showBack
onBack={selectedTab === 'users' && usersSubpageState ? usersSubpageState.onBack : handleBack}
onBack={handleBack}
onClose={handleClose}
/>
{currentTabContent}
@@ -416,9 +438,7 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
aria-label="Settings"
>
<SettingsHeader
title={selectedTab === 'users' && usersHeaderTitle ? usersHeaderTitle : 'Settings'}
showBack={selectedTab === 'users' && !!usersSubpageState}
onBack={selectedTab === 'users' && usersSubpageState ? usersSubpageState.onBack : undefined}
title="Settings"
onClose={handleClose}
/>
@@ -11,7 +11,7 @@ import {
useUsersPanelState,
} from './users';
import { SettingsContent } from './SettingsContent';
import { SettingsSubpage } from './shared';
import { SettingsSaveBar } from './shared';
interface UsersPanelProps {
authMode: string;
@@ -23,7 +23,6 @@ interface UsersPanelProps {
isSaving: boolean;
hasChanges: boolean;
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
onSubpageStateChange?: (state: { title: string; onBack: () => void } | null) => void;
}
export const UsersPanel = ({
@@ -36,7 +35,6 @@ export const UsersPanel = ({
isSaving,
hasChanges,
onShowToast,
onSubpageStateChange,
}: UsersPanelProps) => {
const { route, openCreate, openEdit, openEditOverrides, backToList } = useUsersPanelState();
@@ -63,6 +61,7 @@ export const UsersPanel = ({
isUserOverridable,
userSettings,
setUserSettings,
hasUserSettingsChanges,
beginEditing,
applyUserEditContext,
resetEditContext,
@@ -82,6 +81,7 @@ export const UsersPanel = ({
} = useUserMutations({
onShowToast,
fetchUsers,
users,
createForm,
resetCreateForm,
editingUser,
@@ -145,36 +145,25 @@ export const UsersPanel = ({
backToList();
};
useEffect(() => {
if (!onSubpageStateChange) {
return undefined;
}
if (route.kind === 'edit-overrides') {
const username = editingUser && editingUser.id === route.userId
? editingUser.username
: 'User';
onSubpageStateChange({
title: `Users / User Preferences: ${username}`,
onBack: handleBackToEdit,
});
} else {
onSubpageStateChange(null);
}
return () => {
onSubpageStateChange(null);
};
}, [editingUser, handleBackToEdit, onSubpageStateChange, route]);
useEffect(() => {
if (route.kind === 'create' && !canCreateLocalUsers) {
backToList();
}
}, [backToList, canCreateLocalUsers, route.kind]);
const handleSave = async () => {
const ok = await saveEditedUser();
const handleSaveUserEdit = async () => {
const ok = await saveEditedUser({ includeSettings: false });
if (ok) {
backToList();
}
};
const handleSaveUserOverrides = async () => {
const ok = await saveEditedUser({
includeProfile: false,
includePassword: false,
includeSettings: true,
});
if (ok) {
backToList();
}
@@ -213,65 +202,79 @@ export const UsersPanel = ({
}
return (
<UserOverridesView
onSave={handleSave}
saving={saving}
onBack={handleBackToEdit}
deliveryPreferences={deliveryPreferences}
isUserOverridable={isUserOverridable}
userSettings={userSettings}
setUserSettings={(updater) => setUserSettings(updater)}
/>
<div className="flex-1 flex flex-col min-h-0">
<UserOverridesView
hasChanges={hasUserSettingsChanges}
onBack={handleBackToEdit}
deliveryPreferences={deliveryPreferences}
isUserOverridable={isUserOverridable}
userSettings={userSettings}
setUserSettings={(updater) => setUserSettings(updater)}
/>
{hasUserSettingsChanges && (
<SettingsSaveBar onSave={handleSaveUserOverrides} isSaving={saving} />
)}
</div>
);
}
return (
<SettingsSubpage>
<div>
<UserListView
authMode={authMode}
users={users}
onCreate={openCreate}
showCreateForm={route.kind === 'create'}
createForm={createForm}
onCreateFormChange={setCreateForm}
creating={creating}
isFirstUser={users.length === 0}
onCreateSubmit={handleCreate}
onCancelCreate={handleCancelCreate}
showEditForm={route.kind === 'edit'}
activeEditUserId={route.kind === 'edit' ? route.userId : null}
editingUser={route.kind === 'edit' ? editingUser : null}
onEditingUserChange={setEditingUser}
onEditSave={handleSave}
saving={saving}
onCancelEdit={handleBackToList}
editPassword={editPassword}
onEditPasswordChange={setEditPassword}
editPasswordConfirm={editPasswordConfirm}
onEditPasswordConfirmChange={setEditPasswordConfirm}
downloadDefaults={downloadDefaults}
onOpenOverrides={handleOpenOverrides}
onEdit={handleEdit}
onDelete={deleteUser}
deletingUserId={deletingUserId}
onSyncCwa={handleSyncCwa}
syncingCwa={syncingCwa}
/>
<div className="pt-5 mt-4 border-t border-black/10 dark:border-white/10">
<SettingsContent
tab={tab}
values={values}
onChange={onChange}
onSave={onSave}
onAction={onAction}
isSaving={isSaving}
hasChanges={hasChanges}
embedded
<div className="flex-1 flex flex-col min-h-0">
<div
className="flex-1 overflow-y-auto p-6"
style={{ paddingBottom: hasChanges ? 'calc(5rem + env(safe-area-inset-bottom))' : '1.5rem' }}
>
<div>
<UserListView
authMode={authMode}
users={users}
onCreate={openCreate}
showCreateForm={route.kind === 'create'}
createForm={createForm}
onCreateFormChange={setCreateForm}
creating={creating}
isFirstUser={users.length === 0}
onCreateSubmit={handleCreate}
onCancelCreate={handleCancelCreate}
showEditForm={route.kind === 'edit'}
activeEditUserId={route.kind === 'edit' ? route.userId : null}
editingUser={route.kind === 'edit' ? editingUser : null}
onEditingUserChange={setEditingUser}
onEditSave={handleSaveUserEdit}
saving={saving}
onCancelEdit={handleBackToList}
editPassword={editPassword}
onEditPasswordChange={setEditPassword}
editPasswordConfirm={editPasswordConfirm}
onEditPasswordConfirmChange={setEditPasswordConfirm}
downloadDefaults={downloadDefaults}
onOpenOverrides={handleOpenOverrides}
onEdit={handleEdit}
onDelete={deleteUser}
deletingUserId={deletingUserId}
onSyncCwa={handleSyncCwa}
syncingCwa={syncingCwa}
/>
<div className="pt-5 mt-4 border-t border-black/10 dark:border-white/10">
<SettingsContent
tab={tab}
values={values}
onChange={onChange}
onSave={onSave}
onAction={onAction}
isSaving={isSaving}
hasChanges={false}
embedded
/>
</div>
</div>
</div>
</SettingsSubpage>
{hasChanges && (
<SettingsSaveBar onSave={onSave} isSaving={isSaving} />
)}
</div>
);
};
@@ -0,0 +1,43 @@
interface SettingsSaveBarProps {
onSave: () => void | Promise<void>;
isSaving: boolean;
}
export const SettingsSaveBar = ({ onSave, isSaving }: SettingsSaveBarProps) => (
<div
className="flex-shrink-0 px-6 py-4 border-t border-[var(--border-muted)] bg-[var(--bg)] animate-slide-up"
style={{ paddingBottom: 'calc(1rem + env(safe-area-inset-bottom))' }}
>
<button
onClick={() => { void onSave(); }}
disabled={isSaving}
className="w-full py-2.5 px-4 rounded-lg font-medium transition-colors
bg-sky-600 text-white hover:bg-sky-700
disabled:opacity-50 disabled:cursor-not-allowed"
>
{isSaving ? (
<span className="flex items-center justify-center gap-2">
<svg className="animate-spin h-4 w-4" viewBox="0 0 24 24">
<circle
className="opacity-25"
cx="12"
cy="12"
r="10"
stroke="currentColor"
strokeWidth="4"
fill="none"
/>
<path
className="opacity-75"
fill="currentColor"
d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4z"
/>
</svg>
Saving...
</span>
) : (
'Save Changes'
)}
</button>
</div>
);
@@ -2,13 +2,18 @@ import { ReactNode } from 'react';
interface SettingsSubpageProps {
children: ReactNode;
hasBottomSaveBar?: boolean;
}
export const SettingsSubpage = ({
children,
hasBottomSaveBar = false,
}: SettingsSubpageProps) => {
return (
<div className="flex-1 overflow-y-auto p-6">
<div
className="flex-1 overflow-y-auto p-6"
style={{ paddingBottom: hasBottomSaveBar ? 'calc(5rem + env(safe-area-inset-bottom))' : '1.5rem' }}
>
{children}
</div>
);
@@ -1,3 +1,4 @@
export { EnvLockBadge } from './EnvLockBadge';
export { FieldWrapper } from './FieldWrapper';
export { SettingsSaveBar } from './SettingsSaveBar';
export { SettingsSubpage } from './SettingsSubpage';
@@ -165,7 +165,11 @@ interface UserEditFieldsProps {
editPasswordConfirm: string;
onEditPasswordConfirmChange: (value: string) => void;
downloadDefaults: DownloadDefaults | null;
onEditOverrides?: () => void;
onDelete?: () => void;
onConfirmDelete?: () => void;
onCancelDelete?: () => void;
isDeletePending?: boolean;
deleting?: boolean;
}
export const UserEditFields = ({
@@ -179,7 +183,11 @@ export const UserEditFields = ({
editPasswordConfirm,
onEditPasswordConfirmChange,
downloadDefaults,
onEditOverrides,
onDelete,
onConfirmDelete,
onCancelDelete,
isDeletePending = false,
deleting = false,
}: UserEditFieldsProps) => {
const capabilities = user.edit_capabilities;
const { authSource, canSetPassword, canEditRole, canEditEmail, canEditDisplayName } = capabilities;
@@ -246,29 +254,53 @@ export const UserEditFields = ({
</>
)}
<div className="flex flex-wrap gap-2 pt-2">
<button
onClick={onSave}
disabled={saving}
className="px-4 py-2 rounded-lg text-sm font-medium text-white bg-sky-600 hover:bg-sky-700 transition-colors disabled:opacity-60 disabled:cursor-not-allowed"
>
{saving ? 'Saving...' : 'Save Changes'}
</button>
<button
onClick={onCancel}
className="px-4 py-2 rounded-lg text-sm font-medium border border-[var(--border-muted)]
bg-[var(--bg)] hover:bg-[var(--hover-surface)] transition-colors"
>
Cancel
</button>
{onEditOverrides && (
<div className="flex flex-col gap-2 pt-3 border-t border-[var(--border-muted)] sm:flex-row sm:items-center">
<div className="flex flex-wrap gap-2">
<button
onClick={onEditOverrides}
className="ml-auto px-4 py-2 rounded-lg text-sm font-medium border border-[var(--border-muted)]
onClick={onSave}
disabled={saving}
className="px-4 py-2 rounded-lg text-sm font-medium text-white bg-sky-600 hover:bg-sky-700 transition-colors disabled:opacity-60 disabled:cursor-not-allowed"
>
{saving ? 'Saving...' : 'Save Changes'}
</button>
<button
onClick={onCancel}
className="px-4 py-2 rounded-lg text-sm font-medium border border-[var(--border-muted)]
bg-[var(--bg)] hover:bg-[var(--hover-surface)] transition-colors"
>
User Preferences
Cancel
</button>
</div>
{onDelete && (
<div className="flex flex-wrap gap-2 sm:ml-auto">
{isDeletePending ? (
<>
<button
onClick={onConfirmDelete}
disabled={deleting}
className="px-4 py-2 rounded-lg text-sm font-medium text-white bg-red-600 hover:bg-red-700 transition-colors disabled:opacity-60 disabled:cursor-not-allowed"
>
{deleting ? 'Deleting...' : 'Confirm Delete'}
</button>
<button
onClick={onCancelDelete}
disabled={deleting}
className="px-4 py-2 rounded-lg text-sm font-medium border border-[var(--border-muted)]
bg-[var(--bg)] hover:bg-[var(--hover-surface)] transition-colors disabled:opacity-60 disabled:cursor-not-allowed"
>
Cancel
</button>
</>
) : (
<button
onClick={onDelete}
className="px-4 py-2 rounded-lg text-sm font-medium transition-colors
border border-red-500/40 text-red-600 hover:bg-red-500/10"
>
Delete User
</button>
)}
</div>
)}
</div>
</>
@@ -74,6 +74,7 @@ export const UserListView = ({
const [confirmDelete, setConfirmDelete] = useState<number | null>(null);
const canCreateLocalUsers = canCreateLocalUsersForAuthMode(authMode);
const isCwaMode = String(authMode || 'none').toLowerCase() === 'cwa';
const toggleButtonClasses = 'p-2 rounded-full hover-action transition-colors text-gray-500 hover:text-gray-900 dark:text-gray-400 dark:hover:text-gray-100';
const usersHeading: HeadingFieldConfig = {
key: 'users_heading',
type: 'HeadingField',
@@ -113,7 +114,9 @@ export const UserListView = ({
key={user.id}
className={`rounded-lg border border-[var(--border-muted)] bg-[var(--bg-soft)] transition-colors ${active ? '' : 'opacity-60'}`}
>
<div className={`flex flex-col gap-3 sm:flex-row sm:items-center sm:justify-between p-3 ${isEditingRow ? 'border-b border-[var(--border-muted)]' : ''}`}>
<div
className={`flex flex-col gap-3 sm:flex-row sm:items-center sm:justify-between p-3 ${isEditingRow ? 'border-b border-[var(--border-muted)]' : ''}`}
>
<div className="flex items-center gap-3 min-w-0 flex-1">
<div
className={`w-8 h-8 rounded-full flex items-center justify-center text-sm font-medium shrink-0
@@ -150,72 +153,44 @@ export const UserListView = ({
{roleLabel}
</span>
{!isEditingRow && (
<>
<button
onClick={() => onEdit(user)}
className="p-2 rounded-full hover-action transition-colors text-gray-500 hover:text-gray-900 dark:text-gray-400 dark:hover:text-gray-100"
aria-label={`Edit ${user.username}`}
title="Edit user"
>
<svg
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
strokeWidth={1.5}
stroke="currentColor"
className="w-[18px] h-[18px]"
>
<path
strokeLinecap="round"
strokeLinejoin="round"
d="m16.862 4.487 1.687-1.688a1.875 1.875 0 1 1 2.652 2.652L10.582 16.07a4.5 4.5 0 0 1-1.897 1.13L6 18l.8-2.685a4.5 4.5 0 0 1 1.13-1.897l8.932-8.931Zm0 0L19.5 7.125M18 14v4.75A2.25 2.25 0 0 1 15.75 21H5.25A2.25 2.25 0 0 1 3 18.75V8.25A2.25 2.25 0 0 1 5.25 6H10"
/>
</svg>
</button>
{confirmDelete === user.id ? (
<div className="flex items-center gap-1">
<button
onClick={() => handleDelete(user.id)}
disabled={deletingUserId === user.id}
className="text-xs font-medium px-2.5 py-1.5 rounded-lg bg-red-600 text-white hover:bg-red-700 transition-colors disabled:opacity-60 disabled:cursor-not-allowed"
>
{deletingUserId === user.id ? 'Deleting...' : 'Confirm'}
</button>
<button
onClick={() => setConfirmDelete(null)}
className="text-xs font-medium px-2.5 py-1.5 rounded-lg border border-[var(--border-muted)]
bg-[var(--bg)] hover:bg-[var(--hover-surface)] transition-colors"
>
Cancel
</button>
</div>
) : (
<button
onClick={() => setConfirmDelete(user.id)}
className="p-2 rounded-full hover-action transition-colors text-red-500 hover:text-red-600 dark:text-red-400 dark:hover:text-red-300"
aria-label={`Delete ${user.username}`}
title="Delete user"
>
<svg
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
strokeWidth={1.5}
stroke="currentColor"
className="w-[18px] h-[18px]"
>
<path
strokeLinecap="round"
strokeLinejoin="round"
d="m14.74 9-.346 9m-4.788 0L9.26 9m9.968-3.21c.342.052.682.107 1.022.166m-1.022-.165L18.16 19.673a2.25 2.25 0 0 1-2.244 2.077H8.084a2.25 2.25 0 0 1-2.244-2.077L4.772 5.79m14.456 0a48.108 48.108 0 0 0-3.478-.397m-12 .562c.34-.059.68-.114 1.022-.165m0 0a48.11 48.11 0 0 1 3.478-.397m7.5 0v-.916c0-1.18-.91-2.164-2.09-2.201a51.964 51.964 0 0 0-3.32 0c-1.18.037-2.09 1.022-2.09 2.201v.916m7.5 0a48.667 48.667 0 0 0-7.5 0"
/>
</svg>
</button>
)}
</>
{isEditingRow && (
<button
onClick={onOpenOverrides}
className="px-4 py-2 rounded-lg text-sm font-medium text-white
bg-sky-600 hover:bg-sky-700 transition-colors"
>
User Preferences
</button>
)}
<button
onClick={() => {
setConfirmDelete(null);
if (isEditingRow) {
onCancelEdit();
} else {
onEdit(user);
}
}}
className={toggleButtonClasses}
aria-label={isEditingRow ? 'Collapse user editor' : `Expand ${user.username} editor`}
title={isEditingRow ? 'Collapse editor' : 'Expand editor'}
>
<svg
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
strokeWidth={1.5}
stroke="currentColor"
className={`w-[18px] h-[18px] transition-transform duration-200 ${isEditingRow ? 'rotate-180' : ''}`}
>
<path
strokeLinecap="round"
strokeLinejoin="round"
d="m19.5 8.25-7.5 7.5-7.5-7.5"
/>
</svg>
</button>
</div>
</div>
@@ -233,7 +208,11 @@ export const UserListView = ({
editPasswordConfirm={editPasswordConfirm}
onEditPasswordConfirmChange={onEditPasswordConfirmChange}
downloadDefaults={downloadDefaults}
onEditOverrides={onOpenOverrides}
onDelete={() => setConfirmDelete(user.id)}
onConfirmDelete={() => handleDelete(user.id)}
onCancelDelete={() => setConfirmDelete(null)}
isDeletePending={confirmDelete === user.id}
deleting={deletingUserId === user.id}
/>
) : (
<div className="text-sm opacity-60">Loading user details...</div>
@@ -44,9 +44,8 @@ const fallbackDestinationAudiobookField: TextFieldConfig = {
type: 'TextField',
key: 'DESTINATION_AUDIOBOOK',
label: 'Destination',
description: 'Directory where downloaded audiobook files are saved. Use {User} for per-user folders and leave empty to use the books destination.',
description: 'Directory for this user\'s audiobook downloads.',
value: '',
placeholder: '/audiobooks',
};
const fallbackBookloreLibraryField: SelectFieldConfig = {
@@ -141,7 +140,7 @@ const audiobooksHeading: HeadingFieldConfig = {
type: 'HeadingField',
key: 'delivery_preferences_audiobooks_heading',
title: 'Audiobooks',
description: 'Audiobooks always use folder output. Set a user-specific destination or leave empty to inherit books.',
description: 'Audiobooks always use folder output. Use Reset to inherit the global audiobook destination.',
};
const BOOK_PREFERENCE_KEYS: DeliverySettingKey[] = [
@@ -169,12 +168,26 @@ export const UserOverridesSection = ({
const destinationAudiobookField = getFieldByKey<TextFieldConfig>(fields, 'DESTINATION_AUDIOBOOK', fallbackDestinationAudiobookField);
const bookloreLibraryField = getFieldByKey<SelectFieldConfig>(fields, 'BOOKLORE_LIBRARY_ID', fallbackBookloreLibraryField);
const booklorePathField = getFieldByKey<SelectFieldConfig>(fields, 'BOOKLORE_PATH_ID', fallbackBooklorePathField);
const emailRecipientField = getFieldByKey<TextFieldConfig>(fields, 'EMAIL_RECIPIENT', fallbackEmailRecipientField);
const emailRecipientFieldSource = getFieldByKey<TextFieldConfig>(fields, 'EMAIL_RECIPIENT', fallbackEmailRecipientField);
const emailRecipientField: TextFieldConfig = {
...emailRecipientFieldSource,
label: 'Email Recipient',
description: 'Email address used for this user in Email output mode.',
};
const isOverridden = (key: DeliverySettingKey): boolean =>
Object.prototype.hasOwnProperty.call(userSettings, key) &&
userSettings[key] !== null &&
userSettings[key] !== undefined;
const isOverridden = (key: DeliverySettingKey): boolean => {
if (
!Object.prototype.hasOwnProperty.call(userSettings, key) ||
userSettings[key] === null ||
userSettings[key] === undefined
) {
return false;
}
const userValue = toStringValue(userSettings[key]);
const globalValue = toStringValue(globalValues[key as string]);
return userValue !== globalValue;
};
const resetKeys = (keys: DeliverySettingKey[]) => {
setUserSettings((prev) => {
@@ -242,24 +255,7 @@ export const UserOverridesSection = ({
<SelectField
field={outputModeField}
value={outputModeValue}
onChange={(value) => {
setUserSettings((prev) => {
const next: PerUserSettings = { ...prev, BOOKS_OUTPUT_MODE: value };
if (value === 'folder') {
delete next.BOOKLORE_LIBRARY_ID;
delete next.BOOKLORE_PATH_ID;
delete next.EMAIL_RECIPIENT;
} else if (value === 'booklore') {
delete next.DESTINATION;
delete next.EMAIL_RECIPIENT;
} else if (value === 'email') {
delete next.DESTINATION;
delete next.BOOKLORE_LIBRARY_ID;
delete next.BOOKLORE_PATH_ID;
}
return next;
});
}}
onChange={(value) => setUserSettings((prev) => ({ ...prev, BOOKS_OUTPUT_MODE: value }))}
disabled={Boolean(outputModeField.fromEnv)}
/>
</FieldWrapper>
@@ -370,12 +366,12 @@ export const UserOverridesSection = ({
) : undefined
}
>
<TextField
field={destinationAudiobookField}
value={destinationAudiobookValue}
onChange={(value) => setUserSettings((prev) => ({ ...prev, DESTINATION_AUDIOBOOK: value }))}
disabled={Boolean(destinationAudiobookField.fromEnv)}
/>
<TextField
field={destinationAudiobookField}
value={destinationAudiobookValue}
onChange={(value) => setUserSettings((prev) => ({ ...prev, DESTINATION_AUDIOBOOK: value }))}
disabled={Boolean(destinationAudiobookField.fromEnv)}
/>
</FieldWrapper>
</>
)}
@@ -4,8 +4,7 @@ import { SettingsSubpage } from '../shared';
import { UserOverridesSection } from './UserOverridesSection';
interface UserOverridesViewProps {
onSave: () => void;
saving: boolean;
hasChanges: boolean;
onBack: () => void;
deliveryPreferences: DeliveryPreferencesResponse | null;
isUserOverridable: (key: keyof PerUserSettings) => boolean;
@@ -14,39 +13,41 @@ interface UserOverridesViewProps {
}
export const UserOverridesView = ({
onSave,
saving,
hasChanges,
onBack,
deliveryPreferences,
isUserOverridable,
userSettings,
setUserSettings,
}: UserOverridesViewProps) => (
<SettingsSubpage>
<SettingsSubpage hasBottomSaveBar={hasChanges}>
<div className="space-y-5">
<div>
<button
onClick={onBack}
className="inline-flex items-center gap-2 px-4 py-2 rounded-lg text-sm font-medium border border-[var(--border-muted)]
bg-[var(--bg)] hover:bg-[var(--hover-surface)] transition-colors"
>
<svg
className="w-4 h-4"
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
strokeWidth={1.5}
stroke="currentColor"
>
<path strokeLinecap="round" strokeLinejoin="round" d="M15.75 19.5 8.25 12l7.5-7.5" />
</svg>
Back to User
</button>
</div>
<UserOverridesSection
deliveryPreferences={deliveryPreferences}
isUserOverridable={isUserOverridable}
userSettings={userSettings}
setUserSettings={setUserSettings}
/>
<div className="flex gap-2 pt-2">
<button
onClick={onSave}
disabled={saving}
className="px-4 py-2 rounded-lg text-sm font-medium text-white bg-sky-600 hover:bg-sky-700 transition-colors disabled:opacity-60 disabled:cursor-not-allowed"
>
{saving ? 'Saving...' : 'Save Changes'}
</button>
<button
onClick={onBack}
className="px-4 py-2 rounded-lg text-sm font-medium border border-[var(--border-muted)]
bg-[var(--bg-soft)] hover:bg-[var(--hover-surface)] transition-colors"
>
Back
</button>
</div>
</div>
</SettingsSubpage>
);
@@ -0,0 +1,38 @@
import { DeliveryPreferencesResponse } from '../../../services/api';
import { PerUserSettings } from './types';
const normalizeComparableValue = (value: unknown): string => {
if (value === null || value === undefined) {
return '';
}
return String(value);
};
export const buildUserSettingsPayload = (
userSettings: PerUserSettings,
userOverridableSettings: Set<string>,
deliveryPreferences: DeliveryPreferencesResponse | null,
): Record<string, unknown> =>
(deliveryPreferences?.keys || [...userOverridableSettings])
.map(String)
.sort()
.reduce<Record<string, unknown>>((payload, key) => {
const typedKey = key as keyof PerUserSettings;
const hasUserValue = Object.prototype.hasOwnProperty.call(userSettings, typedKey)
&& userSettings[typedKey] !== null
&& userSettings[typedKey] !== undefined;
if (!hasUserValue) {
payload[key] = null;
return payload;
}
const userValue = userSettings[typedKey];
const globalValue = deliveryPreferences?.globalValues?.[key];
const isDifferentFromGlobal = deliveryPreferences
? normalizeComparableValue(userValue) !== normalizeComparableValue(globalValue)
: true;
payload[key] = isDifferentFromGlobal ? userValue : null;
return payload;
}, {});
@@ -2,6 +2,19 @@ import { useState } from 'react';
import { AdminUser, DeliveryPreferencesResponse, DownloadDefaults } from '../../../services/api';
import { CreateUserFormState, INITIAL_CREATE_FORM, PerUserSettings } from './types';
import { UserEditContext } from './useUsersFetch';
import { buildUserSettingsPayload } from './settingsPayload';
const normalizeUserSettings = (settings: PerUserSettings): PerUserSettings => {
const normalized: PerUserSettings = {};
Object.keys(settings).sort().forEach((key) => {
const typedKey = key as keyof PerUserSettings;
const value = settings[typedKey];
if (value !== null && value !== undefined) {
normalized[typedKey] = value;
}
});
return normalized;
};
export const useUserForm = () => {
const [createForm, setCreateForm] = useState<CreateUserFormState>({ ...INITIAL_CREATE_FORM });
@@ -11,6 +24,7 @@ export const useUserForm = () => {
const [downloadDefaults, setDownloadDefaults] = useState<DownloadDefaults | null>(null);
const [deliveryPreferences, setDeliveryPreferences] = useState<DeliveryPreferencesResponse | null>(null);
const [userSettings, setUserSettings] = useState<PerUserSettings>({});
const [originalUserSettings, setOriginalUserSettings] = useState<PerUserSettings>({});
const [userOverridableSettings, setUserOverridableSettings] = useState<Set<string>>(new Set());
const resetCreateForm = () => setCreateForm({ ...INITIAL_CREATE_FORM });
@@ -19,6 +33,7 @@ export const useUserForm = () => {
setDownloadDefaults(null);
setDeliveryPreferences(null);
setUserSettings({});
setOriginalUserSettings({});
setUserOverridableSettings(new Set());
};
@@ -29,10 +44,12 @@ export const useUserForm = () => {
};
const applyUserEditContext = (context: UserEditContext) => {
const normalizedSettings = normalizeUserSettings(context.userSettings);
setEditingUser({ ...context.user });
setDownloadDefaults(context.downloadDefaults);
setDeliveryPreferences(context.deliveryPreferences);
setUserSettings(context.userSettings);
setUserSettings(normalizedSettings);
setOriginalUserSettings(normalizedSettings);
setUserOverridableSettings(new Set(context.userOverridableSettings));
};
@@ -44,6 +61,9 @@ export const useUserForm = () => {
};
const isUserOverridable = (key: keyof PerUserSettings) => userOverridableSettings.has(String(key));
const hasUserSettingsChanges =
JSON.stringify(buildUserSettingsPayload(userSettings, userOverridableSettings, deliveryPreferences))
!== JSON.stringify(buildUserSettingsPayload(originalUserSettings, userOverridableSettings, deliveryPreferences));
return {
createForm,
@@ -63,6 +83,7 @@ export const useUserForm = () => {
deliveryPreferences,
userSettings,
setUserSettings,
hasUserSettingsChanges,
userOverridableSettings,
isUserOverridable,
};
@@ -8,11 +8,13 @@ import {
updateAdminUser,
} from '../../../services/api';
import { CreateUserFormState, PerUserSettings } from './types';
import { buildUserSettingsPayload } from './settingsPayload';
const MIN_PASSWORD_LENGTH = 4;
interface UseUserMutationsParams {
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
fetchUsers: () => Promise<void>;
fetchUsers: () => Promise<AdminUser[]>;
users: AdminUser[];
createForm: CreateUserFormState;
resetCreateForm: () => void;
editingUser: AdminUser | null;
@@ -24,24 +26,32 @@ interface UseUserMutationsParams {
onEditSaveSuccess?: () => void;
}
interface SaveEditedUserOptions {
includeProfile?: boolean;
includePassword?: boolean;
includeSettings?: boolean;
}
const getPasswordError = (password: string, passwordConfirm: string) => {
if (!password) return null;
if (password.length < MIN_PASSWORD_LENGTH) return `Password must be at least ${MIN_PASSWORD_LENGTH} characters`;
return password === passwordConfirm ? null : 'Passwords do not match';
};
const buildSettingsPayload = (userSettings: PerUserSettings, userOverridableSettings: Set<string>, deliveryPreferences: DeliveryPreferencesResponse | null) =>
(deliveryPreferences?.keys || [...userOverridableSettings]).reduce<Record<string, unknown>>((payload, key) => {
const typedKey = key as keyof PerUserSettings;
payload[key] = Object.prototype.hasOwnProperty.call(userSettings, typedKey) && userSettings[typedKey] !== null && userSettings[typedKey] !== undefined
? (userSettings[typedKey] ?? '')
: null;
return payload;
}, {});
const countLocalPasswordAdmins = (users: AdminUser[]): number =>
users.filter((user) => user.auth_source === 'builtin' && user.role === 'admin').length;
const authSourceLabel: Record<AdminUser['auth_source'], string> = {
builtin: 'Local',
oidc: 'OIDC',
proxy: 'Proxy',
cwa: 'CWA',
};
export const useUserMutations = ({
onShowToast,
fetchUsers,
users,
createForm,
resetCreateForm,
editingUser,
@@ -82,43 +92,91 @@ export const useUserMutations = ({
}
};
const saveEditedUser = async () => {
const saveEditedUser = async ({
includeProfile = true,
includePassword = true,
includeSettings = true,
}: SaveEditedUserOptions = {}) => {
if (!editingUser) return false;
const passwordError = getPasswordError(editPassword, editPasswordConfirm);
const passwordError = includePassword ? getPasswordError(editPassword, editPasswordConfirm) : null;
if (passwordError) return fail(passwordError);
const localAdminsBeforeSave = includeProfile ? countLocalPasswordAdmins(users) : 0;
const caps = editingUser.edit_capabilities;
const settingsPayload = buildSettingsPayload(userSettings, userOverridableSettings, deliveryPreferences);
const settingsPayload = includeSettings
? buildUserSettingsPayload(userSettings, userOverridableSettings, deliveryPreferences)
: null;
const updatePayload: Partial<Pick<AdminUser, 'role' | 'email' | 'display_name'>> & {
password?: string;
settings?: Record<string, unknown>;
} = {};
if (includeProfile) {
if (caps.canEditEmail) {
updatePayload.email = editingUser.email;
}
if (caps.canEditDisplayName) {
updatePayload.display_name = editingUser.display_name;
}
if (caps.canEditRole) {
updatePayload.role = editingUser.role;
}
}
if (includePassword && caps.canSetPassword && editPassword) {
updatePayload.password = editPassword;
}
if (settingsPayload && Object.keys(settingsPayload).length > 0) {
updatePayload.settings = settingsPayload;
}
setSaving(true);
try {
await updateAdminUser(editingUser.id, {
...(caps.canEditEmail ? { email: editingUser.email } : {}),
...(caps.canEditDisplayName ? { display_name: editingUser.display_name } : {}),
...(caps.canEditRole ? { role: editingUser.role } : {}),
...(caps.canSetPassword && editPassword ? { password: editPassword } : {}),
...(Object.keys(settingsPayload).length > 0 ? { settings: settingsPayload } : {}),
});
await updateAdminUser(editingUser.id, updatePayload);
onEditSaveSuccess?.();
onShowToast?.('User updated', 'success');
await fetchUsers();
onShowToast?.(
includeSettings && !includeProfile && !includePassword ? 'User preferences updated' : 'User updated',
'success',
);
const refreshedUsers = await fetchUsers();
if (includeProfile && localAdminsBeforeSave > 0 && countLocalPasswordAdmins(refreshedUsers) === 0) {
onShowToast?.(
"No local admin accounts remain. Authentication will fall back to 'No Authentication' until a local admin is created.",
'info',
);
}
return true;
} catch {
return fail('Failed to update user');
} catch (err) {
const message = err instanceof Error ? err.message : 'Failed to update user';
return fail(`Failed to update user: ${message}`);
} finally {
setSaving(false);
}
};
const deleteUser = async (userId: number) => {
const deletedUser = users.find((user) => user.id === userId) || null;
const localAdminsBeforeDelete = countLocalPasswordAdmins(users);
setDeletingUserId(userId);
try {
await deleteAdminUser(userId);
onShowToast?.('User deleted', 'success');
await fetchUsers();
const refreshedUsers = await fetchUsers();
if (deletedUser && deletedUser.auth_source !== 'builtin') {
onShowToast?.(
`${authSourceLabel[deletedUser.auth_source]} users may be re-provisioned by your authentication source on a future login or sync.`,
'info',
);
}
if (localAdminsBeforeDelete > 0 && countLocalPasswordAdmins(refreshedUsers) === 0) {
onShowToast?.(
"No local admin accounts remain. Authentication will fall back to 'No Authentication' until a local admin is created.",
'info',
);
}
return true;
} catch {
return fail('Failed to delete user');
} catch (err) {
const message = err instanceof Error ? err.message : 'Failed to delete user';
return fail(`Failed to delete user: ${message}`);
} finally {
setDeletingUserId(null);
}
@@ -27,16 +27,23 @@ export const useUsersFetch = ({ onShowToast }: UseUsersFetchParams) => {
const [loading, setLoading] = useState(true);
const [loadError, setLoadError] = useState<string | null>(null);
const fetchUsers = useCallback(async () => {
const shouldSuppressAccessToast = (message: string): boolean =>
message.toLowerCase().includes('admin access required');
const fetchUsers = useCallback(async (): Promise<AdminUser[]> => {
try {
setLoading(true);
setLoadError(null);
const data = await getAdminUsers();
setUsers(data);
return data;
} catch (err) {
const message = err instanceof Error ? err.message : 'Failed to load users';
setLoadError(message);
onShowToast?.(message, 'error');
if (!shouldSuppressAccessToast(message)) {
onShowToast?.(message, 'error');
}
return [];
} finally {
setLoading(false);
}
+48
View File
@@ -0,0 +1,48 @@
def _base_email_mode_values() -> dict[str, object]:
return {
"BOOKS_OUTPUT_MODE": "email",
"EMAIL_SMTP_HOST": "smtp.example.com",
"EMAIL_FROM": "Shelfmark <mail@example.com>",
}
def test_on_save_downloads_allows_empty_default_email_recipient(monkeypatch):
from shelfmark.config.settings import _on_save_downloads
monkeypatch.setattr("shelfmark.config.settings.load_config_file", lambda _tab: {})
values = {
**_base_email_mode_values(),
"EMAIL_RECIPIENT": "",
}
result = _on_save_downloads(values)
assert result["error"] is False
assert result["values"]["EMAIL_RECIPIENT"] == ""
def test_on_save_downloads_validates_default_email_recipient_format(monkeypatch):
from shelfmark.config.settings import _on_save_downloads
monkeypatch.setattr("shelfmark.config.settings.load_config_file", lambda _tab: {})
values = {
**_base_email_mode_values(),
"EMAIL_RECIPIENT": "Reader <reader@example.com>",
}
result = _on_save_downloads(values)
assert result["error"] is True
assert "valid plain email address" in result["message"]
def test_download_settings_email_recipient_field_uses_default_label():
from shelfmark.config.settings import download_settings
fields = download_settings()
email_field = next(field for field in fields if getattr(field, "key", None) == "EMAIL_RECIPIENT")
assert email_field.label == "Default Email Recipient"
assert "Optional fallback" in email_field.description
+50 -2
View File
@@ -420,6 +420,23 @@ class TestAdminUserUpdateEndpoint:
updated = user_db.get_user(user_id=user["id"])
assert updated["role"] == "admin"
def test_demote_last_admin_allowed(self, admin_client, user_db):
user = user_db.create_user(
username="onlyadmin",
role="admin",
password_hash="hashed_pw",
)
resp = admin_client.put(
f"/api/admin/users/{user['id']}",
json={"role": "user"},
)
assert resp.status_code == 200
updated = user_db.get_user(user_id=user["id"])
assert updated is not None
assert updated["role"] == "user"
def test_update_user_email(self, admin_client, user_db):
user = user_db.create_user(username="alice")
@@ -744,6 +761,12 @@ class TestAdminSyncCwaUsersEndpoint:
role="admin",
auth_source="builtin",
)
stale_cwa = user_db.create_user(
username="stale__cwa",
email="stale@example.com",
role="user",
auth_source="cwa",
)
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"):
with patch("shelfmark.core.admin_routes.CWA_DB_PATH", cwa_db_path):
@@ -753,6 +776,7 @@ class TestAdminSyncCwaUsersEndpoint:
assert resp.json["success"] is True
assert resp.json["created"] == 1
assert resp.json["updated"] == 1
assert resp.json["deleted"] == 1
assert resp.json["total"] == 2
alice_linked = user_db.get_user(user_id=local_email_match["id"])
@@ -775,6 +799,7 @@ class TestAdminSyncCwaUsersEndpoint:
)
assert bob_cwa["username"].startswith("bob__cwa")
assert bob_cwa["role"] == "user"
assert user_db.get_user(user_id=stale_cwa["id"]) is None
def test_sync_cwa_users_rejected_when_not_in_cwa_mode(self, admin_client):
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"):
@@ -1129,14 +1154,23 @@ class TestAdminUserDeleteEndpoint:
assert len(resp.json) == 1
assert resp.json[0]["username"] == "bob"
def test_delete_active_proxy_user_rejected(self, admin_client, user_db):
def test_delete_active_proxy_user_allowed(self, admin_client, user_db):
user = user_db.create_user(username="proxyuser", auth_source="proxy")
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="proxy"):
resp = admin_client.delete(f"/api/admin/users/{user['id']}")
assert resp.status_code == 200
assert resp.json["success"] is True
def test_delete_active_cwa_user_rejected(self, admin_client, user_db):
user = user_db.create_user(username="cwauser", auth_source="cwa")
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"):
resp = admin_client.delete(f"/api/admin/users/{user['id']}")
assert resp.status_code == 400
assert "Cannot delete active PROXY users" in resp.json["error"]
assert "Cannot delete active CWA users" in resp.json["error"]
def test_delete_inactive_proxy_user_allowed(self, admin_client, user_db):
user = user_db.create_user(username="proxyuser", auth_source="proxy")
@@ -1160,6 +1194,20 @@ class TestAdminUserDeleteEndpoint:
assert resp.status_code == 200
assert resp.json["success"] is True
def test_delete_last_local_admin_allowed(self, admin_client, user_db):
user = user_db.create_user(
username="onlyadmin",
password_hash="hashed_pw",
role="admin",
)
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"):
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
# ---------------------------------------------------------------------------
# OIDC lockout prevention (security on_save handler)
+17
View File
@@ -60,3 +60,20 @@ def test_get_ignores_user_override_for_non_overridable_field(monkeypatch):
)
assert config.get("FILE_ORGANIZATION", "rename", user_id=10) == "rename"
def test_get_respects_empty_user_override_for_destination_audiobook(monkeypatch):
monkeypatch.setattr(config, "_ensure_loaded", lambda: None)
monkeypatch.setattr(config, "_cache", {"DESTINATION_AUDIOBOOK": "/global/audiobooks"})
monkeypatch.setattr(
config,
"_field_map",
{"DESTINATION_AUDIOBOOK": (_DummyField(env_supported=True, user_overridable=True), "downloads")},
)
monkeypatch.setattr(config, "_get_user_override", lambda user_id, key: "")
monkeypatch.setattr(
"shelfmark.core.config._get_registry",
lambda: SimpleNamespace(is_value_from_env=lambda field: False),
)
assert config.get("DESTINATION_AUDIOBOOK", "/default", user_id=10) == ""
+39 -1
View File
@@ -5,7 +5,7 @@ import tempfile
import pytest
from shelfmark.core.cwa_user_sync import upsert_cwa_user
from shelfmark.core.cwa_user_sync import sync_cwa_users_from_rows, upsert_cwa_user
from shelfmark.core.user_db import UserDB
@@ -93,3 +93,41 @@ def test_upsert_updates_existing_cwa_user_by_username_before_email(user_db):
assert user["auth_source"] == "cwa"
assert user["email"] == "new@example.com"
assert user["role"] == "admin"
def test_sync_prunes_cwa_users_missing_from_source(user_db):
active_cwa = user_db.create_user(
username="active_cwa",
email="active@example.com",
role="user",
auth_source="cwa",
)
stale_cwa = user_db.create_user(
username="stale_cwa",
email="stale@example.com",
role="admin",
auth_source="cwa",
)
local_builtin = user_db.create_user(
username="local_user",
email="local@example.com",
role="admin",
auth_source="builtin",
)
summary = sync_cwa_users_from_rows(
user_db,
rows=[("active_cwa", 1, "active@example.com")],
)
assert summary["created"] == 0
assert summary["updated"] == 1
assert summary["deleted"] == 1
assert summary["total"] == 1
active_after = user_db.get_user(user_id=active_cwa["id"])
assert active_after is not None
assert active_after["role"] == "admin"
assert user_db.get_user(user_id=stale_cwa["id"]) is None
assert user_db.get_user(user_id=local_builtin["id"]) is not None
+22
View File
@@ -2,8 +2,10 @@
from shelfmark.core.auth_modes import (
determine_auth_mode,
get_settings_tab_from_path,
get_auth_check_admin_status,
is_settings_or_onboarding_path,
requires_admin_for_settings_access,
should_restrict_settings_to_admin,
)
@@ -72,6 +74,26 @@ class TestSettingsRestrictionPolicy:
assert should_restrict_settings_to_admin({"RESTRICT_SETTINGS_TO_ADMIN": True}) is True
assert should_restrict_settings_to_admin({"RESTRICT_SETTINGS_TO_ADMIN": False}) is False
def test_extracts_settings_tab_from_path(self):
assert get_settings_tab_from_path("/api/settings/security") == "security"
assert get_settings_tab_from_path("/api/settings/users/action/open_users_tab") == "users"
assert get_settings_tab_from_path("/api/settings") is None
def test_security_and_users_tabs_always_require_admin(self):
users_config = {"RESTRICT_SETTINGS_TO_ADMIN": False}
assert requires_admin_for_settings_access("/api/settings/security", users_config) is True
assert requires_admin_for_settings_access("/api/settings/users", users_config) is True
def test_other_tabs_follow_global_toggle(self):
assert requires_admin_for_settings_access(
"/api/settings/general",
{"RESTRICT_SETTINGS_TO_ADMIN": False},
) is False
assert requires_admin_for_settings_access(
"/api/settings/general",
{"RESTRICT_SETTINGS_TO_ADMIN": True},
) is True
class TestAuthCheckAdminStatus:
def test_authenticated_admin_when_restricted(self):
@@ -84,3 +84,82 @@ def test_queue_release_uses_user_specific_books_output_mode(monkeypatch):
assert task.output_mode == "email"
assert task.output_args == {"to": "alice@example.com"}
assert ("BOOKS_OUTPUT_MODE", 42) in config_calls
def test_queue_book_email_mode_without_recipient_is_queued(monkeypatch):
import shelfmark.download.orchestrator as orchestrator
captured: dict[str, object] = {}
def fake_get_book_info(_book_id, fetch_download_count=False):
assert fetch_download_count is False
return SimpleNamespace(
title="Test Book",
author="Tester",
format="epub",
size="1 MB",
preview=None,
content="book (fiction)",
)
def fake_config_get(key, default=None, user_id=None):
if key == "BOOKS_OUTPUT_MODE":
return "email" if user_id == 42 else "folder"
if key == "EMAIL_RECIPIENT":
return ""
return default
def fake_add(task):
captured["task"] = task
return True
monkeypatch.setattr(orchestrator.direct_download, "get_book_info", fake_get_book_info)
monkeypatch.setattr(orchestrator.config, "get", fake_config_get)
monkeypatch.setattr(orchestrator.book_queue, "add", fake_add)
monkeypatch.setattr(orchestrator, "ws_manager", None)
success, error = orchestrator.queue_book("book-1", user_id=42, username="alice")
assert success is True
assert error is None
task = captured["task"]
assert task.output_mode == "email"
assert task.output_args == {}
def test_queue_release_email_mode_without_recipient_is_queued(monkeypatch):
import shelfmark.download.orchestrator as orchestrator
captured: dict[str, object] = {}
def fake_config_get(key, default=None, user_id=None):
if key == "BOOKS_OUTPUT_MODE":
return "email" if user_id == 42 else "folder"
if key == "EMAIL_RECIPIENT":
return ""
return default
def fake_add(task):
captured["task"] = task
return True
monkeypatch.setattr(orchestrator.config, "get", fake_config_get)
monkeypatch.setattr(orchestrator.book_queue, "add", fake_add)
monkeypatch.setattr(orchestrator, "ws_manager", None)
release_data = {
"source": "direct_download",
"source_id": "release-1",
"title": "Release Title",
"content_type": "book (fiction)",
"format": "epub",
"size": "1 MB",
}
success, error = orchestrator.queue_release(release_data, user_id=42, username="alice")
assert success is True
assert error is None
task = captured["task"]
assert task.output_mode == "email"
assert task.output_args == {}
+32
View File
@@ -205,6 +205,38 @@ class TestLoginRequiredDecorator:
assert resp[0]["success"] is True
def test_security_tab_always_blocks_non_admin_even_when_toggle_off(self, main_module, view):
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
with patch(
"shelfmark.core.settings_registry.load_config_file",
return_value={"RESTRICT_SETTINGS_TO_ADMIN": False},
):
with main_module.app.test_request_context("/api/settings/security"):
main_module.session["user_id"] = "user"
main_module.session["is_admin"] = False
decorated = main_module.login_required(view)
resp = _as_response(decorated())
data = resp.get_json()
assert resp.status_code == 403
assert "Admin access required" in (data.get("error") or "")
def test_users_tab_always_blocks_non_admin_even_when_toggle_off(self, main_module, view):
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
with patch(
"shelfmark.core.settings_registry.load_config_file",
return_value={"RESTRICT_SETTINGS_TO_ADMIN": False},
):
with main_module.app.test_request_context("/api/settings/users"):
main_module.session["user_id"] = "user"
main_module.session["is_admin"] = False
decorated = main_module.login_required(view)
resp = _as_response(decorated())
data = resp.get_json()
assert resp.status_code == 403
assert "Admin access required" in (data.get("error") or "")
def test_proxy_admin_restriction_blocks_non_admin(self, main_module, view):
with patch.object(main_module, "get_auth_mode", return_value="proxy"):
with patch(