From 60a1530d32394dbfe63fcb2b4b9ae2c08ade9222 Mon Sep 17 00:00:00 2001 From: Carina Ursu Date: Mon, 20 May 2024 13:07:57 -0500 Subject: [PATCH 1/2] Refresh auth on 401, better error messaging Signed-off-by: Carina Ursu --- packages/flyte-api/package.json | 2 +- packages/oss-console/package.json | 2 +- .../oss-console/src/App/ApplicationRouter.tsx | 58 ++++--- packages/oss-console/src/App/index.tsx | 6 +- .../src/components/Errors/ErrorHandler.tsx | 142 ++++++++++++++++++ .../src/components/Errors/GenericError.tsx | 102 +++++++++++++ .../src/components/common/ErrorBoundary.tsx | 12 +- .../data/QueryAuthorizationObserver.tsx | 13 +- .../src/components/data/axiosClient.tsx | 54 +++++++ .../src/components/data/globalQueryClient.ts | 3 + .../src/components/data/loadChunkWithAuth.tsx | 32 ++++ .../src/models/AdminEntity/AdminEntity.ts | 5 +- website/console/package.json | 1 + yarn.lock | 41 +++-- 14 files changed, 422 insertions(+), 51 deletions(-) create mode 100644 packages/oss-console/src/components/Errors/ErrorHandler.tsx create mode 100644 packages/oss-console/src/components/Errors/GenericError.tsx create mode 100644 packages/oss-console/src/components/data/axiosClient.tsx create mode 100644 packages/oss-console/src/components/data/globalQueryClient.ts create mode 100644 packages/oss-console/src/components/data/loadChunkWithAuth.tsx diff --git a/packages/flyte-api/package.json b/packages/flyte-api/package.json index e236fac74..6ddf514a4 100644 --- a/packages/flyte-api/package.json +++ b/packages/flyte-api/package.json @@ -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" } diff --git a/packages/oss-console/package.json b/packages/oss-console/package.json index df8d6a6d7..89b9f247c 100644 --- a/packages/oss-console/package.json +++ b/packages/oss-console/package.json @@ -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", diff --git a/packages/oss-console/src/App/ApplicationRouter.tsx b/packages/oss-console/src/App/ApplicationRouter.tsx index 0ae0fd3de..152ed418c 100644 --- a/packages/oss-console/src/App/ApplicationRouter.tsx +++ b/packages/oss-console/src/App/ApplicationRouter.tsx @@ -13,34 +13,52 @@ import { PrettyError } from '../components/Errors/PrettyError'; import { useDomainPathUpgrade } from '../routes/useDomainPathUpgrade'; import { Routes } from '../routes/routes'; import AnimateRoute from '../routes/AnimateRoute'; +import { loadChunkWithAuth } from '../components/data/loadChunkWithAuth'; -const SelectProject = lazy( - () => import(/* webpackChunkName: "SelectProject" */ '../components/SelectProject'), +const SelectProject = lazy(() => + loadChunkWithAuth( + () => import(/* webpackChunkName: "SelectProject" */ '../components/SelectProject'), + ), ); -const ListProjectEntities = lazy( - () => import(/* webpackChunkName: "ListProjectEntities" */ '../components/ListProjectEntities'), +const ListProjectEntities = lazy(() => + loadChunkWithAuth( + () => import(/* webpackChunkName: "ListProjectEntities" */ '../components/ListProjectEntities'), + ), ); -const ExecutionDetails = lazy( - () => - import(/* webpackChunkName: "ExecutionDetails" */ '../components/Executions/ExecutionDetails'), +const ExecutionDetails = lazy(() => + loadChunkWithAuth( + () => + 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')), ); -const LaunchPlanDetails = lazy( - () => - import( - /* webpackChunkName: "LaunchPlanDetails" */ '../components/LaunchPlan/LaunchPlanDetails' - ), +const WorkflowDetails = lazy(() => + loadChunkWithAuth( + () => + import(/* webpackChunkName: "WorkflowDetails" */ '../components/Workflow/WorkflowDetails'), + ), ); -const EntityVersionsDetailsContainer = lazy( - () => - import( - /* webpackChunkName: "EntityVersionsDetailsContainer" */ '../components/Entities/VersionDetails/EntityVersionDetailsContainer' - ), +const LaunchPlanDetails = lazy(() => + loadChunkWithAuth( + () => + import( + /* webpackChunkName: "LaunchPlanDetails" */ '../components/LaunchPlan/LaunchPlanDetails' + ), + ), +); +const EntityVersionsDetailsContainer = lazy(() => + loadChunkWithAuth( + () => + import( + /* webpackChunkName: "EntityVersionsDetailsContainer" */ '../components/Entities/VersionDetails/EntityVersionDetailsContainer' + ), + ), ); export const ApplicationRouter: React.FC = () => { diff --git a/packages/oss-console/src/App/index.tsx b/packages/oss-console/src/App/index.tsx index b8b4fbee5..9cd49421a 100644 --- a/packages/oss-console/src/App/index.tsx +++ b/packages/oss-console/src/App/index.tsx @@ -16,7 +16,6 @@ import { FeatureFlagsProvider } from '../basics/FeatureFlags'; 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'; @@ -30,12 +29,11 @@ import TopLevelLayoutProvider from '../components/common/TopLevelLayout/TopLevel import ApplicationRouter from './ApplicationRouter'; import { ErrorBoundary } from '../components/common/ErrorBoundary'; import DownForMaintenance from '../components/Errors/DownForMaintenance'; +import { globalQueryClient } from '../components/data/globalQueryClient'; const QueryClientProvider: React.FC> = QueryClientProviderImport; -const queryClient = createQueryClient(); - export const AppComponent: React.FC = () => { if (env.NODE_ENV === 'development') { debug.enable(`${debugPrefix}*:*`); @@ -61,7 +59,7 @@ export const AppComponent: React.FC = () => { anchorOrigin={{ vertical: 'top', horizontal: 'right' }} TransitionComponent={Collapse} > - + diff --git a/packages/oss-console/src/components/Errors/ErrorHandler.tsx b/packages/oss-console/src/components/Errors/ErrorHandler.tsx new file mode 100644 index 000000000..fdaf3ef83 --- /dev/null +++ b/packages/oss-console/src/components/Errors/ErrorHandler.tsx @@ -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 = ({ error }) => { + const { getLoginUrl } = useFlyteApi(); + + const contactSupport = ( + <> + + Please join the Slack community and explore its history on{' '} + + discuss.flyte.org + + {', '} + or file a GitHub issue on{' '} + + flyteorg/flyte + {' '} + if the problem persists. + + + ); + + if ( + error instanceof NotFoundError || + (error instanceof AxiosError && error.response?.status === 404) + ) { + return ( + + + The requested resource was not found + + + + {contactSupport} + + } + /> + ); + } + + if ( + error instanceof NotAuthorizedError || + (error instanceof AxiosError && error.response?.status === 401) + ) { + return ( + + + + You do not have the proper authentication to access this page + + + + + {contactSupport} + + + + + } + /> + ); + } + + if (error instanceof AxiosError && error.response?.status === 403) { + return ( + + + + You don't have permission to access this page + + + + + {contactSupport} + + } + /> + ); + } + + return ( + + + + {contactSupport} + + } + /> + ); +}; diff --git a/packages/oss-console/src/components/Errors/GenericError.tsx b/packages/oss-console/src/components/Errors/GenericError.tsx new file mode 100644 index 000000000..87a055874 --- /dev/null +++ b/packages/oss-console/src/components/Errors/GenericError.tsx @@ -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} +
+ + )); +}; + +export const GenericError: React.FC = ({ + title, + description, + content, + Icon = NotFoundLogo, +}) => { + return ( + <> + + + + + t.spacing(2), + paddingBottom: (t) => t.spacing(2), + }} + > + + {title} + + + {makeDescriptionSpans(description)} + + + {content} + + + {Icon && ( + + theme.palette.common.grays[20], + }} + /> + + )} + + + + ); +}; diff --git a/packages/oss-console/src/components/common/ErrorBoundary.tsx b/packages/oss-console/src/components/common/ErrorBoundary.tsx index a14a601ea..9e37f5fc5 100644 --- a/packages/oss-console/src/components/common/ErrorBoundary.tsx +++ b/packages/oss-console/src/components/common/ErrorBoundary.tsx @@ -6,10 +6,12 @@ import Typography from '@mui/material/Typography'; 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; @@ -58,8 +60,12 @@ export class ErrorBoundary extends React.Component< render() { const { fixed = false } = this.props; if (this.state.error) { - if (this.state.error instanceof NotFoundError) { - return ; + if ( + this.state.error instanceof NotFoundError || + this.state.error instanceof NotAuthorizedError || + (this.state.error as AxiosError).response?.status === 403 + ) { + return ; } return ; diff --git a/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx b/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx index 29b4d6628..a42c9cead 100644 --- a/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx +++ b/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx @@ -1,26 +1,19 @@ import * as React from 'react'; import { onlineManager, Query, useQueryClient } from 'react-query'; import { useFlyteApi } from '@clients/flyte-api/ApiProvider'; -import NotAuthorizedError from '@clients/common/Errors/NotAuthorizedError'; /** Watches all queries to detect a NotAuthorized error, disabling future queries * and triggering the login refresh flow. * Note: Should be placed just below the QueryClient and ApiContext providers. */ -// TODO: narusina - move this one to flyte-api too export const QueryAuthorizationObserver: React.FC = () => { const queryCache = useQueryClient().getQueryCache(); const apiContext = useFlyteApi(); React.useEffect(() => { - const unsubscribe = queryCache.subscribe((query?: Query | undefined) => { - if (!query || !query.state.error) { - return; - } - if (query.state.error instanceof NotAuthorizedError) { - // Stop all in-progress and future requests - onlineManager.setOnline(false); - // Trigger auth flow + const unsubscribe = queryCache.subscribe(async (_query?: Query | undefined) => { + if (!onlineManager.isOnline()) { + // trigger sign in modal apiContext.loginStatus.setExpired(true); } }); diff --git a/packages/oss-console/src/components/data/axiosClient.tsx b/packages/oss-console/src/components/data/axiosClient.tsx new file mode 100644 index 000000000..8e7020329 --- /dev/null +++ b/packages/oss-console/src/components/data/axiosClient.tsx @@ -0,0 +1,54 @@ +import { env } from '@clients/common/environment'; +import axios, { AxiosError, AxiosResponse } from 'axios'; +import { onlineManager } from 'react-query'; +import createAuthRefreshInterceptor from 'axios-auth-refresh'; + +export const axioClient = axios.create({ + baseURL: env.ADMIN_API_URL, + withCredentials: true, + maxRedirects: 0, +}); + +export const refreshAuth = async (axiosError?: AxiosError, isChunk?: boolean) => { + if (axiosError?.response?.status !== 401 && !isChunk) { + return; + } + return axioClient + .get(`${env.ADMIN_API_URL}/login?redirect_url=${env.BASE_URL}/select-project`, { + withCredentials: true, + headers: { + Accept: 'text/html', + }, + responseType: 'text', + maxRedirects: 5, + }) + .then((res) => { + const redirectUrl = `${env.ADMIN_API_URL}${env.BASE_URL}/select-project`; + if (res.request.responseURL.includes(redirectUrl)) { + onlineManager.setOnline(true); + return res; + } + + // throw error if not redirected to the console app + throw new Error(); + }) + .catch((_error) => { + onlineManager.setOnline(false); + + const unauthError = isChunk + ? new AxiosError('Not Authorized', '401', undefined, undefined, { + status: 401, + statusText: 'Not Authorized', + } as AxiosResponse) + : axiosError; + + return Promise.reject(unauthError); + }); +}; + +createAuthRefreshInterceptor(axioClient, refreshAuth, { + statusCodes: [401], // default: [ 401 ] + pauseInstanceWhileRefreshing: true, + retryInstance: axioClient, + interceptNetworkError: true, +}); diff --git a/packages/oss-console/src/components/data/globalQueryClient.ts b/packages/oss-console/src/components/data/globalQueryClient.ts new file mode 100644 index 000000000..95f182db3 --- /dev/null +++ b/packages/oss-console/src/components/data/globalQueryClient.ts @@ -0,0 +1,3 @@ +import { createQueryClient } from './queryCache'; + +export const globalQueryClient = createQueryClient(); diff --git a/packages/oss-console/src/components/data/loadChunkWithAuth.tsx b/packages/oss-console/src/components/data/loadChunkWithAuth.tsx new file mode 100644 index 000000000..eb2e42781 --- /dev/null +++ b/packages/oss-console/src/components/data/loadChunkWithAuth.tsx @@ -0,0 +1,32 @@ +import React from 'react'; +import NotAuthorizedError from '@clients/common/Errors/NotAuthorizedError'; +import { refreshAuth } from './axiosClient'; +import { ErrorHandler } from '../Errors/ErrorHandler'; + +type ImportFunction = () => Promise<{ default: React.ComponentType }>; + +export const loadChunkWithAuth = ( + importFunction: ImportFunction, +): Promise<{ default: React.ComponentType }> => { + return new Promise((resolve, _reject) => { + importFunction() + .then((res) => { + resolve(res); + }) + .catch((_error) => { + refreshAuth(undefined, true) + .then((_res) => { + importFunction().then((res) => { + resolve(res); + }); + }) + + .catch((_err) => { + // if loading chunk failed, show unauthorized error + resolve({ + default: () => , + }); + }); + }); + }); +}; diff --git a/packages/oss-console/src/models/AdminEntity/AdminEntity.ts b/packages/oss-console/src/models/AdminEntity/AdminEntity.ts index f67314c57..c776ebed2 100644 --- a/packages/oss-console/src/models/AdminEntity/AdminEntity.ts +++ b/packages/oss-console/src/models/AdminEntity/AdminEntity.ts @@ -1,4 +1,4 @@ -import axios, { AxiosRequestConfig, Method } from 'axios'; +import { AxiosRequestConfig, Method } from 'axios'; import { AdminEntityTransformer, DecodableType, @@ -7,6 +7,7 @@ import { } from '@clients/common/types/adminEntityTypes'; import { decodeProtoResponse } from '@clients/common/Utils/decodeProtoResponse'; import { transformRequestError } from '@clients/flyte-api/utils/transformRequestError'; +import { axioClient } from '@clients/oss-console/components/data/axiosClient'; import { generateAdminApiQuery } from './AdminApiQuery'; import { adminApiUrl, encodeProtoPayload, logProtoResponse } from './utils'; @@ -54,7 +55,7 @@ async function request( }; try { - const { data } = await axios.request(finalOptions); + const { data } = await axioClient.request(finalOptions); return data; } catch (e) { throw transformRequestError(e, endpoint, true); diff --git a/website/console/package.json b/website/console/package.json index cafccc8b9..3f89a1db7 100644 --- a/website/console/package.json +++ b/website/console/package.json @@ -25,6 +25,7 @@ "@patternfly/react-core": "^4.276.8", "@patternfly/react-log-viewer": "^4.87.100", "@tanstack/react-table": "^8.10.1", + "axios-auth-refresh": "^3.3.6", "chartjs-plugin-datalabels": "2.0.0", "cron-parser": "^4.9.0", "moment": "^2.29.4", diff --git a/yarn.lock b/yarn.lock index 2bd7e9dca..8d1ce1572 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2134,6 +2134,7 @@ __metadata: "@patternfly/react-core": ^4.276.8 "@patternfly/react-log-viewer": ^4.87.100 "@tanstack/react-table": ^8.10.1 + axios-auth-refresh: ^3.3.6 chartjs-plugin-datalabels: 2.0.0 cron-parser: ^4.9.0 moment: ^2.29.4 @@ -2172,7 +2173,7 @@ __metadata: version: 0.0.0-use.local resolution: "@clients/flyte-api@workspace:packages/flyte-api" dependencies: - axios: ^0.27.2 + axios: ^1.7.1 camelcase-keys: ^7.0.2 snakecase-keys: ^5.4.2 peerDependencies: @@ -2232,7 +2233,7 @@ __metadata: "@types/serve-static": ^1.7.31 "@types/shallowequal": ^0.2.3 "@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 @@ -9605,13 +9606,12 @@ __metadata: languageName: node linkType: hard -"axios@npm:^0.27.2": - version: 0.27.2 - resolution: "axios@npm:0.27.2" - dependencies: - follow-redirects: ^1.14.9 - form-data: ^4.0.0 - checksum: 38cb7540465fe8c4102850c4368053c21683af85c5fdf0ea619f9628abbcb59415d1e22ebc8a6390d2bbc9b58a9806c874f139767389c862ec9b772235f06854 +"axios-auth-refresh@npm:^3.3.6": + version: 3.3.6 + resolution: "axios-auth-refresh@npm:3.3.6" + peerDependencies: + axios: ">= 0.18 < 0.19.0 || >= 0.19.1" + checksum: 74206569065a4ece84830210d4f21900aa9690259c3c21743efb195309bcbcb6c3f5e59ce973ea8fe5d9155f5b241001c35448e527da9f32d7ee37d030d51544 languageName: node linkType: hard @@ -9626,6 +9626,17 @@ __metadata: languageName: node linkType: hard +"axios@npm:^1.7.1": + version: 1.7.1 + resolution: "axios@npm:1.7.1" + dependencies: + follow-redirects: ^1.15.6 + form-data: ^4.0.0 + proxy-from-env: ^1.1.0 + checksum: 77760d94b3812e07d4a5b02468a55eed5c8435ef4d605d159f2808775bdd15da60ab5b15b665a6f72800b5d261875d808b410cd3cb1d7571cdc6ec5e0108025a + languageName: node + linkType: hard + "axobject-query@npm:^3.2.1": version: 3.2.1 resolution: "axobject-query@npm:3.2.1" @@ -15022,7 +15033,7 @@ __metadata: languageName: node linkType: hard -"follow-redirects@npm:^1.0.0, follow-redirects@npm:^1.14.9": +"follow-redirects@npm:^1.0.0": version: 1.15.2 resolution: "follow-redirects@npm:1.15.2" peerDependenciesMeta: @@ -15042,6 +15053,16 @@ __metadata: languageName: node linkType: hard +"follow-redirects@npm:^1.15.6": + version: 1.15.6 + resolution: "follow-redirects@npm:1.15.6" + peerDependenciesMeta: + debug: + optional: true + checksum: a62c378dfc8c00f60b9c80cab158ba54e99ba0239a5dd7c81245e5a5b39d10f0c35e249c3379eae719ff0285fff88c365dd446fab19dee771f1d76252df1bbf5 + languageName: node + linkType: hard + "for-each@npm:^0.3.3": version: 0.3.3 resolution: "for-each@npm:0.3.3" From c756cebc489bc6fb3ab416ecf2b0c95bbfc712e0 Mon Sep 17 00:00:00 2001 From: Carina Ursu Date: Mon, 20 May 2024 13:17:25 -0500 Subject: [PATCH 2/2] fix tests Signed-off-by: Carina Ursu --- packages/oss-console/src/test/setupTests.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/oss-console/src/test/setupTests.ts b/packages/oss-console/src/test/setupTests.ts index 9f7d0ed11..808b7d01d 100644 --- a/packages/oss-console/src/test/setupTests.ts +++ b/packages/oss-console/src/test/setupTests.ts @@ -6,7 +6,18 @@ jest.mock('react-syntax-highlighter/dist/esm/styles/prism', () => ({ // mock axios to avoid open handles const axiosMock = jest.mock('axios', () => ({ - request: jest.fn().mockResolvedValue({ data: {} }), + create: jest.fn().mockReturnValue({ + request: jest.fn().mockResolvedValue({ response: {} }), + get: jest.fn().mockResolvedValue({ data: {} }), + interceptors: { + request: { + use: jest.fn(), + }, + response: { + use: jest.fn(), + }, + } + }), defaults: { transformRequest: [], transformResponse: [],