From 8934f1e361294e3d1b37bf90619ce54bb8429c76 Mon Sep 17 00:00:00 2001 From: Vivek Ratnavel Subramanian Date: Mon, 16 May 2022 17:36:53 -0700 Subject: [PATCH] Security: Fix Azure SSO and support refresh tokens (#4988) --- .../authenticators/MsalAuthenticator.test.tsx | 49 +++++++++++ .../authenticators/MsalAuthenticator.tsx | 83 +++++++++---------- 2 files changed, 87 insertions(+), 45 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.test.tsx diff --git a/openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.test.tsx b/openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.test.tsx new file mode 100644 index 00000000000..f86fb33e412 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.test.tsx @@ -0,0 +1,49 @@ +/* + * Copyright 2021 Collate + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import MsalAuthenticator from './MsalAuthenticator'; + +jest.mock('../auth-provider/AuthProvider', () => { + return { + useAuthContext: jest.fn(() => ({ + authConfig: {}, + setIsAuthenticated: jest.fn(), + onLogoutHandler: jest.fn(), + })), + }; +}); + +describe('Test MsalAuthenticator component', () => { + it('Checks if the MsalAuthenticator renders', async () => { + const onLogoutMock = jest.fn(); + const onLoginMock = jest.fn(); + const authenticatorRef = null; + render( + +

+ , + { + wrapper: MemoryRouter, + } + ); + const children = screen.getByTestId('children'); + + expect(children).toBeInTheDocument(); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.tsx b/openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.tsx index 5992a845974..45a973709fe 100644 --- a/openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/authentication/authenticators/MsalAuthenticator.tsx @@ -37,8 +37,6 @@ interface Props { onLogoutSuccess: () => void; } -export type Ref = ReactNode | HTMLElement | string; - const MsalAuthenticator = forwardRef( ({ children, onLoginSuccess, onLogoutSuccess }: Props, ref) => { const { setIsAuthenticated, setLoadingIndicator } = useAuthContext(); @@ -46,7 +44,7 @@ const MsalAuthenticator = forwardRef( const isMsalAuthenticated = useIsAuthenticated(); const account = useAccount(accounts[0] || {}); - const handleOnLogoutSucess = () => { + const handleOnLogoutSuccess = () => { for (const key in localStorage) { if (key.includes('-login.windows.net-') || key.startsWith('msal.')) { localStorage.removeItem(key); @@ -55,52 +53,27 @@ const MsalAuthenticator = forwardRef( onLogoutSuccess(); }; - const login = (loginType = 'popup') => { + const login = () => { setLoadingIndicator(true); - if (loginType === 'popup') { - instance - .loginPopup(msalLoginRequest) - .finally(() => setLoadingIndicator(false)); - } else if (loginType === 'redirect') { - instance - .loginRedirect(msalLoginRequest) - .finally(() => setLoadingIndicator(false)); - } + instance + .loginPopup(msalLoginRequest) + .finally(() => setLoadingIndicator(false)); }; const logout = () => { - // TODO: Uncomment if need to logout from browser - // if (logoutType === 'popup') { - // instance - // .logoutPopup({ - // mainWindowRedirectUri: ROUTES.HOME, - // }) - // .then(() => { - // handleOnLogoutSucess(); - // }); - // } else if (logoutType === 'redirect') { - // instance.logoutRedirect().then(() => { - // handleOnLogoutSucess(); - // }); - // } setLoadingIndicator(false); - handleOnLogoutSucess(); + handleOnLogoutSuccess(); }; - useEffect(() => { - const oidcUserToken = localStorage.getItem(oidcTokenKey); - if ( - !oidcUserToken && - inProgress === InteractionStatus.None && - (accounts.length > 0 || account?.idTokenClaims) - ) { - const tokenRequest = { - account: account || accounts[0], // This is an example - Select account based on your app's requirements - scopes: msalLoginRequest.scopes, - }; + const fetchIdToken = (isRenewal = false): Promise => { + const tokenRequest = { + account: account || accounts[0], // This is an example - Select account based on your app's requirements + scopes: msalLoginRequest.scopes, + }; - // Acquire an access token + return new Promise((resolve, reject) => { + // Acquire access token instance - .acquireTokenSilent(tokenRequest) + .ssoSilent(tokenRequest) .then((response) => { // Call your API with the access token and return the data you need to save in state const { idToken, scopes, account } = response; @@ -116,16 +89,36 @@ const MsalAuthenticator = forwardRef( }; setIsAuthenticated(isMsalAuthenticated); localStorage.setItem(oidcTokenKey, idToken); - onLoginSuccess(user); + if (!isRenewal) { + onLoginSuccess(user); + } + + resolve(''); }) .catch(async (e) => { // Catch interaction_required errors and call interactive method to resolve if (e instanceof InteractionRequiredAuthError) { await instance.acquireTokenRedirect(tokenRequest); - } - throw e; + resolve(''); + } else { + // eslint-disable-next-line no-console + console.error(e); + + reject(e); + } }); + }); + }; + + useEffect(() => { + const oidcUserToken = localStorage.getItem(oidcTokenKey); + if ( + !oidcUserToken && + inProgress === InteractionStatus.None && + (accounts.length > 0 || account?.idTokenClaims) + ) { + fetchIdToken(); } }, [inProgress, accounts, instance, account]); @@ -137,7 +130,7 @@ const MsalAuthenticator = forwardRef( logout(); }, renewIdToken() { - return Promise.resolve(''); + return fetchIdToken(true); }, }));