From 633aa82b2286ad3759e4cac48e294a7252c9609f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 29 Jun 2026 08:09:18 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20un-break=20Spotify=20auth=20=E2=80=94=20?= =?UTF-8?q?revert=20the=20write=20scope=20+=20fix=20the=20OAuth=20token-ca?= =?UTF-8?q?che=20mismatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two compounding bugs broke Spotify auth for every user on the nightly (reported by wolf39us): 1. TRIGGER (regression from #945 increment 2): adding playlist-modify-* to the global SPOTIFY_OAUTH_SCOPE invalidated every existing token. Spotipy's validate_token treats a cached token as invalid the moment the requested scope stops being a subset of the token's granted scope, so growing the scope forced a re-auth on upgrade ("token refresh may have failed"). Reverted: the write scope is OUT of the global scope; Spotify export must request it on-demand (incremental auth) instead of breaking everyone on upgrade. 2. LATENT bug the trigger exposed: both global OAuth callbacks wrote the freshly-exchanged token to the legacy FILE cache (config/.spotify_cache) while the client reads DatabaseTokenCache (the DB store added for the earlier "unauthenticating daily" fix), which only imports the file when the DB is empty. So a re-auth's new token never reached the client → "token exchange succeeded but authentication validation failed", and re-auth was a dead end. Both callbacks now write DatabaseTokenCache — the same store the client reads. The scope revert alone re-validates existing tokens (no re-auth needed); the cache fix makes any future re-auth actually take effect. Tests: scope must not contain playlist-modify (the forced-re-auth guard) + the read scopes stay; global callbacks must use DatabaseTokenCache, not the file. 271 spotify/oauth tests green, ruff clean. NOTE: with the write scope gone, "Sync to Spotify" export can't get write access yet — needs a follow-up on-demand grant. Deezer export is unaffected. --- core/spotify_client.py | 13 +++++++++---- tests/test_spotify_client.py | 29 +++++++++++++++++++++++++++++ web_server.py | 14 +++++++++++--- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/core/spotify_client.py b/core/spotify_client.py index 66514c2d..001e7cef 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -17,12 +17,17 @@ logger = get_logger("spotify_client") # Single source of truth for the Spotify OAuth scope. Used by EVERY SpotifyOAuth # construction (the client, the per-profile registry, and all web_server callbacks) so # the authorize URL and token exchange can never request different scopes — a mismatch -# silently re-prompts or denies. The `playlist-modify-*` pair powers exporting a mirrored -# playlist back to Spotify (#945); existing users re-auth once to grant it. +# silently re-prompts or denies. +# +# IMPORTANT — do NOT add scopes here lightly. Spotipy's validate_token treats a cached +# token as invalid the moment the requested scope is no longer a subset of the token's +# granted scope, so GROWING this string invalidates EVERY existing user's token and forces +# a re-auth on upgrade. `playlist-modify-*` (for exporting a playlist back to Spotify, #945) +# was pulled back out for exactly that reason — it broke all Spotify users on upgrade. The +# Spotify export must request write access on-demand (incremental auth) instead. SPOTIFY_OAUTH_SCOPE = ( "user-library-read user-read-private playlist-read-private " - "playlist-read-collaborative user-read-email user-follow-read " - "playlist-modify-public playlist-modify-private" + "playlist-read-collaborative user-read-email user-follow-read" ) diff --git a/tests/test_spotify_client.py b/tests/test_spotify_client.py index 4dc18dfb..0716658e 100644 --- a/tests/test_spotify_client.py +++ b/tests/test_spotify_client.py @@ -164,3 +164,32 @@ def test_insufficient_scope_says_reconnect(): raise Exception('403 Forbidden: insufficient client scope') res = _spotify_with(_ScopeErr()).create_or_update_playlist('X', ['a']) assert not res['success'] and 'Reconnect Spotify' in res['error'] + + +# ── Spotify auth regression hotfix: scope must not force re-auth; callbacks must write +# the DB store the client reads (else a re-auth never takes effect) ── + +import os as _os + + +def test_oauth_scope_has_no_write_scope_that_forces_reauth(): + """Spotipy invalidates a cached token the moment the requested scope stops being a subset + of the token's granted scope — so GROWING the global scope forces every user to re-auth on + upgrade (it broke all Spotify users). The write scope (playlist-modify) must NOT live in the + global scope; request it on-demand instead.""" + from core.spotify_client import SPOTIFY_OAUTH_SCOPE + assert 'playlist-modify' not in SPOTIFY_OAUTH_SCOPE + # the read scopes existing tokens already carry must stay + for s in ('user-library-read', 'user-read-private', 'playlist-read-private', + 'playlist-read-collaborative', 'user-read-email', 'user-follow-read'): + assert s in SPOTIFY_OAUTH_SCOPE + + +def test_global_oauth_callbacks_use_db_token_cache_not_file(): + """The OAuth callbacks wrote the new token to the legacy file cache while the client reads + DatabaseTokenCache, so a re-auth never reached the client ("validation failed" despite a good + exchange). The global callbacks must write the same DB-backed store the client uses.""" + root = _os.path.dirname(_os.path.dirname(_os.path.abspath(__file__))) + src = open(_os.path.join(root, 'web_server.py'), encoding='utf-8').read() + assert "cache_path='config/.spotify_cache'" not in src # no global file-cache writes + assert src.count('cache_handler=DatabaseTokenCache(config_manager)') >= 2 diff --git a/web_server.py b/web_server.py index 83e88a87..701e6b44 100644 --- a/web_server.py +++ b/web_server.py @@ -5229,12 +5229,16 @@ def spotify_callback(): configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback") logger.info(f"Using redirect_uri for token exchange: {configured_uri}") + # Write the freshly-exchanged token to the SAME store the client reads + # (DatabaseTokenCache), not the legacy file — otherwise a re-auth never reaches the + # client and "validation failed" even though the exchange succeeded. + from core.spotify_token_cache import DatabaseTokenCache auth_manager = SpotifyOAuth( client_id=config['client_id'], client_secret=config['client_secret'], redirect_uri=configured_uri, scope=SPOTIFY_OAUTH_SCOPE, - cache_path='config/.spotify_cache' + cache_handler=DatabaseTokenCache(config_manager) ) token_info = auth_manager.get_access_token(auth_code) @@ -36111,13 +36115,17 @@ def start_oauth_callback_servers(): configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback") _oauth_logger.info(f"Using redirect_uri for token exchange: {configured_uri}") - # Create auth manager and exchange code for token + # Create auth manager and exchange code for token. Use the SAME store + # the client reads (DatabaseTokenCache), not the legacy file — a re-auth + # written to the file never reaches the DB-backed client, so it would + # report "validation failed" despite a successful exchange. + from core.spotify_token_cache import DatabaseTokenCache auth_manager = SpotifyOAuth( client_id=config['client_id'], client_secret=config['client_secret'], redirect_uri=configured_uri, scope=SPOTIFY_OAUTH_SCOPE, - cache_path='config/.spotify_cache' + cache_handler=DatabaseTokenCache(config_manager) ) # Extract the authorization code and exchange it for tokens