diff --git a/openmetadata-ui/src/main/resources/ui/package.json b/openmetadata-ui/src/main/resources/ui/package.json index 90a182f6902..0c92f33024c 100644 --- a/openmetadata-ui/src/main/resources/ui/package.json +++ b/openmetadata-ui/src/main/resources/ui/package.json @@ -109,7 +109,6 @@ "react-antd-column-resize": "1.0.3", "react-awesome-query-builder": "5.1.2", "react-codemirror2": "^7.2.1", - "react-context-mutex": "^2.0.0", "react-dnd": "14.0.2", "react-dnd-html5-backend": "14.0.2", "react-dom": "^17.0.2", 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 8c68868e741..d905d9956cc 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 @@ -14,7 +14,6 @@ import { AuthenticationResult, InteractionRequiredAuthError, - InteractionStatus, } from '@azure/msal-browser'; import { useAccount, useMsal } from '@azure/msal-react'; import { AxiosError } from 'axios'; @@ -23,10 +22,8 @@ import React, { forwardRef, Fragment, ReactNode, - useEffect, useImperativeHandle, } from 'react'; -import { useMutex } from 'react-context-mutex'; import { toast } from 'react-toastify'; import { useApplicationStore } from '../../../hooks/useApplicationStore'; import { msalLoginRequest } from '../../../utils/AuthProvider.util'; @@ -50,34 +47,9 @@ const MsalAuthenticator = forwardRef( { children, onLoginSuccess, onLogoutSuccess, onLoginFailure }: Props, ref ) => { - const { setOidcToken, getOidcToken } = useApplicationStore(); - const { instance, accounts, inProgress } = useMsal(); + const { setOidcToken } = useApplicationStore(); + const { instance, accounts } = useMsal(); const account = useAccount(accounts[0] || {}); - const MutexRunner = useMutex(); - const mutex = new MutexRunner('fetchIdToken'); - - const handleOnLogoutSuccess = () => { - for (const key in localStorage) { - if (key.includes('-login.windows.net-') || key.startsWith('msal.')) { - localStorage.removeItem(key); - } - } - onLogoutSuccess(); - }; - - const login = () => { - try { - instance.loginPopup(msalLoginRequest); - } catch (error) { - // eslint-disable-next-line no-console - console.error(error); - onLoginFailure(error as AxiosError); - } - }; - - const logout = () => { - handleOnLogoutSuccess(); - }; const parseResponse = (response: AuthenticationResult): OidcUser => { // Call your API with the access token and return the data you need to save in state @@ -104,6 +76,29 @@ const MsalAuthenticator = forwardRef( return user; }; + const handleOnLogoutSuccess = () => { + for (const key in localStorage) { + if (key.includes('-login.windows.net-') || key.startsWith('msal.')) { + localStorage.removeItem(key); + } + } + onLogoutSuccess(); + }; + + const login = async () => { + try { + const response = await instance.loginPopup(msalLoginRequest); + + onLoginSuccess(parseResponse(response)); + } catch (error) { + onLoginFailure(error as AxiosError); + } + }; + + const logout = () => { + handleOnLogoutSuccess(); + }; + const fetchIdToken = async ( shouldFallbackToPopup = false ): Promise => { @@ -154,34 +149,11 @@ const MsalAuthenticator = forwardRef( }; const renewIdToken = async () => { - const user = await fetchIdToken(); + const user = await fetchIdToken(true); return user.id_token; }; - useEffect(() => { - const oidcUserToken = getOidcToken(); - if ( - !oidcUserToken && - inProgress === InteractionStatus.None && - (accounts.length > 0 || account?.idTokenClaims) - ) { - mutex.run(async () => { - mutex.lock(); - fetchIdToken(true) - .then((user) => { - if ((user as OidcUser).id_token) { - onLoginSuccess(user as OidcUser); - } - }) - .catch(onLoginFailure) - .finally(() => { - mutex.unlock(); - }); - }); - } - }, [inProgress, accounts, instance, account]); - useImperativeHandle(ref, () => ({ invokeLogin: login, invokeLogout: logout, 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 75ae4d22109..35f6297f6ce 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 @@ -25,7 +25,6 @@ import { InternalAxiosRequestConfig, } from 'axios'; import { CookieStorage } from 'cookie-storage'; -import { compare } from 'fast-json-patch'; import { isEmpty, isNil, isNumber } from 'lodash'; import Qs from 'qs'; import React, { @@ -63,7 +62,7 @@ import { fetchAuthenticationConfig, fetchAuthorizerConfig, } from '../../../rest/miscAPI'; -import { getLoggedInUser, updateUserDetail } from '../../../rest/userAPI'; +import { getLoggedInUser } from '../../../rest/userAPI'; import TokenService from '../../../utils/Auth/TokenService/TokenServiceUtil'; import { extractDetailsFromToken, @@ -76,10 +75,7 @@ import { import { getPathNameFromWindowLocation } from '../../../utils/RouterUtils'; import { escapeESReservedCharacters } from '../../../utils/StringsUtils'; import { showErrorToast, showInfoToast } from '../../../utils/ToastUtils'; -import { - getUserDataFromOidc, - matchUserDetails, -} from '../../../utils/UserDataUtils'; +import { checkIfUpdateRequired } from '../../../utils/UserDataUtils'; import { resetWebAnalyticSession } from '../../../utils/WebAnalyticsUtils'; import Loader from '../../common/Loader/Loader'; import Auth0Authenticator from '../AppAuthenticators/Auth0Authenticator'; @@ -269,29 +265,6 @@ export const AuthProvider = ({ } }; - 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); - - 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'), - }) - ); - } - }; - /** * Renew Id Token handler for all the SSOs. * This method will be called when the id token is about to expire. @@ -428,12 +401,8 @@ export const AuthProvider = ({ const res = await getLoggedInUser({ fields }); if (res) { - const updatedUserData = getUserDataFromOidc(res, newUser); - if (!matchUserDetails(res, updatedUserData, ['profile', 'email'])) { - getUpdatedUser(updatedUserData, res); - } else { - setCurrentUser(res); - } + const userDetails = await checkIfUpdateRequired(res, newUser); + setCurrentUser(userDetails); // Fetch domains at the start await fetchDomainList(); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Users/UserProfileIcon/UserProfileIcon.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Users/UserProfileIcon/UserProfileIcon.component.tsx index 57eb53f8e8f..664ea56e4ff 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Settings/Users/UserProfileIcon/UserProfileIcon.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Settings/Users/UserProfileIcon/UserProfileIcon.component.tsx @@ -47,7 +47,7 @@ type ListMenuItemProps = { listItems: EntityReference[]; labelRenderer: (item: EntityReference) => ReactNode; readMoreLabelRenderer: (count: number) => ReactNode; - readMoreKey?: string; + readMoreKey: string; sizeLimit?: number; }; @@ -66,7 +66,7 @@ const renderLimitedListMenuItem = ({ const items = listItems.slice(0, sizeLimit); return isEmpty(items) - ? [{ label: NO_DATA_PLACEHOLDER, key: 'no-teams' }] + ? [{ label: NO_DATA_PLACEHOLDER, key: readMoreKey.replace('more', 'no') }] : [ ...(items?.map((item) => ({ label: labelRenderer(item), diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts b/openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts index 4c028bd1724..25c2b850b55 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/Auth/TokenService/TokenServiceUtil.ts @@ -73,6 +73,7 @@ class TokenService { response = await this.renewToken(); this.tokeUpdateInProgress = false; } catch (error) { + useApplicationStore.getState().onLogoutHandler(); // Do nothing } finally { this.tokeUpdateInProgress = false; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.test.ts index 5a7599d60de..bf2e5dab32f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.test.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.test.ts @@ -10,8 +10,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { OidcUser } from '../components/Auth/AuthProviders/AuthProvider.interface'; import { User } from '../generated/entity/teams/user'; -import { getUserWithImage } from './UserDataUtils'; +import { checkIfUpdateRequired, getUserWithImage } from './UserDataUtils'; describe('getUserWithImage', () => { it('should return the correct user based on profile and isBot status', () => { @@ -88,3 +89,64 @@ describe('getUserWithImage', () => { expect(result).toEqual(userWithoutProfile); }); }); + +describe('checkIfUpdateRequired', () => { + it('should return the updated user details if update is required', async () => { + const existingUserDetails: User = { + email: 'a@a.com', + id: '1', + name: 'user', + isBot: false, + }; + + const newUser: OidcUser = { + id_token: 'idToken', + scope: 'scope', + profile: { + email: 'a@a.com', + name: 'updatedUser', + picture: '', + preferred_username: 'preferred_username', + sub: 'sub', + }, + }; + + const updatedUserDetails = await checkIfUpdateRequired( + existingUserDetails, + newUser + ); + + expect(updatedUserDetails).toEqual({ + ...existingUserDetails, + name: 'user', + }); + }); + + it('should return the existing user details if update is not required', async () => { + const existingUserDetails: User = { + email: 'a@a.com', + id: '1', + name: 'user', + isBot: false, + }; + + const newUser: OidcUser = { + id_token: 'idToken', + scope: 'scope', + profile: { + email: 'a@a.com', + name: 'user', + picture: '', + preferred_username: 'preferred_username', + sub: 'sub', + }, + }; + + const updatedUserDetails = await checkIfUpdateRequired( + existingUserDetails, + newUser + ); + + expect(updatedUserDetails).toEqual(existingUserDetails); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.ts index c02e8a0629c..cf32a968958 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/UserDataUtils.ts @@ -11,6 +11,8 @@ * limitations under the License. */ +import { AxiosError } from 'axios'; +import { compare } from 'fast-json-patch'; import { isEqual } from 'lodash'; import { OidcUser } from '../components/Auth/AuthProviders/AuthProvider.interface'; import { WILD_CARD_CHAR } from '../constants/char.constants'; @@ -18,13 +20,16 @@ import { SettledStatus } from '../enums/Axios.enum'; import { SearchIndex } from '../enums/search.enum'; import { SearchResponse } from '../interface/search.interface'; import { getSearchedTeams, getSearchedUsers } from '../rest/miscAPI'; +import { updateUserDetail } from '../rest/userAPI'; import { User } from './../generated/entity/teams/user'; import { formatTeamsResponse, formatUsersResponse } from './APIUtils'; import { getImages } from './CommonUtils'; +import i18n from './i18next/LocalUtil'; import { getImageWithResolutionAndFallback, ImageQuality, } from './ProfilerUtils'; +import { showErrorToast } from './ToastUtils'; import userClassBase from './UserClassBase'; export const getUserDataFromOidc = ( @@ -44,7 +49,7 @@ export const getUserDataFromOidc = ( ...userData, email, displayName: oidcUser.profile.name, - profile: images ? { images } : userData.profile, + profile: images ? { ...userData.profile, images } : undefined, }; }; @@ -124,3 +129,44 @@ export const getUserWithImage = (user: User) => { return user; }; + +export const checkIfUpdateRequired = async ( + existingUserDetails: User, + newUser: OidcUser +): Promise => { + const updatedUserData = getUserDataFromOidc(existingUserDetails, newUser); + + if (existingUserDetails.email !== updatedUserData.email) { + return existingUserDetails; + } else if ( + existingUserDetails.email === updatedUserData.email && + // We only want to update images / profile info not any other information + updatedUserData.profile?.images && + !matchUserDetails(existingUserDetails, updatedUserData, ['profile']) + ) { + const finalData = { + ...existingUserDetails, + // We want to override any profile information that is coming from the OIDC provider + profile: { + ...existingUserDetails.profile, + ...updatedUserData.profile + }, + }; + const jsonPatch = compare(existingUserDetails, finalData); + + try { + const res = await updateUserDetail(existingUserDetails.id, jsonPatch); + + return res; + } catch (error) { + showErrorToast( + error as AxiosError, + i18n.t('server.entity-updating-error', { + entity: i18n.t('label.admin-profile'), + }) + ); + } + } + + return existingUserDetails; +}; diff --git a/openmetadata-ui/src/main/resources/ui/webpack.config.dev.js b/openmetadata-ui/src/main/resources/ui/webpack.config.dev.js index c9b3dac652b..7c4b0f7d2c6 100644 --- a/openmetadata-ui/src/main/resources/ui/webpack.config.dev.js +++ b/openmetadata-ui/src/main/resources/ui/webpack.config.dev.js @@ -205,6 +205,14 @@ module.exports = { from: path.join(__dirname, 'public/favicon.png'), to: outputPath, }, + { + from: path.join(__dirname, 'public/favicons/favicon-16x16.png'), + to: outputPath, + }, + { + from: path.join(__dirname, 'public/favicons/favicon-32x32.png'), + to: outputPath, + }, { from: path.join(__dirname, 'public/logo192.png'), to: outputPath, diff --git a/openmetadata-ui/src/main/resources/ui/webpack.config.prod.js b/openmetadata-ui/src/main/resources/ui/webpack.config.prod.js index ef445bb4dbf..a92b0690d8b 100644 --- a/openmetadata-ui/src/main/resources/ui/webpack.config.prod.js +++ b/openmetadata-ui/src/main/resources/ui/webpack.config.prod.js @@ -199,6 +199,14 @@ module.exports = { from: path.join(__dirname, 'public/favicon.png'), to: outputPath, }, + { + from: path.join(__dirname, 'public/favicons/favicon-16x16.png'), + to: outputPath, + }, + { + from: path.join(__dirname, 'public/favicons/favicon-32x32.png'), + to: outputPath, + }, { from: path.join(__dirname, 'public/logo192.png'), to: outputPath, diff --git a/openmetadata-ui/src/main/resources/ui/yarn.lock b/openmetadata-ui/src/main/resources/ui/yarn.lock index b3d7c3d050a..b602af1190d 100644 --- a/openmetadata-ui/src/main/resources/ui/yarn.lock +++ b/openmetadata-ui/src/main/resources/ui/yarn.lock @@ -12186,11 +12186,6 @@ react-codemirror2@^7.2.1: resolved "https://registry.yarnpkg.com/react-codemirror2/-/react-codemirror2-7.2.1.tgz#38dab492fcbe5fb8ebf5630e5bb7922db8d3a10c" integrity sha512-t7YFmz1AXdlImgHXA9Ja0T6AWuopilub24jRaQdPVbzUJVNKIYuy3uCFZYa7CE5S3UW6SrSa5nAqVQvtzRF9gw== -react-context-mutex@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/react-context-mutex/-/react-context-mutex-2.0.0.tgz#4060bc8fbced6d2680ab093139c727201d402b88" - integrity sha512-Qss0YfJAKQ/+CYdVjHB3w7wRLdeetSrp2IMEEouXEpoTdohaCXIKZtFyNMMKDytLVWqHiiXg0RDA2kj/DQbeMQ== - react-dnd-html5-backend@14.0.2: version "14.0.2" resolved "https://registry.yarnpkg.com/react-dnd-html5-backend/-/react-dnd-html5-backend-14.0.2.tgz#25019388f6abdeeda3a6fea835dff155abb2085c"