From 96a123f553f80ae7454daaf139b33e1397d9e3f7 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Mon, 21 Mar 2022 10:19:21 -0700 Subject: [PATCH] feat: SupersetClient config to override 401 behavior (#19144) * wip * feat: make 401 responses configurable in SupersetClient * sort * guest unauthorized handler * add toast container to embedded app * add option for toast presenter to go at the top * remove confusing comms logging * lint * Update superset-frontend/src/embedded/index.tsx * type correction --- .../src/connection/SupersetClientClass.ts | 21 ++-- .../superset-ui-core/src/connection/types.ts | 2 + .../connection/SupersetClientClass.test.ts | 110 +++++++++++++----- ...{ToastContainer.jsx => ToastContainer.tsx} | 6 +- .../MessageToasts/ToastPresenter.tsx | 29 +++-- .../components/MessageToasts/withToasts.tsx | 3 +- superset-frontend/src/embedded/index.tsx | 45 +++++-- 7 files changed, 154 insertions(+), 62 deletions(-) rename superset-frontend/src/components/MessageToasts/{ToastContainer.jsx => ToastContainer.tsx} (90%) 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 fa75148afe..bf0c02a81d 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 0ea6a957a8..0ab382917e 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 ae6ac138d5..921061b22f 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({}); + afterEach(() => { + authSpy.mockReset(); + window.location = originalLocation; + }); - 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('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 d61920de57..0157ff8a4d 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 8fcd4c44a9..0be676ad78 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 2d0486a65e..f7dc7c7217 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 c3e8f6a34f..3cd89a88be 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; }