diff --git a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts index fa75148afe624..bf0c02a81d789 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts @@ -33,6 +33,12 @@ import { } from './types'; import { DEFAULT_FETCH_RETRY_OPTIONS, DEFAULT_BASE_URL } from './constants'; +const defaultUnauthorizedHandler = () => { + window.location.href = `/login?next=${ + window.location.pathname + window.location.search + }`; +}; + export default class SupersetClientClass { credentials: Credentials; @@ -58,6 +64,8 @@ export default class SupersetClientClass { timeout: ClientTimeout; + handleUnauthorized: () => void; + constructor({ baseUrl = DEFAULT_BASE_URL, host, @@ -70,6 +78,7 @@ export default class SupersetClientClass { csrfToken = undefined, guestToken = undefined, guestTokenHeaderName = 'X-GuestToken', + unauthorizedHandler = defaultUnauthorizedHandler, }: ClientConfig = {}) { const url = new URL( host || protocol @@ -100,6 +109,7 @@ export default class SupersetClientClass { if (guestToken) { this.headers[guestTokenHeaderName] = guestToken; } + this.handleUnauthorized = unauthorizedHandler; } async init(force = false): CsrfPromise { @@ -151,6 +161,7 @@ export default class SupersetClientClass { headers, timeout, fetchRetryOptions, + ignoreUnauthorized, ...rest }: RequestConfig & { parseMethod?: T }) { await this.ensureAuth(); @@ -163,8 +174,8 @@ export default class SupersetClientClass { timeout: timeout ?? this.timeout, fetchRetryOptions: fetchRetryOptions ?? this.fetchRetryOptions, }).catch(res => { - if (res?.status === 401) { - this.redirectUnauthorized(); + if (res?.status === 401 && !ignoreUnauthorized) { + this.handleUnauthorized(); } return Promise.reject(res); }); @@ -230,10 +241,4 @@ export default class SupersetClientClass { endpoint[0] === '/' ? endpoint.slice(1) : endpoint }`; } - - redirectUnauthorized() { - window.location.href = `/login?next=${ - window.location.pathname + window.location.search - }`; - } } diff --git a/superset-frontend/packages/superset-ui-core/src/connection/types.ts b/superset-frontend/packages/superset-ui-core/src/connection/types.ts index 0ea6a957a8739..0ab382917e170 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/types.ts @@ -81,6 +81,7 @@ export interface RequestBase { fetchRetryOptions?: FetchRetryOptions; headers?: Headers; host?: Host; + ignoreUnauthorized?: boolean; mode?: Mode; method?: Method; jsonPayload?: Payload; @@ -136,6 +137,7 @@ export interface ClientConfig { headers?: Headers; mode?: Mode; timeout?: ClientTimeout; + unauthorizedHandler?: () => void; } export interface SupersetClientInterface diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts index ae6ac138d5a67..921061b22f297 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts @@ -499,42 +499,92 @@ describe('SupersetClientClass', () => { }); }); - it('should redirect Unauthorized', async () => { + describe('when unauthorized', () => { + let originalLocation: any; + let authSpy: jest.SpyInstance; const mockRequestUrl = 'https://host/get/url'; const mockRequestPath = '/get/url'; const mockRequestSearch = '?param=1¶m=2'; - const { location } = window; - // @ts-ignore - delete window.location; - // @ts-ignore - window.location = { - pathname: mockRequestPath, - search: mockRequestSearch, - }; - const authSpy = jest - .spyOn(SupersetClientClass.prototype, 'ensureAuth') - .mockImplementation(); - const rejectValue = { status: 401 }; - fetchMock.get(mockRequestUrl, () => Promise.reject(rejectValue), { - overwriteRoutes: true, + const mockHref = `http://localhost${mockRequestPath + mockRequestSearch}`; + + beforeEach(() => { + originalLocation = window.location; + // @ts-ignore + delete window.location; + // @ts-ignore + window.location = { + pathname: mockRequestPath, + search: mockRequestSearch, + href: mockHref, + }; + authSpy = jest + .spyOn(SupersetClientClass.prototype, 'ensureAuth') + .mockImplementation(); + const rejectValue = { status: 401 }; + fetchMock.get(mockRequestUrl, () => Promise.reject(rejectValue), { + overwriteRoutes: true, + }); }); - const client = new SupersetClientClass({}); - - let error; - try { - await client.request({ url: mockRequestUrl, method: 'GET' }); - } catch (err) { - error = err; - } finally { - const redirectURL = window.location.href; - expect(redirectURL).toBe( - `/login?next=${mockRequestPath + mockRequestSearch}`, - ); - expect(error.status).toBe(401); - } + afterEach(() => { + authSpy.mockReset(); + window.location = originalLocation; + }); + + it('should redirect', async () => { + const client = new SupersetClientClass({}); - authSpy.mockReset(); - window.location = location; + let error; + try { + await client.request({ url: mockRequestUrl, method: 'GET' }); + } catch (err) { + error = err; + } finally { + const redirectURL = window.location.href; + expect(redirectURL).toBe( + `/login?next=${mockRequestPath + mockRequestSearch}`, + ); + expect(error.status).toBe(401); + } + }); + + it('does nothing if instructed to ignoreUnauthorized', async () => { + const client = new SupersetClientClass({}); + + let error; + try { + await client.request({ + url: mockRequestUrl, + method: 'GET', + ignoreUnauthorized: true, + }); + } catch (err) { + error = err; + } finally { + // unchanged href, no redirect + expect(window.location.href).toBe(mockHref); + expect(error.status).toBe(401); + } + }); + + it('accepts an unauthorizedHandler to override redirect behavior', async () => { + const unauthorizedHandler = jest.fn(); + const client = new SupersetClientClass({ unauthorizedHandler }); + + let error; + try { + await client.request({ + url: mockRequestUrl, + method: 'GET', + }); + } catch (err) { + error = err; + } finally { + // unchanged href, no redirect + expect(window.location.href).toBe(mockHref); + expect(error.status).toBe(401); + expect(unauthorizedHandler).toHaveBeenCalledTimes(1); + } + }); }); }); diff --git a/superset-frontend/src/components/MessageToasts/ToastContainer.jsx b/superset-frontend/src/components/MessageToasts/ToastContainer.tsx similarity index 90% rename from superset-frontend/src/components/MessageToasts/ToastContainer.jsx rename to superset-frontend/src/components/MessageToasts/ToastContainer.tsx index d61920de579ee..0157ff8a4d988 100644 --- a/superset-frontend/src/components/MessageToasts/ToastContainer.jsx +++ b/superset-frontend/src/components/MessageToasts/ToastContainer.tsx @@ -22,7 +22,9 @@ import ToastPresenter from './ToastPresenter'; import { removeToast } from './actions'; -export default connect( - ({ messageToasts: toasts }) => ({ toasts }), +const ToastContainer = connect( + ({ messageToasts: toasts }: any) => ({ toasts }), dispatch => bindActionCreators({ removeToast }, dispatch), )(ToastPresenter); + +export default ToastContainer; diff --git a/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx b/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx index 8fcd4c44a90e3..0be676ad788e5 100644 --- a/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx +++ b/superset-frontend/src/components/MessageToasts/ToastPresenter.tsx @@ -21,10 +21,14 @@ import { styled } from '@superset-ui/core'; import { ToastMeta } from 'src/components/MessageToasts/types'; import Toast from './Toast'; -const StyledToastPresenter = styled.div` +export interface VisualProps { + position: 'bottom' | 'top'; +} + +const StyledToastPresenter = styled.div` max-width: 600px; position: fixed; - bottom: 0px; + ${({ position }) => (position === 'bottom' ? 'bottom' : 'top')}: 0px; right: 0px; margin-right: 50px; margin-bottom: 50px; @@ -69,22 +73,25 @@ const StyledToastPresenter = styled.div` } `; -type ToastPresenterProps = { +type ToastPresenterProps = Partial & { toasts: Array; - removeToast: () => void; + removeToast: () => any; }; export default function ToastPresenter({ toasts, removeToast, + position = 'top', }: ToastPresenterProps) { return ( - toasts.length > 0 && ( - - {toasts.map(toast => ( - - ))} - - ) + <> + {toasts.length > 0 && ( + + {toasts.map(toast => ( + + ))} + + )} + ); } diff --git a/superset-frontend/src/components/MessageToasts/withToasts.tsx b/superset-frontend/src/components/MessageToasts/withToasts.tsx index 2d0486a65e347..f7dc7c72176f8 100644 --- a/superset-frontend/src/components/MessageToasts/withToasts.tsx +++ b/superset-frontend/src/components/MessageToasts/withToasts.tsx @@ -35,7 +35,8 @@ export interface ToastProps { addWarningToast: typeof addWarningToast; } -const toasters = { +/** just action creators, these do not dispatch */ +export const toasters = { addInfoToast, addSuccessToast, addWarningToast, diff --git a/superset-frontend/src/embedded/index.tsx b/superset-frontend/src/embedded/index.tsx index c3e8f6a34f7db..3cd89a88be225 100644 --- a/superset-frontend/src/embedded/index.tsx +++ b/superset-frontend/src/embedded/index.tsx @@ -19,12 +19,16 @@ import React, { lazy, Suspense } from 'react'; import ReactDOM from 'react-dom'; import { BrowserRouter as Router, Route } from 'react-router-dom'; +import { t } from '@superset-ui/core'; import { Switchboard } from '@superset-ui/switchboard'; import { bootstrapData } from 'src/preamble'; import setupClient from 'src/setup/setupClient'; import { RootContextProviders } from 'src/views/RootContextProviders'; +import { store } from 'src/views/store'; import ErrorBoundary from 'src/components/ErrorBoundary'; import Loading from 'src/components/Loading'; +import { addDangerToast } from 'src/components/MessageToasts/actions'; +import ToastContainer from 'src/components/MessageToasts/ToastContainer'; const debugMode = process.env.WEBPACK_MODE === 'development'; @@ -49,6 +53,7 @@ const EmbeddedApp = () => ( + @@ -75,23 +80,43 @@ if (!window.parent) { // ); // } +let displayedUnauthorizedToast = false; + +/** + * If there is a problem with the guest token, we will start getting + * 401 errors from the api and SupersetClient will call this function. + */ +function guestUnauthorizedHandler() { + if (displayedUnauthorizedToast) return; // no need to display this message every time we get another 401 + displayedUnauthorizedToast = true; + // If a guest user were sent to a login screen on 401, they would have no valid login to use. + // For embedded it makes more sense to just display a message + // and let them continue accessing the page, to whatever extent they can. + store.dispatch( + addDangerToast( + t( + 'This session has encountered an interruption, and some controls may not work as intended. If you are the developer of this app, please check that the guest token is being generated correctly.', + ), + { + duration: -1, // stay open until manually closed + noDuplicate: true, + }, + ), + ); +} + +/** + * Configures SupersetClient with the correct settings for the embedded dashboard page. + */ function setupGuestClient(guestToken: string) { - // need to reconfigure SupersetClient to use the guest token setupClient({ guestToken, guestTokenHeaderName: bootstrapData.config?.GUEST_TOKEN_HEADER_NAME, + unauthorizedHandler: guestUnauthorizedHandler, }); } function validateMessageEvent(event: MessageEvent) { - if ( - event.data?.type === 'webpackClose' || - event.data?.source === '@devtools-page' - ) { - // sometimes devtools use the messaging api and we want to ignore those - throw new Error("Sir, this is a Wendy's"); - } - // if (!ALLOW_ORIGINS.includes(event.origin)) { // throw new Error('Message origin is not in the allowed list'); // } @@ -105,7 +130,7 @@ window.addEventListener('message', function embeddedPageInitializer(event) { try { validateMessageEvent(event); } catch (err) { - log('ignoring message', err, event); + log('ignoring message unrelated to embedded comms', err, event); return; }