mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-09-24 18:40:29 +01:00
Fix: User DB hardening and apprise tweak (#668)
This commit is contained in:
@@ -24,7 +24,11 @@ def _require_authenticated(resolve_auth_mode: Callable[[], str]):
|
||||
return None
|
||||
|
||||
|
||||
def _resolve_db_user_id(require_in_auth_mode: bool = True):
|
||||
def _resolve_db_user_id(
|
||||
require_in_auth_mode: bool = True,
|
||||
*,
|
||||
user_db: UserDB | None = None,
|
||||
):
|
||||
raw_db_user_id = session.get("db_user_id")
|
||||
if raw_db_user_id is None:
|
||||
if not require_in_auth_mode:
|
||||
@@ -39,8 +43,10 @@ def _resolve_db_user_id(require_in_auth_mode: bool = True):
|
||||
403,
|
||||
)
|
||||
try:
|
||||
return int(raw_db_user_id), None
|
||||
parsed_db_user_id = int(raw_db_user_id)
|
||||
except (TypeError, ValueError):
|
||||
if not require_in_auth_mode:
|
||||
return None, None
|
||||
return None, (
|
||||
jsonify(
|
||||
{
|
||||
@@ -51,6 +57,40 @@ def _resolve_db_user_id(require_in_auth_mode: bool = True):
|
||||
403,
|
||||
)
|
||||
|
||||
if parsed_db_user_id < 1:
|
||||
if not require_in_auth_mode:
|
||||
return None, None
|
||||
return None, (
|
||||
jsonify(
|
||||
{
|
||||
"error": "User identity unavailable for activity workflow",
|
||||
"code": "user_identity_unavailable",
|
||||
}
|
||||
),
|
||||
403,
|
||||
)
|
||||
|
||||
if user_db is not None:
|
||||
try:
|
||||
db_user = user_db.get_user(user_id=parsed_db_user_id)
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to validate activity db identity %s: %s", parsed_db_user_id, exc)
|
||||
db_user = None
|
||||
if db_user is None:
|
||||
if not require_in_auth_mode:
|
||||
return None, None
|
||||
return None, (
|
||||
jsonify(
|
||||
{
|
||||
"error": "User identity unavailable for activity workflow",
|
||||
"code": "user_identity_unavailable",
|
||||
}
|
||||
),
|
||||
403,
|
||||
)
|
||||
|
||||
return parsed_db_user_id, None
|
||||
|
||||
|
||||
def _ensure_no_auth_activity_user_id(user_db: UserDB) -> int | None:
|
||||
"""Resolve a stable users.db identity for no-auth activity state."""
|
||||
@@ -83,14 +123,23 @@ def _resolve_activity_actor_user_id(
|
||||
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, (
|
||||
jsonify(
|
||||
{
|
||||
"error": "User identity unavailable for activity workflow",
|
||||
"code": "user_identity_unavailable",
|
||||
}
|
||||
),
|
||||
403,
|
||||
)
|
||||
|
||||
db_user_id, db_gate = _resolve_db_user_id(user_db=user_db)
|
||||
if db_user_id is not None:
|
||||
return db_user_id, None
|
||||
|
||||
return None, db_gate
|
||||
|
||||
@@ -360,9 +409,13 @@ def register_activity_routes(
|
||||
403,
|
||||
)
|
||||
|
||||
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":
|
||||
if resolve_auth_mode() == "none":
|
||||
viewer_db_user_id = _ensure_no_auth_activity_user_id(user_db)
|
||||
else:
|
||||
viewer_db_user_id, _ = _resolve_db_user_id(
|
||||
require_in_auth_mode=False,
|
||||
user_db=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(
|
||||
|
||||
@@ -74,6 +74,13 @@ def _normalize_urls(value: Any) -> list[str]:
|
||||
seen: set[str] = set()
|
||||
for raw_url in raw_values:
|
||||
url = str(raw_url or "").strip()
|
||||
if not url:
|
||||
continue
|
||||
# Strip invisible/non-ASCII characters that can sneak in via copy-paste
|
||||
# (zero-width spaces, smart quotes, non-breaking spaces, etc.).
|
||||
# These pass Apprise URL validation but cause UnicodeEncodeError when
|
||||
# requests tries to latin-1 encode credentials for Basic Auth headers.
|
||||
url = url.encode("ascii", errors="ignore").decode("ascii").strip()
|
||||
if not url:
|
||||
continue
|
||||
if url in seen:
|
||||
@@ -98,7 +105,7 @@ def _extract_url_schemes(urls: Iterable[str]) -> list[str]:
|
||||
class _AppriseLogCapture(logging.Handler):
|
||||
def __init__(self, *, thread_id: int):
|
||||
super().__init__(level=logging.INFO)
|
||||
self.records: list[tuple[int, str, str]] = []
|
||||
self.records: list[tuple[int, str, str, str]] = []
|
||||
self._thread_id = thread_id
|
||||
|
||||
def emit(self, record: logging.LogRecord) -> None:
|
||||
@@ -107,11 +114,19 @@ class _AppriseLogCapture(logging.Handler):
|
||||
|
||||
message = record.getMessage()
|
||||
if message:
|
||||
self.records.append((record.levelno, record.name, str(message)))
|
||||
exception_summary = ""
|
||||
if record.exc_info and record.exc_info[0]:
|
||||
exc_type = getattr(record.exc_info[0], "__name__", "Exception")
|
||||
exc = record.exc_info[1]
|
||||
exception_summary = f"{exc_type}: {exc}"
|
||||
elif record.exc_text:
|
||||
exception_summary = str(record.exc_text).strip()
|
||||
|
||||
self.records.append((record.levelno, record.name, str(message), exception_summary))
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _capture_apprise_logs(*, min_level: int = logging.INFO) -> Iterator[list[tuple[int, str, str]]]:
|
||||
def _capture_apprise_logs(*, min_level: int = logging.INFO) -> Iterator[list[tuple[int, str, str, str]]]:
|
||||
apprise_logger = logging.getLogger(_APPRISE_LOGGER_NAME)
|
||||
previous_level = apprise_logger.level
|
||||
handler = _AppriseLogCapture(thread_id=threading.get_ident())
|
||||
@@ -127,22 +142,25 @@ def _capture_apprise_logs(*, min_level: int = logging.INFO) -> Iterator[list[tup
|
||||
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:
|
||||
def _log_apprise_records(records: Iterable[tuple[int, str, str, str]]) -> None:
|
||||
seen: set[tuple[int, str, str, str]] = set()
|
||||
for level, source, raw_message, raw_exception_summary in records:
|
||||
message = str(raw_message or "").strip()
|
||||
source_name = str(source or "").strip() or _APPRISE_LOGGER_NAME
|
||||
key = (int(level), source_name, message)
|
||||
exception_summary = str(raw_exception_summary or "").strip()
|
||||
key = (int(level), source_name, message, exception_summary)
|
||||
if not message or key in seen:
|
||||
continue
|
||||
seen.add(key)
|
||||
|
||||
full_message = message if not exception_summary else f"{message} ({exception_summary})"
|
||||
|
||||
if level >= logging.ERROR:
|
||||
logger.error("Apprise source [%s]: %s", source_name, message)
|
||||
logger.error("Apprise source [%s]: %s", source_name, full_message)
|
||||
elif level >= logging.WARNING:
|
||||
logger.warning("Apprise source [%s]: %s", source_name, message)
|
||||
logger.warning("Apprise source [%s]: %s", source_name, full_message)
|
||||
else:
|
||||
logger.info("Apprise source [%s]: %s", source_name, message)
|
||||
logger.info("Apprise source [%s]: %s", source_name, full_message)
|
||||
|
||||
|
||||
def _normalize_routes(value: Any) -> list[dict[str, str]]:
|
||||
|
||||
@@ -243,6 +243,65 @@ class TestActivityRoutes:
|
||||
assert history_one.status_code == 200
|
||||
assert any(row["item_key"] == item_key for row in history_one.json)
|
||||
|
||||
def test_no_auth_dismiss_many_ignores_stale_session_db_identity(self, main_module, client):
|
||||
stale_db_user_id = 999999999
|
||||
_set_session(client, user_id="stale-session-user", db_user_id=stale_db_user_id, is_admin=False)
|
||||
|
||||
item_key = f"download:no-auth-stale-{uuid.uuid4().hex[:8]}"
|
||||
with patch.object(main_module, "get_auth_mode", return_value="none"):
|
||||
response = client.post(
|
||||
"/api/activity/dismiss-many",
|
||||
json={"items": [{"item_type": "download", "item_key": item_key}]},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json["status"] == "dismissed"
|
||||
|
||||
no_auth_user = main_module.user_db.get_user(username="__shelfmark_noauth_activity__")
|
||||
assert no_auth_user is not None
|
||||
dismissals = main_module.activity_service.get_dismissal_set(no_auth_user["id"])
|
||||
assert {"item_type": "download", "item_key": item_key} in dismissals
|
||||
|
||||
def test_no_auth_dismiss_many_uses_shared_identity_even_with_valid_session_db_user(
|
||||
self,
|
||||
main_module,
|
||||
client,
|
||||
):
|
||||
existing_user = _create_user(main_module, prefix="legacy-reader")
|
||||
_set_session(
|
||||
client,
|
||||
user_id=existing_user["username"],
|
||||
db_user_id=existing_user["id"],
|
||||
is_admin=False,
|
||||
)
|
||||
|
||||
item_key = f"download:no-auth-valid-{uuid.uuid4().hex[:8]}"
|
||||
other_client = main_module.app.test_client()
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="none"):
|
||||
dismiss_response = client.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_response = other_client.get("/api/activity/snapshot")
|
||||
|
||||
assert dismiss_response.status_code == 200
|
||||
assert snapshot_response.status_code == 200
|
||||
assert {"item_type": "download", "item_key": item_key} in snapshot_response.json["dismissed"]
|
||||
|
||||
def test_dismiss_many_with_stale_db_identity_returns_identity_unavailable(self, main_module, client):
|
||||
_set_session(client, user_id="stale-session-user", db_user_id=999999999, is_admin=False)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
response = client.post(
|
||||
"/api/activity/dismiss-many",
|
||||
json={"items": [{"item_type": "download", "item_key": "download:test-stale"}]},
|
||||
)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert response.json["code"] == "user_identity_unavailable"
|
||||
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user