From a1a9b91a1e5d40f119aa97043f0c8a15c8f2c329 Mon Sep 17 00:00:00 2001 From: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> Date: Sat, 5 Oct 2024 11:57:01 +0530 Subject: [PATCH] fix(ui): MSAL popup auth issue (#18125) (cherry picked from commit e44c8d4f3143831c0aa95fe31d89f93b7b0da638) --- .../src/main/resources/ui/package.json | 4 +- .../AppAuthenticators/MsalAuthenticator.tsx | 77 ++++++++++--------- .../Auth/AuthProviders/AuthProvider.tsx | 15 ++-- .../ui/src/utils/AuthProvider.util.ts | 25 ++++++ .../src/main/resources/ui/yarn.lock | 26 +++---- 5 files changed, 89 insertions(+), 58 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/package.json b/openmetadata-ui/src/main/resources/ui/package.json index 6b2308937c4..939f47d0875 100644 --- a/openmetadata-ui/src/main/resources/ui/package.json +++ b/openmetadata-ui/src/main/resources/ui/package.json @@ -45,8 +45,8 @@ "@ant-design/icons": "^4.7.0", "@apidevtools/json-schema-ref-parser": "^9.0.9", "@auth0/auth0-react": "^1.9.0", - "@azure/msal-browser": "^3.10.0", - "@azure/msal-react": "^2.0.12", + "@azure/msal-browser": "^3.25.0", + "@azure/msal-react": "^2.1.1", "@dagrejs/dagre": "^1.1.2", "@deuex-solutions/react-tour": "^1.2.6", "@fontsource/poppins": "^5.0.0", diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/MsalAuthenticator.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/MsalAuthenticator.tsx index d905d9956cc..dfc9efb7579 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/MsalAuthenticator.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/MsalAuthenticator.tsx @@ -12,27 +12,29 @@ */ import { - AuthenticationResult, InteractionRequiredAuthError, + InteractionStatus, } from '@azure/msal-browser'; import { useAccount, useMsal } from '@azure/msal-react'; import { AxiosError } from 'axios'; -import { get } from 'lodash'; import React, { forwardRef, Fragment, ReactNode, + useEffect, useImperativeHandle, } from 'react'; import { toast } from 'react-toastify'; -import { useApplicationStore } from '../../../hooks/useApplicationStore'; -import { msalLoginRequest } from '../../../utils/AuthProvider.util'; +import { + msalLoginRequest, + parseMSALResponse, +} from '../../../utils/AuthProvider.util'; import { getPopupSettingLink } from '../../../utils/BrowserUtils'; import { Transi18next } from '../../../utils/CommonUtils'; +import Loader from '../../common/Loader/Loader'; import { AuthenticatorRef, OidcUser, - UserProfile, } from '../AuthProviders/AuthProvider.interface'; interface Props { @@ -47,36 +49,10 @@ const MsalAuthenticator = forwardRef( { children, onLoginSuccess, onLogoutSuccess, onLoginFailure }: Props, ref ) => { - const { setOidcToken } = useApplicationStore(); - const { instance, accounts } = useMsal(); + const { instance, accounts, inProgress } = useMsal(); const account = useAccount(accounts[0] || {}); - const parseResponse = (response: AuthenticationResult): OidcUser => { - // Call your API with the access token and return the data you need to save in state - const { idToken, scopes, account } = response; - - const user = { - id_token: idToken, - scope: scopes.join(), - profile: { - email: get(account, 'idTokenClaims.email', ''), - name: account?.name || '', - picture: '', - preferred_username: get( - account, - 'idTokenClaims.preferred_username', - '' - ), - sub: get(account, 'idTokenClaims.sub', ''), - } as UserProfile, - }; - - setOidcToken(idToken); - - return user; - }; - - const handleOnLogoutSuccess = () => { + const handleOnLogoutSuccess = async () => { for (const key in localStorage) { if (key.includes('-login.windows.net-') || key.startsWith('msal.')) { localStorage.removeItem(key); @@ -87,9 +63,8 @@ const MsalAuthenticator = forwardRef( const login = async () => { try { - const response = await instance.loginPopup(msalLoginRequest); - - onLoginSuccess(parseResponse(response)); + // Use login with redirect to avoid popup issue with maximized browser window + await instance.loginRedirect(); } catch (error) { onLoginFailure(error as AxiosError); } @@ -109,7 +84,7 @@ const MsalAuthenticator = forwardRef( try { const response = await instance.ssoSilent(tokenRequest); - return parseResponse(response); + return parseMSALResponse(response); } catch (error) { if ( error instanceof InteractionRequiredAuthError && @@ -138,7 +113,7 @@ const MsalAuthenticator = forwardRef( throw e; }); - return parseResponse(response); + return parseMSALResponse(response); } else { // eslint-disable-next-line no-console console.error(error); @@ -160,6 +135,32 @@ const MsalAuthenticator = forwardRef( renewIdToken: renewIdToken, })); + // Need to capture redirect and parse ID token + // Call login success callback + const handleRedirect = async () => { + try { + const response = await instance.handleRedirectPromise(); + + if (response) { + const user = parseMSALResponse(response); + + onLoginSuccess(user); + } + } catch (error) { + onLoginFailure(error as AxiosError); + } + }; + + // To add redirect callback + useEffect(() => { + instance && handleRedirect(); + }, [instance]); + + // Show loader until the interaction is completed + if (inProgress !== InteractionStatus.None) { + return ; + } + return {children}; } ); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx index 2a74a5afd5e..ac12d2dec34 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx @@ -459,16 +459,21 @@ export const AuthProvider = ({ } }, [location.pathname, storeRedirectPath]); - const updateAuthInstance = (configJson: AuthenticationConfiguration) => { + const updateAuthInstance = async ( + configJson: AuthenticationConfiguration + ) => { const { provider, ...otherConfigs } = configJson; switch (provider) { case AuthProviderEnum.Azure: { - setMsalInstance( - new PublicClientApplication( - otherConfigs as unknown as Configuration - ) + const instance = new PublicClientApplication( + otherConfigs as unknown as Configuration ); + + // Need to initialize the instance before setting it + await instance.initialize(); + + setMsalInstance(instance); } break; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.ts b/openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.ts index d8d84242f13..892b6150f1d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/AuthProvider.util.ts @@ -12,6 +12,7 @@ */ import { + AuthenticationResult, BrowserCacheLocation, Configuration, PopupRequest, @@ -32,6 +33,7 @@ import { ClientType, } from '../generated/configuration/authenticationConfiguration'; import { AuthProvider } from '../generated/settings/settings'; +import { useApplicationStore } from '../hooks/useApplicationStore'; import { isDev } from './EnvironmentUtils'; const cookieStorage = new CookieStorage(); @@ -412,3 +414,26 @@ export const prepareUserProfileFromClaims = ({ return newUser; }; + +// Responsible for parsing the response from MSAL AuthenticationResult +export const parseMSALResponse = (response: AuthenticationResult): OidcUser => { + // Call your API with the access token and return the data you need to save in state + const { idToken, scopes, account } = response; + const { setOidcToken } = useApplicationStore.getState(); + + const user = { + id_token: idToken, + scope: scopes.join(), + profile: { + email: get(account, 'idTokenClaims.email', ''), + name: account?.name ?? '', + picture: '', + preferred_username: get(account, 'idTokenClaims.preferred_username', ''), + sub: get(account, 'idTokenClaims.sub', ''), + } as UserProfile, + }; + + setOidcToken(idToken); + + return user; +}; diff --git a/openmetadata-ui/src/main/resources/ui/yarn.lock b/openmetadata-ui/src/main/resources/ui/yarn.lock index 186cf8310db..37fda356141 100644 --- a/openmetadata-ui/src/main/resources/ui/yarn.lock +++ b/openmetadata-ui/src/main/resources/ui/yarn.lock @@ -160,22 +160,22 @@ promise-polyfill "^8.2.1" unfetch "^4.2.0" -"@azure/msal-browser@^3.10.0": - version "3.10.0" - resolved "https://registry.yarnpkg.com/@azure/msal-browser/-/msal-browser-3.10.0.tgz#8925659e8d1a4bd21e389cca4683eb52658c778e" - integrity sha512-mnmi8dCXVNZI+AGRq0jKQ3YiodlIC4W9npr6FCB9WN6NQT+6rq+cIlxgUb//BjLyzKsnYo+i4LROGeMyU+6v1A== +"@azure/msal-browser@^3.25.0": + version "3.25.0" + resolved "https://registry.yarnpkg.com/@azure/msal-browser/-/msal-browser-3.25.0.tgz#7ce0949977bc9e0c58319f7090c44fe5537104d4" + integrity sha512-a0Y7pmSy8SC1s9bvwr+REvyAA1nQcITlZvkElM2gNUPYFTTNUTEdcpg73TmawNucyMdZ9xb/GFcuhrLOqYAzwg== dependencies: - "@azure/msal-common" "14.7.1" + "@azure/msal-common" "14.15.0" -"@azure/msal-common@14.7.1": - version "14.7.1" - resolved "https://registry.yarnpkg.com/@azure/msal-common/-/msal-common-14.7.1.tgz#b13443fbacc87ce2019a91e81a6582ea73847c75" - integrity sha512-v96btzjM7KrAu4NSEdOkhQSTGOuNUIIsUdB8wlyB9cdgl5KqEKnTonHUZ8+khvZ6Ap542FCErbnTyDWl8lZ2rA== +"@azure/msal-common@14.15.0": + version "14.15.0" + resolved "https://registry.yarnpkg.com/@azure/msal-common/-/msal-common-14.15.0.tgz#0e27ac0bb88fe100f4f8d1605b64d5c268636a55" + integrity sha512-ImAQHxmpMneJ/4S8BRFhjt1MZ3bppmpRPYYNyzeQPeFN288YKbb8TmmISQEbtfkQ1BPASvYZU5doIZOPBAqENQ== -"@azure/msal-react@^2.0.12": - version "2.0.12" - resolved "https://registry.yarnpkg.com/@azure/msal-react/-/msal-react-2.0.12.tgz#f33c84a57663307fcf3707ca17d9ce2d7934a048" - integrity sha512-23HKdajBWQ5SSzcwwFKHAWaOCpq4UCthoOBgKpab3UoHx0OuFMQiq6CrNBzBKtBFdyxCjadBGzWshRgl0Nvk1g== +"@azure/msal-react@^2.1.1": + version "2.1.1" + resolved "https://registry.yarnpkg.com/@azure/msal-react/-/msal-react-2.1.1.tgz#d61cd8698d3963ffff685377ece487d7ad5bc111" + integrity sha512-XOBgAR0fbkfUUkQZyhIlwAZiy8WIzNWxAAFacJgyL8LiE2Y5pgM9var9X0Jh7Oz/1Oy5lhTQDtCYWSMHmTZ84Q== "@babel/code-frame@7.12.11": version "7.12.11"