Fix Spotify auth validation flow
- use a shared Spotify OAuth scope everywhere so callback tokens and runtime validation expect the same permissions - store Spotify OAuth caches at absolute project paths for global and per-profile auth flows - validate post-callback success with is_spotify_authenticated() instead of the fallback-friendly is_authenticated() - clear any active Spotify rate-limit cooldown before immediate post-auth validation - improve auth probe logging so post-callback failures expose the real exception
This commit is contained in:
Antti Kettunen
parent
d16aaab0aa
commit
718c5cb3d7
2 changed files with 55 additions and 34 deletions
|
|
@ -3,6 +3,7 @@ from spotipy.oauth2 import SpotifyOAuth, SpotifyClientCredentials
|
|||
from typing import Dict, List, Optional, Any
|
||||
import time
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from functools import wraps
|
||||
from dataclasses import dataclass
|
||||
from utils.logging_config import get_logger
|
||||
|
|
@ -56,6 +57,17 @@ _ESCALATION_WINDOW = 3600 # 1 hour — if re-limited within this, escalate
|
|||
_ESCALATION_MAX = 14400 # 4 hours max ban
|
||||
_BASE_UNKNOWN_BAN = 1800 # 30 min default when Retry-After header is missing
|
||||
_BASE_MAX_RETRIES_BAN = 14400 # 4 hours default when spotipy exhausted all retries (severe rate limit)
|
||||
SPOTIFY_USER_SCOPE = (
|
||||
"user-library-read user-read-private playlist-read-private "
|
||||
"playlist-read-collaborative user-read-email user-follow-read"
|
||||
)
|
||||
|
||||
def get_spotify_cache_path(profile_id: Optional[int] = None) -> str:
|
||||
"""Return an absolute cache path so OAuth tokens persist regardless of process CWD."""
|
||||
cache_dir = Path(config_manager.base_dir) / "config"
|
||||
cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
cache_name = ".spotify_cache" if not profile_id or profile_id == 1 else f".spotify_cache_profile_{profile_id}"
|
||||
return str(cache_dir / cache_name)
|
||||
|
||||
class SpotifyRateLimitError(Exception):
|
||||
"""Raised when Spotify API calls are blocked due to active global rate limit ban."""
|
||||
|
|
@ -542,8 +554,8 @@ class SpotifyClient:
|
|||
client_id=config['client_id'],
|
||||
client_secret=config['client_secret'],
|
||||
redirect_uri=config.get('redirect_uri', "http://127.0.0.1:8888/callback"),
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read",
|
||||
cache_path='config/.spotify_cache'
|
||||
scope=SPOTIFY_USER_SCOPE,
|
||||
cache_path=get_spotify_cache_path()
|
||||
)
|
||||
|
||||
self.sp = spotipy.Spotify(auth_manager=auth_manager, retries=0, requests_timeout=15)
|
||||
|
|
@ -633,7 +645,7 @@ class SpotifyClient:
|
|||
logger.warning(f"Auth probe rate limited — activating {ban_duration}s global ban")
|
||||
result = True
|
||||
else:
|
||||
logger.debug(f"Spotify authentication check failed: {e}")
|
||||
logger.warning(f"Spotify authentication check failed: {type(e).__name__}: {e}")
|
||||
result = False
|
||||
|
||||
with self._auth_cache_lock:
|
||||
|
|
@ -644,16 +656,16 @@ class SpotifyClient:
|
|||
|
||||
def disconnect(self):
|
||||
"""Disconnect Spotify: clear client, delete cache, invalidate auth cache, clear rate limit"""
|
||||
import os
|
||||
self.sp = None
|
||||
self.user_id = None
|
||||
self._invalidate_auth_cache()
|
||||
_clear_rate_limit()
|
||||
|
||||
cache_path = 'config/.spotify_cache'
|
||||
cache_path = get_spotify_cache_path()
|
||||
try:
|
||||
if os.path.exists(cache_path):
|
||||
os.remove(cache_path)
|
||||
cache_file = Path(cache_path)
|
||||
if cache_file.exists():
|
||||
cache_file.unlink()
|
||||
logger.info("Deleted Spotify cache file")
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to delete Spotify cache: {e}")
|
||||
|
|
@ -1600,4 +1612,4 @@ class SpotifyClient:
|
|||
except Exception as e:
|
||||
logger.error(f"Error in batch artist fetch: {e}")
|
||||
|
||||
return found
|
||||
return found
|
||||
|
|
|
|||
|
|
@ -57,7 +57,14 @@ if not pp_logger.handlers:
|
|||
_pp_handler.setFormatter(_logging.Formatter("%(asctime)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S"))
|
||||
pp_logger.addHandler(_pp_handler)
|
||||
pp_logger.propagate = False
|
||||
from core.spotify_client import SpotifyClient, Playlist as SpotifyPlaylist, Track as SpotifyTrack, _is_globally_rate_limited as _spotify_rate_limited
|
||||
from core.spotify_client import (
|
||||
SpotifyClient,
|
||||
Playlist as SpotifyPlaylist,
|
||||
Track as SpotifyTrack,
|
||||
SPOTIFY_USER_SCOPE,
|
||||
_is_globally_rate_limited as _spotify_rate_limited,
|
||||
get_spotify_cache_path,
|
||||
)
|
||||
from core.plex_client import PlexClient
|
||||
from core.jellyfin_client import JellyfinClient
|
||||
from core.navidrome_client import NavidromeClient
|
||||
|
|
@ -245,8 +252,8 @@ def get_spotify_client_for_profile(profile_id=None):
|
|||
client_id=creds['client_id'],
|
||||
client_secret=creds['client_secret'],
|
||||
redirect_uri=creds.get('redirect_uri', 'http://127.0.0.1:8888/callback'),
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email",
|
||||
cache_path=f'config/.spotify_cache_profile_{profile_id}'
|
||||
scope=SPOTIFY_USER_SCOPE,
|
||||
cache_path=get_spotify_cache_path(profile_id)
|
||||
)
|
||||
|
||||
# Create a bare SpotifyClient and immediately set the profile-specific
|
||||
|
|
@ -6397,14 +6404,15 @@ def auth_spotify():
|
|||
db = get_database()
|
||||
creds = db.get_profile_spotify(profile_id_int)
|
||||
if creds and creds.get('client_id'):
|
||||
from core.spotify_client import get_spotify_cache_path
|
||||
from spotipy.oauth2 import SpotifyOAuth
|
||||
redirect_uri = creds.get('redirect_uri') or config_manager.get_spotify_config().get('redirect_uri', 'http://127.0.0.1:8888/callback')
|
||||
auth_manager = SpotifyOAuth(
|
||||
client_id=creds['client_id'],
|
||||
client_secret=creds['client_secret'],
|
||||
redirect_uri=redirect_uri,
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email",
|
||||
cache_path=f'config/.spotify_cache_profile_{profile_id_int}',
|
||||
scope=SPOTIFY_USER_SCOPE,
|
||||
cache_path=get_spotify_cache_path(profile_id_int),
|
||||
state=f'profile_{profile_id_int}'
|
||||
)
|
||||
auth_url = auth_manager.get_authorize_url()
|
||||
|
|
@ -6710,7 +6718,7 @@ def spotify_callback():
|
|||
pass
|
||||
|
||||
try:
|
||||
from core.spotify_client import SpotifyClient
|
||||
from core.spotify_client import SpotifyClient, get_spotify_cache_path
|
||||
from spotipy.oauth2 import SpotifyOAuth
|
||||
from config.settings import config_manager
|
||||
|
||||
|
|
@ -6724,8 +6732,8 @@ def spotify_callback():
|
|||
client_id=creds['client_id'],
|
||||
client_secret=creds['client_secret'],
|
||||
redirect_uri=redirect_uri,
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email",
|
||||
cache_path=f'config/.spotify_cache_profile_{profile_id_from_state}',
|
||||
scope=SPOTIFY_USER_SCOPE,
|
||||
cache_path=get_spotify_cache_path(profile_id_from_state),
|
||||
state=f'profile_{profile_id_from_state}'
|
||||
)
|
||||
token_info = auth_manager.get_access_token(auth_code)
|
||||
|
|
@ -6747,8 +6755,8 @@ def spotify_callback():
|
|||
client_id=config['client_id'],
|
||||
client_secret=config['client_secret'],
|
||||
redirect_uri=configured_uri,
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email",
|
||||
cache_path='config/.spotify_cache'
|
||||
scope=SPOTIFY_USER_SCOPE,
|
||||
cache_path=get_spotify_cache_path()
|
||||
)
|
||||
|
||||
token_info = auth_manager.get_access_token(auth_code)
|
||||
|
|
@ -6757,12 +6765,12 @@ def spotify_callback():
|
|||
# CRITICAL: update the GLOBAL spotify_client, not a local variable
|
||||
global spotify_client
|
||||
spotify_client = SpotifyClient()
|
||||
if spotify_client.is_authenticated():
|
||||
# Clear any active rate limit ban and post-ban cooldown
|
||||
# so Spotify is immediately usable after re-auth
|
||||
from core.spotify_client import _clear_rate_limit
|
||||
_clear_rate_limit()
|
||||
spotify_client._invalidate_auth_cache()
|
||||
# Clear any active rate limit ban and post-ban cooldown before validation,
|
||||
# otherwise a fresh re-auth can still look unauthenticated during cooldown.
|
||||
from core.spotify_client import _clear_rate_limit
|
||||
_clear_rate_limit()
|
||||
spotify_client._invalidate_auth_cache()
|
||||
if spotify_client.is_spotify_authenticated():
|
||||
# Invalidate status cache so next poll picks up the new connection
|
||||
_status_cache_timestamps['spotify'] = 0
|
||||
# Refresh enrichment worker's client so it picks up new auth
|
||||
|
|
@ -6772,7 +6780,7 @@ def spotify_callback():
|
|||
add_activity_item("✅", "Spotify Auth Complete", "Successfully authenticated with Spotify", "Now")
|
||||
return "<h1>Spotify Authentication Successful!</h1><p>You can close this window.</p>"
|
||||
else:
|
||||
raise Exception("Token exchange succeeded but authentication validation failed")
|
||||
raise Exception("Token exchange succeeded but Spotify is still not authenticated")
|
||||
else:
|
||||
raise Exception("Failed to exchange authorization code for access token")
|
||||
except Exception as e:
|
||||
|
|
@ -47678,7 +47686,7 @@ def start_oauth_callback_servers():
|
|||
|
||||
# Manually trigger the token exchange using spotipy's auth manager
|
||||
try:
|
||||
from core.spotify_client import SpotifyClient
|
||||
from core.spotify_client import SpotifyClient, get_spotify_cache_path
|
||||
from spotipy.oauth2 import SpotifyOAuth
|
||||
from config.settings import config_manager
|
||||
|
||||
|
|
@ -47692,8 +47700,8 @@ def start_oauth_callback_servers():
|
|||
client_id=config['client_id'],
|
||||
client_secret=config['client_secret'],
|
||||
redirect_uri=configured_uri,
|
||||
scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email",
|
||||
cache_path='config/.spotify_cache'
|
||||
scope=SPOTIFY_USER_SCOPE,
|
||||
cache_path=get_spotify_cache_path()
|
||||
)
|
||||
|
||||
# Extract the authorization code and exchange it for tokens
|
||||
|
|
@ -47704,11 +47712,12 @@ def start_oauth_callback_servers():
|
|||
global spotify_client
|
||||
spotify_client = SpotifyClient()
|
||||
|
||||
if spotify_client.is_authenticated():
|
||||
# Clear rate limit ban + post-ban cooldown so Spotify is usable immediately
|
||||
from core.spotify_client import _clear_rate_limit
|
||||
_clear_rate_limit()
|
||||
spotify_client._invalidate_auth_cache()
|
||||
# Clear rate limit ban + post-ban cooldown before validation.
|
||||
from core.spotify_client import _clear_rate_limit
|
||||
_clear_rate_limit()
|
||||
spotify_client._invalidate_auth_cache()
|
||||
|
||||
if spotify_client.is_spotify_authenticated():
|
||||
# Invalidate status cache so next poll picks up the new connection
|
||||
_status_cache_timestamps['spotify'] = 0
|
||||
# Refresh enrichment worker's client so it picks up new auth
|
||||
|
|
@ -47721,7 +47730,7 @@ def start_oauth_callback_servers():
|
|||
self.end_headers()
|
||||
self.wfile.write(b'<h1>Spotify Authentication Successful!</h1><p>You can close this window.</p>')
|
||||
else:
|
||||
raise Exception("Token exchange succeeded but authentication validation failed")
|
||||
raise Exception("Token exchange succeeded but Spotify is still not authenticated")
|
||||
else:
|
||||
raise Exception("Failed to exchange authorization code for access token")
|
||||
except Exception as e:
|
||||
|
|
|
|||
Loading…
Reference in a new issue