mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-15 12:38:32 +00:00
Merge commit from fork
A user could set any value for their own overridable settings through
PUT /api/users/me. DESTINATION and DESTINATION_AUDIOBOOK accepted any
string, so a download (including one an admin fulfilled for that user)
could be written to any directory the container can reach, and
get_destination created it on demand.
A user's own destination must now resolve inside the global one (with
{User} expanded on both sides). Absolute paths outside it and any ".."
segment are rejected at save time, and get_destination applies the same
check when the path is used, so a stored or hand-written value outside
the global destination is ignored with a warning and the global
destination is used instead. A relative value is taken relative to the
global destination.
Other self-service values are checked as well: select fields must use
one of their options, EMAIL_RECIPIENT must be a plain address (the same
check the orchestrator and the downloads tab use, now shared from
request_helpers), and Grimmory library and path ids must be non-negative
integers. Admin saves on a user's behalf are unchanged.
The self-service 400 response now carries a message line so the reason
reaches the user in the toast.
Co-authored-by: CaliBrain <calibrain@l4n.xyz>
This commit is contained in:
1 parent
c269230ef3
commit
cb72ce807c
9 files changed
+529
-34
No files matched your search
@@ -878,15 +878,13 @@ def _on_save_downloads(values: dict[str, Any]) -> dict[str, Any]:
|
|||||||
if books_output_mode == "email":
|
if books_output_mode == "email":
|
||||||
from email.utils import parseaddr
|
from email.utils import parseaddr
|
||||||
|
|
||||||
def _is_plain_email_address(addr: str) -> bool:
|
from shelfmark.core.request_helpers import is_plain_email_address
|
||||||
parsed = parseaddr(addr or "")[1]
|
|
||||||
return bool(parsed) and "@" in parsed and parsed == addr
|
|
||||||
|
|
||||||
# Preferred model: single recipient for global default and per-user override.
|
# Preferred model: single recipient for global default and per-user override.
|
||||||
raw_recipient = str(effective.get("EMAIL_RECIPIENT", "") or "").strip()
|
raw_recipient = str(effective.get("EMAIL_RECIPIENT", "") or "").strip()
|
||||||
|
|
||||||
# Optional global fallback: validate only when a default recipient is provided.
|
# Optional global fallback: validate only when a default recipient is provided.
|
||||||
if raw_recipient and not _is_plain_email_address(raw_recipient):
|
if raw_recipient and not is_plain_email_address(raw_recipient):
|
||||||
return {
|
return {
|
||||||
"error": True,
|
"error": True,
|
||||||
"message": "Email recipient must be a valid plain email address.",
|
"message": "Email recipient must be a valid plain email address.",
|
||||||
|
|||||||
@@ -16,8 +16,14 @@ from shelfmark.config.users_settings import (
|
|||||||
validate_search_preference_value,
|
validate_search_preference_value,
|
||||||
)
|
)
|
||||||
from shelfmark.core.config import config as app_config
|
from shelfmark.core.config import config as app_config
|
||||||
|
from shelfmark.core.request_helpers import coerce_int, is_plain_email_address
|
||||||
from shelfmark.core.request_policy import parse_policy_mode, validate_policy_rules
|
from shelfmark.core.request_policy import parse_policy_mode, validate_policy_rules
|
||||||
from shelfmark.core.settings_registry import load_config_file
|
from shelfmark.core.settings_registry import (
|
||||||
|
FieldBase,
|
||||||
|
SelectField,
|
||||||
|
TextField,
|
||||||
|
load_config_file,
|
||||||
|
)
|
||||||
from shelfmark.core.user_settings_overrides import (
|
from shelfmark.core.user_settings_overrides import (
|
||||||
build_user_preferences_payload as _build_user_preferences_payload,
|
build_user_preferences_payload as _build_user_preferences_payload,
|
||||||
)
|
)
|
||||||
@@ -27,36 +33,117 @@ from shelfmark.core.user_settings_overrides import (
|
|||||||
from shelfmark.core.user_settings_overrides import (
|
from shelfmark.core.user_settings_overrides import (
|
||||||
get_settings_registry as _get_settings_registry,
|
get_settings_registry as _get_settings_registry,
|
||||||
)
|
)
|
||||||
|
from shelfmark.core.utils import get_global_destination, resolve_user_destination_override
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from collections.abc import Callable
|
from collections.abc import Callable, Mapping
|
||||||
|
|
||||||
from flask.typing import ResponseReturnValue
|
from flask.typing import ResponseReturnValue
|
||||||
|
|
||||||
from shelfmark.core.user_db import UserDB
|
from shelfmark.core.user_db import UserDB
|
||||||
|
|
||||||
|
|
||||||
|
_USER_DESTINATION_KEYS = {"DESTINATION", "DESTINATION_AUDIOBOOK"}
|
||||||
|
_BOOKLORE_ID_KEYS = {"BOOKLORE_LIBRARY_ID", "BOOKLORE_PATH_ID"}
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_self_service_value(
|
||||||
|
key: str,
|
||||||
|
value: Any,
|
||||||
|
field: FieldBase,
|
||||||
|
user: Mapping[str, Any],
|
||||||
|
) -> tuple[Any, str | None]:
|
||||||
|
"""Check a value a user saves for themselves; admins are not limited this way.
|
||||||
|
|
||||||
|
Returns (normalized_value, error). The value is returned unchanged when no
|
||||||
|
rule applies to the field.
|
||||||
|
"""
|
||||||
|
if key in _USER_DESTINATION_KEYS:
|
||||||
|
if not isinstance(value, str):
|
||||||
|
return None, f"Invalid value for {key}: must be a string"
|
||||||
|
if not value.strip():
|
||||||
|
return value, None
|
||||||
|
base = get_global_destination(is_audiobook=key == "DESTINATION_AUDIOBOOK")
|
||||||
|
raw_user_id = user.get("id")
|
||||||
|
resolved = resolve_user_destination_override(
|
||||||
|
value,
|
||||||
|
base,
|
||||||
|
user_id=raw_user_id if isinstance(raw_user_id, int) else None,
|
||||||
|
username=str(user.get("username") or ""),
|
||||||
|
)
|
||||||
|
if resolved is None:
|
||||||
|
return None, f"Invalid value for {key}: must be a folder inside {base}"
|
||||||
|
return value, None
|
||||||
|
|
||||||
|
if key in _BOOKLORE_ID_KEYS:
|
||||||
|
parsed = coerce_int(value, -1)
|
||||||
|
if parsed < 0 or str(parsed) != str(value).strip():
|
||||||
|
return None, f"Invalid value for {key}: must be a non-negative integer"
|
||||||
|
return value, None
|
||||||
|
|
||||||
|
if key == "EMAIL_RECIPIENT":
|
||||||
|
if not isinstance(value, str):
|
||||||
|
return None, f"Invalid value for {key}: must be a string"
|
||||||
|
recipient = value.strip()
|
||||||
|
if recipient and not is_plain_email_address(recipient):
|
||||||
|
return None, f"Invalid value for {key}: must be a plain email address"
|
||||||
|
return recipient, None
|
||||||
|
|
||||||
|
# Search preferences have their own validator (case-insensitive values).
|
||||||
|
if (
|
||||||
|
isinstance(field, SelectField)
|
||||||
|
and key not in SEARCH_PREFERENCE_VALIDATABLE_KEYS
|
||||||
|
and not callable(field.options)
|
||||||
|
):
|
||||||
|
options = field.options if isinstance(field.options, list) else []
|
||||||
|
allowed = {option.get("value") for option in options if isinstance(option, dict)}
|
||||||
|
if not isinstance(value, str) or value not in allowed:
|
||||||
|
return None, f"Invalid value for {key}: {value!r} is not one of the options"
|
||||||
|
return value, None
|
||||||
|
|
||||||
|
if isinstance(field, TextField) and not isinstance(value, str):
|
||||||
|
return None, f"Invalid value for {key}: must be a string"
|
||||||
|
|
||||||
|
return value, None
|
||||||
|
|
||||||
|
|
||||||
def validate_user_settings(
|
def validate_user_settings(
|
||||||
settings: dict[str, Any],
|
settings: dict[str, Any],
|
||||||
|
*,
|
||||||
|
self_service_user: Mapping[str, Any] | None = None,
|
||||||
) -> tuple[dict[str, Any], list[str]]:
|
) -> tuple[dict[str, Any], list[str]]:
|
||||||
"""Validate and normalize per-user settings overrides."""
|
"""Validate and normalize per-user settings overrides.
|
||||||
|
|
||||||
|
``self_service_user`` is the account saving its own settings; its values are
|
||||||
|
held to the user-scope rules (destinations inside the global one, option
|
||||||
|
lists, plain email). Admin saves on a user's behalf pass None.
|
||||||
|
"""
|
||||||
settings_registry = _get_settings_registry()
|
settings_registry = _get_settings_registry()
|
||||||
field_map = settings_registry.get_settings_field_map()
|
field_map = settings_registry.get_settings_field_map()
|
||||||
overridable_map = settings_registry.get_user_overridable_fields()
|
overridable_map = settings_registry.get_user_overridable_fields()
|
||||||
|
|
||||||
valid: dict[str, Any] = {}
|
valid: dict[str, Any] = {}
|
||||||
errors: list[str] = []
|
errors: list[str] = []
|
||||||
for key, value in settings.items():
|
for key, raw_value in settings.items():
|
||||||
if key not in field_map:
|
if key not in field_map:
|
||||||
errors.append(f"Unknown setting: {key}")
|
errors.append(f"Unknown setting: {key}")
|
||||||
elif key not in overridable_map:
|
elif key not in overridable_map:
|
||||||
errors.append(f"Setting not user-overridable: {key}")
|
errors.append(f"Setting not user-overridable: {key}")
|
||||||
else:
|
else:
|
||||||
# null means "clear the per-user override; use global default"
|
# null means "clear the per-user override; use global default"
|
||||||
if value is None:
|
if raw_value is None:
|
||||||
valid[key] = None
|
valid[key] = None
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
value = raw_value
|
||||||
|
if self_service_user is not None:
|
||||||
|
value, self_service_error = _validate_self_service_value(
|
||||||
|
key, raw_value, overridable_map[key][0], self_service_user
|
||||||
|
)
|
||||||
|
if self_service_error:
|
||||||
|
errors.append(self_service_error)
|
||||||
|
continue
|
||||||
|
|
||||||
if (
|
if (
|
||||||
key in {"REQUEST_POLICY_DEFAULT_EBOOK", "REQUEST_POLICY_DEFAULT_AUDIOBOOK"}
|
key in {"REQUEST_POLICY_DEFAULT_EBOOK", "REQUEST_POLICY_DEFAULT_AUDIOBOOK"}
|
||||||
and parse_policy_mode(value) is None
|
and parse_policy_mode(value) is None
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from datetime import UTC, datetime
|
from datetime import UTC, datetime
|
||||||
|
from email.utils import parseaddr
|
||||||
from typing import Any, Protocol, SupportsIndex, SupportsInt, TypeGuard
|
from typing import Any, Protocol, SupportsIndex, SupportsInt, TypeGuard
|
||||||
|
|
||||||
from shelfmark.core.config import config as app_config
|
from shelfmark.core.config import config as app_config
|
||||||
@@ -115,6 +116,12 @@ def coerce_int(value: object, default: int) -> int:
|
|||||||
return default
|
return default
|
||||||
|
|
||||||
|
|
||||||
|
def is_plain_email_address(value: str) -> bool:
|
||||||
|
"""Return True for a bare address like ``reader@example.com`` (no display name)."""
|
||||||
|
parsed = parseaddr(value or "")[1]
|
||||||
|
return bool(parsed) and "@" in parsed and parsed == value
|
||||||
|
|
||||||
|
|
||||||
def normalize_optional_text(value: object) -> str | None:
|
def normalize_optional_text(value: object) -> str | None:
|
||||||
"""Return a trimmed string or None for empty/non-string input."""
|
"""Return a trimmed string or None for empty/non-string input."""
|
||||||
if not isinstance(value, str):
|
if not isinstance(value, str):
|
||||||
|
|||||||
@@ -454,11 +454,14 @@ def register_self_user_routes(
|
|||||||
}
|
}
|
||||||
), 400
|
), 400
|
||||||
|
|
||||||
validated_settings, validation_errors = validate_user_settings(settings_payload)
|
validated_settings, validation_errors = validate_user_settings(
|
||||||
|
settings_payload, self_service_user=user
|
||||||
|
)
|
||||||
if validation_errors:
|
if validation_errors:
|
||||||
return jsonify(
|
return jsonify(
|
||||||
{
|
{
|
||||||
"error": "Invalid settings payload",
|
"error": "Invalid settings payload",
|
||||||
|
"message": "; ".join(validation_errors),
|
||||||
"details": validation_errors,
|
"details": validation_errors,
|
||||||
}
|
}
|
||||||
), 400
|
), 400
|
||||||
|
|||||||
+107
-18
@@ -10,11 +10,14 @@ from threading import Lock
|
|||||||
from typing import TYPE_CHECKING
|
from typing import TYPE_CHECKING
|
||||||
from urllib.parse import urlparse
|
from urllib.parse import urlparse
|
||||||
|
|
||||||
|
from shelfmark.core.logger import setup_logger
|
||||||
from shelfmark.core.request_helpers import normalize_optional_text
|
from shelfmark.core.request_helpers import normalize_optional_text
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from types import ModuleType
|
from types import ModuleType
|
||||||
|
|
||||||
|
logger = setup_logger(__name__)
|
||||||
|
|
||||||
|
|
||||||
def normalize_http_url(
|
def normalize_http_url(
|
||||||
url: str | None,
|
url: str | None,
|
||||||
@@ -226,38 +229,124 @@ def _expand_user_destination_placeholder(
|
|||||||
return _USER_PLACEHOLDER_PATTERN.sub(resolved_username, path_value)
|
return _USER_PLACEHOLDER_PATTERN.sub(resolved_username, path_value)
|
||||||
|
|
||||||
|
|
||||||
|
def get_global_destination(*, is_audiobook: bool = False) -> str:
|
||||||
|
"""Return the admin-configured destination template, before {User} expansion.
|
||||||
|
|
||||||
|
The audiobook destination falls back to the books destination when unset.
|
||||||
|
"""
|
||||||
|
from shelfmark.core.config import config
|
||||||
|
|
||||||
|
if is_audiobook:
|
||||||
|
audiobook_dest = str(config.get("DESTINATION_AUDIOBOOK", "") or "").strip()
|
||||||
|
if audiobook_dest:
|
||||||
|
return audiobook_dest
|
||||||
|
return str(config.get("DESTINATION", "") or config.get("INGEST_DIR", "/books") or "").strip()
|
||||||
|
|
||||||
|
|
||||||
|
def resolve_user_destination_override(
|
||||||
|
override: object,
|
||||||
|
base: str,
|
||||||
|
*,
|
||||||
|
user_id: int | None = None,
|
||||||
|
username: str | None = None,
|
||||||
|
) -> Path | None:
|
||||||
|
"""Resolve a per-user destination against the global one it must stay inside.
|
||||||
|
|
||||||
|
``{User}`` is expanded in both values. A relative override is taken relative
|
||||||
|
to the global destination. Returns None when the override is empty, contains
|
||||||
|
a ``..`` segment, or does not resolve to the global destination or a path
|
||||||
|
below it; callers then fall back to the global destination.
|
||||||
|
"""
|
||||||
|
if not isinstance(override, str) or not override.strip():
|
||||||
|
return None
|
||||||
|
|
||||||
|
base_expanded = _expand_user_destination_placeholder(
|
||||||
|
base.strip(), user_id=user_id, username=username
|
||||||
|
)
|
||||||
|
base_path = Path(os.path.normpath(base_expanded))
|
||||||
|
if not base_path.is_absolute():
|
||||||
|
return None
|
||||||
|
|
||||||
|
candidate = _expand_user_destination_placeholder(
|
||||||
|
override.strip(), user_id=user_id, username=username
|
||||||
|
)
|
||||||
|
if ".." in Path(candidate).parts:
|
||||||
|
return None
|
||||||
|
candidate_path = Path(os.path.normpath(candidate))
|
||||||
|
if not candidate_path.is_absolute():
|
||||||
|
candidate_path = base_path / candidate_path
|
||||||
|
if not candidate_path.is_relative_to(base_path):
|
||||||
|
return None
|
||||||
|
return candidate_path
|
||||||
|
|
||||||
|
|
||||||
|
def _get_user_destination_override(
|
||||||
|
key: str,
|
||||||
|
base: str,
|
||||||
|
*,
|
||||||
|
user_id: int,
|
||||||
|
username: str | None,
|
||||||
|
) -> Path | None:
|
||||||
|
"""Return the user's own destination for ``key`` when it stays inside ``base``."""
|
||||||
|
from shelfmark.core.config import config
|
||||||
|
|
||||||
|
user_value = config.get(key, "", user_id=user_id)
|
||||||
|
if user_value == config.get(key, ""):
|
||||||
|
return None
|
||||||
|
if not isinstance(user_value, str) or not user_value.strip():
|
||||||
|
return None
|
||||||
|
|
||||||
|
resolved = resolve_user_destination_override(
|
||||||
|
user_value, base, user_id=user_id, username=username
|
||||||
|
)
|
||||||
|
if resolved is None:
|
||||||
|
logger.warning(
|
||||||
|
"Ignoring %s override for user %s: %r is outside the configured destination %r",
|
||||||
|
key,
|
||||||
|
user_id,
|
||||||
|
user_value,
|
||||||
|
base,
|
||||||
|
)
|
||||||
|
return resolved
|
||||||
|
|
||||||
|
|
||||||
def get_destination(
|
def get_destination(
|
||||||
*,
|
*,
|
||||||
is_audiobook: bool = False,
|
is_audiobook: bool = False,
|
||||||
user_id: int | None = None,
|
user_id: int | None = None,
|
||||||
username: str | None = None,
|
username: str | None = None,
|
||||||
) -> Path:
|
) -> Path:
|
||||||
"""Get base destination directory. Audiobooks fall back to main destination."""
|
"""Get base destination directory. Audiobooks fall back to main destination.
|
||||||
from shelfmark.core.config import config
|
|
||||||
|
A per-user override is only honoured while it resolves inside the global
|
||||||
|
destination; anything else falls back to the global value.
|
||||||
|
"""
|
||||||
|
books_base = get_global_destination()
|
||||||
|
|
||||||
if is_audiobook:
|
if is_audiobook:
|
||||||
# Audiobook destination with fallback to main destination
|
audiobook_base = get_global_destination(is_audiobook=True)
|
||||||
audiobook_dest = config.get("DESTINATION_AUDIOBOOK", "", user_id=user_id)
|
if user_id is not None:
|
||||||
if audiobook_dest:
|
user_audiobook = _get_user_destination_override(
|
||||||
|
"DESTINATION_AUDIOBOOK", audiobook_base, user_id=user_id, username=username
|
||||||
|
)
|
||||||
|
if user_audiobook is not None:
|
||||||
|
return user_audiobook
|
||||||
|
if audiobook_base != books_base:
|
||||||
return Path(
|
return Path(
|
||||||
_expand_user_destination_placeholder(
|
_expand_user_destination_placeholder(
|
||||||
str(audiobook_dest),
|
audiobook_base, user_id=user_id, username=username
|
||||||
user_id=user_id,
|
|
||||||
username=username,
|
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
# Main destination (also fallback for audiobooks)
|
if user_id is not None:
|
||||||
# Check new setting first, then legacy INGEST_DIR
|
user_destination = _get_user_destination_override(
|
||||||
destination = config.get("DESTINATION", "", user_id=user_id) or config.get(
|
"DESTINATION", books_base, user_id=user_id, username=username
|
||||||
"INGEST_DIR", "/books"
|
|
||||||
)
|
|
||||||
return Path(
|
|
||||||
_expand_user_destination_placeholder(
|
|
||||||
str(destination),
|
|
||||||
user_id=user_id,
|
|
||||||
username=username,
|
|
||||||
)
|
)
|
||||||
|
if user_destination is not None:
|
||||||
|
return user_destination
|
||||||
|
|
||||||
|
return Path(
|
||||||
|
_expand_user_destination_placeholder(books_base, user_id=user_id, username=username)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ from shelfmark.core.logger import setup_logger
|
|||||||
from shelfmark.core.models import DownloadTask, QueueStatus, SearchMode, normalize_format
|
from shelfmark.core.models import DownloadTask, QueueStatus, SearchMode, normalize_format
|
||||||
from shelfmark.core.queue import book_queue
|
from shelfmark.core.queue import book_queue
|
||||||
from shelfmark.core.request_helpers import (
|
from shelfmark.core.request_helpers import (
|
||||||
|
is_plain_email_address,
|
||||||
normalize_optional_text,
|
normalize_optional_text,
|
||||||
normalize_positive_int,
|
normalize_positive_int,
|
||||||
)
|
)
|
||||||
@@ -90,11 +91,6 @@ _PROGRESS_BROADCAST_COMPLETE_PERCENT = 99
|
|||||||
_PROGRESS_BROADCAST_MIN_DELTA = 10
|
_PROGRESS_BROADCAST_MIN_DELTA = 10
|
||||||
|
|
||||||
|
|
||||||
def _is_plain_email_address(value: str) -> bool:
|
|
||||||
parsed = parseaddr(value or "")[1]
|
|
||||||
return bool(parsed) and "@" in parsed and parsed == value
|
|
||||||
|
|
||||||
|
|
||||||
def _resolve_email_destination(
|
def _resolve_email_destination(
|
||||||
user_id: int | None = None,
|
user_id: int | None = None,
|
||||||
) -> tuple[str | None, str | None]:
|
) -> tuple[str | None, str | None]:
|
||||||
@@ -106,7 +102,7 @@ def _resolve_email_destination(
|
|||||||
"""
|
"""
|
||||||
configured_recipient = str(config.get("EMAIL_RECIPIENT", "", user_id=user_id) or "").strip()
|
configured_recipient = str(config.get("EMAIL_RECIPIENT", "", user_id=user_id) or "").strip()
|
||||||
if configured_recipient:
|
if configured_recipient:
|
||||||
if _is_plain_email_address(configured_recipient):
|
if is_plain_email_address(configured_recipient):
|
||||||
return configured_recipient, None
|
return configured_recipient, None
|
||||||
return None, "Configured email recipient is invalid"
|
return None, "Configured email recipient is invalid"
|
||||||
|
|
||||||
|
|||||||
@@ -699,6 +699,19 @@ class TestAdminUserUpdateEndpoint:
|
|||||||
assert resp.json["settings"]["DESTINATION"] == "/books/alice"
|
assert resp.json["settings"]["DESTINATION"] == "/books/alice"
|
||||||
assert resp.json["settings"]["BOOKLORE_LIBRARY_ID"] == "2"
|
assert resp.json["settings"]["BOOKLORE_LIBRARY_ID"] == "2"
|
||||||
|
|
||||||
|
def test_update_settings_admin_is_not_limited_to_global_destination(
|
||||||
|
self, admin_client, user_db
|
||||||
|
):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
|
||||||
|
resp = admin_client.put(
|
||||||
|
f"/api/admin/users/{user['id']}",
|
||||||
|
json={"settings": {"DESTINATION": "/other/alice", "BOOKS_OUTPUT_MODE": "custom"}},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert resp.json["settings"]["DESTINATION"] == "/other/alice"
|
||||||
|
assert resp.json["settings"]["BOOKS_OUTPUT_MODE"] == "custom"
|
||||||
|
|
||||||
def test_update_response_includes_settings(self, admin_client, user_db):
|
def test_update_response_includes_settings(self, admin_client, user_db):
|
||||||
user = user_db.create_user(username="alice")
|
user = user_db.create_user(username="alice")
|
||||||
user_db.set_user_settings(user["id"], {"DESTINATION": "/books/alice"})
|
user_db.set_user_settings(user["id"], {"DESTINATION": "/books/alice"})
|
||||||
|
|||||||
@@ -5,8 +5,11 @@ Tests that DownloadTask has a user_id field and that the queue
|
|||||||
can be filtered by user.
|
can be filtered by user.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import logging
|
||||||
import sqlite3
|
import sqlite3
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from shelfmark.core.models import DownloadTask, QueueStatus
|
from shelfmark.core.models import DownloadTask, QueueStatus
|
||||||
from shelfmark.core.queue import BookQueue
|
from shelfmark.core.queue import BookQueue
|
||||||
|
|
||||||
@@ -372,3 +375,143 @@ class TestTaskToDictUsername:
|
|||||||
|
|
||||||
result = _task_to_dict(task, current_status=QueueStatus.COMPLETE)
|
result = _task_to_dict(task, current_status=QueueStatus.COMPLETE)
|
||||||
assert result["status"] == QueueStatus.COMPLETE.value
|
assert result["status"] == QueueStatus.COMPLETE.value
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def destination_warnings():
|
||||||
|
"""Collect warnings from the utils logger.
|
||||||
|
|
||||||
|
setup_logger builds its loggers outside the standard hierarchy, so caplog's root
|
||||||
|
handler never sees them.
|
||||||
|
"""
|
||||||
|
from shelfmark.core import utils
|
||||||
|
|
||||||
|
messages: list[str] = []
|
||||||
|
|
||||||
|
class _Capture(logging.Handler):
|
||||||
|
def emit(self, record: logging.LogRecord) -> None:
|
||||||
|
if record.levelno >= logging.WARNING:
|
||||||
|
messages.append(record.getMessage())
|
||||||
|
|
||||||
|
handler = _Capture()
|
||||||
|
utils.logger.addHandler(handler)
|
||||||
|
try:
|
||||||
|
yield messages
|
||||||
|
finally:
|
||||||
|
utils.logger.removeHandler(handler)
|
||||||
|
|
||||||
|
|
||||||
|
class TestUserDestinationContainment:
|
||||||
|
"""A stored per-user destination only applies while it stays inside the global one."""
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _config_get(global_values: dict, user_values: dict):
|
||||||
|
def fake_config_get(key, default=None, user_id=None):
|
||||||
|
if user_id is not None and key in user_values:
|
||||||
|
return user_values[key]
|
||||||
|
return global_values.get(key, default)
|
||||||
|
|
||||||
|
return fake_config_get
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"override",
|
||||||
|
["/config", "/books/../config", "/books/{User}/../../etc", "/booksmark", "../x"],
|
||||||
|
)
|
||||||
|
def test_override_outside_global_is_ignored_with_warning(
|
||||||
|
self, monkeypatch, destination_warnings, override
|
||||||
|
):
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from shelfmark.core.config import config
|
||||||
|
from shelfmark.core.utils import get_destination
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
config,
|
||||||
|
"get",
|
||||||
|
self._config_get({"DESTINATION": "/books"}, {"DESTINATION": override}),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = get_destination(is_audiobook=False, user_id=42, username="alice")
|
||||||
|
|
||||||
|
assert result == Path("/books")
|
||||||
|
assert any("DESTINATION" in message for message in destination_warnings)
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("global_destination", "override", "expected"),
|
||||||
|
[
|
||||||
|
("/books", "/books/alice", "/books/alice"),
|
||||||
|
("/books", "/books/{User}/fiction", "/books/alice/fiction"),
|
||||||
|
("/books", "fiction", "/books/fiction"),
|
||||||
|
("/books/{User}", "/books/alice/fiction", "/books/alice/fiction"),
|
||||||
|
("/books/{User}", "/books/{User}", "/books/alice"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_override_inside_global_is_used(
|
||||||
|
self, monkeypatch, global_destination, override, expected
|
||||||
|
):
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from shelfmark.core.config import config
|
||||||
|
from shelfmark.core.utils import get_destination
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
config,
|
||||||
|
"get",
|
||||||
|
self._config_get({"DESTINATION": global_destination}, {"DESTINATION": override}),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = get_destination(is_audiobook=False, user_id=42, username="alice")
|
||||||
|
assert result == Path(expected)
|
||||||
|
|
||||||
|
def test_audiobook_override_is_checked_against_books_destination(self, monkeypatch):
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from shelfmark.core.config import config
|
||||||
|
from shelfmark.core.utils import get_destination
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
config,
|
||||||
|
"get",
|
||||||
|
self._config_get(
|
||||||
|
{"DESTINATION": "/books", "DESTINATION_AUDIOBOOK": ""},
|
||||||
|
{"DESTINATION_AUDIOBOOK": "/audio/alice"},
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = get_destination(is_audiobook=True, user_id=42, username="alice")
|
||||||
|
assert result == Path("/books")
|
||||||
|
|
||||||
|
def test_audiobook_override_inside_global_audiobook_destination(self, monkeypatch):
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from shelfmark.core.config import config
|
||||||
|
from shelfmark.core.utils import get_destination
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
config,
|
||||||
|
"get",
|
||||||
|
self._config_get(
|
||||||
|
{"DESTINATION": "/books", "DESTINATION_AUDIOBOOK": "/audio"},
|
||||||
|
{"DESTINATION_AUDIOBOOK": "/audio/alice"},
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = get_destination(is_audiobook=True, user_id=42, username="alice")
|
||||||
|
assert result == Path("/audio/alice")
|
||||||
|
|
||||||
|
def test_empty_override_uses_global(self, monkeypatch, destination_warnings):
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from shelfmark.core.config import config
|
||||||
|
from shelfmark.core.utils import get_destination
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
config,
|
||||||
|
"get",
|
||||||
|
self._config_get({"DESTINATION": "/books"}, {"DESTINATION": ""}),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = get_destination(is_audiobook=False, user_id=42, username="alice")
|
||||||
|
|
||||||
|
assert result == Path("/books")
|
||||||
|
assert destination_warnings == []
|
||||||
@@ -301,6 +301,165 @@ def test_users_me_update_rejects_non_object_settings_payload(app, user_db):
|
|||||||
assert resp.json["error"] == "Settings must be an object"
|
assert resp.json["error"] == "Settings must be an object"
|
||||||
|
|
||||||
|
|
||||||
|
def _config_get_with_values(values: dict[str, object]):
|
||||||
|
def _get(key: str, default: object = None, user_id: int | None = None) -> object:
|
||||||
|
del user_id
|
||||||
|
return values.get(key, default)
|
||||||
|
|
||||||
|
return _get
|
||||||
|
|
||||||
|
|
||||||
|
def _put_self_settings(
|
||||||
|
app, user: dict, settings: dict, config_values: dict[str, object] | None = None
|
||||||
|
):
|
||||||
|
values = {"VISIBLE_SELF_SETTINGS_SECTIONS": ["delivery"]}
|
||||||
|
values.update(config_values or {})
|
||||||
|
client = _authed_client_for_user(app, user)
|
||||||
|
with patch("shelfmark.core.self_user_routes.load_active_auth_mode", return_value="builtin"):
|
||||||
|
with patch(
|
||||||
|
"shelfmark.core.self_user_routes.app_config.get",
|
||||||
|
side_effect=_config_get_with_values(values),
|
||||||
|
):
|
||||||
|
return client.put("/api/users/me", json={"settings": settings})
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("global_destination", "override"),
|
||||||
|
[
|
||||||
|
("/books", "/config"),
|
||||||
|
("/books", "/books/../config"),
|
||||||
|
("/books", "/books/{User}/../../config"),
|
||||||
|
("/books", "../config"),
|
||||||
|
("/books", "/booksmark"),
|
||||||
|
("/books/{User}", "/books/bob"),
|
||||||
|
("/books/{User}", "/books/{User}/../bob"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_users_me_update_rejects_destination_outside_global(
|
||||||
|
app, user_db, global_destination, override
|
||||||
|
):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
|
||||||
|
resp = _put_self_settings(
|
||||||
|
app, user, {"DESTINATION": override}, {"DESTINATION": global_destination}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 400
|
||||||
|
assert resp.json["error"] == "Invalid settings payload"
|
||||||
|
assert any("DESTINATION" in detail for detail in resp.json["details"])
|
||||||
|
assert "DESTINATION" in resp.json["message"]
|
||||||
|
assert user_db.get_user_settings(user["id"]) == {}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("global_destination", "override"),
|
||||||
|
[
|
||||||
|
("/books", "/books/alice"),
|
||||||
|
("/books", "/books"),
|
||||||
|
("/books", "/books/{User}/fiction"),
|
||||||
|
("/books", "fiction"),
|
||||||
|
("/books/{User}", "/books/alice/fiction"),
|
||||||
|
("/books/{User}", "/books/{User}/fiction"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_users_me_update_accepts_destination_inside_global(
|
||||||
|
app, user_db, global_destination, override
|
||||||
|
):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
|
||||||
|
resp = _put_self_settings(
|
||||||
|
app, user, {"DESTINATION": override}, {"DESTINATION": global_destination}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert user_db.get_user_settings(user["id"])["DESTINATION"] == override
|
||||||
|
|
||||||
|
|
||||||
|
def test_users_me_update_rejects_non_string_destination(app, user_db):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
|
||||||
|
resp = _put_self_settings(
|
||||||
|
app, user, {"DESTINATION": ["/books/alice"]}, {"DESTINATION": "/books"}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 400
|
||||||
|
assert user_db.get_user_settings(user["id"]) == {}
|
||||||
|
|
||||||
|
|
||||||
|
def test_users_me_update_audiobook_destination_falls_back_to_books_base(app, user_db):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
config_values = {"DESTINATION": "/books", "DESTINATION_AUDIOBOOK": ""}
|
||||||
|
|
||||||
|
rejected = _put_self_settings(
|
||||||
|
app, user, {"DESTINATION_AUDIOBOOK": "/audio/alice"}, config_values
|
||||||
|
)
|
||||||
|
assert rejected.status_code == 400
|
||||||
|
assert any("DESTINATION_AUDIOBOOK" in detail for detail in rejected.json["details"])
|
||||||
|
assert user_db.get_user_settings(user["id"]) == {}
|
||||||
|
|
||||||
|
accepted = _put_self_settings(
|
||||||
|
app, user, {"DESTINATION_AUDIOBOOK": "/books/alice/audio"}, config_values
|
||||||
|
)
|
||||||
|
assert accepted.status_code == 200
|
||||||
|
assert user_db.get_user_settings(user["id"])["DESTINATION_AUDIOBOOK"] == "/books/alice/audio"
|
||||||
|
|
||||||
|
|
||||||
|
def test_users_me_update_audiobook_destination_uses_global_audiobook_base(app, user_db):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
config_values = {"DESTINATION": "/books", "DESTINATION_AUDIOBOOK": "/audio"}
|
||||||
|
|
||||||
|
resp = _put_self_settings(app, user, {"DESTINATION_AUDIOBOOK": "/audio/alice"}, config_values)
|
||||||
|
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert user_db.get_user_settings(user["id"])["DESTINATION_AUDIOBOOK"] == "/audio/alice"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("settings", "expected_key"),
|
||||||
|
[
|
||||||
|
({"BOOKS_OUTPUT_MODE": "ftp"}, "BOOKS_OUTPUT_MODE"),
|
||||||
|
({"BOOKS_OUTPUT_MODE": ["email"]}, "BOOKS_OUTPUT_MODE"),
|
||||||
|
({"EMAIL_RECIPIENT": "not-an-email"}, "EMAIL_RECIPIENT"),
|
||||||
|
({"EMAIL_RECIPIENT": "Alice <alice@example.com>"}, "EMAIL_RECIPIENT"),
|
||||||
|
({"EMAIL_RECIPIENT": ["alice@example.com"]}, "EMAIL_RECIPIENT"),
|
||||||
|
({"BOOKLORE_LIBRARY_ID": -1}, "BOOKLORE_LIBRARY_ID"),
|
||||||
|
({"BOOKLORE_LIBRARY_ID": "abc"}, "BOOKLORE_LIBRARY_ID"),
|
||||||
|
({"BOOKLORE_LIBRARY_ID": True}, "BOOKLORE_LIBRARY_ID"),
|
||||||
|
({"BOOKLORE_LIBRARY_ID": 2.5}, "BOOKLORE_LIBRARY_ID"),
|
||||||
|
({"BOOKLORE_PATH_ID": "-3"}, "BOOKLORE_PATH_ID"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_users_me_update_rejects_invalid_delivery_values(app, user_db, settings, expected_key):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
|
||||||
|
resp = _put_self_settings(app, user, settings)
|
||||||
|
|
||||||
|
assert resp.status_code == 400
|
||||||
|
assert resp.json["error"] == "Invalid settings payload"
|
||||||
|
assert any(expected_key in detail for detail in resp.json["details"])
|
||||||
|
assert user_db.get_user_settings(user["id"]) == {}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("settings", "expected"),
|
||||||
|
[
|
||||||
|
({"BOOKS_OUTPUT_MODE": "email"}, {"BOOKS_OUTPUT_MODE": "email"}),
|
||||||
|
({"EMAIL_RECIPIENT": " alice@example.com "}, {"EMAIL_RECIPIENT": "alice@example.com"}),
|
||||||
|
({"EMAIL_RECIPIENT": ""}, {"EMAIL_RECIPIENT": ""}),
|
||||||
|
({"BOOKLORE_LIBRARY_ID": "3"}, {"BOOKLORE_LIBRARY_ID": "3"}),
|
||||||
|
({"BOOKLORE_LIBRARY_ID": 3}, {"BOOKLORE_LIBRARY_ID": 3}),
|
||||||
|
({"BOOKLORE_PATH_ID": 0}, {"BOOKLORE_PATH_ID": 0}),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_users_me_update_accepts_valid_delivery_values(app, user_db, settings, expected):
|
||||||
|
user = user_db.create_user(username="alice")
|
||||||
|
|
||||||
|
resp = _put_self_settings(app, user, settings)
|
||||||
|
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert user_db.get_user_settings(user["id"]) == expected
|
||||||
|
|
||||||
|
|
||||||
def test_users_me_update_rejects_oidc_email_change(app, user_db):
|
def test_users_me_update_rejects_oidc_email_change(app, user_db):
|
||||||
user = user_db.create_user(
|
user = user_db.create_user(
|
||||||
username="oidc-user",
|
username="oidc-user",
|
||||||
|
|||||||
Reference in new issue
Block a user