mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 13:31:16 +01:00
Fix: Apprise logging and no_auth hardening (#667)
- Passes apprise logging into shelfmark logs - Update UI activity dismissal when no authentication is active
This commit is contained in:
@@ -12,6 +12,8 @@ from shelfmark.core.user_db import UserDB
|
||||
|
||||
logger = setup_logger(__name__)
|
||||
|
||||
_NO_AUTH_ACTIVITY_USERNAME = "__shelfmark_noauth_activity__"
|
||||
|
||||
|
||||
def _require_authenticated(resolve_auth_mode: Callable[[], str]):
|
||||
auth_mode = resolve_auth_mode()
|
||||
@@ -50,6 +52,49 @@ def _resolve_db_user_id(require_in_auth_mode: bool = True):
|
||||
)
|
||||
|
||||
|
||||
def _ensure_no_auth_activity_user_id(user_db: UserDB) -> int | None:
|
||||
"""Resolve a stable users.db identity for no-auth activity state."""
|
||||
try:
|
||||
user = user_db.get_user(username=_NO_AUTH_ACTIVITY_USERNAME)
|
||||
if user is None:
|
||||
try:
|
||||
user_db.create_user(
|
||||
username=_NO_AUTH_ACTIVITY_USERNAME,
|
||||
display_name="No-auth Activity",
|
||||
role="admin",
|
||||
)
|
||||
except ValueError:
|
||||
# Another request may have created it between lookup and insert.
|
||||
pass
|
||||
user = user_db.get_user(username=_NO_AUTH_ACTIVITY_USERNAME)
|
||||
|
||||
if user is None:
|
||||
return None
|
||||
user_id = int(user.get("id"))
|
||||
return user_id if user_id > 0 else None
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to resolve no-auth activity identity: %s", exc)
|
||||
return None
|
||||
|
||||
|
||||
def _resolve_activity_actor_user_id(
|
||||
*,
|
||||
user_db: UserDB,
|
||||
resolve_auth_mode: Callable[[], str],
|
||||
) -> tuple[int | None, Any | None]:
|
||||
"""Resolve acting user identity for activity mutations."""
|
||||
db_user_id, db_gate = _resolve_db_user_id()
|
||||
if db_user_id is not None:
|
||||
return db_user_id, None
|
||||
|
||||
if resolve_auth_mode() == "none":
|
||||
no_auth_user_id = _ensure_no_auth_activity_user_id(user_db)
|
||||
if no_auth_user_id is not None:
|
||||
return no_auth_user_id, None
|
||||
|
||||
return None, db_gate
|
||||
|
||||
|
||||
def _emit_activity_event(ws_manager: Any | None, *, room: str, payload: dict[str, Any]) -> None:
|
||||
if ws_manager is None:
|
||||
return
|
||||
@@ -316,6 +361,8 @@ def register_activity_routes(
|
||||
)
|
||||
|
||||
viewer_db_user_id, _ = _resolve_db_user_id(require_in_auth_mode=False)
|
||||
if viewer_db_user_id is None and resolve_auth_mode() == "none":
|
||||
viewer_db_user_id = _ensure_no_auth_activity_user_id(user_db)
|
||||
scoped_user_id = None if is_admin else db_user_id
|
||||
status = queue_status(user_id=scoped_user_id)
|
||||
updated_requests = sync_request_delivery_states(
|
||||
@@ -371,7 +418,10 @@ def register_activity_routes(
|
||||
if auth_gate is not None:
|
||||
return auth_gate
|
||||
|
||||
db_user_id, db_gate = _resolve_db_user_id()
|
||||
db_user_id, db_gate = _resolve_activity_actor_user_id(
|
||||
user_db=user_db,
|
||||
resolve_auth_mode=resolve_auth_mode,
|
||||
)
|
||||
if db_gate is not None or db_user_id is None:
|
||||
return db_gate
|
||||
|
||||
@@ -437,7 +487,10 @@ def register_activity_routes(
|
||||
if auth_gate is not None:
|
||||
return auth_gate
|
||||
|
||||
db_user_id, db_gate = _resolve_db_user_id()
|
||||
db_user_id, db_gate = _resolve_activity_actor_user_id(
|
||||
user_db=user_db,
|
||||
resolve_auth_mode=resolve_auth_mode,
|
||||
)
|
||||
if db_gate is not None or db_user_id is None:
|
||||
return db_gate
|
||||
|
||||
@@ -517,7 +570,10 @@ def register_activity_routes(
|
||||
if auth_gate is not None:
|
||||
return auth_gate
|
||||
|
||||
db_user_id, db_gate = _resolve_db_user_id()
|
||||
db_user_id, db_gate = _resolve_activity_actor_user_id(
|
||||
user_db=user_db,
|
||||
resolve_auth_mode=resolve_auth_mode,
|
||||
)
|
||||
if db_gate is not None or db_user_id is None:
|
||||
return db_gate
|
||||
|
||||
@@ -536,7 +592,10 @@ def register_activity_routes(
|
||||
if auth_gate is not None:
|
||||
return auth_gate
|
||||
|
||||
db_user_id, db_gate = _resolve_db_user_id()
|
||||
db_user_id, db_gate = _resolve_activity_actor_user_id(
|
||||
user_db=user_db,
|
||||
resolve_auth_mode=resolve_auth_mode,
|
||||
)
|
||||
if db_gate is not None or db_user_id is None:
|
||||
return db_gate
|
||||
|
||||
|
||||
@@ -2,10 +2,13 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import threading
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from contextlib import contextmanager
|
||||
from dataclasses import dataclass
|
||||
from enum import Enum
|
||||
from typing import Any, Iterable
|
||||
from typing import Any, Iterable, Iterator
|
||||
from urllib.parse import urlsplit
|
||||
|
||||
try:
|
||||
@@ -26,6 +29,7 @@ _APPRISE_APP_DESC = "Shelfmark notifications"
|
||||
_APPRISE_LOGO_URL = (
|
||||
"https://raw.githubusercontent.com/calibrain/shelfmark/main/src/frontend/public/logo.png"
|
||||
)
|
||||
_APPRISE_LOGGER_NAME = "apprise"
|
||||
|
||||
|
||||
class NotificationEvent(str, Enum):
|
||||
@@ -91,6 +95,56 @@ def _extract_url_schemes(urls: Iterable[str]) -> list[str]:
|
||||
return schemes
|
||||
|
||||
|
||||
class _AppriseLogCapture(logging.Handler):
|
||||
def __init__(self, *, thread_id: int):
|
||||
super().__init__(level=logging.INFO)
|
||||
self.records: list[tuple[int, str, str]] = []
|
||||
self._thread_id = thread_id
|
||||
|
||||
def emit(self, record: logging.LogRecord) -> None:
|
||||
if record.thread != self._thread_id:
|
||||
return
|
||||
|
||||
message = record.getMessage()
|
||||
if message:
|
||||
self.records.append((record.levelno, record.name, str(message)))
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _capture_apprise_logs(*, min_level: int = logging.INFO) -> Iterator[list[tuple[int, str, str]]]:
|
||||
apprise_logger = logging.getLogger(_APPRISE_LOGGER_NAME)
|
||||
previous_level = apprise_logger.level
|
||||
handler = _AppriseLogCapture(thread_id=threading.get_ident())
|
||||
apprise_logger.addHandler(handler)
|
||||
|
||||
if previous_level == logging.NOTSET or previous_level > min_level:
|
||||
apprise_logger.setLevel(min_level)
|
||||
|
||||
try:
|
||||
yield handler.records
|
||||
finally:
|
||||
apprise_logger.removeHandler(handler)
|
||||
apprise_logger.setLevel(previous_level)
|
||||
|
||||
|
||||
def _log_apprise_records(records: Iterable[tuple[int, str, str]]) -> None:
|
||||
seen: set[tuple[int, str, str]] = set()
|
||||
for level, source, raw_message in records:
|
||||
message = str(raw_message or "").strip()
|
||||
source_name = str(source or "").strip() or _APPRISE_LOGGER_NAME
|
||||
key = (int(level), source_name, message)
|
||||
if not message or key in seen:
|
||||
continue
|
||||
seen.add(key)
|
||||
|
||||
if level >= logging.ERROR:
|
||||
logger.error("Apprise source [%s]: %s", source_name, message)
|
||||
elif level >= logging.WARNING:
|
||||
logger.warning("Apprise source [%s]: %s", source_name, message)
|
||||
else:
|
||||
logger.info("Apprise source [%s]: %s", source_name, message)
|
||||
|
||||
|
||||
def _normalize_routes(value: Any) -> list[dict[str, str]]:
|
||||
if not isinstance(value, list):
|
||||
return []
|
||||
@@ -251,45 +305,52 @@ def _dispatch_to_apprise(
|
||||
apobj = _create_apprise_client()
|
||||
if apobj is None:
|
||||
return {"success": False, "message": "Apprise is not installed"}
|
||||
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 as exc:
|
||||
with _capture_apprise_logs(min_level=logging.INFO) as apprise_records:
|
||||
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 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:
|
||||
_log_apprise_records(apprise_records)
|
||||
scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown"
|
||||
logger.warning(
|
||||
"Failed to register notification route URL for scheme '%s': %s",
|
||||
scheme,
|
||||
exc,
|
||||
"No valid Apprise notification routes after registration for scheme(s): %s",
|
||||
scheme_summary,
|
||||
)
|
||||
added = False
|
||||
if added:
|
||||
valid_urls += 1
|
||||
else:
|
||||
invalid_urls += 1
|
||||
logger.warning("Apprise rejected notification route URL for scheme '%s'", scheme)
|
||||
return {
|
||||
"success": False,
|
||||
"message": "No valid notification URLs configured",
|
||||
}
|
||||
|
||||
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",
|
||||
}
|
||||
|
||||
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}"}
|
||||
try:
|
||||
delivered = bool(apobj.notify(title=title, body=body, notify_type=notify_type))
|
||||
except Exception as exc:
|
||||
_log_apprise_records(apprise_records)
|
||||
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:
|
||||
_log_apprise_records(apprise_records)
|
||||
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)",
|
||||
@@ -299,6 +360,8 @@ def _dispatch_to_apprise(
|
||||
)
|
||||
return {"success": False, "message": "Notification delivery failed"}
|
||||
|
||||
_log_apprise_records(apprise_records)
|
||||
|
||||
message = f"Notification sent to {valid_urls} URL(s)"
|
||||
if invalid_urls:
|
||||
message += f" ({invalid_urls} invalid URL(s) skipped)"
|
||||
|
||||
@@ -214,6 +214,35 @@ class TestActivityRoutes:
|
||||
to=f"user_{user['id']}",
|
||||
)
|
||||
|
||||
def test_no_auth_dismiss_many_and_history_use_shared_identity(self, main_module):
|
||||
item_key = f"download:no-auth-{uuid.uuid4().hex[:10]}"
|
||||
|
||||
client_one = main_module.app.test_client()
|
||||
client_two = main_module.app.test_client()
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="none"):
|
||||
dismiss_many_response = client_one.post(
|
||||
"/api/activity/dismiss-many",
|
||||
json={"items": [{"item_type": "download", "item_key": item_key}]},
|
||||
)
|
||||
with patch.object(main_module.backend, "queue_status", return_value=_sample_status_payload()):
|
||||
snapshot_one = client_one.get("/api/activity/snapshot")
|
||||
snapshot_two = client_two.get("/api/activity/snapshot")
|
||||
history_one = client_one.get("/api/activity/history?limit=10&offset=0")
|
||||
|
||||
assert dismiss_many_response.status_code == 200
|
||||
assert dismiss_many_response.json["status"] == "dismissed"
|
||||
assert dismiss_many_response.json["count"] == 1
|
||||
|
||||
assert snapshot_one.status_code == 200
|
||||
assert {"item_type": "download", "item_key": item_key} in snapshot_one.json["dismissed"]
|
||||
|
||||
assert snapshot_two.status_code == 200
|
||||
assert {"item_type": "download", "item_key": item_key} in snapshot_two.json["dismissed"]
|
||||
|
||||
assert history_one.status_code == 200
|
||||
assert any(row["item_key"] == item_key for row in history_one.json)
|
||||
|
||||
def test_queue_clear_does_not_set_request_delivery_state_to_cleared(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
"""Tests for core notification rendering and dispatch helpers."""
|
||||
|
||||
import logging
|
||||
|
||||
from shelfmark.core import notifications as notifications_module
|
||||
|
||||
|
||||
@@ -24,6 +26,8 @@ class _FakeAppriseClient:
|
||||
self.add_calls = []
|
||||
self.notify_calls = []
|
||||
self.notify_result = True
|
||||
self.notify_warning_messages: list[str] = []
|
||||
self.notify_info_messages: list[str] = []
|
||||
|
||||
def add(self, url):
|
||||
self.add_calls.append(url)
|
||||
@@ -31,6 +35,10 @@ class _FakeAppriseClient:
|
||||
|
||||
def notify(self, **kwargs):
|
||||
self.notify_calls.append(kwargs)
|
||||
for message in self.notify_info_messages:
|
||||
logging.getLogger("apprise.plugins.pushover").info(message)
|
||||
for message in self.notify_warning_messages:
|
||||
logging.getLogger("apprise.plugins.pushover").warning(message)
|
||||
return self.notify_result
|
||||
|
||||
|
||||
@@ -183,6 +191,36 @@ 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_logs_captured_apprise_info_messages(monkeypatch):
|
||||
fake_apprise = _FakeAppriseModule()
|
||||
fake_apprise.client.notify_info_messages = [
|
||||
"Sent Pushover notification to ALL_DEVICES."
|
||||
]
|
||||
monkeypatch.setattr(notifications_module, "apprise", fake_apprise)
|
||||
|
||||
info_messages: list[str] = []
|
||||
|
||||
def _fake_info(message, *args, **kwargs):
|
||||
_ = kwargs
|
||||
info_messages.append(message % args if args else str(message))
|
||||
|
||||
monkeypatch.setattr(notifications_module.logger, "info", _fake_info)
|
||||
|
||||
result = notifications_module._dispatch_to_apprise(
|
||||
["ntfys://ntfy.sh/shelfmark"],
|
||||
title="Test",
|
||||
body="Body",
|
||||
notify_type=_FakeNotifyType.INFO,
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
assert any(
|
||||
"Apprise source [apprise.plugins.pushover]: Sent Pushover notification to ALL_DEVICES."
|
||||
in message
|
||||
for message in info_messages
|
||||
)
|
||||
|
||||
|
||||
def test_dispatch_to_apprise_notify_false_returns_generic_failure_and_logs(monkeypatch):
|
||||
fake_apprise = _FakeAppriseModule()
|
||||
fake_apprise.client.notify_result = False
|
||||
@@ -208,6 +246,37 @@ def test_dispatch_to_apprise_notify_false_returns_generic_failure_and_logs(monke
|
||||
assert any("scheme(s): pover" in message for message in warning_messages)
|
||||
|
||||
|
||||
def test_dispatch_to_apprise_logs_captured_apprise_warning_messages(monkeypatch):
|
||||
fake_apprise = _FakeAppriseModule()
|
||||
fake_apprise.client.notify_result = False
|
||||
fake_apprise.client.notify_warning_messages = [
|
||||
"Failed to send Pushover notification to ALL_DEVICES: Unauthorized - Invalid Token., error=401."
|
||||
]
|
||||
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 any(
|
||||
"Apprise source [apprise.plugins.pushover]: Failed to send Pushover notification"
|
||||
in msg
|
||||
for msg 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":
|
||||
|
||||
Reference in New Issue
Block a user