fix: OAuth2 trigger

superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx

@@ -20,7 +20,7 @@
 import * as reduxHooks from 'react-redux';
 import { Provider } from 'react-redux';
 import { createStore } from 'redux';
-import { render, fireEvent, waitFor } from 'spec/helpers/testing-library';
+import { render, waitFor } from 'spec/helpers/testing-library';
 import { ErrorLevel, ErrorSource, ErrorTypeEnum } from '@superset-ui/core';
 import { reRunQuery } from 'src/SqlLab/actions/sqlLab';
 import { triggerQuery } from 'src/components/Chart/chartAction';
@@ -58,25 +58,24 @@ jest.mock('src/dashboard/actions/dashboardState', () => ({
 const mockDispatch = jest.fn();
 jest.spyOn(reduxHooks, 'useDispatch').mockReturnValue(mockDispatch);
 
-// Mock global window functions
-const mockOpen = jest.spyOn(window, 'open').mockImplementation(() => null);
-const mockAddEventListener = jest.spyOn(window, 'addEventListener');
-const mockRemoveEventListener = jest.spyOn(window, 'removeEventListener');
-
-// Mock window.postMessage
-const originalPostMessage = window.postMessage;
+// Capture the channel instance created by the component so tests can drive its
+// onmessage handler and assert it gets closed on unmount.
+let capturedChannel: {
+  onmessage: ((event: any) => void) | null;
+  close: jest.Mock;
+};
+const channelCloseMock = jest.fn();
 
 beforeEach(() => {
-  window.postMessage = jest.fn();
+  jest.clearAllMocks();
+  capturedChannel = { onmessage: null, close: channelCloseMock };
+  (global as any).BroadcastChannel = jest
+    .fn()
+    .mockImplementation(() => capturedChannel);
 });
 
-afterEach(() => {
-  window.postMessage = originalPostMessage;
-});
-
-function simulateMessageEvent(data: any, origin: string) {
-  const messageEvent = new MessageEvent('message', { data, origin });
-  window.dispatchEvent(messageEvent);
+function simulateBroadcastMessage(data: any) {
+  capturedChannel.onmessage?.({ data });
 }
 
 const defaultProps = {
@@ -108,27 +107,26 @@ describe('OAuth2RedirectMessage Component', () => {
     expect(getByText(/provide authorization/i)).toBeInTheDocument();
   });
 
-  test('opens a new window with the correct URL when the link is clicked', () => {
+  test('renders the authorization link pointing at the OAuth2 URL', () => {
     const { getByText } = render(setup());
 
-    const linkElement = getByText(/provide authorization/i);
-    fireEvent.click(linkElement);
-
-    expect(mockOpen).toHaveBeenCalledWith('https://example.com', '_blank');
+    const linkElement = getByText(/provide authorization/i).closest('a');
+    expect(linkElement).toHaveAttribute('href', 'https://example.com');
+    expect(linkElement).toHaveAttribute('target', '_blank');
   });
 
-  test('cleans up the message event listener on unmount', () => {
+  test('closes the BroadcastChannel on unmount', () => {
     const { unmount } = render(setup());
 
-    expect(mockAddEventListener).toHaveBeenCalled();
+    expect((global as any).BroadcastChannel).toHaveBeenCalledWith('oauth');
     unmount();
-    expect(mockRemoveEventListener).toHaveBeenCalled();
+    expect(channelCloseMock).toHaveBeenCalled();
   });
 
   test('dispatches reRunQuery action when a message with correct tab ID is received for SQL Lab', async () => {
     render(setup());
 
-    simulateMessageEvent({ tabId: 'tabId' }, 'https://redirect.example.com');
+    simulateBroadcastMessage({ tabId: 'tabId' });
 
     await waitFor(() => {
       expect(reRunQuery).toHaveBeenCalledWith({ sql: 'SELECT * FROM table' });
@@ -138,7 +136,7 @@ describe('OAuth2RedirectMessage Component', () => {
   test('dispatches triggerQuery action for explore source upon receiving a correct message', async () => {
     render(setup({ source: 'explore' }));
 
-    simulateMessageEvent({ tabId: 'tabId' }, 'https://redirect.example.com');
+    simulateBroadcastMessage({ tabId: 'tabId' });
 
     await waitFor(() => {
       expect(triggerQuery).toHaveBeenCalledWith(true, 123);
@@ -148,11 +146,19 @@ describe('OAuth2RedirectMessage Component', () => {
   test('dispatches onRefresh action for dashboard source upon receiving a correct message', async () => {
     render(setup({ source: 'dashboard' }));
 
-    simulateMessageEvent({ tabId: 'tabId' }, 'https://redirect.example.com');
+    simulateBroadcastMessage({ tabId: 'tabId' });
 
     await waitFor(() => {
       // Chart IDs are converted to numbers by the component via chartList.map(Number)
       expect(onRefresh).toHaveBeenCalledWith([1, 2], true, 0, 'dashboard-id');
     });
   });
+
+  test('ignores messages with a mismatched tab ID', () => {
+    render(setup());
+
+    simulateBroadcastMessage({ tabId: 'someOtherTab' });
+
+    expect(reRunQuery).not.toHaveBeenCalled();
+  });
 });

superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useEffect, useRef, MouseEvent } from 'react';
+import { useEffect } from 'react';
 
 import { useDispatch, useSelector } from 'react-redux';
 import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types';
@@ -34,7 +34,6 @@ import { ErrorAlert } from './ErrorAlert';
 interface OAuth2RedirectExtra {
   url: string;
   tab_id: string;
-  redirect_uri: string;
 }
 
 /*
@@ -52,29 +51,20 @@ interface OAuth2RedirectExtra {
  * be used in subsequent connections. If a refresh token is also present in the response,
  * it will also be stored.
  *
- * After the token has been stored, the opened tab will send a message to the original
- * tab and close itself. This component, running on the original tab, will listen for
- * message events, and once it receives the success message from the opened tab it will
- * re-run the query for the user, be it in SQL Lab, Explore, or a dashboard. In order to
- * communicate securely, both tabs share a "tab ID", which is a UUID that is generated
- * by the backend and sent from the opened tab to the original tab. For extra security,
- * we also check that the source of the message is the opened tab via a ref.
+ * After the token has been stored, the opened tab will broadcast a message to the
+ * original tab and close itself. This component, running on the original tab, listens
+ * on a same-origin BroadcastChannel and re-runs the query for the user once it
+ * receives the success message — be it in SQL Lab, Explore, or a dashboard. Both tabs
+ * share a "tab ID" (a UUID generated by the backend) which is echoed back through the
+ * channel so the original tab only reacts to its own OAuth2 flow.
  */
 export function OAuth2RedirectMessage({
   error,
   source,
   closable,
 }: ErrorMessageComponentProps<OAuth2RedirectExtra>) {
-  const oAuthTab = useRef<Window | null>(null);
   const { extra, level } = error;
 
-  // store a reference to the OAuth2 browser tab, so we can check that the success
-  // message is coming from it
-  const handleOAuthClick = (event: MouseEvent<HTMLAnchorElement>) => {
-    event.preventDefault();
-    oAuthTab.current = window.open(extra.url, '_blank');
-  };
-
   // state needed for re-running the SQL Lab query
   const queries = useSelector<
     SqlLabRootState,
@@ -107,43 +97,24 @@ export function OAuth2RedirectMessage({
   const dispatch = useDispatch();
 
   useEffect(() => {
-    /* Listen for messages from the OAuth2 tab.
-     *
-     * After OAuth2 is successful the opened tab will send a message before
-     * closing itself. Once we receive the message we can retrigger the
-     * original query in SQL Lab, explore, or in a dashboard.
-     */
-    const redirectUrl = new URL(extra.redirect_uri);
-    const handleMessage = (event: MessageEvent) => {
-      if (
-        event.origin === redirectUrl.origin &&
-        event.data.tabId === extra.tab_id &&
-        event.source === oAuthTab.current
-      ) {
-        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));
-        }
+    const channel = new BroadcastChannel('oauth');
+    channel.onmessage = event => {
+      if (event.data.tabId !== extra.tab_id) {
+        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));
       }
     };
-    window.addEventListener('message', handleMessage);
 
     return () => {
-      window.removeEventListener('message', handleMessage);
+      channel.close();
     };
-  }, [
-    source,
-    extra.redirect_uri,
-    extra.tab_id,
-    dispatch,
-    query,
-    chartId,
-    chartList,
-    dashboardId,
-  ]);
+  }, [source, extra.tab_id, dispatch, query, chartId, chartList, dashboardId]);
 
   const body = (
     <p>
@@ -155,12 +126,7 @@ export function OAuth2RedirectMessage({
   const subtitle = (
     <>
       {t('You need to')}{' '}
-      <a
-        href={extra.url}
-        onClick={handleOAuthClick}
-        target="_blank"
-        rel="noreferrer"
-      >
+      <a href={extra.url} target="_blank" rel="noreferrer">
         {t('provide authorization')}
       </a>{' '}
       {t('in order to run this operation.')}

superset/templates/superset/oauth2.html

@@ -23,7 +23,9 @@ under the License.
   </head>
   <body>
     <script>
-      window.opener.postMessage({ tabId: '{{ tab_id }}' });
+      const channel = new BroadcastChannel('oauth');
+      channel.postMessage({ tabId: '{{ tab_id }}' });
+      channel.close();
       window.close();
     </script>
     <p>You can close this window and re-run the query.</p>