You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2026/05/13 19:30:28 UTC
(superset) 01/01: fix: OAuth2 trigger
This is an automated email from the ASF dual-hosted git repository.
betodealmeida pushed a commit to branch fix-oauth-start
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4e6d5b57c6681543fabdee49d56ab667d2f4e1d7
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed May 13 15:30:04 2026 -0400
fix: OAuth2 trigger
---
.../ErrorMessage/OAuth2RedirectMessage.test.tsx | 60 +++++++++--------
.../ErrorMessage/OAuth2RedirectMessage.tsx | 76 ++++++----------------
superset/templates/superset/oauth2.html | 4 +-
3 files changed, 57 insertions(+), 83 deletions(-)
diff --git a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx
index a69089b91ae..3152d81e5c3 100644
--- a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx
+++ b/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();
+ });
});
diff --git a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx
index 2dc655fdec0..fd010a04c8c 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, 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.')}
diff --git a/superset/templates/superset/oauth2.html b/superset/templates/superset/oauth2.html
index e3562758db8..47fb9a20ecf 100644
--- a/superset/templates/superset/oauth2.html
+++ b/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>