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

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) {
+      if (res?.status === 401 && !ignoreUnauthorized) {
-        this.redirectUnauthorized();
+        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
-    }`;
-  }
 }

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

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&param=2';
-    const { location } = window;
+    const mockHref = `http://localhost${mockRequestPath + mockRequestSearch}`;
-    // @ts-ignore
+
-    delete window.location;
+    beforeEach(() => {
-    // @ts-ignore
+      originalLocation = window.location;
-    window.location = {
+      // @ts-ignore
-      pathname: mockRequestPath,
+      delete window.location;
-      search: mockRequestSearch,
+      // @ts-ignore
-    };
+      window.location = {
-    const authSpy = jest
+        pathname: mockRequestPath,
-      .spyOn(SupersetClientClass.prototype, 'ensureAuth')
+        search: mockRequestSearch,
-      .mockImplementation();
+        href: mockHref,
-    const rejectValue = { status: 401 };
+      };
-    fetchMock.get(mockRequestUrl, () => Promise.reject(rejectValue), {
+      authSpy = jest
-      overwriteRoutes: true,
+        .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;
+    it('should redirect', async () => {
-    try {
+      const client = new SupersetClientClass({});
-      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);
-    }
 
-    authSpy.mockReset();
+      let error;
-    window.location = location;
+      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);
+      }
+    });
   });
 });

superset-frontend/src/components/MessageToasts/ToastContainer.tsx

@@ -22,7 +22,9 @@ import ToastPresenter from './ToastPresenter';
 
 import { removeToast } from './actions';
 
-export default connect(
+const ToastContainer = connect(
-  ({ messageToasts: toasts }) => ({ toasts }),
+  ({ messageToasts: toasts }: any) => ({ toasts }),
   dispatch => bindActionCreators({ removeToast }, dispatch),
 )(ToastPresenter);
+
+export default ToastContainer;

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<VisualProps>`
   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<VisualProps> & {
   toasts: Array<ToastMeta>;
-  removeToast: () => void;
+  removeToast: () => any;
 };
 
 export default function ToastPresenter({
   toasts,
   removeToast,
+  position = 'top',
 }: ToastPresenterProps) {
   return (
-    toasts.length > 0 && (
+    <>
-      <StyledToastPresenter id="toast-presenter">
+      {toasts.length > 0 && (
-        {toasts.map(toast => (
+        <StyledToastPresenter id="toast-presenter" position={position}>
-          <Toast key={toast.id} toast={toast} onCloseToast={removeToast} />
+          {toasts.map(toast => (
-        ))}
+            <Toast key={toast.id} toast={toast} onCloseToast={removeToast} />
-      </StyledToastPresenter>
+          ))}
-    )
+        </StyledToastPresenter>
+      )}
+    </>
   );
 }

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,

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 = () => (
           <ErrorBoundary>
             <LazyDashboardPage />
           </ErrorBoundary>
+          <ToastContainer position="top" />
         </RootContextProviders>
       </Suspense>
     </Route>
@@ -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;
   }