One more fix for oAuth flow

diff --git a/music_assistant/controllers/webserver/helpers/auth_providers.py b/music_assistant/controllers/webserver/helpers/auth_providers.py
index e69c0d35bdba23491c88be60d803b450040c19cd..8df9579d7404eea69906b5d229e077e567314855 100644 (file)
--- a/music_assistant/controllers/webserver/helpers/auth_providers.py
+++ b/music_assistant/controllers/webserver/helpers/auth_providers.py
@@ -416,6 +416,18 @@ class BuiltinLoginProvider(LoginProvider):
 class HomeAssistantOAuthProvider(LoginProvider):
     """Home Assistant OAuth login provider."""
 
+    def __init__(self, mass: MusicAssistant, provider_id: str, config: LoginProviderConfig) -> None:
+        """
+        Initialize Home Assistant OAuth provider.
+
+        :param mass: MusicAssistant instance.
+        :param provider_id: Unique identifier for this provider instance.
+        :param config: Provider-specific configuration.
+        """
+        super().__init__(mass, provider_id, config)
+        # Store OAuth state -> return_url mapping to support concurrent sessions
+        self._oauth_sessions: dict[str, str | None] = {}
+
     @property
     def provider_type(self) -> AuthProviderType:
         """Return the provider type."""
@@ -528,9 +540,9 @@ class HomeAssistantOAuthProvider(LoginProvider):
             ha_url = inferred_ha_url
 
         state = secrets.token_urlsafe(32)
-        # Store state and return_url for verification and final redirect
-        self._oauth_state = state
-        self._oauth_return_url = return_url
+        # Store return_url keyed by state to support concurrent OAuth sessions
+        # This prevents race conditions when multiple users/sessions login simultaneously
+        self._oauth_sessions[state] = return_url
 
         # Use base_url of callback as client_id (same as HA provider does)
         client_id = base_url(redirect_uri)
@@ -681,9 +693,12 @@ class HomeAssistantOAuthProvider(LoginProvider):
         :param state: OAuth state parameter.
         :param redirect_uri: The callback URL.
         """
-        # Verify state
-        if not hasattr(self, "_oauth_state") or state != self._oauth_state:
-            return AuthResult(success=False, error="Invalid state parameter")
+        # Verify state and retrieve return_url from session
+        if state not in self._oauth_sessions:
+            return AuthResult(success=False, error="Invalid or expired state parameter")
+
+        # Retrieve and remove the return_url for this session (cleanup)
+        return_url = self._oauth_sessions.pop(state)
 
         # Get the correct HA URL (external URL if running as add-on)
         # This must be the same URL used in get_authorization_url
@@ -731,9 +746,6 @@ class HomeAssistantOAuthProvider(LoginProvider):
             # Get or create user
             user = await self._get_or_create_user(username, display_name, ha_user_id)
 
-            # Get stored return_url from OAuth state
-            return_url = getattr(self, "_oauth_return_url", None)
-
             if not user:
                 return AuthResult(
                     success=False,

diff --git a/music_assistant/helpers/resources/oauth_callback.html b/music_assistant/helpers/resources/oauth_callback.html
index ef030e90cd4f4299e627a20210e9a1998fa19685..8c7beb0c04da9c16ac4e3a3ab778077da5224125 100644 (file)
--- a/music_assistant/helpers/resources/oauth_callback.html
+++ b/music_assistant/helpers/resources/oauth_callback.html
@@ -242,27 +242,45 @@
             }
 
             if (isPopup) {
-                statusEl.textContent = 'Login Complete!';
-                messageEl.textContent = 'Closing popup...';
-
-                if (window.opener && !window.opener.closed) {
-                    try {
-                        window.opener.postMessage({
-                            type: 'oauth_success',
-                            token: token,
-                            redirectUrl: redirectUrl
-                        }, window.location.origin);
-                    } catch (e) {
-                        // Silently fail
+                const isExternalRedirect = redirectUrl && !redirectUrl.startsWith(window.location.origin);
+
+                if (isExternalRedirect) {
+                    // External redirect - must redirect to complete OAuth flow
+                    statusEl.textContent = 'Authentication Successful';
+                    messageEl.textContent = 'Redirecting...';
+
+                    if (isValidRedirectUrl(redirectUrl)) {
+                        setTimeout(() => {
+                            window.location.href = redirectUrl;
+                        }, 500);
+                    } else {
+                        messageEl.textContent = 'Redirect failed. Please close this window.';
+                        showManualCloseButton();
+                    }
+                } else {
+                    // Internal redirect - close popup and post message
+                    statusEl.textContent = 'Login Complete!';
+                    messageEl.textContent = 'Closing popup...';
+
+                    if (window.opener && !window.opener.closed) {
+                        try {
+                            window.opener.postMessage({
+                                type: 'oauth_success',
+                                token: token,
+                                redirectUrl: redirectUrl
+                            }, window.location.origin);
+                        } catch (e) {
+                            // Silently fail
+                        }
                     }
-                }
 
-                setTimeout(() => {
-                    attemptAutoClose();
                     setTimeout(() => {
-                        showManualCloseButton();
-                    }, 300);
-                }, 100);
+                        attemptAutoClose();
+                        setTimeout(() => {
+                            showManualCloseButton();
+                        }, 300);
+                    }, 100);
+                }
             } else {
                 statusEl.textContent = 'Authentication Successful';
                 messageEl.textContent = 'Redirecting...';