Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Refresh auth on 401, better error messaging #874

Merged
merged 2 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/flyte-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"tslib": "^2.4.1"
},
"dependencies": {
"axios": "^0.27.2",
"axios": "^1.7.1",
"camelcase-keys": "^7.0.2",
"snakecase-keys": "^5.4.2"
}
Expand Down
2 changes: 1 addition & 1 deletion packages/oss-console/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"@tanstack/react-virtual": "^3.0.0-alpha.0",
"@types/d3-shape": "^1.2.6",
"@xstate/react": "^1.0.0",
"axios": "^0.27.2",
"axios": "^1.7.1",
"copy-to-clipboard": "^3.0.8",
"cronstrue": "^1.31.0",
"d3-dag": "^0.3.4",
Expand Down
58 changes: 38 additions & 20 deletions packages/oss-console/src/App/ApplicationRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,52 @@
import { useDomainPathUpgrade } from '../routes/useDomainPathUpgrade';
import { Routes } from '../routes/routes';
import AnimateRoute from '../routes/AnimateRoute';
import { loadChunkWithAuth } from '../components/data/loadChunkWithAuth';

Check warning on line 16 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L16

Added line #L16 was not covered by tests

const SelectProject = lazy(
() => import(/* webpackChunkName: "SelectProject" */ '../components/SelectProject'),
const SelectProject = lazy(() =>
loadChunkWithAuth(
() => import(/* webpackChunkName: "SelectProject" */ '../components/SelectProject'),

Check warning on line 20 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L18-L20

Added lines #L18 - L20 were not covered by tests
),
);
const ListProjectEntities = lazy(
() => import(/* webpackChunkName: "ListProjectEntities" */ '../components/ListProjectEntities'),
const ListProjectEntities = lazy(() =>
loadChunkWithAuth(
() => import(/* webpackChunkName: "ListProjectEntities" */ '../components/ListProjectEntities'),

Check warning on line 25 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L23-L25

Added lines #L23 - L25 were not covered by tests
),
);

const ExecutionDetails = lazy(
() =>
import(/* webpackChunkName: "ExecutionDetails" */ '../components/Executions/ExecutionDetails'),
const ExecutionDetails = lazy(() =>
loadChunkWithAuth(
() =>

Check warning on line 31 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L29-L31

Added lines #L29 - L31 were not covered by tests
import(
/* webpackChunkName: "ExecutionDetails" */ '../components/Executions/ExecutionDetails'
),
),
);

const TaskDetails = lazy(() => import(/* webpackChunkName: "TaskDetails" */ '../components/Task'));
const WorkflowDetails = lazy(
() => import(/* webpackChunkName: "WorkflowDetails" */ '../components/Workflow/WorkflowDetails'),
const TaskDetails = lazy(() =>
loadChunkWithAuth(() => import(/* webpackChunkName: "TaskDetails" */ '../components/Task')),

Check warning on line 39 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L38-L39

Added lines #L38 - L39 were not covered by tests
);
const LaunchPlanDetails = lazy(
() =>
import(
/* webpackChunkName: "LaunchPlanDetails" */ '../components/LaunchPlan/LaunchPlanDetails'
),
const WorkflowDetails = lazy(() =>
loadChunkWithAuth(
() =>

Check warning on line 43 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L41-L43

Added lines #L41 - L43 were not covered by tests
import(/* webpackChunkName: "WorkflowDetails" */ '../components/Workflow/WorkflowDetails'),
),
);
const EntityVersionsDetailsContainer = lazy(
() =>
import(
/* webpackChunkName: "EntityVersionsDetailsContainer" */ '../components/Entities/VersionDetails/EntityVersionDetailsContainer'
),
const LaunchPlanDetails = lazy(() =>
loadChunkWithAuth(
() =>

Check warning on line 49 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L47-L49

Added lines #L47 - L49 were not covered by tests
import(
/* webpackChunkName: "LaunchPlanDetails" */ '../components/LaunchPlan/LaunchPlanDetails'
),
),
);
const EntityVersionsDetailsContainer = lazy(() =>
loadChunkWithAuth(
() =>

Check warning on line 57 in packages/oss-console/src/App/ApplicationRouter.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/ApplicationRouter.tsx#L55-L57

Added lines #L55 - L57 were not covered by tests
import(
/* webpackChunkName: "EntityVersionsDetailsContainer" */ '../components/Entities/VersionDetails/EntityVersionDetailsContainer'
),
),
);

export const ApplicationRouter: React.FC = () => {
Expand Down
6 changes: 2 additions & 4 deletions packages/oss-console/src/App/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import { debug, debugPrefix } from '../common/log';
import { APIContext, useAPIState } from '../components/data/apiContext';
import { QueryAuthorizationObserver } from '../components/data/QueryAuthorizationObserver';
import { createQueryClient } from '../components/data/queryCache';
import { SystemStatusBanner } from '../components/Notifications/SystemStatusBanner';
import { history } from '../routes/history';
import { LocalCacheProvider } from '../basics/LocalCache/ContextProvider';
Expand All @@ -30,12 +29,11 @@
import ApplicationRouter from './ApplicationRouter';
import { ErrorBoundary } from '../components/common/ErrorBoundary';
import DownForMaintenance from '../components/Errors/DownForMaintenance';
import { globalQueryClient } from '../components/data/globalQueryClient';

Check warning on line 32 in packages/oss-console/src/App/index.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/App/index.tsx#L32

Added line #L32 was not covered by tests

const QueryClientProvider: React.FC<PropsWithChildren<QueryClientProviderProps>> =
QueryClientProviderImport;

const queryClient = createQueryClient();

export const AppComponent: React.FC<unknown> = () => {
if (env.NODE_ENV === 'development') {
debug.enable(`${debugPrefix}*:*`);
Expand All @@ -61,7 +59,7 @@
anchorOrigin={{ vertical: 'top', horizontal: 'right' }}
TransitionComponent={Collapse}
>
<QueryClientProvider client={queryClient}>
<QueryClientProvider client={globalQueryClient}>
<FlyteApiProvider flyteApiDomain={env.ADMIN_API} disableAutomaticLogin>
<APIContext.Provider value={apiState}>
<QueryAuthorizationObserver />
Expand Down
142 changes: 142 additions & 0 deletions packages/oss-console/src/components/Errors/ErrorHandler.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import NotAuthorizedError from '@clients/common/Errors/NotAuthorizedError';
import NotFoundError from '@clients/common/Errors/NotFoundError';
import { AxiosError } from 'axios';
import React from 'react';
import Box from '@mui/material/Box';
import Typography from '@mui/material/Typography';
import Link from '@mui/material/Link';
import Button from '@mui/material/Button';
import { useFlyteApi } from '@clients/flyte-api/ApiProvider';
import { GenericError } from './GenericError';

export interface ErrorHandlerProps {
error: NotFoundError | NotAuthorizedError | AxiosError | Error;
}

export const ErrorHandler: React.FC<ErrorHandlerProps> = ({ error }) => {
const { getLoginUrl } = useFlyteApi();

Check warning on line 17 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L17

Added line #L17 was not covered by tests

const contactSupport = (
<>

Check warning on line 20 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L20

Added line #L20 was not covered by tests
<Typography variant="label">
Please join the Slack community and explore its history on{' '}
<Link
color="inherit"
variant="label"
sx={{
fontSize: 'inherit',
}}
href="https://discuss.flyte.org/"
target="_blank"
>
discuss.flyte.org
</Link>
{', '}
or file a GitHub issue on{' '}
<Link
color="inherit"
variant="label"
sx={{
fontSize: 'inherit',
}}
href="https://github.com/flyteorg/flyte"
target="_blank"
>
flyteorg/flyte
</Link>{' '}
if the problem persists.
</Typography>
</>
);

if (
error instanceof NotFoundError ||
(error instanceof AxiosError && error.response?.status === 404)

Check warning on line 54 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L52-L54

Added lines #L52 - L54 were not covered by tests
) {
return (

Check warning on line 56 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L56

Added line #L56 was not covered by tests
<GenericError
title="404"
description="Not Found"
content={
<>
<Box py={1} />
<Typography variant="body2">The requested resource was not found</Typography>

<Box py={1} />

{contactSupport}
</>
}
/>
);
}

if (
error instanceof NotAuthorizedError ||
(error instanceof AxiosError && error.response?.status === 401)

Check warning on line 76 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L74-L76

Added lines #L74 - L76 were not covered by tests
) {
return (

Check warning on line 78 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L78

Added line #L78 was not covered by tests
<GenericError
title="401"
description="Unauthorized"
content={
<>
<Box py={1} />
<Typography variant="body2">
<strong>You do not have the proper authentication to access this page</strong>
</Typography>

<Box py={1} />

{contactSupport}
<Box py={1} />

<Button
variant="contained"
href={getLoginUrl()}
autoFocus
data-cy="login-button-overlay"
>
Log in
</Button>
</>
}
/>
);
}

if (error instanceof AxiosError && error.response?.status === 403) {
return (

Check warning on line 109 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L108-L109

Added lines #L108 - L109 were not covered by tests
<GenericError
title="403"
description="Forbidden"
content={
<>
<Box py={1} />
<Typography variant="body2">
<strong>You don't have permission to access this page</strong>
</Typography>

<Box py={1} />

{contactSupport}
</>
}
/>
);
}

return (

Check warning on line 129 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L129

Added line #L129 was not covered by tests
<GenericError
title={(error as AxiosError)?.response?.status || 'Oops!'}

Check warning on line 131 in packages/oss-console/src/components/Errors/ErrorHandler.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/ErrorHandler.tsx#L131

Added line #L131 was not covered by tests
description="Something went wrong."
content={
<>
<Box py={1} />

{contactSupport}
</>
}
/>
);
};
102 changes: 102 additions & 0 deletions packages/oss-console/src/components/Errors/GenericError.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import React from 'react';
import { type SvgIconProps } from '@mui/material/SvgIcon';
import PageMeta from '@clients/primitives/PageMeta';
import Container from '@mui/material/Container';
import NotFoundLogo from '@clients/ui-atoms/NotFoundLogo';
import Grid from '@mui/material/Grid';
import Typography from '@mui/material/Typography';

export interface GenericErrorProps {
title?: string | number;
description?: string;
content?: React.ReactNode;
Icon?: (props: SvgIconProps) => React.JSX.Element;
}

/**
* React prints the \n as "\n" in the DOM,
* so we need to split on that to make new lines
*/
const makeDescriptionSpans = (description: string = '') => {
const descriptionSpans = description.split('\\n').filter((span) => !!span);
return descriptionSpans.map((span) => (
<span key={span}>

Check warning on line 23 in packages/oss-console/src/components/Errors/GenericError.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/GenericError.tsx#L21-L23

Added lines #L21 - L23 were not covered by tests
{span}
<br />
</span>
));
};

export const GenericError: React.FC<GenericErrorProps> = ({
title,
description,
content,
Icon = NotFoundLogo,
}) => {
return (

Check warning on line 36 in packages/oss-console/src/components/Errors/GenericError.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/GenericError.tsx#L34-L36

Added lines #L34 - L36 were not covered by tests
<>
<PageMeta title={title?.toString()} />
<Container
sx={{
height: '100%',
flexGrow: 1,
display: 'flex',
alignItems: 'center',
}}
>
<Grid
container
gap={1}
sx={{
flexDirection: { xs: 'column-reverse', md: 'row' },
}}
>
<Grid item xs={12} md={2} />
<Grid
item
xs={12}
md={4}
sx={{
paddingRight: (t) => t.spacing(2),
paddingBottom: (t) => t.spacing(2),

Check warning on line 61 in packages/oss-console/src/components/Errors/GenericError.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/GenericError.tsx#L60-L61

Added lines #L60 - L61 were not covered by tests
}}
>
<Typography
variant="h3"
pb={1}
sx={{
fontSize: '22px',
}}
>
{title}
</Typography>
<Typography
variant="h1"
sx={{
fontSize: '42px',
}}
>
{makeDescriptionSpans(description)}
</Typography>

{content}
</Grid>
<Grid item xs={12} md={1} py={3} sx={{ display: 'flex' }} />
{Icon && (
<Grid item xs={4} md={4} sx={{ display: 'flex' }}>

Check warning on line 86 in packages/oss-console/src/components/Errors/GenericError.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/GenericError.tsx#L85-L86

Added lines #L85 - L86 were not covered by tests
<Icon
sx={{
width: '100%',
height: '100%',
maxWidth: { xs: '150px', md: '250px' },
maxHeight: { xs: '150px', md: '250px' },
color: (theme) => theme.palette.common.grays[20],

Check warning on line 93 in packages/oss-console/src/components/Errors/GenericError.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/Errors/GenericError.tsx#L93

Added line #L93 was not covered by tests
}}
/>
</Grid>
)}
</Grid>
</Container>
</>
);
};
12 changes: 9 additions & 3 deletions packages/oss-console/src/components/common/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
import Card from '@mui/material/Card';
import ErrorOutline from '@mui/icons-material/ErrorOutline';
import NotFoundError from '@clients/common/Errors/NotFoundError';
import NotAuthorizedError from '@clients/common/Errors/NotAuthorizedError';
import { AxiosError } from 'axios';
import { log } from '../../common/log';
import { useCommonStyles } from './styles';
import { PrettyError } from '../Errors/PrettyError';
import { NonIdealState } from './NonIdealState';
import { ErrorHandler } from '../Errors/ErrorHandler';

interface ErrorBoundaryState {
error?: Error;
Expand Down Expand Up @@ -58,8 +60,12 @@
render() {
const { fixed = false } = this.props;
if (this.state.error) {
if (this.state.error instanceof NotFoundError) {
return <PrettyError />;
if (
this.state.error instanceof NotFoundError ||
this.state.error instanceof NotAuthorizedError ||
(this.state.error as AxiosError).response?.status === 403

Check warning on line 66 in packages/oss-console/src/components/common/ErrorBoundary.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/common/ErrorBoundary.tsx#L63-L66

Added lines #L63 - L66 were not covered by tests
) {
return <ErrorHandler error={this.state.error} />;

Check warning on line 68 in packages/oss-console/src/components/common/ErrorBoundary.tsx

View check run for this annotation

Codecov / codecov/patch

packages/oss-console/src/components/common/ErrorBoundary.tsx#L68

Added line #L68 was not covered by tests
}

return <RenderError error={this.state.error} fixed={fixed} />;
Expand Down
Loading
Loading