diff --git a/shelfmark/config/migrations.py b/shelfmark/config/migrations.py index 77f86446..8e78d427 100644 --- a/shelfmark/config/migrations.py +++ b/shelfmark/config/migrations.py @@ -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: diff --git a/shelfmark/core/notifications.py b/shelfmark/core/notifications.py index ef3ff773..59e48dfd 100644 --- a/shelfmark/core/notifications.py +++ b/shelfmark/core/notifications.py @@ -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)" diff --git a/shelfmark/main.py b/shelfmark/main.py index d510e680..7e284c53 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -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 diff --git a/src/frontend/src/components/ReleaseModal.tsx b/src/frontend/src/components/ReleaseModal.tsx index 9660a167..3581a040 100644 --- a/src/frontend/src/components/ReleaseModal.tsx +++ b/src/frontend/src/components/ReleaseModal.tsx @@ -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(); - -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(); - -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, path: string): unknown { - return path.split('.').reduce((current, key) => { - if (current && typeof current === 'object' && key in (current as Record)) { - return (current as Record)[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, sortKey); - const bVal = getNestedSortValue(b as unknown as Record, 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[] { - const seen = new Set(); - 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)) { - 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]; diff --git a/src/frontend/src/utils/releaseCache.ts b/src/frontend/src/utils/releaseCache.ts new file mode 100644 index 00000000..39f4b634 --- /dev/null +++ b/src/frontend/src/utils/releaseCache.ts @@ -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(); + +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(); + +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); +} diff --git a/src/frontend/src/utils/releaseScoring.ts b/src/frontend/src/utils/releaseScoring.ts new file mode 100644 index 00000000..49ee81b1 --- /dev/null +++ b/src/frontend/src/utils/releaseScoring.ts @@ -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[] { + const seen = new Set(); + 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)) { + 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); +} diff --git a/src/frontend/src/utils/releaseSort.ts b/src/frontend/src/utils/releaseSort.ts new file mode 100644 index 00000000..461725fb --- /dev/null +++ b/src/frontend/src/utils/releaseSort.ts @@ -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, path: string): unknown { + return path.split('.').reduce((current, key) => { + if (current && typeof current === 'object' && key in (current as Record)) { + return (current as Record)[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, sortKey); + const bVal = getNestedSortValue(b as unknown as Record, 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; + }); +} diff --git a/tests/config/test_security.py b/tests/config/test_security.py index 04342bc7..f9f26e4c 100644 --- a/tests/config/test_security.py +++ b/tests/config/test_security.py @@ -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()): diff --git a/tests/core/test_notifications.py b/tests/core/test_notifications.py index a14f7c01..29c935da 100644 --- a/tests/core/test_notifications.py +++ b/tests/core/test_notifications.py @@ -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": diff --git a/tests/e2e/test_proxy_auth_middleware.py b/tests/e2e/test_proxy_auth_middleware.py index 685201e0..acfd2f63 100644 --- a/tests/e2e/test_proxy_auth_middleware.py +++ b/tests/e2e/test_proxy_auth_middleware.py @@ -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(