This is an automated email from the ASF dual-hosted git repository. betodealmeida pushed a commit to branch fix-oauth-followup in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3331146643fc59aaaa1dd37a60cf83d195f009b3 Author: Beto Dealmeida <[email protected]> AuthorDate: Wed May 20 08:57:31 2026 -0400 fix(oauth2): make OAuth2 completion idempotent and listener resilient --- .../ErrorMessage/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 ae1519e2b41..b7704d2bae5 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 c76a95db4d7..225a68af2d2 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 aff43f6d4e9..9708a7d4b01 100644 --- a/superset/templates/superset/oauth2.html +++ b/superset/templates/superset/oauth2.html @@ -25,13 +25,25 @@ under the License. <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>
