Skip to content

Commit

Permalink
feat: SupersetClient config to override 401 behavior (apache#19144)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
suddjian authored and michael_hoffman committed Mar 23, 2022
1 parent 81f68c2 commit 7f5b772
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -58,6 +64,8 @@ export default class SupersetClientClass {

timeout: ClientTimeout;

handleUnauthorized: () => void;

constructor({
baseUrl = DEFAULT_BASE_URL,
host,
Expand All @@ -70,6 +78,7 @@ export default class SupersetClientClass {
csrfToken = undefined,
guestToken = undefined,
guestTokenHeaderName = 'X-GuestToken',
unauthorizedHandler = defaultUnauthorizedHandler,
}: ClientConfig = {}) {
const url = new URL(
host || protocol
Expand Down Expand Up @@ -100,6 +109,7 @@ export default class SupersetClientClass {
if (guestToken) {
this.headers[guestTokenHeaderName] = guestToken;
}
this.handleUnauthorized = unauthorizedHandler;
}

async init(force = false): CsrfPromise {
Expand Down Expand Up @@ -151,6 +161,7 @@ export default class SupersetClientClass {
headers,
timeout,
fetchRetryOptions,
ignoreUnauthorized,
...rest
}: RequestConfig & { parseMethod?: T }) {
await this.ensureAuth();
Expand All @@ -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);
});
Expand Down Expand Up @@ -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
}`;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export interface RequestBase {
fetchRetryOptions?: FetchRetryOptions;
headers?: Headers;
host?: Host;
ignoreUnauthorized?: boolean;
mode?: Mode;
method?: Method;
jsonPayload?: Payload;
Expand Down Expand Up @@ -136,6 +137,7 @@ export interface ClientConfig {
headers?: Headers;
mode?: Mode;
timeout?: ClientTimeout;
unauthorizedHandler?: () => void;
}

export interface SupersetClientInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
// @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);
}
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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;
29 changes: 18 additions & 11 deletions superset-frontend/src/components/MessageToasts/ToastPresenter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.map(toast => (
<Toast key={toast.id} toast={toast} onCloseToast={removeToast} />
))}
</StyledToastPresenter>
)
<>
{toasts.length > 0 && (
<StyledToastPresenter id="toast-presenter" position={position}>
{toasts.map(toast => (
<Toast key={toast.id} toast={toast} onCloseToast={removeToast} />
))}
</StyledToastPresenter>
)}
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
45 changes: 35 additions & 10 deletions superset-frontend/src/embedded/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -49,6 +53,7 @@ const EmbeddedApp = () => (
<ErrorBoundary>
<LazyDashboardPage />
</ErrorBoundary>
<ToastContainer position="top" />
</RootContextProviders>
</Suspense>
</Route>
Expand All @@ -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');
// }
Expand All @@ -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;
}

Expand Down

0 comments on commit 7f5b772

Please sign in to comment.