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 fa75148afe62..bf0c02a81d78 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 0ea6a957a873..0ab382917e17 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 ae6ac138d5a6..921061b22f29 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 d61920de579e..0157ff8a4d98 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 8fcd4c44a90e..0be676ad788e 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 2d0486a65e34..f7dc7c72176f 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 c3e8f6a34f7d..3cd89a88be22 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; }