From 3331146643fc59aaaa1dd37a60cf83d195f009b3 Mon Sep 17 00:00:00 2001
From: Beto Dealmeida <roberto@dealmeida.net>
Date: Wed, 20 May 2026 08:57:31 -0400
Subject: [PATCH] fix(oauth2): make OAuth2 completion idempotent and listener
 resilient

---
 .../OAuth2RedirectMessage.test.tsx            | 25 +++++++
 .../ErrorMessage/OAuth2RedirectMessage.tsx    | 68 ++++++++++++++-----
 superset/templates/superset/oauth2.html       | 22 ++++--
 3 files changed, 92 insertions(+), 23 deletions(-)

diff --git a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx
index ae1519e2b41c..b7704d2bae56 100644
--- a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx
+++ b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx
@@ -180,4 +180,29 @@ describe('OAuth2RedirectMessage Component', () => {
 
     expect(reRunQuery).not.toHaveBeenCalled();
   });
+
+  test('dispatches only once when both BroadcastChannel and storage signals arrive', async () => {
+    render(setup());
+
+    simulateBroadcastMessage({ tabId: 'tabId' });
+    simulateStorageMessage({ tabId: 'tabId' });
+
+    await waitFor(() => {
+      expect(reRunQuery).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  test('falls back to storage events when BroadcastChannel construction throws', async () => {
+    (global as any).BroadcastChannel = jest.fn().mockImplementation(() => {
+      throw new Error('blocked');
+    });
+
+    render(setup());
+
+    simulateStorageMessage({ tabId: 'tabId' });
+
+    await waitFor(() => {
+      expect(reRunQuery).toHaveBeenCalledWith({ sql: 'SELECT * FROM table' });
+    });
+  });
 });
diff --git a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx
index c76a95db4d79..225a68af2d2c 100644
--- a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx
+++ b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useEffect } from 'react';
+import { useEffect, useRef } from 'react';
 
 import { useDispatch, useSelector } from 'react-redux';
 import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types';
@@ -99,29 +99,61 @@ export function OAuth2RedirectMessage({
 
   const dispatch = useDispatch();
 
+  // `chartList` is rebuilt from `Object.keys` on every store update; keeping
+  // the listener state in a ref avoids tearing down/recreating the
+  // BroadcastChannel on each render and the race window that comes with it.
+  const latestStateRef = useRef({
+    source,
+    query,
+    chartId,
+    chartList,
+    dashboardId,
+  });
+  latestStateRef.current = {
+    source,
+    query,
+    chartId,
+    chartList,
+    dashboardId,
+  };
+
   useEffect(() => {
+    // Guard against duplicate dispatches if both the BroadcastChannel and the
+    // storage fallback ever deliver the same completion.
+    let handled = false;
     const handleOAuthComplete = (tabId?: string) => {
-      if (tabId !== extra.tab_id) {
+      if (tabId !== extra.tab_id || handled) {
         return;
       }
-      if (source === 'sqllab' && query) {
-        dispatch(reRunQuery(query));
-      } else if (source === 'explore' && chartId) {
-        dispatch(triggerQuery(true, chartId));
-      } else if (source === 'dashboard') {
-        dispatch(onRefresh(chartList.map(Number), true, 0, dashboardId));
+      handled = true;
+      const {
+        source: src,
+        query: q,
+        chartId: cId,
+        chartList: cList,
+        dashboardId: dId,
+      } = latestStateRef.current;
+      if (src === 'sqllab' && q) {
+        dispatch(reRunQuery(q));
+      } else if (src === 'explore' && cId) {
+        dispatch(triggerQuery(true, cId));
+      } else if (src === 'dashboard') {
+        dispatch(onRefresh(cList.map(Number), true, 0, dId));
       }
     };
 
-    const channel =
-      typeof BroadcastChannel !== 'undefined'
-        ? new BroadcastChannel(OAUTH_CHANNEL_NAME)
-        : null;
-
-    if (channel) {
-      channel.onmessage = event => {
-        handleOAuthComplete(event.data?.tabId);
-      };
+    // `BroadcastChannel` may exist on `window` but throw at construction time
+    // in restricted contexts; fall back to the storage listener if so.
+    let channel: BroadcastChannel | null = null;
+    if (typeof BroadcastChannel !== 'undefined') {
+      try {
+        channel = new BroadcastChannel(OAUTH_CHANNEL_NAME);
+        channel.onmessage = event => {
+          handleOAuthComplete(event.data?.tabId);
+        };
+      } catch {
+        channel = null;
+      }
     }
 
     const handleStorage = (event: StorageEvent) => {
@@ -143,7 +175,7 @@ export function OAuth2RedirectMessage({
       window.removeEventListener('storage', handleStorage);
       channel?.close();
     };
-  }, [source, extra.tab_id, dispatch, query, chartId, chartList, dashboardId]);
+  }, [extra.tab_id, dispatch]);
 
   const body = (
     <p>
diff --git a/superset/templates/superset/oauth2.html b/superset/templates/superset/oauth2.html
index aff43f6d4e9d..9708a7d4b01c 100644
--- a/superset/templates/superset/oauth2.html
+++ b/superset/templates/superset/oauth2.html
@@ -25,13 +25,25 @@
   <body>
     <script nonce="{{ macros.get_nonce() }}">
       const message = { tabId: '{{ tab_id }}' };
+      let broadcasted = false;
       if (typeof BroadcastChannel !== 'undefined') {
-        const channel = new BroadcastChannel('oauth');
-        channel.postMessage(message);
-        channel.close();
+        try {
+          const channel = new BroadcastChannel('oauth');
+          channel.postMessage(message);
+          channel.close();
+          broadcasted = true;
+        } catch (e) {
+          // fall through to the localStorage fallback below
+        }
+      }
+      if (!broadcasted) {
+        try {
+          localStorage.setItem('oauth2_auth_complete', JSON.stringify(message));
+          localStorage.removeItem('oauth2_auth_complete');
+        } catch (e) {
+          // storage may be unavailable (e.g. blocked or restricted); ignore
+        }
       }
-      localStorage.setItem('oauth2_auth_complete', JSON.stringify(message));
-      localStorage.removeItem('oauth2_auth_complete');
       window.close();
     </script>
     <p>You can close this window and re-run the query.</p>