From 55603be14c482871867da1b03d64f76543a82c11 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 1 May 2026 11:23:00 +0300 Subject: [PATCH] Clarify Spotify auth flow and sync UI - Send Spotify auth completion back to the opener so the settings page refreshes immediately - Make the local auth flow go straight through to Spotify instead of showing the temporary instruction page - Keep the remote/docker instruction page available for manual callback setups - Sync Spotify status, connect/disconnect buttons, and metadata source selection after auth and disconnect - Keep the disconnect behavior aligned with the active primary metadata source --- core/metadata/registry.py | 5 +- tests/metadata/test_metadata_registry.py | 12 +- web_server.py | 222 ++++++++++++----------- webui/index.html | 2 +- webui/static/helper.js | 4 +- webui/static/init.js | 4 +- webui/static/settings.js | 27 ++- webui/static/shared-helpers.js | 33 ++++ 8 files changed, 193 insertions(+), 116 deletions(-) diff --git a/core/metadata/registry.py b/core/metadata/registry.py index d1162b35..214aa3f6 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -300,9 +300,10 @@ def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = return source -def get_spotify_disconnect_source() -> str: +def get_spotify_disconnect_source(configured_source: Optional[str] = None) -> str: """Return the active metadata source after Spotify is disconnected.""" - source = get_primary_source() + source = configured_source if configured_source is not None else _get_config_value("metadata.fallback_source", "deezer") + source = source or "deezer" return "deezer" if source == "spotify" else source diff --git a/tests/metadata/test_metadata_registry.py b/tests/metadata/test_metadata_registry.py index ea3a33d0..e4701d99 100644 --- a/tests/metadata/test_metadata_registry.py +++ b/tests/metadata/test_metadata_registry.py @@ -6,16 +6,12 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[2])) from core.metadata import registry -def test_spotify_disconnect_source_uses_deezer_when_spotify_is_primary(monkeypatch): - monkeypatch.setattr(registry, "get_primary_source", lambda: "spotify") - - assert registry.get_spotify_disconnect_source() == "deezer" +def test_spotify_disconnect_source_uses_deezer_when_spotify_is_primary(): + assert registry.get_spotify_disconnect_source("spotify") == "deezer" -def test_spotify_disconnect_source_keeps_non_spotify_primary(monkeypatch): - monkeypatch.setattr(registry, "get_primary_source", lambda: "discogs") - - assert registry.get_spotify_disconnect_source() == "discogs" +def test_spotify_disconnect_source_keeps_non_spotify_primary(): + assert registry.get_spotify_disconnect_source("discogs") == "discogs" def test_metadata_source_label_maps_known_sources(): diff --git a/web_server.py b/web_server.py index d42781f8..1dfd3ad0 100644 --- a/web_server.py +++ b/web_server.py @@ -3426,7 +3426,7 @@ def get_status(): is_rate_limited = spotify_client.is_rate_limited() if spotify_client else False rate_limit_info = spotify_client.get_rate_limit_info() if (spotify_client and is_rate_limited) else None cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining() if spotify_client else 0 - spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) + spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False # Read configured source once — no auth validation here, we do that explicitly below configured_source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer' @@ -3447,7 +3447,7 @@ def get_status(): music_source = configured_source _status_cache['spotify'] = { - 'connected': True, # Always true — iTunes fallback is always available + 'connected': spotify_session_active, 'authenticated': spotify_session_active, 'response_time': round(spotify_response_time, 1), 'source': music_source, @@ -4777,7 +4777,7 @@ def test_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': - spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) + spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False _status_cache['spotify']['connected'] = True _status_cache['spotify']['authenticated'] = spotify_session_active _status_cache['spotify']['source'] = _get_metadata_fallback_source() @@ -4945,7 +4945,7 @@ def test_dashboard_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': - spotify_session_active = bool(spotify_client and getattr(spotify_client, 'sp', None) is not None) + spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False _status_cache['spotify']['connected'] = True _status_cache['spotify']['authenticated'] = spotify_session_active _status_cache['spotify']['source'] = _get_metadata_fallback_source() @@ -5480,101 +5480,85 @@ def auth_spotify(): '127.0.0.1' not in configured_uri and 'localhost' not in configured_uri ) - if is_remote or is_docker: - # Show instructions for remote/docker access - if uses_main_port: - # redirect_uri already points to port 8008 or a custom domain — - # callback will come through the main Flask app, no manual steps needed - return f''' - - - - - -

Spotify Authentication

-

Click the link below to authenticate with Spotify:

-

Authenticate with Spotify

-
- Redirect URI: {configured_uri}
- After authorizing, Spotify will redirect back automatically. Make sure this URL matches your Spotify Dashboard redirect URI. -
-

After authentication completes, you can close this window and return to SoulSync.

- - - ''' - else: - # redirect_uri points to the standalone callback server — show manual steps AND suggest switching - import re as _re - _port_match = _re.search(r':(\d+)/', configured_uri) - callback_server_port = _port_match.group(1) if _port_match else str(os.environ.get('SOULSYNC_SPOTIFY_CALLBACK_PORT', '8888')) - return f''' - - - - - -

Spotify Authentication (Remote/Docker)

+ if not (is_remote or is_docker): + return redirect(auth_url) -
- Using a reverse proxy? Your redirect URI is set to {configured_uri} - which uses port {callback_server_port}. If you're behind a reverse proxy (Caddy, Nginx, Traefik), change the - redirect URI in SoulSync settings to use your proxy URL on the main port instead, e.g.:
- https://{host}/callback
- Then update the same URI in your Spotify Dashboard. - This avoids the need for manual URL editing below. -
+ if uses_main_port: + # The OAuth callback returns to the app itself, so there is no + # need to keep an intermediate page open. + return redirect(auth_url) -

Step 1: Click the link below to authenticate with Spotify

-

{auth_url}

-
-

Step 2: After authorizing, you'll see a blank page. The URL will look like:

- http://127.0.0.1:{callback_server_port}/callback?code=... -

Step 3: Change 127.0.0.1 to {host} and press Enter: - -

- http://{host}:{callback_server_port}/callback?code=... -

Authentication will then complete!

+ # redirect_uri points to the standalone callback server — show manual steps AND suggest switching + import re as _re + _port_match = _re.search(r':(\d+)/', configured_uri) + callback_server_port = _port_match.group(1) if _port_match else str(os.environ.get('SOULSYNC_SPOTIFY_CALLBACK_PORT', '8888')) + return f''' + + + + + +

Spotify Authentication (Remote/Docker)

- - - - ''' - else: - # Local access - simple message - return f'

Spotify Authentication

Click the link below to authenticate:

{auth_url}

After authentication, return to the app.

' +
+ Using a reverse proxy? Your redirect URI is set to {configured_uri} + which uses port {callback_server_port}. If you're behind a reverse proxy (Caddy, Nginx, Traefik), change the + redirect URI in SoulSync settings to use your proxy URL on the main port instead, e.g.:
+ https://{host}/callback
+ Then update the same URI in your Spotify Dashboard. + This avoids the need for manual URL editing below. +
+ +

Step 1: Click the link below to authenticate with Spotify

+

{auth_url}

+
+

Step 2: After authorizing, you'll see a blank page. The URL will look like:

+ http://127.0.0.1:{callback_server_port}/callback?code=... +

Step 3: Change 127.0.0.1 to {host} and press Enter: + +

+ http://{host}:{callback_server_port}/callback?code=... +

Authentication will then complete!

+ + + + + ''' else: return "

Spotify Authentication Failed

Could not initialize Spotify client. Check your credentials.

", 400 except Exception as e: @@ -5735,6 +5719,33 @@ def auth_tidal(): return f"

Tidal Authentication Error

{str(e)}

", 500 +def _spotify_auth_success_page(detail_text: str) -> str: + """Return the post-auth success page and notify the opener.""" + return f""" + + + + Spotify Authentication Successful + + +

Spotify Authentication Successful

+

{detail_text}

+ + +""" + + @app.route('/callback') def spotify_callback(): """ @@ -5796,7 +5807,7 @@ def spotify_callback(): # Invalidate cached profile client so it gets recreated with new tokens metadata_registry.clear_cached_profile_spotify_client(profile_id_from_state) add_activity_item("", "Spotify Auth Complete", f"Profile {profile_id_from_state} authenticated with Spotify", "Now") - return "

Spotify Authentication Successful!

Your personal Spotify account is now connected. You can close this window.

" + return _spotify_auth_success_page("Your personal Spotify account is now connected. You can close this window.") else: raise Exception("Failed to exchange authorization code for access token") @@ -5833,7 +5844,7 @@ def spotify_callback(): spotify_enrichment_worker.client.reload_config() spotify_enrichment_worker.client._invalidate_auth_cache() add_activity_item("", "Spotify Auth Complete", "Successfully authenticated with Spotify", "Now") - return "

Spotify Authentication Successful!

You can close this window.

" + return _spotify_auth_success_page("You can close this window.") else: raise Exception("Token exchange succeeded but authentication validation failed") else: @@ -5849,13 +5860,16 @@ def spotify_disconnect(): """Disconnect Spotify and keep using the active primary metadata source.""" global spotify_client try: + configured_source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer' # Pause enrichment worker before disconnecting to prevent it from hammering API if spotify_enrichment_worker: spotify_enrichment_worker.pause() spotify_client.disconnect() # Immediately update status cache so UI reflects the change - active_source = get_spotify_disconnect_source() + active_source = get_spotify_disconnect_source(configured_source) source_label = get_metadata_source_label(active_source) + if configured_source == 'spotify': + config_manager.set('metadata.fallback_source', active_source) _status_cache['spotify'] = { 'connected': False, 'authenticated': False, @@ -5867,7 +5881,13 @@ def spotify_disconnect(): } _status_cache_timestamps['spotify'] = time.time() add_activity_item("", "Spotify Disconnected", f"Using {source_label} for metadata", "Now") - return jsonify({'success': True, 'message': f'Spotify disconnected. Using {source_label} for metadata.', 'source': active_source, 'authenticated': False}) + return jsonify({ + 'success': True, + 'message': f'Spotify disconnected. Using {source_label} for metadata.', + 'source': active_source, + 'authenticated': False, + 'primary_source_changed': configured_source == 'spotify' + }) except Exception as e: logger.error(f"Error disconnecting Spotify: {e}") return jsonify({'success': False, 'error': str(e)}), 500 @@ -31970,7 +31990,7 @@ def start_oauth_callback_servers(): self.send_response(200) self.send_header('Content-type', 'text/html') self.end_headers() - self.wfile.write(b'

Spotify Authentication Successful!

You can close this window.

') + self.wfile.write(_spotify_auth_success_page("You can close this window.").encode("utf-8")) else: raise Exception("Token exchange succeeded but authentication validation failed") else: diff --git a/webui/index.html b/webui/index.html index 7cd8bb17..4652f7e6 100644 --- a/webui/index.html +++ b/webui/index.html @@ -3702,7 +3702,7 @@
-
Choose the primary source for artist, album, and track metadata. Spotify auth is optional and only needed for Spotify-specific actions. Discogs requires a personal token.
+
Choose the primary source for artist, album, and track metadata. Spotify can only be selected while an active Spotify session exists. Discogs requires a personal token.
diff --git a/webui/static/helper.js b/webui/static/helper.js index 46994799..6be9cfb0 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -2979,8 +2979,8 @@ async function _checkSetupStatus() { const resp = await fetch('/status'); if (resp.ok) { const data = await resp.json(); - // Metadata source: spotify.connected is always true (iTunes fallback), check .source - if (data.spotify?.connected && data.spotify?.source) { + // Metadata source is available when status reports a source. + if (data.spotify?.source) { results['metadata-source'] = results['metadata-source'] || Date.now(); _markSetupComplete('metadata-source'); } diff --git a/webui/static/init.js b/webui/static/init.js index 2680518a..6b3b11eb 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -1970,6 +1970,9 @@ function initApp() { initExpandedPlayer(); initializeSyncPage(); initializeWatchlist(); + if (typeof initializeSpotifyAuthCompletionListener === 'function') { + initializeSpotifyAuthCompletionListener(); + } // Initialize WebSocket connection (falls back to HTTP polling if unavailable) @@ -2371,4 +2374,3 @@ async function loadPageData(pageId) { // Old updateStatusIndicator function removed - replaced by updateSidebarServiceStatus // =============================== - diff --git a/webui/static/settings.js b/webui/static/settings.js index cd74a3e8..bb4e1e9d 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -50,6 +50,13 @@ function handleManualSaveClick() { saveSettings(false); } +function syncMetadataSourceSelection(source) { + const select = document.getElementById('metadata-fallback-source'); + if (!select || !source) return; + const option = select.querySelector(`option[value="${source}"]`); + if (option) select.value = source; +} + function initializeSettings() { // This function is called when the settings page is loaded. // It attaches event listeners to all interactive elements on the page. @@ -102,6 +109,11 @@ function initializeSettings() { // Test connection buttons // Test button event listeners removed - they use onclick attributes in HTML to avoid double firing + + if (typeof syncSpotifyMetadataSourceAvailability === 'function') { + syncSpotifyMetadataSourceAvailability(_lastServiceStatus?.spotify || null); + } + syncMetadataSourceSelection(_lastServiceStatus?.spotify?.source); } function resetFileOrganizationTemplates() { @@ -2400,6 +2412,17 @@ async function saveSettings(quiet = false) { activeServer = 'soulsync'; } + const metadataSourceSelect = document.getElementById('metadata-fallback-source'); + let metadataSource = metadataSourceSelect?.value || 'itunes'; + const spotifyDisconnected = _lastServiceStatus?.spotify?.connected === false; + if (metadataSource === 'spotify' && spotifyDisconnected) { + metadataSource = 'deezer'; + if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; + if (!quiet) { + showToast('Spotify is disconnected, so Deezer is used as the primary metadata source.', 'warning'); + } + } + const settings = { active_media_server: activeServer, spotify: { @@ -2472,7 +2495,7 @@ async function saveSettings(quiet = false) { token: document.getElementById('discogs-token').value, }, metadata: { - fallback_source: document.getElementById('metadata-fallback-source').value || 'itunes' + fallback_source: metadataSource }, hydrabase: { url: document.getElementById('hydrabase-url').value, @@ -3078,6 +3101,7 @@ async function disconnectSpotify() { const data = await response.json(); if (data.success) { showToast(data.message || 'Spotify disconnected.', 'success'); + syncMetadataSourceSelection(data.source || 'deezer'); // Immediately refresh status to update UI await fetchAndUpdateServiceStatus(); } else { @@ -3178,6 +3202,7 @@ async function disconnectSpotifyFromRateLimit() { if (data.success) { _spotifyRateLimitShown = false; showToast(data.message || 'Spotify disconnected.', 'success'); + syncMetadataSourceSelection(data.source || 'deezer'); await fetchAndUpdateServiceStatus(); if (currentPage === 'discover') { loadDiscoverPage(); diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index a0f822a0..299f90ac 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3107,6 +3107,24 @@ async function _forceServiceStatusRefresh() { } } +let _spotifyAuthCompletionListenerInstalled = false; + +function initializeSpotifyAuthCompletionListener() { + if (_spotifyAuthCompletionListenerInstalled) return; + _spotifyAuthCompletionListenerInstalled = true; + + window.addEventListener('message', async event => { + if (event.origin !== window.location.origin) return; + if (!event.data || event.data.type !== 'spotify-auth-complete') return; + + try { + await _forceServiceStatusRefresh(); + } catch (error) { + console.warn('Could not refresh Spotify status after auth completion:', error); + } + }); +} + async function fetchAndUpdateServiceStatus() { if (document.hidden) return; // Skip polling when tab is not visible if (socketConnected) return; // WebSocket is pushing updates — skip HTTP poll @@ -3160,6 +3178,19 @@ async function fetchAndUpdateServiceStatus() { } } +function syncSpotifyMetadataSourceAvailability(statusData) { + const select = document.getElementById('metadata-fallback-source'); + if (!select) return; + if (!statusData) return; + + const spotifyOption = select.querySelector('option[value="spotify"]'); + if (!spotifyOption) return; + + const spotifyAvailable = statusData?.connected === true; + spotifyOption.disabled = !spotifyAvailable; + spotifyOption.dataset.unavailable = spotifyAvailable ? 'false' : 'true'; +} + function updateServiceStatus(service, statusData) { const indicator = document.getElementById(`${service}-status-indicator`); const statusText = document.getElementById(`${service}-status-text`); @@ -3204,6 +3235,8 @@ function updateServiceStatus(service, statusData) { if (disconnectBtn) { disconnectBtn.style.display = spotifySessionActive ? '' : 'none'; } + + syncSpotifyMetadataSourceAvailability(statusData); } // Update download source title on dashboard card