diff --git a/core/navidrome_client.py b/core/navidrome_client.py index 7ec9d286..765153bc 100644 --- a/core/navidrome_client.py +++ b/core/navidrome_client.py @@ -3,6 +3,7 @@ import hashlib import secrets from typing import List, Optional, Dict, Any from datetime import datetime +from urllib.parse import urlencode import json from utils.logging_config import get_logger from config.settings import config_manager @@ -316,6 +317,42 @@ class NavidromeClient(MediaServerClient): 'f': 'json' # Response format } + # Fixed salt for cover-art URLs ONLY. Subsonic token auth (t=md5(password + # +salt), s=salt) does not require a unique salt per request, and a stable + # one makes the cover URL deterministic — so the image cache and the + # browser cache actually HIT. The rotating salt from _generate_auth_params + # would make every request a unique URL → cache miss every time + a dead, + # never-reused cache row per fetch (#766 review). The password is never + # exposed either way (only its salted md5). + _COVER_ART_SALT = 'soulsync-cover' + + def build_cover_art_url(self, cover_id, size=None) -> Optional[str]: + """Absolute, Subsonic-authenticated getCoverArt URL for ``cover_id``. + + Deterministic for a given (server, password, cover_id) so it caches. + The web layer proxies this to the browser (sync editor + modals). + Returns ``None`` when not connected or no id was supplied. #766: the + ``/api/navidrome/cover/`` route had no working URL behind it, so + every Navidrome cover came back blank.""" + if not self.base_url or not cover_id: + return None + if not self.username or not self.password: + return None + salt = self._COVER_ART_SALT + token = hashlib.md5((self.password + salt).encode()).hexdigest() + params = { + 'u': self.username, + 't': token, + 's': salt, + 'v': '1.16.1', + 'c': 'SoulSync', + 'f': 'json', # harmless for getCoverArt — it returns image binary + 'id': str(cover_id), + } + if size: + params['size'] = str(size) + return f"{self.base_url}/rest/getCoverArt?{urlencode(params)}" + # Subsonic endpoints that modify data — use POST to avoid URL length limits _WRITE_ENDPOINTS = frozenset({ 'createPlaylist', 'updatePlaylist', 'deletePlaylist', diff --git a/tests/test_navidrome_cover_url.py b/tests/test_navidrome_cover_url.py new file mode 100644 index 00000000..67894bbb --- /dev/null +++ b/tests/test_navidrome_cover_url.py @@ -0,0 +1,83 @@ +"""Tests for Navidrome cover-art URL building (#766). + +The sync editor + modals referenced /api/navidrome/cover/ but no route +served it, and the URL behind it had to be a fully-authenticated Subsonic +getCoverArt URL. build_cover_art_url is that builder — these pin its shape and +the not-connected guards (the token/salt are random per call, so we assert +structure + required params rather than an exact string). +""" + +from __future__ import annotations + +from urllib.parse import parse_qs, urlsplit + +from core.navidrome_client import NavidromeClient + + +def _connected_client(): + c = NavidromeClient() + c.base_url = "https://nav.example.com" + c.username = "boulder" + c.password = "hunter2" + return c + + +def test_builds_authenticated_cover_url(): + url = _connected_client().build_cover_art_url("al-123") + parts = urlsplit(url) + assert parts.scheme == "https" + assert parts.netloc == "nav.example.com" + assert parts.path == "/rest/getCoverArt" + q = parse_qs(parts.query) + assert q["id"] == ["al-123"] + assert q["u"] == ["boulder"] + # Subsonic token auth: salted md5, never the raw password. + assert q["t"] and q["t"][0] != "hunter2" + assert q["s"] # salt present + assert "hunter2" not in url + for required in ("t", "s", "v", "c"): + assert required in q + + +def test_cover_url_is_deterministic_so_the_cache_hits(): + # #766 review: the URL must be stable for a given (server, password, + # cover_id) — otherwise the image cache keys on a rotating salt and misses + # every request, re-fetching Navidrome each time + leaking dead rows. + c = _connected_client() + assert c.build_cover_art_url("al-123") == c.build_cover_art_url("al-123") + # ...and different covers still produce different URLs. + assert c.build_cover_art_url("al-123") != c.build_cover_art_url("al-999") + + +def test_cover_url_changes_with_password(): + # A password change must invalidate the cached URL (new token). + c1 = _connected_client() + c2 = _connected_client() + c2.password = "different" + assert c1.build_cover_art_url("al-1") != c2.build_cover_art_url("al-1") + + +def test_size_param_optional(): + assert "size" not in (_connected_client().build_cover_art_url("x") or "") + assert "size=300" in _connected_client().build_cover_art_url("x", size=300) + + +def test_cover_id_is_stringified(): + url = _connected_client().build_cover_art_url(12345) + assert "id=12345" in url + + +def test_returns_none_when_not_connected(): + c = NavidromeClient() # base_url is None + assert c.build_cover_art_url("al-1") is None + + +def test_returns_none_for_empty_cover_id(): + assert _connected_client().build_cover_art_url("") is None + assert _connected_client().build_cover_art_url(None) is None + + +def test_returns_none_without_credentials(): + c = NavidromeClient() + c.base_url = "https://nav.example.com" # but no username/password + assert c.build_cover_art_url("al-1") is None diff --git a/web_server.py b/web_server.py index dbb363e1..2849ac1c 100644 --- a/web_server.py +++ b/web_server.py @@ -4133,6 +4133,33 @@ def select_jellyfin_music_library(): logger.error(f"Error setting Jellyfin music library: {e}") return jsonify({"success": False, "error": str(e)}), 500 +@app.route('/api/navidrome/cover/', methods=['GET']) +def navidrome_cover(cover_id): + """Proxy a Navidrome (Subsonic) cover-art image to the browser. + + The sync editor and other modals reference /api/navidrome/cover/, + but no route served it — so every Navidrome cover came back blank (#766). + We build the authenticated getCoverArt URL server-side (keeping Subsonic + credentials off the client) and stream it through the shared image cache. + """ + try: + client = media_server_engine.client('navidrome') + if not client: + return '', 404 + url = client.build_cover_art_url(cover_id) + if not url: + return '', 404 + from core.image_cache import get_image_cache + cached = get_image_cache().get_url(url) + response = send_file(cached.path, mimetype=cached.mime_type, conditional=True) + max_age = int(config_manager.get("image_cache.ttl_seconds", 2592000)) + response.headers['Cache-Control'] = f'private, max-age={max_age}' + return response + except Exception as exc: + logger.debug("navidrome cover proxy failed for %s: %s", cover_id, exc) + return '', 502 + + @app.route('/api/navidrome/music-folders', methods=['GET']) def get_navidrome_music_folders(): """Get list of available music folders from Navidrome"""