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

Handle 401 unauthorized error in the UI #3640

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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@
highly-available when we have multiple Prometheus instances
(PR[#3573](https://github.com/scality/metalk8s/pull/3573))

- Handle 401 unauthorized error in MetalK8s UI
(PR[#3640](https://github.com/scality/metalk8s/pull/3640))
## Bug fixes

- [#3601](https://github.com/scality/metalk8s/issues/3601) - Marks
Expand Down
4 changes: 2 additions & 2 deletions shell-ui/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion shell-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"webpack-dev-server": "^3.11.2"
},
"dependencies": {
"@scality/core-ui": "github:scality/core-ui.git#v0.24.2",
"@scality/core-ui": "github:scality/core-ui.git#v0.25.0",
"@scality/module-federation": "github:scality/module-federation.git#1.0.0",
"oidc-client": "^1.11.3",
"oidc-react": "^1.1.5",
Expand Down
24 changes: 12 additions & 12 deletions shell-ui/src/FederatedApp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,7 @@ import {
useConfigRetriever,
useDiscoveredViews,
} from './initFederation/ConfigurationProviders';
import {
Route,
Switch,
Router,
} from 'react-router-dom';
import { Route, Switch, Router } from 'react-router-dom';
import {
ShellConfigProvider,
useShellConfig,
Expand Down Expand Up @@ -203,13 +199,17 @@ function WithInitFederationProviders({ children }: { children: Node }) {
export default function App(): Node {
return (
<ScrollbarWrapper>
<QueryClientProvider client={queryClient} contextSharing={true}>
<ShellConfigProvider shellConfigUrl={'/shell/config.json'}>
<WithInitFederationProviders>
<InternalApp />
</WithInitFederationProviders>
</ShellConfigProvider>
</QueryClientProvider>
<div
style={{ height: '100vh', display: 'flex', flexDirection: 'column' }}
>
<QueryClientProvider client={queryClient} contextSharing={true}>
<ShellConfigProvider shellConfigUrl={'/shell/config.json'}>
<WithInitFederationProviders>
<InternalApp />
</WithInitFederationProviders>
</ShellConfigProvider>
</QueryClientProvider>
</div>
</ScrollbarWrapper>
);
}
4 changes: 2 additions & 2 deletions ui/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"@fortawesome/free-solid-svg-icons": "^5.15.3",
"@fortawesome/react-fontawesome": "^0.1.14",
"@kubernetes/client-node": "github:scality/kubernetes-client-javascript.git#browser-0.10.2-63-g579d066",
"@scality/core-ui": "github:scality/core-ui.git#v0.24.2",
"@scality/core-ui": "github:scality/core-ui.git#v0.25.0",
"@scality/module-federation": "github:scality/module-federation.git#1.0.0",
"axios": "^0.21.1",
"formik": "2.2.5",
Expand Down
10 changes: 9 additions & 1 deletion ui/src/FederableApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,21 @@ import { useTypedSelector } from './hooks';
import { setHistory as setReduxHistory } from './ducks/history';
import { setApiConfigAction } from './ducks/config';
import { initToggleSideBarAction } from './ducks/app/layout';
import { authErrorAction } from './ducks/app/authError';
import { AuthError } from './services/errorhandler';

const composeEnhancers =
typeof window === 'object' && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__
? window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({})
: compose;

const sagaMiddleware = createSagaMiddleware();
const sagaMiddleware = createSagaMiddleware({
onError: (error) => {
if (error instanceof AuthError) {
store.dispatch(authErrorAction());
}
},
});
const enhancer = composeEnhancers(applyMiddleware(sagaMiddleware));
export const store = createStore(reducer, enhancer);

Expand Down
17 changes: 15 additions & 2 deletions ui/src/containers/PrivateRoute.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useMemo } from 'react';
import { Route } from 'react-router-dom';
import { ErrorPage500, ErrorPageAuth } from '@scality/core-ui';
import { useHistory } from 'react-router';
import { ErrorPage500, ErrorPageAuth, ErrorPage401 } from '@scality/core-ui';
import { useTypedSelector } from '../hooks';
import { ComponentWithFederatedImports } from '@scality/module-federation';
import { useDispatch } from 'react-redux';
Expand All @@ -13,16 +14,28 @@ const InternalPrivateRoute = ({
...rest
}) => {
const { language, api } = useTypedSelector((state) => state.config);
const { isAuthError } = useTypedSelector((state) => state.app.authError);
const url_support = api?.url_support;
const { userData } = moduleExports['./auth/AuthProvider'].useAuth();
const dispatch = useDispatch();
const history = useHistory();

useMemo(() => {
dispatch(updateAPIConfigAction(userData));
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [!userData]);

if (userData.token && userData.username) {
if (isAuthError) {
return (
<ErrorPage401
supportLink={url_support}
locale={language}
onReturnHomeClick={() => {
history.push('/');
}}
/>
);
} else if (userData.token && userData.username) {
return <Route {...rest} component={component} />;
} else {
return (
Expand Down
29 changes: 29 additions & 0 deletions ui/src/ducks/app/authError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//@flow
export type AuthErrorState = {
isAuthError: boolean,
};

type AuthErrorAction = {
type: 'AUTH_ERROR',
};

const defaultState: AuthErrorState = {
isAuthError: false,
};

export default function reducer(
state: AuthErrorState = defaultState,
action: AuthErrorAction,
) {
switch (action.type) {
case 'AUTH_ERROR': {
return { ...state, isAuthError: true };
}
default:
return { ...state };
}
}
Comment on lines +14 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not familiar with the way it work but what i see from the code, there is no way to remove the isAuthError. And we check in privateRoute.js if isAuthError is true. Are we sure that if the user relogin the isAuthError is reset ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @gaudiauj ,
It's a very good concern! but thanks to the authentication workflow, the isAuthError should be reset if we re-login again.

Once the user triggers logout action, the page will be redirected to the IDP, either dex or Keycloak.

Given the page is fully reloaded, the redux store will be re-initiated as well.


export function authErrorAction() {
return { type: 'AUTH_ERROR' };
}
4 changes: 4 additions & 0 deletions ui/src/ducks/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import pods from './app/pods';
import type { PodsState } from './app/pods';
import volumes from './app/volumes';
import type { VolumesState } from './app/volumes';
import authError from './app/authError';
import type { AuthErrorState } from './app/authError';
import login from './login';
import type { LoginState } from './login';
import layout from './app/layout';
Expand All @@ -31,6 +33,7 @@ const rootReducer = combineReducers({
salt,
monitoring,
volumes,
authError,
}),
oidc: oidcReducer,
history: historyReducer,
Expand All @@ -49,6 +52,7 @@ export type RootState = {
layout: LayoutState,
salt: SaltState,
monitoring: MonitoringState,
authError: AuthErrorState,
},
};

Expand Down
13 changes: 13 additions & 0 deletions ui/src/services/errorhandler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export class AuthError extends Error {}

export function handleUnAuthorizedError({ error }) {
if (
error?.response?.statusCode === 401 ||
error?.response?.statusCode === 403 ||
error?.response?.status === 401 ||
error?.response?.status === 403
) {
throw new AuthError();
}
return { error };
}
23 changes: 12 additions & 11 deletions ui/src/services/k8s/core.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import { handleUnAuthorizedError } from '../errorhandler';
import { coreV1, appsV1 } from './api';

export async function getNodes() {
try {
return await coreV1.listNode();
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

export async function getPods() {
try {
return await coreV1.listPodForAllNamespaces();
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

Expand All @@ -25,15 +26,15 @@ export async function getKubeSystemNamespace() {
'metadata.name=kube-system',
);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

export async function createNode(payload) {
try {
return await coreV1.createNode(payload);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

Expand All @@ -57,7 +58,7 @@ export async function listNamespaces({
options,
);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

Expand All @@ -72,7 +73,7 @@ export async function queryPodInNamespace(namespace, podLabel) {
`app=${podLabel}`,
);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

Expand All @@ -86,7 +87,7 @@ export async function createNamespacedConfigMap(name, namespace, restProps) {
try {
return await coreV1.createNamespacedConfigMap(namespace, body);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

Expand Down Expand Up @@ -115,30 +116,30 @@ export async function patchNamespacedConfigMap(
{ headers: { 'Content-Type': cTypeHeader } },
);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

export async function getNamespacedDeployment(name, namespace) {
try {
return await appsV1.readNamespacedDeployment(name, namespace);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

export async function readNode(name) {
try {
return await coreV1.readNode(name);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

export async function readNamespacedConfigMap(nameConfigMap, namespace) {
try {
return await coreV1.readNamespacedConfigMap(nameConfigMap, namespace);
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}
7 changes: 4 additions & 3 deletions ui/src/services/k8s/volumes.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@flow
import { handleUnAuthorizedError } from '../errorhandler';
import { coreV1, storage } from './api';

export async function getPersistentVolumes() {
Expand All @@ -8,7 +9,7 @@ export async function getPersistentVolumes() {
try {
return await coreV1.listPersistentVolume();
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

Expand All @@ -19,7 +20,7 @@ export async function getStorageClass() {
try {
return await storage.listStorageClass();
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}

Expand All @@ -30,6 +31,6 @@ export async function getPersistentVolumeClaims() {
try {
return await coreV1.listPersistentVolumeClaimForAllNamespaces();
} catch (error) {
return { error };
return handleUnAuthorizedError({ error });
}
}
Loading