diff --git a/Makefile b/Makefile index e15ca60..16cd35b 100644 --- a/Makefile +++ b/Makefile @@ -60,8 +60,8 @@ NPM ?= pnpm frontend-test-monitored-artists \ frontend-test-playlist-detail \ frontend-test-queuehelpers \ - project-map rebuild \ - test tests check lint ci + rebuild \ + test tests check lint format ci # Help @@ -260,9 +260,6 @@ frontend-test-queuehelpers: ## Run queue helper regressions # Utilities -project-map: ## Refresh the project map block - cd "$(ROOT_DIR)" && $(PYTHON) scripts/gen-project-map.py - rebuild: ## Rebuild the application cd "$(ROOT_DIR)" && ./manage.sh --rebuild @@ -276,4 +273,8 @@ check: backend-test frontend-check ## Run backend tests and frontend type checks lint: backend-lint frontend-lint ## Run linting targets +format: ## Auto-format backend (ruff --fix) and frontend (prettier) + cd "$(ROOT_DIR)" && $(BACKEND_VENV_DIR)/bin/ruff check --fix backend + cd "$(FRONTEND_DIR)" && $(NPM) run format + ci: backend-test backend-lint frontend-check frontend-lint frontend-format-check frontend-test-server ## Run the local CI checks diff --git a/backend/repositories/lastfm_repository.py b/backend/repositories/lastfm_repository.py index 6fea154..edf6c83 100644 --- a/backend/repositories/lastfm_repository.py +++ b/backend/repositories/lastfm_repository.py @@ -71,8 +71,9 @@ LASTFM_ERROR_MAP: dict[int, tuple[type[Exception], str]] = { 9: (ConfigurationError, "Session key expired - please re-authorize with Last.fm"), 10: (ConfigurationError, "Invalid API key - check your Last.fm API key"), 11: (ExternalServiceError, "Last.fm service is temporarily offline"), - 26: (ConfigurationError, "API key has been suspended - contact Last.fm support"), 14: (TokenNotAuthorizedError, "Token not yet authorized"), + 17: (ConfigurationError, "Authentication required - re-authorize Last.fm or make your listening history public"), + 26: (ConfigurationError, "API key has been suspended - contact Last.fm support"), 29: (ExternalServiceError, "Rate limit exceeded"), } @@ -107,6 +108,10 @@ class LastFmRepository: self._shared_secret = shared_secret self._session_key = session_key + @property + def _can_sign(self) -> bool: + return bool(self._shared_secret) and bool(self._session_key) + def configure(self, api_key: str, shared_secret: str, session_key: str = "") -> None: self._api_key = api_key self._shared_secret = shared_secret @@ -320,6 +325,7 @@ class LastFmRepository: data = await self._request( "user.getTopArtists", params={"user": username, "period": period, "limit": str(limit)}, + signed=self._can_sign, ) artists = [ parse_top_artist(item) @@ -340,6 +346,7 @@ class LastFmRepository: data = await self._request( "user.getTopAlbums", params={"user": username, "period": period, "limit": str(limit)}, + signed=self._can_sign, ) albums = [ parse_top_album(item) @@ -360,6 +367,7 @@ class LastFmRepository: data = await self._request( "user.getTopTracks", params={"user": username, "period": period, "limit": str(limit)}, + signed=self._can_sign, ) tracks = [ parse_top_track(item) @@ -378,6 +386,7 @@ class LastFmRepository: data = await self._request( "user.getRecentTracks", params={"user": username, "limit": str(limit), "extended": "0"}, + signed=self._can_sign, ) tracks = [ parse_recent_track(item) @@ -396,6 +405,7 @@ class LastFmRepository: data = await self._request( "user.getLovedTracks", params={"user": username, "limit": str(limit)}, + signed=self._can_sign, ) tracks = [ parse_loved_track(item) @@ -414,6 +424,7 @@ class LastFmRepository: data = await self._request( "user.getWeeklyArtistChart", params={"user": username}, + signed=self._can_sign, ) artists = [ parse_top_artist(item) @@ -432,6 +443,7 @@ class LastFmRepository: data = await self._request( "user.getWeeklyAlbumChart", params={"user": username}, + signed=self._can_sign, ) albums = [ parse_weekly_album_chart_item(item) diff --git a/backend/tests/services/test_lastfm_repository.py b/backend/tests/services/test_lastfm_repository.py index 66fc17e..e440390 100644 --- a/backend/tests/services/test_lastfm_repository.py +++ b/backend/tests/services/test_lastfm_repository.py @@ -88,6 +88,14 @@ class TestHandleErrorResponse: with pytest.raises(ExternalServiceError, match="Last.fm error \\(999\\)"): repo._handle_error_response({"error": 999, "message": "weird"}) + def test_error_17_raises_configuration_error(self): + repo = _make_repo() + with pytest.raises(ConfigurationError, match="re-authorize Last.fm"): + repo._handle_error_response({ + "error": 17, + "message": "Login: User required to be logged in", + }) + class TestConfigureMethod: def test_configure_updates_credentials(self): @@ -107,6 +115,123 @@ class TestConstructorDefaults: assert repo._session_key == "" +class TestCanSignProperty: + def test_true_when_both_credentials_present(self): + repo = _make_repo(shared_secret="sec", session_key="sk") + assert repo._can_sign is True + + def test_false_without_shared_secret(self): + repo = _make_repo(shared_secret="", session_key="sk") + assert repo._can_sign is False + + def test_false_without_session_key(self): + repo = _make_repo(shared_secret="sec", session_key="") + assert repo._can_sign is False + + def test_false_with_neither(self): + repo = _make_repo(shared_secret="", session_key="") + assert repo._can_sign is False + + def test_configure_enables_can_sign(self): + repo = _make_repo(shared_secret="", session_key="") + assert repo._can_sign is False + repo.configure(api_key="k", shared_secret="s", session_key="sk") + assert repo._can_sign is True + + +class TestSignedUserRequests: + @pytest.mark.asyncio + async def test_get_recent_tracks_sends_signed_request(self): + cache = _make_cache() + http_client = AsyncMock(spec=httpx.AsyncClient) + http_client.get = AsyncMock( + return_value=MagicMock( + status_code=200, + json=lambda: {"recenttracks": {"track": []}}, + text="", + ) + ) + repo = LastFmRepository( + http_client=http_client, + cache=cache, + api_key="key", + shared_secret="sec", + session_key="sk-1", + ) + await repo.get_user_recent_tracks("user1") + call_params = http_client.get.call_args.kwargs.get("params", {}) + assert "api_sig" in call_params + assert call_params["sk"] == "sk-1" + + @pytest.mark.asyncio + async def test_get_recent_tracks_unsigned_without_session_key(self): + cache = _make_cache() + http_client = AsyncMock(spec=httpx.AsyncClient) + http_client.get = AsyncMock( + return_value=MagicMock( + status_code=200, + json=lambda: {"recenttracks": {"track": []}}, + text="", + ) + ) + repo = LastFmRepository( + http_client=http_client, + cache=cache, + api_key="key", + ) + await repo.get_user_recent_tracks("user1") + call_params = http_client.get.call_args.kwargs.get("params", {}) + assert "api_sig" not in call_params + assert "sk" not in call_params + + @pytest.mark.asyncio + async def test_get_top_artists_sends_signed_request(self): + cache = _make_cache() + http_client = AsyncMock(spec=httpx.AsyncClient) + http_client.get = AsyncMock( + return_value=MagicMock( + status_code=200, + json=lambda: {"topartists": {"artist": []}}, + text="", + ) + ) + repo = LastFmRepository( + http_client=http_client, + cache=cache, + api_key="key", + shared_secret="sec", + session_key="sk-1", + ) + await repo.get_user_top_artists("user1") + call_params = http_client.get.call_args.kwargs.get("params", {}) + assert "api_sig" in call_params + assert call_params["sk"] == "sk-1" + + @pytest.mark.asyncio + async def test_error_17_through_signed_request_raises_configuration_error(self): + cache = _make_cache() + http_client = AsyncMock(spec=httpx.AsyncClient) + http_client.get = AsyncMock( + return_value=MagicMock( + status_code=200, + json=lambda: { + "error": 17, + "message": "Login: User required to be logged in", + }, + text="", + ) + ) + repo = LastFmRepository( + http_client=http_client, + cache=cache, + api_key="key", + shared_secret="sec", + session_key="sk-1", + ) + with pytest.raises(ConfigurationError, match="re-authorize Last.fm"): + await repo.get_user_recent_tracks("user1") + + class TestUpdateNowPlaying: @pytest.mark.asyncio async def test_posts_with_required_params(self): diff --git a/frontend/src/lib/components/settings/SettingsLastFm.svelte b/frontend/src/lib/components/settings/SettingsLastFm.svelte index 4e426ef..640de76 100644 --- a/frontend/src/lib/components/settings/SettingsLastFm.svelte +++ b/frontend/src/lib/components/settings/SettingsLastFm.svelte @@ -46,9 +46,10 @@ credsPersisted = !!(form.data?.api_key && form.data?.shared_secret); } - async function save() { + async function save(): Promise { const ok = await form.save(); if (ok && form.data?.api_key && form.data?.shared_secret) credsPersisted = true; + return ok; } async function saveAndTest() { @@ -129,7 +130,7 @@

Last.fm

- Connect Last.fm to track listens and improve recommendations. + Connect Last.fm to scrobble listens and improve recommendations.

{#if form.loading} @@ -138,7 +139,7 @@
{:else if form.data} @@ -160,13 +161,11 @@ {#if showForm}
-

Step 1 — API Credentials

+

Step 1: API credentials

{#if !step1Complete}
-

- You will need a Last.fm API key and shared secret: -

+

You'll need a Last.fm API key and shared secret:

  1. Copy the API Key and Shared Secret from that page
    2. -
  2. Paste them here, then click Save & Test
    3. +
  3. Paste them here, then select Save & Test
{/if} @@ -197,7 +196,7 @@ type="text" bind:value={form.data.api_key} class="input input-bordered w-full" - placeholder="Your Last.fm API key" + placeholder="Last.fm API key" /> {#if step1Complete}
@@ -224,7 +223,7 @@ type={showSecret ? 'text' : 'password'} bind:value={form.data.shared_secret} class="input input-bordered join-item flex-1" - placeholder="Your Last.fm shared secret" + placeholder="Last.fm shared secret" />
-

Step 2 — Authorize

+

Step 2: Authorize

{#if step2Complete && !pendingToken}
@@ -298,7 +297,7 @@ {:else if !pendingToken}

- Open Last.fm in a new tab, approve access, then come back here. + Open Last.fm in a new tab, approve access, then return here.

-

Step 3 — Enable

+

Step 3: Enable

ListenBrainz

- Connect to ListenBrainz for personalized recommendations and listening stats. + Connect ListenBrainz for tailored recommendations and listening stats.

{#if form.loading} @@ -83,11 +84,11 @@ {#if showForm}
-

Step 1 — Credentials

+

Step 1: Credentials

{#if !step1Complete}
-

To get started:

+

Getting started:

  1. Create a free account at @@ -103,7 +104,7 @@
  2. Enter your username below
    3. -
  3. Optionally add your User Token for private statistics
    4. +
  4. Add a User Token if you want private listening stats
{/if} @@ -117,7 +118,7 @@ type="text" bind:value={form.data.username} class="input input-bordered w-full" - placeholder="Your ListenBrainz username" + placeholder="ListenBrainz username" />
@@ -131,7 +132,7 @@ type={showToken ? 'text' : 'password'} bind:value={form.data.user_token} class="input input-bordered join-item flex-1" - placeholder="For private statistics" + placeholder="Needed for private stats" />
-

Step 2 — Enable

+

Step 2: Enable

- To scrobble your listening activity to ListenBrainz, + To scrobble your listening to ListenBrainz, enable it in the Scrobbling tab →turn it on in the Scrobbling tab