From 71cd030484a407408073cbcf5a795088704ec1da Mon Sep 17 00:00:00 2001 From: splitsec2 <35583321+splitsec2@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:35:02 -0600 Subject: [PATCH] Merge commit from fork GET /api/users/me/edit-context served every user-overridable key with the admin's global value in globalValues and effective, so any user could read the global destination paths, the admin's EMAIL_RECIPIENT, the Booklore library and path ids and the global USER_NOTIFICATION_ROUTES table, whose Apprise URLs embed tokens. build_user_preferences_payload gains a for_self_service mode that replaces a global value with None unless the user set the key themselves or the field is a checkbox, number, select or multi-select switch the form needs to lay itself out (BOOKLORE_LIBRARY_ID and BOOKLORE_PATH_ID are masked despite being selects). The self-service route uses it; the admin routes keep full data. --- shelfmark/core/self_user_routes.py | 6 +- shelfmark/core/user_settings_overrides.py | 46 ++++++++++++-- tests/core/test_config_user_overrides.py | 77 +++++++++++++++++++++++ tests/core/test_self_user_routes.py | 72 +++++++++++++++++++++ 4 files changed, 196 insertions(+), 5 deletions(-) diff --git a/shelfmark/core/self_user_routes.py b/shelfmark/core/self_user_routes.py index a3f88f70..2603a032 100644 --- a/shelfmark/core/self_user_routes.py +++ b/shelfmark/core/self_user_routes.py @@ -119,7 +119,11 @@ def _build_optional_user_preferences( preference_label: str, ) -> tuple[dict[str, Any] | None, tuple[Response, int] | None]: try: - return _build_user_preferences_payload(user_db, user_id, tab_name), None + # Self-service: the user only gets to see values they set themselves. + return ( + _build_user_preferences_payload(user_db, user_id, tab_name, for_self_service=True), + None, + ) except ValueError as exc: if str(exc) == missing_tab_error: return None, (jsonify({"error": missing_tab_error}), 500) diff --git a/shelfmark/core/user_settings_overrides.py b/shelfmark/core/user_settings_overrides.py index c1f7325a..506c6119 100644 --- a/shelfmark/core/user_settings_overrides.py +++ b/shelfmark/core/user_settings_overrides.py @@ -3,7 +3,13 @@ from importlib import import_module from typing import TYPE_CHECKING, Any -from shelfmark.core.settings_registry import load_config_file +from shelfmark.core.settings_registry import ( + CheckboxField, + MultiSelectField, + NumberField, + SelectField, + load_config_file, +) if TYPE_CHECKING: from types import ModuleType @@ -33,8 +39,36 @@ def get_ordered_user_overridable_fields(tab_name: str) -> list[tuple[str, Any]]: return [(field.key, field) for field in tab.fields if field.key in overridable_map] -def build_user_preferences_payload(user_db: UserDB, user_id: int, tab_name: str) -> dict[str, Any]: - """Build the effective user-preferences payload for a settings tab.""" +# Global values a self-service user may see when they have not set the key +# themselves. These are mode switches and option picks the settings form needs +# to lay itself out, and they carry nothing private. Everything else (free text +# such as paths and email addresses, tables such as notification routes whose +# URLs embed tokens) stays with the admin. +_SELF_SERVICE_VISIBLE_FIELD_TYPES = (CheckboxField, NumberField, SelectField, MultiSelectField) +# Select fields whose value identifies something in the admin's own accounts. +_SELF_SERVICE_HIDDEN_KEYS = frozenset({"BOOKLORE_LIBRARY_ID", "BOOKLORE_PATH_ID"}) + + +def is_global_value_visible_to_user(key: str, field: object) -> bool: + """Return whether a self-service user may see the global value of a field.""" + if key in _SELF_SERVICE_HIDDEN_KEYS: + return False + return isinstance(field, _SELF_SERVICE_VISIBLE_FIELD_TYPES) + + +def build_user_preferences_payload( + user_db: UserDB, + user_id: int, + tab_name: str, + *, + for_self_service: bool = False, +) -> dict[str, Any]: + """Build the effective user-preferences payload for a settings tab. + + With ``for_self_service`` the payload is meant for the user it describes: + global values they did not set themselves are replaced with ``None`` unless + `is_global_value_visible_to_user` allows them. Admin callers get full data. + """ from shelfmark.core.config import config as app_config settings_registry = get_settings_registry() @@ -59,7 +93,8 @@ def build_user_preferences_payload(user_db: UserDB, user_id: int, tab_name: str) ) fields_payload.append(serialized) - global_values[key] = app_config.get(key, field.default) + show_global = not for_self_service or is_global_value_visible_to_user(key, field) + global_values[key] = app_config.get(key, field.default) if show_global else None source = "default" value = app_config.get(key, field.default, user_id=user_id) @@ -71,6 +106,9 @@ def build_user_preferences_payload(user_db: UserDB, user_id: int, tab_name: str) elif key in tab_config: source = "global_config" + if source != "user_override" and not show_global: + value = None + effective[key] = {"value": value, "source": source} user_overrides = { diff --git a/tests/core/test_config_user_overrides.py b/tests/core/test_config_user_overrides.py index 06d8e137..fbba5e85 100644 --- a/tests/core/test_config_user_overrides.py +++ b/tests/core/test_config_user_overrides.py @@ -145,6 +145,83 @@ def test_build_user_preferences_payload_reports_effective_sources(monkeypatch): assert fields_by_key["BOOKS_OUTPUT_MODE"]["fromEnv"] is True +def test_build_user_preferences_payload_masks_admin_globals_for_self_service(): + """Self-service mode hides global values the user did not set themselves.""" + import shelfmark.config.settings # noqa: F401 + from shelfmark.core import settings_registry + from shelfmark.core.user_settings_overrides import build_user_preferences_payload + + user_db = SimpleNamespace(get_user_settings=lambda user_id: {"DESTINATION": "/user/books"}) + admin_globals = { + "DESTINATION": "/global/books", + "DESTINATION_AUDIOBOOK": "/global/audiobooks", + "BOOKS_OUTPUT_MODE": "booklore", + "EMAIL_RECIPIENT": "global@example.com", + "BOOKLORE_LIBRARY_ID": "3", + "BOOKLORE_PATH_ID": "9", + "DOWNLOAD_TO_BROWSER_CONTENT_TYPES": ["ebook"], + "USER_NOTIFICATION_ROUTES": [{"event": ["all"], "url": "ntfys://ntfy.sh/secret-topic"}], + } + + def fake_get(key, default=None, user_id=None): + if user_id is not None and key == "DESTINATION": + return "/user/books" + return admin_globals.get(key, default) + + with ( + patch( + "shelfmark.core.user_settings_overrides.get_settings_registry", + return_value=settings_registry, + ), + patch( + "shelfmark.core.user_settings_overrides.load_config_file", + return_value=dict(admin_globals), + ), + patch.object(config, "get", side_effect=fake_get), + patch.object(settings_registry, "is_value_from_env", return_value=False), + ): + admin_payload = build_user_preferences_payload(user_db, 7, "downloads") + self_payload = build_user_preferences_payload( + user_db, 7, "downloads", for_self_service=True + ) + self_notifications = build_user_preferences_payload( + user_db, 7, "notifications", for_self_service=True + ) + + # Admin routes keep the full picture. + assert admin_payload["globalValues"]["DESTINATION"] == "/global/books" + assert admin_payload["globalValues"]["EMAIL_RECIPIENT"] == "global@example.com" + assert admin_payload["effective"]["EMAIL_RECIPIENT"]["value"] == "global@example.com" + + # Same keys and field metadata, but the admin's values are masked. + assert self_payload["keys"] == admin_payload["keys"] + assert self_payload["fields"] == admin_payload["fields"] + assert self_payload["userOverrides"] == {"DESTINATION": "/user/books"} + for masked_key in ( + "DESTINATION", + "DESTINATION_AUDIOBOOK", + "EMAIL_RECIPIENT", + "BOOKLORE_LIBRARY_ID", + "BOOKLORE_PATH_ID", + ): + assert self_payload["globalValues"][masked_key] is None, masked_key + assert self_payload["effective"]["EMAIL_RECIPIENT"] == { + "value": None, + "source": "global_config", + } + assert self_payload["effective"]["DESTINATION"] == { + "value": "/user/books", + "source": "user_override", + } + # Select and multi-select switches drive the form layout and stay visible. + assert self_payload["globalValues"]["BOOKS_OUTPUT_MODE"] == "booklore" + assert self_payload["globalValues"]["DOWNLOAD_TO_BROWSER_CONTENT_TYPES"] == ["ebook"] + + assert self_notifications["globalValues"]["USER_NOTIFICATION_ROUTES"] is None + assert self_notifications["effective"]["USER_NOTIFICATION_ROUTES"]["value"] is None + assert "secret-topic" not in repr(self_notifications) + + def test_build_user_preferences_payload_carries_the_language_default(): """BOOK_LANGUAGE rides along with the other search preferences on its tab.""" import shelfmark.config.settings # noqa: F401 diff --git a/tests/core/test_self_user_routes.py b/tests/core/test_self_user_routes.py index 0c95bf70..646f5fe2 100644 --- a/tests/core/test_self_user_routes.py +++ b/tests/core/test_self_user_routes.py @@ -115,6 +115,78 @@ def test_users_me_edit_context_includes_search_preferences_when_visible(app, use assert resp.json["userOverridableKeys"] == sorted(resp.json["userOverridableKeys"]) +def _admin_globals_config_get( + visible_sections: object, + admin_globals: dict[str, object], +): + def _get(key: str, default: object = None, user_id: int | None = None) -> object: + del user_id + if key == "VISIBLE_SELF_SETTINGS_SECTIONS": + return visible_sections + return admin_globals.get(key, default) + + return _get + + +def test_users_me_edit_context_hides_admin_global_values(app, user_db): + """A user only sees the values they set; the admin's globals stay masked.""" + user = user_db.create_user(username="alice") + user_db.set_user_settings(user["id"], {"DESTINATION_AUDIOBOOK": "/audiobooks/alice"}) + client = _authed_client_for_user(app, user) + admin_globals: dict[str, object] = { + "BOOKS_OUTPUT_MODE": "email", + "DESTINATION": "/srv/admin-private/books", + "EMAIL_RECIPIENT": "admin-private@example.com", + "BOOKLORE_LIBRARY_ID": "7", + "BOOKLORE_PATH_ID": "11", + "SEARCH_MODE": "universal", + "USER_NOTIFICATION_ROUTES": [ + {"event": ["all"], "url": "discord://admin-webhook-id/admin-webhook-token"} + ], + } + + 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=_admin_globals_config_get( + ["delivery", "search", "notifications"], admin_globals + ), + ): + resp = client.get("/api/users/me/edit-context") + + assert resp.status_code == 200 + body = resp.get_data(as_text=True) + for leaked in ( + "/srv/admin-private/books", + "admin-private@example.com", + "admin-webhook-token", + '"7"', + '"11"', + ): + assert leaked not in body + + delivery = resp.json["deliveryPreferences"] + assert delivery["globalValues"]["DESTINATION"] is None + assert delivery["globalValues"]["EMAIL_RECIPIENT"] is None + assert delivery["globalValues"]["BOOKLORE_LIBRARY_ID"] is None + assert delivery["globalValues"]["BOOKLORE_PATH_ID"] is None + assert delivery["effective"]["DESTINATION"]["value"] is None + assert delivery["effective"]["DESTINATION"]["source"] != "user_override" + # Mode switches are needed to render the form and reveal nothing private. + assert delivery["globalValues"]["BOOKS_OUTPUT_MODE"] == "email" + assert resp.json["searchPreferences"]["globalValues"]["SEARCH_MODE"] == "universal" + # The user's own value is still served in full. + assert delivery["userOverrides"] == {"DESTINATION_AUDIOBOOK": "/audiobooks/alice"} + assert delivery["effective"]["DESTINATION_AUDIOBOOK"] == { + "value": "/audiobooks/alice", + "source": "user_override", + } + + notifications = resp.json["notificationPreferences"] + assert notifications["globalValues"]["USER_NOTIFICATION_ROUTES"] is None + assert notifications["effective"]["USER_NOTIFICATION_ROUTES"]["value"] is None + + def test_users_me_edit_context_falls_back_to_default_sections_for_invalid_config(app, user_db): user = user_db.create_user(username="alice") client = _authed_client_for_user(app, user)