starred/shelfmark
1
0
Fork 0
mirror of https://github.com/calibrain/shelfmark.git synced 2026-09-24 15:00:19 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Fixes: Auth edge cases, apprise logging, scoring and release refactors (#665)

Browse Source 
- Added migration for builtin auth users who used dev builds during
multi-user development
- Display apprise errors in logging
- Fix user provisioning in reverse proxy auth setups
- Refactor scoring and release modal utils
This commit is contained in:
Alex
2026-02-27 10:21:06 +00:00
committed by GitHub
parent ac36d539c8
commit e7d2845235
10 changed files with 524 additions and 306 deletions
Download Patch File Download Diff File Expand all files Collapse all files
shelfmark/config/migrations.py
+12
View File
@@ -79,6 +79,18 @@ def migrate_security_settings(
logger.info("Removed deprecated USE_CWA_AUTH setting (AUTH_METHOD already exists)")
migrated_security = True
# Backfill AUTH_METHOD for configs that have builtin credentials but
# were never migrated from USE_CWA_AUTH (e.g. dev builds that predated
# the AUTH_METHOD field).
if "AUTH_METHOD" not in config:
if config.get("BUILTIN_USERNAME") and config.get("BUILTIN_PASSWORD_HASH"):
config["AUTH_METHOD"] = "builtin"
migrated_security = True
logger.info(
"Backfilled AUTH_METHOD='builtin' from legacy "
"BUILTIN_USERNAME/BUILTIN_PASSWORD_HASH credentials"
)
if "RESTRICT_SETTINGS_TO_ADMIN" not in users_config:
legacy_restrict = _pick_legacy_settings_restriction(config)
if legacy_restrict is not None:
shelfmark/core/notifications.py
+37 -1
View File
@@ -6,6 +6,7 @@ from concurrent.futures import ThreadPoolExecutor
from dataclasses import dataclass
from enum import Enum
from typing import Any, Iterable
from urllib.parse import urlsplit
try:
import apprise
@@ -78,6 +79,18 @@ def _normalize_urls(value: Any) -> list[str]:
return normalized
def _extract_url_schemes(urls: Iterable[str]) -> list[str]:
schemes: list[str] = []
seen: set[str] = set()
for raw_url in urls:
scheme = urlsplit(str(raw_url or "")).scheme.lower()
if not scheme or scheme in seen:
continue
seen.add(scheme)
schemes.append(scheme)
return schemes
def _normalize_routes(value: Any) -> list[dict[str, str]]:
if not isinstance(value, list):
return []
@@ -228,6 +241,7 @@ def _dispatch_to_apprise(
notify_type: Any,
) -> dict[str, Any]:
normalized_urls = _normalize_urls(list(urls))
url_schemes = _extract_url_schemes(normalized_urls)
if not normalized_urls:
return {"success": False, "message": "No notification URLs configured"}
@@ -240,16 +254,25 @@ def _dispatch_to_apprise(
valid_urls = 0
invalid_urls = 0
for url in normalized_urls:
scheme = urlsplit(url).scheme or "unknown"
try:
added = bool(apobj.add(url))
except Exception:
except Exception as exc:
logger.warning(
"Failed to register notification route URL for scheme '%s': %s",
scheme,
exc,
)
added = False
if added:
valid_urls += 1
else:
invalid_urls += 1
logger.warning("Apprise rejected notification route URL for scheme '%s'", scheme)
if valid_urls == 0:
scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown"
logger.warning("No valid Apprise notification routes after registration for scheme(s): %s", scheme_summary)
return {
"success": False,
"message": "No valid notification URLs configured",
@@ -258,9 +281,22 @@ def _dispatch_to_apprise(
try:
delivered = bool(apobj.notify(title=title, body=body, notify_type=notify_type))
except Exception as exc:
scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown"
logger.warning(
"Apprise notify raised %s for scheme(s): %s",
type(exc).__name__,
scheme_summary,
)
return {"success": False, "message": f"Notification send failed: {type(exc).__name__}: {exc}"}
if not delivered:
scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown"
logger.warning(
"Apprise notify returned False for scheme(s): %s (valid_urls=%s invalid_urls=%s)",
scheme_summary,
valid_urls,
invalid_urls,
)
return {"success": False, "message": "Notification delivery failed"}
message = f"Notification sent to {valid_urls} URL(s)"
shelfmark/main.py
+32 -13
View File
@@ -567,20 +567,39 @@ def proxy_auth_middleware():
session['is_admin'] = is_admin
# Provision proxy-authenticated users into users.db for multi-user features.
if user_db is not None and 'db_user_id' not in session:
role = "admin" if is_admin else "user"
db_user, _ = upsert_external_user(
user_db,
auth_source="proxy",
username=username,
role=role,
collision_strategy="takeover",
context="proxy_request",
)
if db_user is None:
raise RuntimeError("Unexpected proxy user sync result: no user returned")
# Re-provision when db_user_id is missing/stale/mismatched to avoid broken
# sessions after DB resets or auth-mode transitions.
if user_db is not None:
raw_db_user_id = session.get('db_user_id')
session_db_user = None
session['db_user_id'] = db_user["id"]
if raw_db_user_id is not None:
try:
session_db_user = user_db.get_user(user_id=int(raw_db_user_id))
except (TypeError, ValueError):
session_db_user = None
session_db_username = str(session_db_user.get("username") or "").strip() if session_db_user else ""
needs_db_user_sync = (
raw_db_user_id is None
or session_db_user is None
or session_db_username != username
)
if needs_db_user_sync:
role = "admin" if is_admin else "user"
db_user, _ = upsert_external_user(
user_db,
auth_source="proxy",
username=username,
role=role,
collision_strategy="takeover",
context="proxy_request",
)
if db_user is None:
raise RuntimeError("Unexpected proxy user sync result: no user returned")
session['db_user_id'] = db_user["id"]
session.permanent = False
src/frontend/src/components/ReleaseModal.tsx
+6 -291
View File
@@ -32,286 +32,10 @@ import {
buildLanguageNormalizer,
} from '../utils/languageFilters';
import { getReleaseFormats } from '../utils/releaseFormats';
import { getBookTitleCandidates, getBookAuthorCandidates, sortReleasesByBookMatch } from '../utils/releaseScoring';
import { getCachedReleases, setCachedReleases, invalidateCachedReleases } from '../utils/releaseCache';
import { SortState, getSavedSort, saveSort, clearSort, inferDefaultDirection, sortReleases } from '../utils/releaseSort';
// Module-level cache for release search results
// Key format: `${provider}:${provider_id}:${source}:${contentType}`
// This persists across modal open/close cycles
const releaseCache = new Map<string, ReleasesResponse>();
function getCacheKey(provider: string, providerId: string, source: string, contentType: string): string {
return `${provider}:${providerId}:${source}:${contentType}`;
}
// Default cache TTL (5 minutes) - sources can override via column_config.cache_ttl_seconds
const DEFAULT_CACHE_TTL_MS = 5 * 60 * 1000;
const cacheTimestamps = new Map<string, number>();
function getCachedReleases(provider: string, providerId: string, source: string, contentType: string): ReleasesResponse | null {
const key = getCacheKey(provider, providerId, source, contentType);
const timestamp = cacheTimestamps.get(key);
const cached = releaseCache.get(key);
if (!timestamp || !cached) {
return null;
}
// Use source-specific TTL if available, otherwise default
const ttlSeconds = cached.column_config?.cache_ttl_seconds;
const ttlMs = ttlSeconds ? ttlSeconds * 1000 : DEFAULT_CACHE_TTL_MS;
// Check if cache entry is not expired
if (Date.now() - timestamp < ttlMs) {
return cached;
}
// Clear expired entry
releaseCache.delete(key);
cacheTimestamps.delete(key);
return null;
}
function setCachedReleases(provider: string, providerId: string, source: string, contentType: string, data: ReleasesResponse): void {
const key = getCacheKey(provider, providerId, source, contentType);
releaseCache.set(key, data);
cacheTimestamps.set(key, Date.now());
}
// LocalStorage helpers for persisting sort preferences per source
const SORT_STORAGE_PREFIX = 'cwa-bd-release-sort-';
interface SortState {
key: string;
direction: 'asc' | 'desc';
}
function getSavedSort(sourceName: string): SortState | null {
try {
const saved = localStorage.getItem(`${SORT_STORAGE_PREFIX}${sourceName}`);
if (saved) {
const parsed = JSON.parse(saved);
if (parsed.key && parsed.direction) {
return parsed as SortState;
}
}
return null;
} catch {
return null;
}
}
function saveSort(sourceName: string, sortState: SortState): void {
try {
localStorage.setItem(`${SORT_STORAGE_PREFIX}${sourceName}`, JSON.stringify(sortState));
} catch {
// localStorage may be unavailable in private browsing
}
}
// Get nested value from an object using dot notation path
function getNestedSortValue(obj: Record<string, unknown>, path: string): unknown {
return path.split('.').reduce<unknown>((current, key) => {
if (current && typeof current === 'object' && key in (current as Record<string, unknown>)) {
return (current as Record<string, unknown>)[key];
}
return undefined;
}, obj);
}
// Infer default sort direction from column render type
function inferDefaultDirection(renderType: string): 'asc' | 'desc' {
// Numeric types sort descending by default (bigger is usually better)
if (renderType === 'size' || renderType === 'number' || renderType === 'peers') {
return 'desc';
}
// Text/badge types sort ascending (alphabetical)
return 'asc';
}
// Sort releases by a column
function sortReleases(
releases: Release[],
sortKey: string,
direction: 'asc' | 'desc'
): Release[] {
return [...releases].sort((a, b) => {
const aVal = getNestedSortValue(a as unknown as Record<string, unknown>, sortKey);
const bVal = getNestedSortValue(b as unknown as Record<string, unknown>, sortKey);
// Handle null/undefined - sort them to the end
if (aVal == null && bVal == null) return 0;
if (aVal == null) return 1;
if (bVal == null) return -1;
// Numeric comparison
if (typeof aVal === 'number' && typeof bVal === 'number') {
return direction === 'asc' ? aVal - bVal : bVal - aVal;
}
// String comparison (case-insensitive)
const aStr = String(aVal).toLowerCase();
const bStr = String(bVal).toLowerCase();
const cmp = aStr.localeCompare(bStr);
return direction === 'asc' ? cmp : -cmp;
});
}
function normalizeMatchText(value: string): string {
return value
.toLowerCase()
.replace(/[^a-z0-9]+/g, ' ')
.replace(/\s+/g, ' ')
.trim();
}
function collectNormalizedStrings(values: Array<string | null | undefined>): string[] {
const seen = new Set<string>();
const normalizedValues: string[] = [];
for (const value of values) {
if (!value) continue;
const normalized = normalizeMatchText(value);
if (!normalized || seen.has(normalized)) continue;
seen.add(normalized);
normalizedValues.push(normalized);
}
return normalizedValues;
}
function getLocalizedTitleValues(raw: unknown): string[] {
if (!raw || typeof raw !== 'object' || Array.isArray(raw)) {
return [];
}
const values: string[] = [];
for (const value of Object.values(raw as Record<string, unknown>)) {
if (typeof value === 'string' && value.trim()) {
values.push(value);
}
}
return values;
}
function splitAuthorString(author: string): string[] {
return author
.split(',')
.map((part) => part.trim())
.filter(Boolean);
}
function getBookTitleCandidates(
uiBook: Book | null,
responseBook: ReleasesResponse['book'] | undefined
): string[] {
return collectNormalizedStrings([
responseBook?.search_title,
responseBook?.title,
...getLocalizedTitleValues(responseBook?.titles_by_language),
uiBook?.search_title,
uiBook?.title,
...getLocalizedTitleValues(uiBook?.titles_by_language),
]);
}
function getBookAuthorCandidates(
uiBook: Book | null,
responseBook: ReleasesResponse['book'] | undefined
): string[] {
const responseAuthors = responseBook?.authors ?? [];
const uiAuthors = uiBook?.authors ?? [];
const uiAuthorParts = uiBook?.author ? splitAuthorString(uiBook.author) : [];
return collectNormalizedStrings([
responseBook?.search_author,
...responseAuthors,
uiBook?.search_author,
...uiAuthors,
...uiAuthorParts,
]);
}
function getReleaseAuthorForMatch(release: Release): string | null {
const rawAuthor = release.extra?.author;
if (typeof rawAuthor !== 'string') {
return null;
}
const normalized = normalizeMatchText(rawAuthor);
return normalized || null;
}
function hasExactAuthorMatch(release: Release, authorCandidates: string[]): boolean {
if (authorCandidates.length === 0) {
return false;
}
const releaseAuthor = getReleaseAuthorForMatch(release);
if (!releaseAuthor) {
return false;
}
return authorCandidates.includes(releaseAuthor);
}
function getTitleMatchScore(title: string, titleCandidate: string): number {
const normalizedTitle = normalizeMatchText(title);
if (!normalizedTitle || !titleCandidate) {
return 0;
}
if (normalizedTitle === titleCandidate) {
return 10000;
}
let score = 0;
if (normalizedTitle.startsWith(titleCandidate)) {
score += 6000;
} else if (normalizedTitle.includes(titleCandidate)) {
score += 3000;
}
const candidateTokens = titleCandidate.split(' ').filter((token) => token.length > 1);
if (candidateTokens.length > 0) {
const titleTokens = new Set(normalizedTitle.split(' '));
const matchedTokens = candidateTokens.filter((token) => (
titleTokens.has(token) || normalizedTitle.includes(token)
)).length;
score += Math.round((matchedTokens / candidateTokens.length) * 2500);
}
// Prefer closer-length titles when match quality is otherwise similar.
score -= Math.abs(normalizedTitle.length - titleCandidate.length);
return score;
}
function sortReleasesByBookMatch(
releases: Release[],
titleCandidates: string[],
authorCandidates: string[]
): Release[] {
if (titleCandidates.length === 0) {
return releases;
}
return releases
.map((release, index) => ({
release,
index,
score: titleCandidates.reduce((best, candidate) => (
Math.max(best, getTitleMatchScore(release.title, candidate))
), 0) + (hasExactAuthorMatch(release, authorCandidates) ? 1500 : 0),
}))
.sort((a, b) => {
const scoreDiff = b.score - a.score;
if (scoreDiff !== 0) {
return scoreDiff;
}
return a.index - b.index;
})
.map(({ release }) => release);
}
// Default column configuration (fallback when backend doesn't provide one)
const DEFAULT_COLUMN_CONFIG: ReleaseColumnConfig = {
@@ -1356,12 +1080,7 @@ export const ReleaseModal = ({
delete next[activeTab];
return next;
});
// Clear from localStorage
try {
localStorage.removeItem(`${SORT_STORAGE_PREFIX}${activeTab}`);
} catch {
// Ignore localStorage errors
}
clearSort(activeTab);
return;
}
@@ -1978,9 +1697,7 @@ export const ReleaseModal = ({
const bookId = book.provider_id;
// Clear cache and state
const key = getCacheKey(provider, bookId, activeTab, contentType);
releaseCache.delete(key);
cacheTimestamps.delete(key);
invalidateCachedReleases(provider, bookId, activeTab, contentType);
setExpandedBySource((prev) => {
const next = { ...prev };
delete next[activeTab];
@@ -2044,9 +1761,7 @@ export const ReleaseModal = ({
const bookId = book.provider_id;
// Clear cache + clear visible results so user gets feedback.
const key = getCacheKey(provider, bookId, activeTab, contentType);
releaseCache.delete(key);
cacheTimestamps.delete(key);
invalidateCachedReleases(provider, bookId, activeTab, contentType);
setExpandedBySource((prev) => {
const next = { ...prev };
delete next[activeTab];
src/frontend/src/utils/releaseCache.ts
+51
View File
@@ -0,0 +1,51 @@
import { ReleasesResponse } from '../types';
// Module-level cache for release search results
// Key format: `${provider}:${provider_id}:${source}:${contentType}`
// This persists across modal open/close cycles
const releaseCache = new Map<string, ReleasesResponse>();
function getCacheKey(provider: string, providerId: string, source: string, contentType: string): string {
return `${provider}:${providerId}:${source}:${contentType}`;
}
// Default cache TTL (5 minutes) - sources can override via column_config.cache_ttl_seconds
const DEFAULT_CACHE_TTL_MS = 5 * 60 * 1000;
const cacheTimestamps = new Map<string, number>();
export function getCachedReleases(provider: string, providerId: string, source: string, contentType: string): ReleasesResponse | null {
const key = getCacheKey(provider, providerId, source, contentType);
const timestamp = cacheTimestamps.get(key);
const cached = releaseCache.get(key);
if (!timestamp || !cached) {
return null;
}
// Use source-specific TTL if available, otherwise default
const ttlSeconds = cached.column_config?.cache_ttl_seconds;
const ttlMs = ttlSeconds ? ttlSeconds * 1000 : DEFAULT_CACHE_TTL_MS;
// Check if cache entry is not expired
if (Date.now() - timestamp < ttlMs) {
return cached;
}
// Clear expired entry
releaseCache.delete(key);
cacheTimestamps.delete(key);
return null;
}
export function setCachedReleases(provider: string, providerId: string, source: string, contentType: string, data: ReleasesResponse): void {
const key = getCacheKey(provider, providerId, source, contentType);
releaseCache.set(key, data);
cacheTimestamps.set(key, Date.now());
}
export function invalidateCachedReleases(provider: string, providerId: string, source: string, contentType: string): void {
const key = getCacheKey(provider, providerId, source, contentType);
releaseCache.delete(key);
cacheTimestamps.delete(key);
}
src/frontend/src/utils/releaseScoring.ts
+172
View File
@@ -0,0 +1,172 @@
import { Book, Release, ReleasesResponse } from '../types';
function normalizeMatchText(value: string): string {
return value
.toLowerCase()
.replace(/[^a-z0-9]+/g, ' ')
.replace(/\s+/g, ' ')
.trim();
}
function collectNormalizedStrings(values: Array<string | null | undefined>): string[] {
const seen = new Set<string>();
const normalizedValues: string[] = [];
for (const value of values) {
if (!value) continue;
const normalized = normalizeMatchText(value);
if (!normalized || seen.has(normalized)) continue;
seen.add(normalized);
normalizedValues.push(normalized);
}
return normalizedValues;
}
function getLocalizedTitleValues(raw: unknown): string[] {
if (!raw || typeof raw !== 'object' || Array.isArray(raw)) {
return [];
}
const values: string[] = [];
for (const value of Object.values(raw as Record<string, unknown>)) {
if (typeof value === 'string' && value.trim()) {
values.push(value);
}
}
return values;
}
function splitAuthorString(author: string): string[] {
return author
.split(',')
.map((part) => part.trim())
.filter(Boolean);
}
export function getBookTitleCandidates(
uiBook: Book | null,
responseBook: ReleasesResponse['book'] | undefined
): string[] {
return collectNormalizedStrings([
responseBook?.search_title,
responseBook?.title,
...getLocalizedTitleValues(responseBook?.titles_by_language),
uiBook?.search_title,
uiBook?.title,
...getLocalizedTitleValues(uiBook?.titles_by_language),
]);
}
export function getBookAuthorCandidates(
uiBook: Book | null,
responseBook: ReleasesResponse['book'] | undefined
): string[] {
const responseAuthors = responseBook?.authors ?? [];
const uiAuthors = uiBook?.authors ?? [];
const uiAuthorParts = uiBook?.author ? splitAuthorString(uiBook.author) : [];
return collectNormalizedStrings([
responseBook?.search_author,
...responseAuthors,
uiBook?.search_author,
...uiAuthors,
...uiAuthorParts,
]);
}
function getReleaseAuthorForMatch(release: Release): string | null {
const rawAuthor = release.extra?.author;
if (typeof rawAuthor !== 'string') {
return null;
}
const normalized = normalizeMatchText(rawAuthor);
return normalized || null;
}
function hasAuthorMatch(release: Release, authorCandidates: string[]): boolean {
if (authorCandidates.length === 0) {
return false;
}
const releaseAuthor = getReleaseAuthorForMatch(release);
if (!releaseAuthor) {
return false;
}
const releaseTokens = new Set(releaseAuthor.split(' ').filter(Boolean));
return authorCandidates.some((candidate) => {
const candidateTokens = candidate.split(' ').filter(Boolean);
return candidateTokens.length > 0 && candidateTokens.every((token) => releaseTokens.has(token));
});
}
const STOP_WORDS = new Set(['a', 'an', 'the', 'and', 'or', 'of', 'in', 'to', 'for', 'on', 'at', 'by', 'is']);
function removeStopWords(text: string): string {
return text.split(' ').filter((w) => !STOP_WORDS.has(w)).join(' ');
}
function getTitleMatchScore(title: string, titleCandidate: string): number {
const normalizedTitle = normalizeMatchText(title);
if (!normalizedTitle || !titleCandidate) {
return 0;
}
// Exact match on full normalized strings (highest score)
if (normalizedTitle === titleCandidate) {
return 10000;
}
// Also check with stop words stripped for substring comparisons
const strippedTitle = removeStopWords(normalizedTitle);
const strippedCandidate = removeStopWords(titleCandidate);
let score = 0;
if (normalizedTitle.startsWith(titleCandidate) || strippedTitle.startsWith(strippedCandidate)) {
score += 6000;
} else if (normalizedTitle.includes(titleCandidate) || strippedTitle.includes(strippedCandidate)) {
score += 3000;
}
// Token overlap uses stop-word-stripped versions so only meaningful words are compared
const candidateTokens = strippedCandidate.split(' ').filter((token) => token.length >= 3);
if (candidateTokens.length > 0) {
const titleTokens = new Set(strippedTitle.split(' '));
const matchedTokens = candidateTokens.filter((token) => titleTokens.has(token)).length;
score += Math.round((matchedTokens / candidateTokens.length) * 2500);
}
// Prefer closer-length titles when match quality is otherwise similar.
score -= Math.min(Math.abs(normalizedTitle.length - titleCandidate.length), 100);
return score;
}
export function sortReleasesByBookMatch(
releases: Release[],
titleCandidates: string[],
authorCandidates: string[]
): Release[] {
if (titleCandidates.length === 0) {
return releases;
}
return releases
.map((release, index) => ({
release,
index,
score: titleCandidates.reduce((best, candidate) => (
Math.max(best, getTitleMatchScore(release.title, candidate))
), 0) + (hasAuthorMatch(release, authorCandidates) ? 1500 : 0),
}))
.sort((a, b) => {
const scoreDiff = b.score - a.score;
if (scoreDiff !== 0) {
return scoreDiff;
}
return a.index - b.index;
})
.map(({ release }) => release);
}
src/frontend/src/utils/releaseSort.ts
+88
View File
@@ -0,0 +1,88 @@
import { Release } from '../types';
export interface SortState {
key: string;
direction: 'asc' | 'desc';
}
// LocalStorage helpers for persisting sort preferences per source
const SORT_STORAGE_PREFIX = 'cwa-bd-release-sort-';
export function getSavedSort(sourceName: string): SortState | null {
try {
const saved = localStorage.getItem(`${SORT_STORAGE_PREFIX}${sourceName}`);
if (saved) {
const parsed = JSON.parse(saved);
if (parsed.key && parsed.direction) {
return parsed as SortState;
}
}
return null;
} catch {
return null;
}
}
export function saveSort(sourceName: string, sortState: SortState): void {
try {
localStorage.setItem(`${SORT_STORAGE_PREFIX}${sourceName}`, JSON.stringify(sortState));
} catch {
// localStorage may be unavailable in private browsing
}
}
export function clearSort(sourceName: string): void {
try {
localStorage.removeItem(`${SORT_STORAGE_PREFIX}${sourceName}`);
} catch {
// localStorage may be unavailable in private browsing
}
}
// Get nested value from an object using dot notation path
function getNestedSortValue(obj: Record<string, unknown>, path: string): unknown {
return path.split('.').reduce<unknown>((current, key) => {
if (current && typeof current === 'object' && key in (current as Record<string, unknown>)) {
return (current as Record<string, unknown>)[key];
}
return undefined;
}, obj);
}
// Infer default sort direction from column render type
export function inferDefaultDirection(renderType: string): 'asc' | 'desc' {
// Numeric types sort descending by default (bigger is usually better)
if (renderType === 'size' || renderType === 'number' || renderType === 'peers') {
return 'desc';
}
// Text/badge types sort ascending (alphabetical)
return 'asc';
}
// Sort releases by a column
export function sortReleases(
releases: Release[],
sortKey: string,
direction: 'asc' | 'desc'
): Release[] {
return [...releases].sort((a, b) => {
const aVal = getNestedSortValue(a as unknown as Record<string, unknown>, sortKey);
const bVal = getNestedSortValue(b as unknown as Record<string, unknown>, sortKey);
// Handle null/undefined - sort them to the end
if (aVal == null && bVal == null) return 0;
if (aVal == null) return 1;
if (bVal == null) return -1;
// Numeric comparison
if (typeof aVal === 'number' && typeof bVal === 'number') {
return direction === 'asc' ? aVal - bVal : bVal - aVal;
}
// String comparison (case-insensitive)
const aStr = String(aVal).toLowerCase();
const bStr = String(bVal).toLowerCase();
const cmp = aStr.localeCompare(bStr);
return direction === 'asc' ? cmp : -cmp;
});
}
tests/config/test_security.py
+33
View File
@@ -199,6 +199,39 @@ class TestSecurityMigration:
assert migrated["AUTH_METHOD"] == "proxy"
assert "USE_CWA_AUTH" not in migrated
def test_migrate_backfills_auth_method_from_legacy_builtin_credentials(
self, temp_config_dir, mock_logger, monkeypatch
):
"""Configs with BUILTIN creds but no AUTH_METHOD should be backfilled to builtin."""
config_root = temp_config_dir.parent
monkeypatch.setenv("CONFIG_DIR", str(config_root))
config_file = temp_config_dir / "config.json"
legacy_config = {
"BUILTIN_USERNAME": "admin",
"BUILTIN_PASSWORD_HASH": "hashed_password",
}
config_file.write_text(json.dumps(legacy_config, indent=2))
with patch("shelfmark.config.security.load_config_file", return_value=legacy_config.copy()):
with patch("shelfmark.core.settings_registry._get_config_file_path", return_value=str(config_file)):
with patch("shelfmark.core.settings_registry._ensure_config_dir"):
with patch("shelfmark.config.security.logger", mock_logger):
from shelfmark.config.security import _migrate_security_settings
_migrate_security_settings()
migrated = json.loads(config_file.read_text())
assert migrated["AUTH_METHOD"] == "builtin"
assert migrated["BUILTIN_USERNAME"] == "admin"
assert migrated["BUILTIN_PASSWORD_HASH"] == "hashed_password"
user_db = UserDB(str(config_root / "users.db"))
user_db.initialize()
user = user_db.get_user(username="admin")
assert user is not None
assert user["role"] == "admin"
def test_migrate_handles_missing_config_file(self, mock_logger):
"""Missing config file should be handled gracefully."""
with patch("shelfmark.config.security.load_config_file", side_effect=FileNotFoundError()):
tests/core/test_notifications.py
+27 -1
View File
@@ -23,6 +23,7 @@ class _FakeAppriseClient:
def __init__(self):
self.add_calls = []
self.notify_calls = []
self.notify_result = True
def add(self, url):
self.add_calls.append(url)
@@ -30,7 +31,7 @@ class _FakeAppriseClient:
def notify(self, **kwargs):
self.notify_calls.append(kwargs)
return True
return self.notify_result
class _FakeAppriseModule:
@@ -182,6 +183,31 @@ def test_dispatch_to_apprise_uses_shelfmark_asset_defaults(monkeypatch):
assert "logo.png" in fake_apprise.asset_kwargs["image_url_logo"]
def test_dispatch_to_apprise_notify_false_returns_generic_failure_and_logs(monkeypatch):
fake_apprise = _FakeAppriseModule()
fake_apprise.client.notify_result = False
monkeypatch.setattr(notifications_module, "apprise", fake_apprise)
warning_messages: list[str] = []
def _fake_warning(message, *args, **kwargs):
_ = kwargs
warning_messages.append(message % args if args else str(message))
monkeypatch.setattr(notifications_module.logger, "warning", _fake_warning)
result = notifications_module._dispatch_to_apprise(
["pover://user_key@app_token"],
title="Test",
body="Body",
notify_type=_FakeNotifyType.INFO,
)
assert result["success"] is False
assert result["message"] == "Notification delivery failed"
assert any("scheme(s): pover" in message for message in warning_messages)
def test_resolve_admin_routes_returns_empty_when_no_routes(monkeypatch):
def _fake_get(key, default=None):
if key == "ADMIN_NOTIFICATION_ROUTES":
tests/e2e/test_proxy_auth_middleware.py
+66
View File
@@ -5,6 +5,7 @@ from __future__ import annotations
import importlib
from typing import Any
from unittest.mock import patch
from uuid import uuid4
import pytest
@@ -123,6 +124,71 @@ class TestProxyAuthMiddleware:
db_user = main_module.user_db.get_user(user_id=db_user_id)
assert db_user["username"] == "proxyuser2"
def test_reprovisions_when_session_db_user_is_stale(self, main_module):
stale_user_id = 99999999
username = f"proxy_stale_{uuid4().hex[:8]}"
assert main_module.user_db.get_user(user_id=stale_user_id) is None
with patch.object(main_module, "get_auth_mode", return_value="proxy"):
with patch(
"shelfmark.core.settings_registry.load_config_file",
return_value={
"PROXY_AUTH_USER_HEADER": "X-Auth-User",
},
):
with main_module.app.test_request_context(
"/api/search",
headers={"X-Auth-User": username},
):
main_module.session["user_id"] = username
main_module.session["db_user_id"] = stale_user_id
result = main_module.proxy_auth_middleware()
assert result is None
assert main_module.session.get("user_id") == username
db_user_id = main_module.session.get("db_user_id")
assert db_user_id is not None
assert db_user_id != stale_user_id
db_user = main_module.user_db.get_user(user_id=db_user_id)
assert db_user is not None
assert db_user["username"] == username
def test_reprovisions_when_session_db_user_points_to_other_username(self, main_module):
username = f"proxy_target_{uuid4().hex[:8]}"
other_user = main_module.user_db.create_user(
username=f"proxy_other_{uuid4().hex[:8]}",
role="user",
auth_source="proxy",
)
with patch.object(main_module, "get_auth_mode", return_value="proxy"):
with patch(
"shelfmark.core.settings_registry.load_config_file",
return_value={
"PROXY_AUTH_USER_HEADER": "X-Auth-User",
},
):
with main_module.app.test_request_context(
"/api/search",
headers={"X-Auth-User": username},
):
main_module.session["user_id"] = username
main_module.session["db_user_id"] = other_user["id"]
result = main_module.proxy_auth_middleware()
assert result is None
assert main_module.session.get("user_id") == username
db_user_id = main_module.session.get("db_user_id")
assert db_user_id is not None
assert db_user_id != other_user["id"]
db_user = main_module.user_db.get_user(user_id=db_user_id)
assert db_user is not None
assert db_user["username"] == username
def test_returns_401_when_header_missing_on_protected_path(self, main_module):
with patch.object(main_module, "get_auth_mode", return_value="proxy"):
with patch(