diff --git a/.env.example b/.env.example index 05680eb..999659a 100644 --- a/.env.example +++ b/.env.example @@ -1,5 +1,4 @@ # Copy to .env and adjust. Without a .env the app starts in demo mode. -APP_MODE=demo +# Credential variables get added here as the features that need them land. -# The redirect URI must exactly match the URI registered in the Spotify dashboard. -SPOTIFY_CLIENT_ID= +APP_MODE=demo diff --git a/backend/app/adapters/spotify/auth.py b/backend/app/adapters/spotify/auth.py deleted file mode 100644 index fb78635..0000000 --- a/backend/app/adapters/spotify/auth.py +++ /dev/null @@ -1,137 +0,0 @@ -"""Spotify Authorization Code with PKCE helpers and token exchange.""" - -import base64 -import hashlib -import secrets -import time -from dataclasses import dataclass -from urllib.parse import urlencode - -import httpx2 - -from app.adapters.spotify.errors import SpotifyAuthenticationError - -AUTHORIZE_URL = "https://accounts.spotify.com/authorize" -TOKEN_URL = "https://accounts.spotify.com/api/token" -SCOPES = ( - "user-top-read user-library-read user-read-recently-played " - "playlist-modify-public playlist-modify-private " - "user-read-playback-state user-modify-playback-state" -) -TOKEN_EXPIRY_SKEW_SECONDS = 60.0 - - -@dataclass(frozen=True) -class TokenSet: - """An access token and its refresh credentials.""" - - access_token: str - refresh_token: str - expires_at: float - - @property - def is_expired(self) -> bool: - """Return whether the access token is expired within the safety skew.""" - return time.monotonic() >= self.expires_at - TOKEN_EXPIRY_SKEW_SECONDS - - -def generate_code_verifier() -> str: - """Create a high-entropy PKCE verifier within the allowed length.""" - return secrets.token_urlsafe(64) - - -def derive_code_challenge(verifier: str) -> str: - """Derive an unpadded base64url S256 challenge from a PKCE verifier.""" - digest = hashlib.sha256(verifier.encode("ascii")).digest() - return base64.urlsafe_b64encode(digest).rstrip(b"=").decode("ascii") - - -def build_authorize_url( - client_id: str, - redirect_uri: str, - state: str, - code_challenge: str, -) -> str: - """Build the Spotify consent URL for the required scopes.""" - query = urlencode( - { - "client_id": client_id, - "response_type": "code", - "redirect_uri": redirect_uri, - "state": state, - "scope": SCOPES, - "code_challenge_method": "S256", - "code_challenge": code_challenge, - } - ) - return f"{AUTHORIZE_URL}?{query}" - - -async def exchange_authorization_code( - http: httpx2.AsyncClient, - *, - client_id: str, - redirect_uri: str, - code: str, - code_verifier: str, -) -> TokenSet: - """Exchange a Spotify authorization code for an initial token set.""" - response = await http.post( - TOKEN_URL, - data={ - "client_id": client_id, - "grant_type": "authorization_code", - "code": code, - "redirect_uri": redirect_uri, - "code_verifier": code_verifier, - }, - ) - return _parse_token_response(response, existing_refresh_token=None) - - -async def refresh_access_token( - http: httpx2.AsyncClient, - *, - client_id: str, - tokens: TokenSet, -) -> TokenSet: - """Refresh an access token while retaining an omitted refresh token.""" - response = await http.post( - TOKEN_URL, - data={ - "client_id": client_id, - "grant_type": "refresh_token", - "refresh_token": tokens.refresh_token, - }, - ) - return _parse_token_response(response, existing_refresh_token=tokens.refresh_token) - - -def _parse_token_response( - response: httpx2.Response, - existing_refresh_token: str | None, -) -> TokenSet: - if not response.is_success: - raise SpotifyAuthenticationError( - f"Spotify token request failed with status {response.status_code}" - ) - - try: - payload = response.json() - access_token = payload["access_token"] - expires_in = payload["expires_in"] - refresh_token = payload.get("refresh_token", existing_refresh_token) - if ( - not isinstance(access_token, str) - or not isinstance(expires_in, int | float) - or not isinstance(refresh_token, str) - ): - raise TypeError - except (TypeError, KeyError, ValueError) as error: - raise SpotifyAuthenticationError("Spotify returned an invalid token response") from error - - return TokenSet( - access_token=access_token, - refresh_token=refresh_token, - expires_at=time.monotonic() + float(expires_in), - ) diff --git a/backend/app/adapters/spotify/client.py b/backend/app/adapters/spotify/client.py deleted file mode 100644 index b077df6..0000000 --- a/backend/app/adapters/spotify/client.py +++ /dev/null @@ -1,145 +0,0 @@ -"""Thin asynchronous client for the Spotify Web API.""" - -import asyncio - -import httpx2 - -from app.adapters.spotify.auth import refresh_access_token -from app.adapters.spotify.errors import ( - SpotifyAuthenticationError, - SpotifyRateLimitedError, - SpotifyRequestError, - SpotifyUnavailableError, -) -from app.adapters.spotify.mapping import ( - CreatedPlaylist, - CurrentUser, - parse_created_playlist, - parse_current_user, - parse_search_tracks, -) -from app.adapters.spotify.session import SpotifySession -from app.config import Settings -from app.domain.models import Track - - -class SpotifyClient: - """Call the supported Spotify endpoints with bounded retry behavior.""" - - def __init__( - self, - http: httpx2.AsyncClient, - session: SpotifySession, - settings: Settings, - ) -> None: - """Bind the shared transport to one authenticated Spotify session.""" - self.http = http - self.session = session - self.settings = settings - - async def search_tracks(self, query: str, limit: int = 10) -> list[Track]: - """Search Spotify tracks and return only valid mapped results.""" - response = await self._request( - "GET", - "/search", - params={"q": query, "type": "track", "limit": limit}, - ) - return parse_search_tracks(response.json()) - - async def fetch_current_user(self) -> CurrentUser: - """Fetch the authenticated Spotify user's stable identity.""" - response = await self._request("GET", "/me") - return parse_current_user(response.json()) - - async def create_playlist(self, name: str, description: str) -> CreatedPlaylist: - """Create a private playlist for the authenticated user.""" - response = await self._request( - "POST", - "/me/playlists", - json={"name": name, "description": description, "public": False}, - ) - return parse_created_playlist(response.json()) - - async def add_tracks_to_playlist(self, playlist_id: str, track_uris: list[str]) -> None: - """Add tracks to a playlist without retrying an ambiguous write.""" - await self._request( - "POST", - f"/playlists/{playlist_id}/items", - json={"uris": track_uris}, - ) - - async def _request( - self, - method: str, - path: str, - *, - params: dict[str, str | int] | None = None, - json: dict[str, object] | None = None, - ) -> httpx2.Response: - has_retried_authentication = False - has_retried_rate_limit = False - - while True: - access_token = await self._access_token() - response = await self.http.request( - method, - f"{self.settings.spotify_api_base_url.rstrip('/')}{path}", - params=params, - json=json, - headers={"Authorization": f"Bearer {access_token}"}, - ) - - if response.status_code == 401: - if has_retried_authentication: - raise SpotifyAuthenticationError("Spotify rejected refreshed authentication") - await self._refresh_if_current(access_token) - has_retried_authentication = True - continue - - if response.status_code == 429: - retry_after_seconds = _parse_retry_after(response) - if ( - method == "GET" - and not has_retried_rate_limit - and retry_after_seconds is not None - and retry_after_seconds <= self.settings.spotify_retry_after_cap_seconds - ): - await asyncio.sleep(retry_after_seconds) - has_retried_rate_limit = True - continue - raise SpotifyRateLimitedError(retry_after_seconds) - - if response.status_code >= 500: - raise SpotifyUnavailableError( - f"Spotify is unavailable with status {response.status_code}" - ) - if response.status_code >= 400: - raise SpotifyRequestError(response.status_code) - return response - - async def _access_token(self) -> str: - access_token = self.session.tokens.access_token - if self.session.tokens.is_expired: - await self._refresh_if_current(access_token) - return self.session.tokens.access_token - - async def _refresh_if_current(self, access_token: str) -> None: - async with self.session.refresh_lock: - if self.session.tokens.access_token != access_token: - return - self.session.tokens = await refresh_access_token( - self.http, - client_id=self.settings.spotify_client_id, - tokens=self.session.tokens, - ) - - -def _parse_retry_after(response: httpx2.Response) -> float | None: - value = response.headers.get("Retry-After") - if value is None: - return None - try: - retry_after_seconds = float(value) - except ValueError: - return None - return retry_after_seconds if retry_after_seconds >= 0 else None diff --git a/backend/app/adapters/spotify/errors.py b/backend/app/adapters/spotify/errors.py deleted file mode 100644 index c1e82de..0000000 --- a/backend/app/adapters/spotify/errors.py +++ /dev/null @@ -1,31 +0,0 @@ -"""Typed failures raised by the Spotify adapter.""" - - -class SpotifyError(Exception): - """Base class for Spotify adapter failures.""" - - -class SpotifyAuthenticationError(SpotifyError): - """Spotify rejected authentication or token refresh.""" - - -class SpotifyRateLimitedError(SpotifyError): - """Spotify rate limited a request that could not be retried.""" - - def __init__(self, retry_after_seconds: float | None) -> None: - """Record Spotify's requested delay without exposing request data.""" - super().__init__("Spotify rate limit exceeded") - self.retry_after_seconds = retry_after_seconds - - -class SpotifyUnavailableError(SpotifyError): - """Spotify returned a server-side failure.""" - - -class SpotifyRequestError(SpotifyError): - """Spotify rejected a non-authenticated API request.""" - - def __init__(self, status_code: int) -> None: - """Record the response status without exposing response content.""" - super().__init__(f"Spotify request failed with status {status_code}") - self.status_code = status_code diff --git a/backend/app/adapters/spotify/mapping.py b/backend/app/adapters/spotify/mapping.py deleted file mode 100644 index 44793dc..0000000 --- a/backend/app/adapters/spotify/mapping.py +++ /dev/null @@ -1,132 +0,0 @@ -"""Map Spotify response data into application-owned models.""" - -from collections.abc import Mapping -from dataclasses import dataclass -from typing import cast - -from app.domain.models import Track - - -@dataclass(frozen=True) -class CurrentUser: - """The stable identity fields returned for the current Spotify user.""" - - account_id: str - display_name: str - - -@dataclass(frozen=True) -class CreatedPlaylist: - """The application-owned result of creating a Spotify playlist.""" - - id: str - url: str - - -def parse_search_tracks(payload: object) -> list[Track]: - """Map valid Spotify search items and discard malformed entries.""" - root = _as_mapping(payload) - tracks = _as_mapping(root.get("tracks")) if root is not None else None - items = tracks.get("items") if tracks is not None else None - if not isinstance(items, list): - return [] - - parsed_tracks: list[Track] = [] - for item in items: - parsed_track = _parse_track(item) - if parsed_track is not None: - parsed_tracks.append(parsed_track) - return parsed_tracks - - -def parse_current_user(payload: object) -> CurrentUser: - """Map a Spotify current-user response into stable identity fields.""" - root = _as_mapping(payload) - account_id = _required_string(root, "account_id") - display_name = _required_string(root, "display_name") - if account_id is None or display_name is None: - raise ValueError("Spotify returned an invalid current-user response") - return CurrentUser(account_id=account_id, display_name=display_name) - - -def parse_created_playlist(payload: object) -> CreatedPlaylist: - """Map a Spotify playlist creation response.""" - root = _as_mapping(payload) - playlist_id = _required_string(root, "id") - external_urls = _as_mapping(root.get("external_urls")) if root is not None else None - url = _required_string(external_urls, "spotify") - if playlist_id is None or url is None: - raise ValueError("Spotify returned an invalid playlist response") - return CreatedPlaylist(id=playlist_id, url=url) - - -def _parse_track(payload: object) -> Track | None: - item = _as_mapping(payload) - track_id = _required_string(item, "id") - uri = _required_string(item, "uri") - title = _required_string(item, "name") - album = _as_mapping(item.get("album")) if item is not None else None - album_name = _required_string(album, "name") - artists = _parse_artists(item.get("artists")) if item is not None else None - - if ( - item is None - or track_id is None - or uri is None - or title is None - or album_name is None - or artists is None - ): - return None - - return Track( - id=track_id, - uri=uri, - title=title, - artists=artists, - album_name=album_name, - album_art_url=_parse_album_art_url(album), - external_url=_parse_external_url(item), - ) - - -def _parse_artists(payload: object) -> tuple[str, ...] | None: - if not isinstance(payload, list) or not payload: - return None - - names: list[str] = [] - for artist_payload in payload: - name = _required_string(_as_mapping(artist_payload), "name") - if name is None: - return None - names.append(name) - return tuple(names) - - -def _parse_album_art_url(album: Mapping[str, object] | None) -> str | None: - images = album.get("images") if album is not None else None - if not isinstance(images, list) or not images: - return None - return _optional_string(_as_mapping(images[0]), "url") - - -def _parse_external_url(item: Mapping[str, object]) -> str | None: - return _optional_string(_as_mapping(item.get("external_urls")), "spotify") - - -def _as_mapping(payload: object) -> Mapping[str, object] | None: - if not isinstance(payload, Mapping): - return None - return cast(Mapping[str, object], payload) - - -def _required_string(payload: Mapping[str, object] | None, key: str) -> str | None: - value = _optional_string(payload, key) - return value if value else None - - -def _optional_string(payload: Mapping[str, object] | None, key: str) -> str | None: - if payload is None: - return None - value = payload.get(key) - return value if isinstance(value, str) else None diff --git a/backend/app/adapters/spotify/session.py b/backend/app/adapters/spotify/session.py deleted file mode 100644 index 613933b..0000000 --- a/backend/app/adapters/spotify/session.py +++ /dev/null @@ -1,65 +0,0 @@ -"""In-memory Spotify login and session state.""" - -import asyncio -import secrets -import time -from dataclasses import dataclass, field - -from app.adapters.spotify.auth import TokenSet - -PENDING_LOGIN_LIFETIME_SECONDS = 600.0 - - -@dataclass -class SpotifySession: - """Authenticated Spotify identity and refresh coordination state.""" - - tokens: TokenSet - account_id: str - display_name: str - refresh_lock: asyncio.Lock = field(default_factory=asyncio.Lock) - - -class SessionStore: - """Store Spotify sessions behind opaque cookie-safe identifiers.""" - - def __init__(self) -> None: - """Create an empty session store.""" - self._sessions: dict[str, SpotifySession] = {} - - def create(self, session: SpotifySession) -> str: - """Store a session and return its opaque identifier.""" - session_id = secrets.token_urlsafe(32) - self._sessions[session_id] = session - return session_id - - def get(self, session_id: str) -> SpotifySession | None: - """Return a session by identifier when present.""" - return self._sessions.get(session_id) - - def remove(self, session_id: str) -> None: - """Remove a session if it exists.""" - self._sessions.pop(session_id, None) - - -class PendingLogins: - """Store short-lived, single-use PKCE verifiers by OAuth state.""" - - def __init__(self) -> None: - """Create an empty pending-login store.""" - self._entries: dict[str, tuple[str, float]] = {} - - def add(self, state: str, code_verifier: str) -> None: - """Store a PKCE verifier for a newly issued OAuth state.""" - self._entries[state] = (code_verifier, time.monotonic()) - - def pop(self, state: str) -> str | None: - """Consume a verifier unless its OAuth state is unknown or expired.""" - entry = self._entries.pop(state, None) - if entry is None: - return None - - code_verifier, created_at = entry - if time.monotonic() - created_at >= PENDING_LOGIN_LIFETIME_SECONDS: - return None - return code_verifier diff --git a/backend/app/api/routes.py b/backend/app/api/routes.py deleted file mode 100644 index d641619..0000000 --- a/backend/app/api/routes.py +++ /dev/null @@ -1,129 +0,0 @@ -"""HTTP routes for Spotify login and session management.""" - -import secrets -from typing import cast - -import httpx2 -from fastapi import APIRouter, Request -from fastapi.responses import JSONResponse, RedirectResponse, Response - -from app.adapters.spotify.auth import ( - build_authorize_url, - derive_code_challenge, - exchange_authorization_code, - generate_code_verifier, -) -from app.adapters.spotify.client import SpotifyClient -from app.adapters.spotify.errors import SpotifyError -from app.adapters.spotify.session import PendingLogins, SessionStore, SpotifySession -from app.config import Settings - -SESSION_COOKIE_NAME = "discovery_session" - -router = APIRouter() - - -@router.get("/api/auth/login") -def login(request: Request) -> RedirectResponse: - """Start Spotify Authorization Code with PKCE login.""" - application_settings = cast(Settings, request.app.state.settings) - pending_logins = cast(PendingLogins, request.app.state.pending_logins) - state = secrets.token_urlsafe(32) - code_verifier = generate_code_verifier() - pending_logins.add(state, code_verifier) - authorize_url = build_authorize_url( - application_settings.spotify_client_id, - application_settings.spotify_redirect_uri, - state, - derive_code_challenge(code_verifier), - ) - return RedirectResponse(authorize_url, status_code=307) - - -@router.get("/callback") -async def callback( - request: Request, - code: str | None = None, - state: str | None = None, - error: str | None = None, -) -> RedirectResponse: - """Complete Spotify login and establish an opaque cookie session.""" - if error is not None or code is None or state is None: - return _login_error_redirect() - - pending_logins = cast(PendingLogins, request.app.state.pending_logins) - code_verifier = pending_logins.pop(state) - if code_verifier is None: - return _login_error_redirect() - - application_settings = cast(Settings, request.app.state.settings) - http = cast(httpx2.AsyncClient, request.app.state.http) - try: - tokens = await exchange_authorization_code( - http, - client_id=application_settings.spotify_client_id, - redirect_uri=application_settings.spotify_redirect_uri, - code=code, - code_verifier=code_verifier, - ) - bootstrap_session = SpotifySession(tokens=tokens, account_id="", display_name="") - current_user = await SpotifyClient( - http, - bootstrap_session, - application_settings, - ).fetch_current_user() - except (SpotifyError, ValueError): - return _login_error_redirect() - - session = SpotifySession( - tokens=bootstrap_session.tokens, - account_id=current_user.account_id, - display_name=current_user.display_name, - ) - session_store = cast(SessionStore, request.app.state.session_store) - session_id = session_store.create(session) - response = RedirectResponse("/", status_code=307) - response.set_cookie( - SESSION_COOKIE_NAME, - session_id, - httponly=True, - samesite="lax", - path="/", - secure=application_settings.session_cookie_secure, - ) - return response - - -@router.get("/api/auth/me") -def current_session(request: Request) -> JSONResponse: - """Return the display name for a valid application session.""" - session_id = request.cookies.get(SESSION_COOKIE_NAME) - session_store = cast(SessionStore, request.app.state.session_store) - session = session_store.get(session_id) if session_id is not None else None - if session is None: - return JSONResponse({"detail": "Not authenticated"}, status_code=401) - return JSONResponse({"display_name": session.display_name}) - - -@router.post("/api/auth/logout", status_code=204) -def logout(request: Request) -> Response: - """Remove the current application session and clear its cookie.""" - session_id = request.cookies.get(SESSION_COOKIE_NAME) - session_store = cast(SessionStore, request.app.state.session_store) - if session_id is not None: - session_store.remove(session_id) - - application_settings = cast(Settings, request.app.state.settings) - response = Response(status_code=204) - response.delete_cookie( - SESSION_COOKIE_NAME, - path="/", - secure=application_settings.session_cookie_secure, - httponly=True, - samesite="lax", - ) - return response - - -def _login_error_redirect() -> RedirectResponse: - return RedirectResponse("/?login=error", status_code=307) diff --git a/backend/app/config.py b/backend/app/config.py index 4171843..95e0aa6 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -18,12 +18,6 @@ class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") app_mode: AppMode = AppMode.DEMO - spotify_client_id: str = "" - spotify_redirect_uri: str = "http://127.0.0.1:8888/callback" - spotify_api_base_url: str = "https://api.spotify.com/v1" - spotify_timeout_seconds: float = 10.0 - spotify_retry_after_cap_seconds: float = 5.0 - session_cookie_secure: bool = False settings = Settings() diff --git a/backend/app/domain/models.py b/backend/app/domain/models.py deleted file mode 100644 index 24e6beb..0000000 --- a/backend/app/domain/models.py +++ /dev/null @@ -1,16 +0,0 @@ -"""Pure domain models shared across application boundaries.""" - -from dataclasses import dataclass - - -@dataclass(frozen=True) -class Track: - """A Spotify track expressed without transport-specific data.""" - - id: str - uri: str - title: str - artists: tuple[str, ...] - album_name: str - album_art_url: str | None - external_url: str | None diff --git a/backend/app/main.py b/backend/app/main.py index ceea6ad..33c84d2 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,53 +1,23 @@ """Application factory and wiring. No logic lives here.""" -from collections.abc import AsyncIterator -from contextlib import asynccontextmanager from pathlib import Path -import httpx2 from fastapi import FastAPI from fastapi.staticfiles import StaticFiles -from app.adapters.spotify.session import PendingLogins, SessionStore -from app.api.routes import router -from app.config import AppMode, Settings, settings +from app.config import settings FRONTEND_DIST = Path(__file__).parent / "static" -def create_app( - application_settings: Settings | None = None, - http_transport: httpx2.AsyncBaseTransport | None = None, -) -> FastAPI: +def create_app() -> FastAPI: """Build the FastAPI app: API routes plus the built SPA on one port.""" - active_settings = application_settings or settings - - @asynccontextmanager - async def lifespan(application: FastAPI) -> AsyncIterator[None]: - if active_settings.app_mode is AppMode.LIVE and not active_settings.spotify_client_id: - raise RuntimeError("SPOTIFY_CLIENT_ID is required in live mode") - async with httpx2.AsyncClient( - timeout=active_settings.spotify_timeout_seconds, - transport=http_transport, - ) as http: - application.state.http = http - application.state.session_store = SessionStore() - application.state.pending_logins = PendingLogins() - application.state.settings = active_settings - yield - - app = FastAPI( - title="discovery-by-llm", - docs_url=None, - redoc_url=None, - lifespan=lifespan, - ) + app = FastAPI(title="discovery-by-llm", docs_url=None, redoc_url=None) @app.get("/api/health") def health() -> dict[str, str]: - return {"status": "ok", "mode": active_settings.app_mode} + return {"status": "ok", "mode": settings.app_mode} - app.include_router(router) if FRONTEND_DIST.is_dir(): app.mount("/", StaticFiles(directory=FRONTEND_DIST, html=True), name="spa") diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 3e999ca..215c438 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -5,13 +5,13 @@ description = "LLM-driven music discovery on the Spotify Web API" requires-python = "==3.13.*" dependencies = [ "fastapi>=0.116", - "httpx2>=2.10", "pydantic-settings>=2.10", "uvicorn[standard]>=0.35", ] [dependency-groups] dev = [ + "httpx2>=0.1", "mypy>=1.17", "pytest>=8.4", "ruff>=0.12", diff --git a/backend/tests/test_auth_routes.py b/backend/tests/test_auth_routes.py deleted file mode 100644 index 8d46f9d..0000000 --- a/backend/tests/test_auth_routes.py +++ /dev/null @@ -1,74 +0,0 @@ -"""End-to-end route tests for the Spotify login surface.""" - -from urllib.parse import parse_qs, urlparse - -import httpx2 -from fastapi.testclient import TestClient - -from app.config import AppMode, Settings -from app.main import create_app - - -def test_login_callback_cookie_and_current_user_flow() -> None: - async def spotify_handler(request: httpx2.Request) -> httpx2.Response: - if request.url.host == "accounts.spotify.com": - return httpx2.Response( - 200, - json={ - "access_token": "access", - "refresh_token": "refresh", - "expires_in": 3600, - }, - ) - assert request.url == "https://api.spotify.com/v1/me" - return httpx2.Response( - 200, - json={"account_id": "stable-account", "display_name": "Ada Listener"}, - ) - - app = create_app( - application_settings=_live_settings(), - http_transport=httpx2.MockTransport(spotify_handler), - ) - with TestClient(app, follow_redirects=False) as client: - unauthenticated_response = client.get("/api/auth/me") - login_response = client.get("/api/auth/login") - - authorize_url = urlparse(login_response.headers["location"]) - authorize_query = parse_qs(authorize_url.query) - state = authorize_query["state"][0] - - assert unauthenticated_response.status_code == 401 - assert login_response.status_code == 307 - assert authorize_url.netloc == "accounts.spotify.com" - assert authorize_url.path == "/authorize" - assert authorize_query["code_challenge"][0] - assert authorize_query["code_challenge_method"] == ["S256"] - - callback_response = client.get("/callback", params={"code": "code", "state": state}) - - assert callback_response.status_code == 307 - assert callback_response.headers["location"] == "/" - assert "discovery_session=" in callback_response.headers["set-cookie"] - assert "HttpOnly" in callback_response.headers["set-cookie"] - assert "SameSite=lax" in callback_response.headers["set-cookie"] - assert client.get("/api/auth/me").json() == {"display_name": "Ada Listener"} - - -def test_unknown_callback_state_redirects_to_login_error() -> None: - async def spotify_handler(request: httpx2.Request) -> httpx2.Response: - raise AssertionError("Spotify must not be called for an unknown state") - - app = create_app( - application_settings=_live_settings(), - http_transport=httpx2.MockTransport(spotify_handler), - ) - with TestClient(app, follow_redirects=False) as client: - response = client.get("/callback", params={"code": "code", "state": "unknown"}) - - assert response.status_code == 307 - assert response.headers["location"] == "/?login=error" - - -def _live_settings() -> Settings: - return Settings(app_mode=AppMode.LIVE, spotify_client_id="client-id") diff --git a/backend/tests/test_spotify_auth.py b/backend/tests/test_spotify_auth.py deleted file mode 100644 index c9f0565..0000000 --- a/backend/tests/test_spotify_auth.py +++ /dev/null @@ -1,45 +0,0 @@ -"""Tests for Spotify PKCE and token handling.""" - -import asyncio -import base64 -import hashlib -import time - -import httpx2 - -from app.adapters.spotify.auth import ( - TokenSet, - derive_code_challenge, - refresh_access_token, -) - - -def test_code_challenge_is_unpadded_base64url_sha256() -> None: - verifier = "a-fixed-code-verifier" - expected = base64.urlsafe_b64encode(hashlib.sha256(verifier.encode()).digest()) - - challenge = derive_code_challenge(verifier) - - assert challenge == expected.rstrip(b"=").decode() - assert "=" not in challenge - - -def test_refresh_keeps_existing_refresh_token_when_omitted() -> None: - async def run() -> None: - async def handler(request: httpx2.Request) -> httpx2.Response: - assert request.url == "https://accounts.spotify.com/api/token" - return httpx2.Response(200, json={"access_token": "new", "expires_in": 3600}) - - tokens = TokenSet("old", "keep-me", time.monotonic() + 3600) - async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as http: - refreshed = await refresh_access_token(http, client_id="client", tokens=tokens) - - assert refreshed.access_token == "new" - assert refreshed.refresh_token == "keep-me" - - asyncio.run(run()) - - -def test_token_expiry_uses_sixty_second_skew() -> None: - assert TokenSet("access", "refresh", time.monotonic() + 59).is_expired - assert not TokenSet("access", "refresh", time.monotonic() + 61).is_expired diff --git a/backend/tests/test_spotify_client.py b/backend/tests/test_spotify_client.py deleted file mode 100644 index 156a677..0000000 --- a/backend/tests/test_spotify_client.py +++ /dev/null @@ -1,189 +0,0 @@ -"""Transport-level tests for the Spotify API client.""" - -import asyncio -import time -from collections.abc import Callable, Coroutine - -import httpx2 -import pytest - -from app.adapters.spotify.auth import TokenSet -from app.adapters.spotify.client import SpotifyClient -from app.adapters.spotify.errors import SpotifyRateLimitedError -from app.adapters.spotify.session import SpotifySession -from app.config import Settings -from app.domain.models import Track - -TransportHandler = Callable[[httpx2.Request], Coroutine[None, None, httpx2.Response]] - - -def test_unauthorized_response_refreshes_once_and_returns_result() -> None: - async def run() -> None: - token_calls = 0 - api_calls = 0 - - async def handler(request: httpx2.Request) -> httpx2.Response: - nonlocal api_calls, token_calls - if request.url.host == "accounts.spotify.com": - token_calls += 1 - return _token_response() - api_calls += 1 - if request.headers["Authorization"] == "Bearer old-access": - return httpx2.Response(401) - return httpx2.Response(200, json=_search_payload()) - - tracks = await _search_with_handler(handler) - - assert [track.id for track in tracks] == ["track-1"] - assert token_calls == 1 - assert api_calls == 2 - - asyncio.run(run()) - - -def test_concurrent_unauthorized_responses_share_one_refresh() -> None: - async def run() -> None: - token_calls = 0 - old_api_calls = 0 - both_old_requests_arrived = asyncio.Event() - - async def handler(request: httpx2.Request) -> httpx2.Response: - nonlocal old_api_calls, token_calls - if request.url.host == "accounts.spotify.com": - token_calls += 1 - return _token_response() - if request.headers["Authorization"] == "Bearer old-access": - old_api_calls += 1 - if old_api_calls == 2: - both_old_requests_arrived.set() - await asyncio.wait_for(both_old_requests_arrived.wait(), timeout=1) - return httpx2.Response(401) - return httpx2.Response(200, json=_search_payload()) - - async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as http: - client = _client(http) - first, second = await asyncio.gather( - client.search_tracks("first"), - client.search_tracks("second"), - ) - - assert first == second - assert token_calls == 1 - - asyncio.run(run()) - - -def test_get_rate_limit_with_small_delay_retries_once() -> None: - async def run() -> None: - api_calls = 0 - - async def handler(request: httpx2.Request) -> httpx2.Response: - nonlocal api_calls - api_calls += 1 - if api_calls == 1: - return httpx2.Response(429, headers={"Retry-After": "0"}) - return httpx2.Response(200, json=_search_payload()) - - tracks = await _search_with_handler(handler) - - assert len(tracks) == 1 - assert api_calls == 2 - - asyncio.run(run()) - - -def test_get_rate_limit_above_cap_raises_without_retry() -> None: - async def run() -> None: - api_calls = 0 - - async def handler(request: httpx2.Request) -> httpx2.Response: - nonlocal api_calls - api_calls += 1 - return httpx2.Response(429, headers={"Retry-After": "6"}) - - with pytest.raises(SpotifyRateLimitedError) as error: - await _search_with_handler(handler) - - assert error.value.retry_after_seconds == 6 - assert api_calls == 1 - - asyncio.run(run()) - - -def test_playlist_write_rate_limit_is_not_retried() -> None: - async def run() -> None: - api_calls = 0 - - async def handler(request: httpx2.Request) -> httpx2.Response: - nonlocal api_calls - api_calls += 1 - assert request.method == "POST" - assert request.url.path == "/v1/playlists/playlist-1/items" - return httpx2.Response(429, headers={"Retry-After": "0"}) - - async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as http: - with pytest.raises(SpotifyRateLimitedError): - await _client(http).add_tracks_to_playlist("playlist-1", ["spotify:track:1"]) - - assert api_calls == 1 - - asyncio.run(run()) - - -def test_search_maps_valid_fields_and_drops_malformed_item() -> None: - async def run() -> None: - async def handler(request: httpx2.Request) -> httpx2.Response: - return httpx2.Response(200, json=_search_payload()) - - tracks = await _search_with_handler(handler) - - assert len(tracks) == 1 - assert tracks[0].id == "track-1" - assert tracks[0].uri == "spotify:track:1" - assert tracks[0].title == "Mapped song" - assert tracks[0].artists == ("First artist", "Second artist") - assert tracks[0].album_name == "Mapped album" - assert tracks[0].album_art_url == "https://images.example/cover.jpg" - assert tracks[0].external_url == "https://open.spotify.com/track/track-1" - - asyncio.run(run()) - - -async def _search_with_handler(handler: TransportHandler) -> list[Track]: - async with httpx2.AsyncClient(transport=httpx2.MockTransport(handler)) as http: - return list(await _client(http).search_tracks("mapped")) - - -def _client(http: httpx2.AsyncClient) -> SpotifyClient: - session = SpotifySession( - tokens=TokenSet("old-access", "refresh", time.monotonic() + 3600), - account_id="account", - display_name="Listener", - ) - return SpotifyClient(http, session, Settings(spotify_client_id="client")) - - -def _token_response() -> httpx2.Response: - return httpx2.Response(200, json={"access_token": "new-access", "expires_in": 3600}) - - -def _search_payload() -> dict[str, object]: - return { - "tracks": { - "total": 0, - "items": [ - { - "id": "track-1", - "uri": "spotify:track:1", - "name": "Mapped song", - "artists": [{"name": "First artist"}, {"name": "Second artist"}], - "album": { - "name": "Mapped album", - "images": [{"url": "https://images.example/cover.jpg"}], - }, - "external_urls": {"spotify": "https://open.spotify.com/track/track-1"}, - }, - {"id": "missing-required-fields"}, - ], - } - } diff --git a/backend/uv.lock b/backend/uv.lock index 31daa2f..0b1db0d 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -103,13 +103,13 @@ version = "0.1.0" source = { virtual = "." } dependencies = [ { name = "fastapi" }, - { name = "httpx2" }, { name = "pydantic-settings" }, { name = "uvicorn", extra = ["standard"] }, ] [package.dev-dependencies] dev = [ + { name = "httpx2" }, { name = "mypy" }, { name = "pytest" }, { name = "ruff" }, @@ -118,13 +118,13 @@ dev = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.116" }, - { name = "httpx2", specifier = ">=2.10" }, { name = "pydantic-settings", specifier = ">=2.10" }, { name = "uvicorn", extras = ["standard"], specifier = ">=0.35" }, ] [package.metadata.requires-dev] dev = [ + { name = "httpx2", specifier = ">=0.1" }, { name = "mypy", specifier = ">=1.17" }, { name = "pytest", specifier = ">=8.4" }, { name = "ruff", specifier = ">=0.12" },