Harden Spotify OAuth callback for Docker/SSH tunnel setups (#220)
Top-level try/except in do_GET ensures an HTTP response is always sent — previously, unhandled exceptions caused BaseHTTPRequestHandler to silently close the connection (ERR_EMPTY_RESPONSE). All callback logging now uses the app logger instead of print() so output appears in app.log rather than only Docker stdout. Added health check at / to verify the callback server is running, and startup now logs the actual bind address to help diagnose port conflicts.
This commit is contained in:
parent
32adc66fe3
commit
edaa55ae82
2 changed files with 124 additions and 83 deletions
|
|
@ -19136,6 +19136,17 @@ def get_version_info():
|
||||||
"title": "What's New in SoulSync",
|
"title": "What's New in SoulSync",
|
||||||
"subtitle": f"Version {SOULSYNC_VERSION} — Latest Changes",
|
"subtitle": f"Version {SOULSYNC_VERSION} — Latest Changes",
|
||||||
"sections": [
|
"sections": [
|
||||||
|
{
|
||||||
|
"title": "🔧 Fix Spotify OAuth ERR_EMPTY_RESPONSE in Docker (#220)",
|
||||||
|
"description": "OAuth callback server hardened for Docker/SSH tunnel setups",
|
||||||
|
"features": [
|
||||||
|
"• Top-level error handler ensures an HTTP response is always sent (no more ERR_EMPTY_RESPONSE)",
|
||||||
|
"• All callback logging now goes to app.log (was only in Docker stdout before)",
|
||||||
|
"• Health check at http://localhost:8888/ to verify the callback server is running",
|
||||||
|
"• Startup logs the actual bind address for diagnosing port conflicts",
|
||||||
|
"• Port-in-use errors now logged clearly with explanation"
|
||||||
|
]
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"title": "📊 Show All Services on Dashboard (#219)",
|
"title": "📊 Show All Services on Dashboard (#219)",
|
||||||
"description": "Dashboard now shows connection status for all external services, not just the core three",
|
"description": "Dashboard now shows connection status for all external services, not just the core three",
|
||||||
|
|
@ -42369,10 +42380,21 @@ def start_oauth_callback_servers():
|
||||||
import urllib.parse
|
import urllib.parse
|
||||||
|
|
||||||
# Spotify callback server (port 8888 — for direct/local access only)
|
# Spotify callback server (port 8888 — for direct/local access only)
|
||||||
|
_oauth_logger = get_logger("oauth_callback")
|
||||||
|
|
||||||
class SpotifyCallbackHandler(BaseHTTPRequestHandler):
|
class SpotifyCallbackHandler(BaseHTTPRequestHandler):
|
||||||
def do_GET(self):
|
def do_GET(self):
|
||||||
|
try:
|
||||||
parsed_url = urllib.parse.urlparse(self.path)
|
parsed_url = urllib.parse.urlparse(self.path)
|
||||||
|
|
||||||
|
# Health check at root — lets users verify the server is running
|
||||||
|
if parsed_url.path == '/':
|
||||||
|
self.send_response(200)
|
||||||
|
self.send_header('Content-type', 'text/plain')
|
||||||
|
self.end_headers()
|
||||||
|
self.wfile.write(b'SoulSync Spotify OAuth callback server is running. Callback URL: /callback')
|
||||||
|
return
|
||||||
|
|
||||||
# Only process requests to /callback — ignore everything else
|
# Only process requests to /callback — ignore everything else
|
||||||
if parsed_url.path != '/callback':
|
if parsed_url.path != '/callback':
|
||||||
self.send_response(404)
|
self.send_response(404)
|
||||||
|
|
@ -42382,11 +42404,11 @@ def start_oauth_callback_servers():
|
||||||
return
|
return
|
||||||
|
|
||||||
query_params = urllib.parse.parse_qs(parsed_url.query)
|
query_params = urllib.parse.parse_qs(parsed_url.query)
|
||||||
print(f"🎵 Spotify callback received on port 8888: {self.path}")
|
_oauth_logger.info(f"Spotify callback received on port 8888: {self.path}")
|
||||||
|
|
||||||
if 'code' in query_params:
|
if 'code' in query_params:
|
||||||
auth_code = query_params['code'][0]
|
auth_code = query_params['code'][0]
|
||||||
print(f"🎵 Received Spotify authorization code: {auth_code[:10]}...")
|
_oauth_logger.info(f"Received Spotify authorization code: {auth_code[:10]}...")
|
||||||
|
|
||||||
# Manually trigger the token exchange using spotipy's auth manager
|
# Manually trigger the token exchange using spotipy's auth manager
|
||||||
try:
|
try:
|
||||||
|
|
@ -42397,7 +42419,7 @@ def start_oauth_callback_servers():
|
||||||
# Get Spotify config
|
# Get Spotify config
|
||||||
config = config_manager.get_spotify_config()
|
config = config_manager.get_spotify_config()
|
||||||
configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback")
|
configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback")
|
||||||
print(f"🎵 Using redirect_uri for token exchange: {configured_uri}")
|
_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
|
||||||
auth_manager = SpotifyOAuth(
|
auth_manager = SpotifyOAuth(
|
||||||
|
|
@ -42432,7 +42454,7 @@ def start_oauth_callback_servers():
|
||||||
else:
|
else:
|
||||||
raise Exception("Failed to exchange authorization code for access token")
|
raise Exception("Failed to exchange authorization code for access token")
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
print(f"🔴 Spotify token processing error: {e}")
|
_oauth_logger.error(f"Spotify token processing error: {e}")
|
||||||
add_activity_item("❌", "Spotify Auth Failed", f"Token processing failed: {str(e)}", "Now")
|
add_activity_item("❌", "Spotify Auth Failed", f"Token processing failed: {str(e)}", "Now")
|
||||||
self.send_response(400)
|
self.send_response(400)
|
||||||
self.send_header('Content-type', 'text/html')
|
self.send_header('Content-type', 'text/html')
|
||||||
|
|
@ -42440,8 +42462,8 @@ def start_oauth_callback_servers():
|
||||||
self.wfile.write(f'<h1>Spotify Authentication Failed</h1><p>{str(e)}</p>'.encode())
|
self.wfile.write(f'<h1>Spotify Authentication Failed</h1><p>{str(e)}</p>'.encode())
|
||||||
elif 'error' in query_params:
|
elif 'error' in query_params:
|
||||||
error = query_params['error'][0]
|
error = query_params['error'][0]
|
||||||
print(f"🔴 Spotify OAuth error returned by Spotify: {error}")
|
_oauth_logger.error(f"Spotify OAuth error returned by Spotify: {error}")
|
||||||
print(f"🔴 Full callback URL: {self.path}")
|
_oauth_logger.error(f"Full callback URL: {self.path}")
|
||||||
add_activity_item("❌", "Spotify Auth Failed", f"Spotify returned error: {error}", "Now")
|
add_activity_item("❌", "Spotify Auth Failed", f"Spotify returned error: {error}", "Now")
|
||||||
self.send_response(400)
|
self.send_response(400)
|
||||||
self.send_header('Content-type', 'text/html')
|
self.send_header('Content-type', 'text/html')
|
||||||
|
|
@ -42449,9 +42471,9 @@ def start_oauth_callback_servers():
|
||||||
self.wfile.write(f'<h1>Spotify Authentication Failed</h1><p>Spotify returned error: {error}</p>'.encode())
|
self.wfile.write(f'<h1>Spotify Authentication Failed</h1><p>Spotify returned error: {error}</p>'.encode())
|
||||||
else:
|
else:
|
||||||
# No code AND no error — callback was hit without OAuth params
|
# No code AND no error — callback was hit without OAuth params
|
||||||
print(f"🔴 Spotify callback received without OAuth parameters (no code or error)")
|
_oauth_logger.error(f"Spotify callback received without OAuth parameters (no code or error)")
|
||||||
print(f"🔴 Path: {self.path} | Query params: {query_params}")
|
_oauth_logger.error(f"Path: {self.path} | Query params: {query_params}")
|
||||||
print(f"🔴 This usually means the redirect lost its query parameters (reverse proxy issue)")
|
_oauth_logger.error(f"This usually means the redirect lost its query parameters (reverse proxy issue)")
|
||||||
self.send_response(400)
|
self.send_response(400)
|
||||||
self.send_header('Content-type', 'text/html')
|
self.send_header('Content-type', 'text/html')
|
||||||
self.end_headers()
|
self.end_headers()
|
||||||
|
|
@ -42463,17 +42485,35 @@ def start_oauth_callback_servers():
|
||||||
'(e.g. <code>https://yourdomain.com/callback</code>) — the main app handles callbacks too.</p>'
|
'(e.g. <code>https://yourdomain.com/callback</code>) — the main app handles callbacks too.</p>'
|
||||||
)
|
)
|
||||||
self.wfile.write(msg.encode())
|
self.wfile.write(msg.encode())
|
||||||
|
except Exception as e:
|
||||||
|
# Top-level catch-all — ensures we ALWAYS send an HTTP response.
|
||||||
|
# Without this, BaseHTTPRequestHandler silently closes the connection
|
||||||
|
# on unhandled exceptions, producing ERR_EMPTY_RESPONSE in the browser.
|
||||||
|
_oauth_logger.error(f"Unhandled error in Spotify callback handler: {e}", exc_info=True)
|
||||||
|
try:
|
||||||
|
self.send_response(500)
|
||||||
|
self.send_header('Content-type', 'text/html')
|
||||||
|
self.end_headers()
|
||||||
|
self.wfile.write(f'<h1>Internal Server Error</h1><p>{str(e)}</p>'.encode())
|
||||||
|
except Exception:
|
||||||
|
pass # Connection already broken, nothing more we can do
|
||||||
|
|
||||||
def log_message(self, format, *args):
|
def log_message(self, format, *args):
|
||||||
pass # Suppress server logs
|
pass # Suppress BaseHTTPRequestHandler access logs (we use our own logger)
|
||||||
|
|
||||||
# Start Spotify callback server
|
# Start Spotify callback server
|
||||||
def run_spotify_server():
|
def run_spotify_server():
|
||||||
try:
|
try:
|
||||||
spotify_server = HTTPServer(('0.0.0.0', 8888), SpotifyCallbackHandler)
|
bind_addr = ('0.0.0.0', 8888)
|
||||||
print("🎵 Started Spotify OAuth callback server on port 8888")
|
spotify_server = HTTPServer(bind_addr, SpotifyCallbackHandler)
|
||||||
|
_oauth_logger.info(f"Spotify OAuth callback server listening on {bind_addr[0]}:{bind_addr[1]}")
|
||||||
|
print(f"🎵 Started Spotify OAuth callback server on {bind_addr[0]}:{bind_addr[1]}")
|
||||||
spotify_server.serve_forever()
|
spotify_server.serve_forever()
|
||||||
|
except OSError as e:
|
||||||
|
_oauth_logger.error(f"Failed to start Spotify callback server on port 8888: {e} — port may already be in use")
|
||||||
|
print(f"🔴 Failed to start Spotify callback server on port 8888: {e}")
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
_oauth_logger.error(f"Failed to start Spotify callback server: {e}")
|
||||||
print(f"🔴 Failed to start Spotify callback server: {e}")
|
print(f"🔴 Failed to start Spotify callback server: {e}")
|
||||||
|
|
||||||
# Tidal callback server
|
# Tidal callback server
|
||||||
|
|
|
||||||
|
|
@ -3403,6 +3403,7 @@ function closeHelperSearch() {
|
||||||
const WHATS_NEW = {
|
const WHATS_NEW = {
|
||||||
'2.1': [
|
'2.1': [
|
||||||
// Newest features first
|
// Newest features first
|
||||||
|
{ title: 'Fix Spotify OAuth Empty Response', desc: 'OAuth callback server now always sends a response in Docker — added health check and proper logging' },
|
||||||
{ title: 'All Services on Dashboard', desc: 'Dashboard shows all enrichment services as live-status chips — click unconfigured ones to jump to Settings. Spotify card no longer shows "Apple Music"', page: 'dashboard' },
|
{ title: 'All Services on Dashboard', desc: 'Dashboard shows all enrichment services as live-status chips — click unconfigured ones to jump to Settings. Spotify card no longer shows "Apple Music"', page: 'dashboard' },
|
||||||
{ title: 'Qobuz on Connections Tab', desc: 'Qobuz credentials now on Settings → Connections for metadata enrichment without needing it as download source' },
|
{ title: 'Qobuz on Connections Tab', desc: 'Qobuz credentials now on Settings → Connections for metadata enrichment without needing it as download source' },
|
||||||
{ title: 'Fix Enrichment Status Widget', desc: 'Enrichment tooltip now shows Rate Limited or Daily Limit instead of stuck on Running' },
|
{ title: 'Fix Enrichment Status Widget', desc: 'Enrichment tooltip now shows Rate Limited or Daily Limit instead of stuck on Running' },
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue