diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx index c1c20cfbfc7..8da33d6e94d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx @@ -19,7 +19,6 @@ import AppContainer from '../../components/AppContainer/AppContainer'; import { ROUTES } from '../../constants/constants'; import { CustomEventTypes } from '../../generated/analytics/webAnalyticEventData'; import { useApplicationStore } from '../../hooks/useApplicationStore'; -import Loader from '../common/Loader/Loader'; import { UnAuthenticatedAppRouter } from './UnAuthenticatedAppRouter'; import withSuspenseFallback from './withSuspenseFallback'; @@ -33,7 +32,7 @@ const AppRouter = () => { // web analytics instance const analytics = useAnalytics(); - const { isAuthenticated, loading } = useApplicationStore(); + const { isAuthenticated } = useApplicationStore(); useEffect(() => { const { pathname } = location; @@ -71,10 +70,6 @@ const AppRouter = () => { return () => targetNode.removeEventListener('click', handleClickEvent); }, [handleClickEvent]); - if (loading) { - return ; - } - return ( {isAuthenticated ? : } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/OidcAuthenticator.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/OidcAuthenticator.tsx index 5f312a8ff4f..879f354ed53 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/OidcAuthenticator.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/OidcAuthenticator.tsx @@ -65,12 +65,10 @@ const OidcAuthenticator = forwardRef( ref ) => { const { - loading, isAuthenticated, setIsAuthenticated, isSigningIn, setIsSigningIn, - setLoadingIndicator, updateAxiosInterceptors, currentUser, newUser, @@ -87,7 +85,6 @@ const OidcAuthenticator = forwardRef( }; const logout = () => { - setLoadingIndicator(true); userManager.removeUser(); onLogoutSuccess(); }; @@ -176,7 +173,7 @@ const OidcAuthenticator = forwardRef( )} - {loading && isSigningIn && } + {isSigningIn && } ); } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.interface.ts index a967f22f68e..ee60fbdefc4 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.interface.ts @@ -56,11 +56,9 @@ export interface IAuthContext { setIsSigningIn: (authenticated: boolean) => void; onLoginHandler: () => void; onLogoutHandler: () => void; - loading: boolean; currentUser?: User; newUser?: UserProfile; updateNewUser: (user: UserProfile) => void; - setLoadingIndicator: (authenticated: boolean) => void; handleSuccessfulLogin: (user: OidcUser) => void; handleFailedLogin: () => void; updateAxiosInterceptors: () => void; 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 ddc26b50c5f..16884f766e3 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 @@ -13,7 +13,11 @@ import { removeSession } from '@analytics/session-utils'; import { Auth0Provider } from '@auth0/auth0-react'; -import { Configuration } from '@azure/msal-browser'; +import { + Configuration, + IPublicClientApplication, + PublicClientApplication, +} from '@azure/msal-browser'; import { MsalProvider } from '@azure/msal-react'; import { AxiosError, @@ -62,8 +66,6 @@ import { getUrlPathnameExpiry, getUserManagerConfig, isProtectedRoute, - msalInstance, - setMsalInstance, } from '../../../utils/AuthProvider.util'; import { escapeESReservedCharacters } from '../../../utils/StringsUtils'; import { showErrorToast } from '../../../utils/ToastUtils'; @@ -107,7 +109,6 @@ export const AuthProvider = ({ setCurrentUser, updateNewUser: setNewUserProfile, setIsAuthenticated: setIsUserAuthenticated, - setLoadingIndicator: setLoading, authConfig, setAuthConfig, setAuthorizerConfig, @@ -117,18 +118,18 @@ export const AuthProvider = ({ removeOidcToken, getOidcToken, getRefreshToken, - urlPathName, - setUrlPathName, } = useApplicationStore(); const { activeDomain } = useDomainStore(); const location = useLocation(); const history = useHistory(); const { t } = useTranslation(); - const [timeoutId, setTimeoutId] = useState(); - const authenticatorRef = useRef(null); - let silentSignInRetries = 0; + const [timeoutId, setTimeoutId] = useState(); + const [loading, setLoading] = useState(false); + const [msalInstance, setMsalInstance] = useState(); + + const authenticatorRef = useRef(null); const userConfig = useMemo( () => (authConfig ? getUserManagerConfig(authConfig) : {}), @@ -173,28 +174,24 @@ export const AuthProvider = ({ } }; - const setLoadingIndicator = (value: boolean) => { - setLoading(value); - }; - /** * Stores redirect URL for successful login */ - const storeRedirectPath = useCallback( - (path?: string) => { - cookieStorage.setItem(REDIRECT_PATHNAME, path ?? urlPathName, { - expires: getUrlPathnameExpiry(), - path: '/', - }); - }, - [urlPathName] - ); + const storeRedirectPath = useCallback((path?: string) => { + if (!path) { + return; + } + cookieStorage.setItem(REDIRECT_PATHNAME, path, { + expires: getUrlPathnameExpiry(), + path: '/', + }); + }, []); const resetUserDetails = (forceLogout = false) => { setCurrentUser({} as User); removeOidcToken(); setIsUserAuthenticated(false); - setLoadingIndicator(false); + setLoading(false); clearTimeout(timeoutId); if (forceLogout) { onLogoutHandler(); @@ -203,55 +200,53 @@ export const AuthProvider = ({ } }; - const getLoggedInUserDetails = () => { + const getLoggedInUserDetails = async () => { setLoading(true); - getLoggedInUser({ fields: userAPIQueryFields }) - .then((res) => { - if (res) { - setCurrentUser(res); - setIsUserAuthenticated(true); - } else { - resetUserDetails(); - } - }) - .catch((err: AxiosError) => { + try { + const res = await getLoggedInUser({ fields: userAPIQueryFields }); + if (res) { + setCurrentUser(res); + setIsUserAuthenticated(true); + } else { resetUserDetails(); - if (err.response?.status !== 404) { - showErrorToast( - err, - t('server.entity-fetch-error', { - entity: t('label.logged-in-user-lowercase'), - }) - ); - } - }) - .finally(() => { - setLoading(false); - }); + } + } catch (error) { + const err = error as AxiosError; + resetUserDetails(); + if (err.response?.status !== 404) { + showErrorToast( + err, + t('server.entity-fetch-error', { + entity: t('label.logged-in-user-lowercase'), + }) + ); + } + } finally { + setLoading(false); + } }; - const getUpdatedUser = (updatedData: User, existingData: User) => { + const getUpdatedUser = async (updatedData: User, existingData: User) => { // PUT method for users api only excepts below fields const updatedUserData = { ...existingData, ...updatedData }; const jsonPatch = compare(existingData, updatedUserData); - updateUserDetail(existingData.id, jsonPatch) - .then((res) => { - if (res) { - setCurrentUser({ ...existingData, ...res }); - } else { - throw t('server.unexpected-response'); - } - }) - .catch((error: AxiosError) => { - setCurrentUser(existingData); - showErrorToast( - error, - t('server.entity-updating-error', { - entity: t('label.admin-profile'), - }) - ); - }); + try { + const res = await updateUserDetail(existingData.id, jsonPatch); + if (res) { + setCurrentUser({ ...existingData, ...res }); + } else { + throw t('server.unexpected-response'); + } + } catch (error) { + setCurrentUser(existingData); + showErrorToast( + error as AxiosError, + t('server.entity-updating-error', { + entity: t('label.admin-profile'), + }) + ); + } }; /** @@ -277,34 +272,40 @@ export const AuthProvider = ({ /** * This method will try to signIn silently when token is about to expire - * It will try for max 3 times if it's not succeed then it will proceed for logout + * if it's not succeed then it will proceed for logout */ - const trySilentSignIn = () => { + const trySilentSignIn = async (forceLogout?: boolean) => { const pathName = location.pathname; // Do not try silent sign in for SignIn or SignUp route - if ([ROUTES.SIGNIN, ROUTES.SIGNUP].indexOf(pathName) === -1) { + if ([ROUTES.SIGNIN, ROUTES.SIGNUP].includes(pathName)) { + return; + } + setLoading(true); + try { // Try to renew token - silentSignInRetries < 3 - ? renewIdToken() - .then(() => { - silentSignInRetries = 0; - // eslint-disable-next-line @typescript-eslint/no-use-before-define - startTokenExpiryTimer(); - }) - .catch((err) => { - if (err.message.includes('Frame window timed out')) { - silentSignInRetries = 0; - // eslint-disable-next-line @typescript-eslint/no-use-before-define - startTokenExpiryTimer(); + const newToken = await renewIdToken(); - return; - } - // eslint-disable-next-line no-console - console.error('Error while attempting for silent signIn. ', err); - silentSignInRetries += 1; - trySilentSignIn(); - }) - : resetUserDetails(); // Logout if we reaches max silent signIn limit; + if (newToken) { + // Start expiry timer on successful silent signIn + // eslint-disable-next-line @typescript-eslint/no-use-before-define + startTokenExpiryTimer(); + } else { + // reset user details if silent signIn fails + resetUserDetails(forceLogout); + } + } catch (error) { + const err = error as AxiosError; + if (err.message.includes('Frame window timed out')) { + // Start expiry timer if silent signIn is timed out + // eslint-disable-next-line @typescript-eslint/no-use-before-define + startTokenExpiryTimer(); + + return; + } + // reset user details if silent signIn fails + resetUserDetails(forceLogout); + } finally { + setLoading(false); } }; @@ -356,55 +357,67 @@ export const AuthProvider = ({ history.push(ROUTES.SIGNIN); }; - const handleSuccessfulLogin = (user: OidcUser) => { + const handleSuccessfulLogin = async (user: OidcUser) => { setLoading(true); setIsUserAuthenticated(true); const fields = authConfig?.provider === AuthProviderEnum.Basic ? userAPIQueryFields + ',' + isEmailVerifyField : userAPIQueryFields; - getLoggedInUser({ fields }) - .then((res) => { - if (res) { - const updatedUserData = getUserDataFromOidc(res, user); - if (!matchUserDetails(res, updatedUserData, ['email'])) { - getUpdatedUser(updatedUserData, res); - } else { - setCurrentUser(res); - } - - handledVerifiedUser(); - // Start expiry timer on successful login - startTokenExpiryTimer(); - } - }) - .catch((err) => { - if (err && err.response && err.response.status === 404) { - setNewUserProfile(user.profile); - setCurrentUser({} as User); - setIsSigningIn(true); - history.push(ROUTES.SIGNUP); + try { + const res = await getLoggedInUser({ fields }); + if (res) { + const updatedUserData = getUserDataFromOidc(res, user); + if (!matchUserDetails(res, updatedUserData, ['email'])) { + getUpdatedUser(updatedUserData, res); } else { - // eslint-disable-next-line no-console - console.error(err); - history.push(ROUTES.SIGNIN); + setCurrentUser(res); } - }) - .finally(() => { - setLoading(false); - }); + + handledVerifiedUser(); + // Start expiry timer on successful login + startTokenExpiryTimer(); + } + } catch (error) { + const err = error as AxiosError; + if (err && err.response && err.response.status === 404) { + setNewUserProfile(user.profile); + setCurrentUser({} as User); + setIsSigningIn(true); + history.push(ROUTES.SIGNUP); + } else { + // eslint-disable-next-line no-console + console.error(err); + history.push(ROUTES.SIGNIN); + } + } finally { + setLoading(false); + } }; const handleSuccessfulLogout = () => { resetUserDetails(); }; + /** + * Stores redirect URL for successful login + */ + const handleStoreProtectedRedirectPath = useCallback(() => { + if (isProtectedRoute(location.pathname)) { + storeRedirectPath(location.pathname); + } + }, [location.pathname, storeRedirectPath]); + const updateAuthInstance = (configJson: AuthenticationConfiguration) => { const { provider, ...otherConfigs } = configJson; switch (provider) { case AuthProviderEnum.Azure: { - setMsalInstance(otherConfigs as unknown as Configuration); + setMsalInstance( + new PublicClientApplication( + otherConfigs as unknown as Configuration + ) + ); } break; @@ -498,8 +511,8 @@ export const AuthProvider = ({ if (error.response) { const { status } = error.response; if (status === ClientErrors.UNAUTHORIZED) { - storeRedirectPath(); - resetUserDetails(true); + handleStoreProtectedRedirectPath(); + trySilentSignIn(true); } } @@ -524,9 +537,7 @@ export const AuthProvider = ({ setAuthorizerConfig(authorizerConfig); updateAuthInstance(configJson); if (!getOidcToken()) { - if (isProtectedRoute(location.pathname)) { - storeRedirectPath(location.pathname); - } + handleStoreProtectedRedirectPath(); setLoading(false); } else { if (location.pathname !== ROUTES.AUTH_CALLBACK) { @@ -558,10 +569,13 @@ export const AuthProvider = ({ }; const getProtectedApp = () => { + // Show loader if application in loading state + const childElement = loading ? : children; + if (clientType === ClientType.Confidential) { return ( - {children} + {childElement} ); } @@ -573,7 +587,7 @@ export const AuthProvider = ({ onLoginFailure={handleFailedLogin} onLoginSuccess={handleSuccessfulLogin}> - {children} + {childElement} ); @@ -589,7 +603,7 @@ export const AuthProvider = ({ - {children} + {childElement} ); @@ -599,7 +613,7 @@ export const AuthProvider = ({ - {children} + {childElement} ); } @@ -609,7 +623,7 @@ export const AuthProvider = ({ - {children} + {childElement} ); @@ -617,7 +631,7 @@ export const AuthProvider = ({ case AuthProviderEnum.Google: case AuthProviderEnum.CustomOidc: case AuthProviderEnum.AwsCognito: { - return authConfig ? ( + return ( - {children} + {childElement} - ) : ( - ); } case AuthProviderEnum.Azure: { @@ -639,7 +651,7 @@ export const AuthProvider = ({ onLoginFailure={handleFailedLogin} onLoginSuccess={handleSuccessfulLogin} onLogoutSuccess={handleSuccessfulLogout}> - {children} + {childElement} ) : ( @@ -668,12 +680,6 @@ export const AuthProvider = ({ return cleanup; }, []); - useEffect(() => { - if (isProtectedRoute(location.pathname)) { - setUrlPathName(location.pathname); - } - }, [location.pathname]); - const isLoading = !authConfig || (authConfig.provider === AuthProviderEnum.Azure && !msalInstance); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx index 48a65f5c5da..fda2d9d9335 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx @@ -90,7 +90,6 @@ const BasicAuthProvider = ({ }: BasicAuthProps) => { const { t } = useTranslation(); const { - setLoadingIndicator, setRefreshToken, setOidcToken, getOidcToken, @@ -142,7 +141,6 @@ const BasicAuthProvider = ({ try { const isEmailAlreadyExists = await checkEmailInUse(request.email); if (!isEmailAlreadyExists) { - setLoadingIndicator(true); await basicAuthRegister(request); showSuccessToast( @@ -166,8 +164,6 @@ const BasicAuthProvider = ({ } else { showErrorToast(err as AxiosError, t('server.unexpected-response')); } - } finally { - setLoadingIndicator(false); } }; @@ -187,14 +183,10 @@ const BasicAuthProvider = ({ }; const handleResetPassword = async (payload: PasswordResetRequest) => { - setLoadingIndicator(true); - const response = await resetPassword(payload); if (response) { showSuccessToast(t('server.reset-password-success')); } - - setLoadingIndicator(false); }; const handleLogout = async () => { diff --git a/openmetadata-ui/src/main/resources/ui/src/context/PermissionProvider/PermissionProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/context/PermissionProvider/PermissionProvider.tsx index f751b787c54..c89f14012bc 100644 --- a/openmetadata-ui/src/main/resources/ui/src/context/PermissionProvider/PermissionProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/context/PermissionProvider/PermissionProvider.tsx @@ -31,7 +31,6 @@ import { getLoggedInUserPermissions, getResourcePermission, } from '../../rest/permissionAPI'; -import { getUrlPathnameExpiryAfterRoute } from '../../utils/AuthProvider.util'; import { getOperationPermissions, getUIPermission, @@ -79,10 +78,7 @@ const PermissionProvider: FC = ({ children }) => { const redirectToStoredPath = useCallback(() => { const urlPathname = cookieStorage.getItem(REDIRECT_PATHNAME); if (urlPathname) { - cookieStorage.setItem(REDIRECT_PATHNAME, urlPathname, { - expires: getUrlPathnameExpiryAfterRoute(), - path: '/', - }); + cookieStorage.removeItem(REDIRECT_PATHNAME); history.push(urlPathname); } }, [history]); diff --git a/openmetadata-ui/src/main/resources/ui/src/hooks/useApplicationStore.ts b/openmetadata-ui/src/main/resources/ui/src/hooks/useApplicationStore.ts index 9505715d82c..ed70ee83e39 100644 --- a/openmetadata-ui/src/main/resources/ui/src/hooks/useApplicationStore.ts +++ b/openmetadata-ui/src/main/resources/ui/src/hooks/useApplicationStore.ts @@ -41,11 +41,9 @@ export const useApplicationStore = create()( jwtPrincipalClaims: [], userProfilePics: {}, cachedEntityData: {}, - urlPathName: '', selectedPersona: {} as EntityReference, oidcIdToken: '', refreshTokenKey: '', - loading: false, theme: { ...DEFAULT_THEME }, setTheme: (theme: ApplicationStore['theme']) => { set({ theme }); @@ -66,11 +64,6 @@ export const useApplicationStore = create()( setApplicationConfig: (config: LogoConfiguration) => { set({ applicationConfig: config }); }, - - setUrlPathName: (urlPathName: string) => { - set({ urlPathName }); - }, - setCurrentUser: (user) => { set({ currentUser: user }); }, @@ -91,9 +84,6 @@ export const useApplicationStore = create()( setIsSigningIn: (signingIn: boolean) => { set({ isSigningIn: signingIn }); }, - setLoadingIndicator: (loading: boolean) => { - set({ loading }); - }, onLoginHandler: () => { // This is a placeholder function that will be replaced by the actual function diff --git a/openmetadata-ui/src/main/resources/ui/src/interface/store.interface.ts b/openmetadata-ui/src/main/resources/ui/src/interface/store.interface.ts index 771c4ed7247..f22787cc35e 100644 --- a/openmetadata-ui/src/main/resources/ui/src/interface/store.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/interface/store.interface.ts @@ -32,7 +32,7 @@ import { EntityReference } from '../generated/entity/type'; export interface HelperFunctions { onLoginHandler: () => void; onLogoutHandler: () => void; - handleSuccessfulLogin: (user: OidcUser) => void; + handleSuccessfulLogin: (user: OidcUser) => Promise; handleFailedLogin: () => void; updateAxiosInterceptors: () => void; } @@ -43,7 +43,6 @@ export interface ApplicationStore LoginConfiguration { userProfilePics: Record; cachedEntityData: Record; - urlPathName: string; selectedPersona: EntityReference; oidcIdToken: string; refreshTokenKey: string; @@ -55,7 +54,6 @@ export interface ApplicationStore searchCriteria: ExploreSearchIndex | ''; setSelectedPersona: (persona: EntityReference) => void; setApplicationConfig: (config: LogoConfiguration) => void; - setUrlPathName: (urlPathName: string) => void; setCurrentUser: (user: User) => void; setAuthConfig: (authConfig: AuthenticationConfigurationWithScope) => void; setAuthorizerConfig: (authorizerConfig: AuthorizerConfiguration) => void; 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 ad47c865338..103a06cae68 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 @@ -14,9 +14,7 @@ import { BrowserCacheLocation, Configuration, - IPublicClientApplication, PopupRequest, - PublicClientApplication, } from '@azure/msal-browser'; import jwtDecode, { JwtPayload } from 'jwt-decode'; import { first, isNil } from 'lodash'; @@ -34,8 +32,6 @@ import { import { AuthProvider } from '../generated/settings/settings'; import { isDev } from './EnvironmentUtils'; -export let msalInstance: IPublicClientApplication; - // 25s for server auth approch export const EXPIRY_THRESHOLD_MILLES = 25 * 1000; // 2 minutes for client auth approch @@ -214,10 +210,6 @@ export const getAuthConfig = ( return config as AuthenticationConfigurationWithScope; }; -export const setMsalInstance = (configs: Configuration) => { - msalInstance = new PublicClientApplication(configs); -}; - // Add here scopes for id token to be used at MS Identity Platform endpoints. export const msalLoginRequest: PopupRequest = { scopes: ['openid', 'profile', 'email', 'offline_access'], @@ -281,6 +273,7 @@ export const isProtectedRoute = (pathname: string) => { ROUTES.ACCOUNT_ACTIVATION, ROUTES.HOME, ROUTES.AUTH_CALLBACK, + ROUTES.NOT_FOUND, ].indexOf(pathname) === -1 ); }; @@ -293,10 +286,6 @@ export const getUrlPathnameExpiry = () => { return new Date(Date.now() + 60 * 60 * 1000); }; -export const getUrlPathnameExpiryAfterRoute = () => { - return new Date(Date.now() + 1000); -}; - /** * @exp expiry of token * @isExpired Whether token is already expired or not