From 4312d98f3d0e7ec00abb48b9ae9db5f0b0c7cf92 Mon Sep 17 00:00:00 2001 From: lucferbux Date: Sat, 14 Dec 2024 00:40:21 +0100 Subject: [PATCH] feat: add user logout utility and custom hooks for user settings; update environment variables and constants --- clients/ui/frontend/.env | 5 +++-- clients/ui/frontend/config/dotenv.js | 2 +- .../cypress/cypress/pages/appChrome.ts | 9 ++++----- .../cypress/support/commands/application.ts | 6 ++---- clients/ui/frontend/src/app/App.tsx | 17 +++++++---------- clients/ui/frontend/src/app/AppRoutes.tsx | 6 +++--- clients/ui/frontend/src/app/NavSidebar.tsx | 3 ++- clients/ui/frontend/src/app/hooks/useUser.ts | 10 ++++++++++ .../RegisterModel/useRegistrationCommonState.ts | 5 +++-- clients/ui/frontend/src/shared/api/apiUtils.ts | 10 ++-------- .../frontend/src/shared/hooks/useSettings.tsx | 6 ++---- .../src/shared/hooks/useTimeBasedRefresh.ts | 4 ++-- clients/ui/frontend/src/shared/types.ts | 4 ---- .../frontend/src/shared/utilities/appUtils.ts | 3 +++ .../ui/frontend/src/shared/utilities/const.ts | 16 ++++++++-------- 15 files changed, 52 insertions(+), 54 deletions(-) create mode 100644 clients/ui/frontend/src/app/hooks/useUser.ts create mode 100644 clients/ui/frontend/src/shared/utilities/appUtils.ts diff --git a/clients/ui/frontend/.env b/clients/ui/frontend/.env index 39fbc83c..cf90978c 100644 --- a/clients/ui/frontend/.env +++ b/clients/ui/frontend/.env @@ -1,9 +1,10 @@ IS_PROJECT_ROOT_DIR=false PORT=${FRONTEND_PORT} -########## Change the following three variables to customize the Dashboard ########## +########## Change the following variables to customize the Dashboard ########## LOGO=logo-light-theme.svg LOGO_DARK=logo-dark-theme.svg FAVICON=favicon.ico -PRODUCT_NAME=Model Registry +PRODUCT_NAME="Model Registry" +STYLE_THEME=mui-theme diff --git a/clients/ui/frontend/config/dotenv.js b/clients/ui/frontend/config/dotenv.js index c8a64d33..c0e765f7 100644 --- a/clients/ui/frontend/config/dotenv.js +++ b/clients/ui/frontend/config/dotenv.js @@ -159,7 +159,7 @@ const setupDotenvFilesForEnv = ({ env }) => { const OUTPUT_ONLY = process.env._OUTPUT_ONLY === 'true'; process.env._RELATIVE_DIRNAME = RELATIVE_DIRNAME; - process.env._UI_IS_PROJECT_ROOT_DIR = IS_ROOT; + process.env._IS_PROJECT_ROOT_DIR = IS_ROOT; process.env._IMAGES_DIRNAME = IMAGES_DIRNAME; process.env._PUBLIC_PATH = PUBLIC_PATH; process.env._SRC_DIR = SRC_DIR; diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts index 5d6afc32..903c3427 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts @@ -9,11 +9,10 @@ class AppChrome { cy.testA11y(); } - // TODO: [Auth-enablement] Uncomment once auth is enabled - // shouldBeUnauthorized() { - // cy.findByTestId('unauthorized-error'); - // return this; - // } + shouldBeUnauthorized() { + cy.findByTestId('unauthorized-error'); + return this; + } findNavToggle() { return cy.get('#page-nav-toggle'); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts index c6100d0c..6a953ab1 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts @@ -1,13 +1,11 @@ import type { MatcherOptions } from '@testing-library/cypress'; import type { Matcher, MatcherOptions as DTLMatcherOptions } from '@testing-library/dom'; -// import type { UserAuthConfig } from '~/__tests__/cypress/cypress/types'; -// import { TEST_USER } from '~/__tests__/cypress/cypress/utils/e2eUsers'; /* eslint-disable @typescript-eslint/no-namespace */ declare global { namespace Cypress { interface Chainable { - // TODO: [Auth-enablement] Uncomment once auth is enabled + // TODO: [Global auth] Uncomment once auth is enabled // /** // * Visits the URL and performs a login if necessary. // * Uses credentials supplied by environment variables if not provided. @@ -121,7 +119,7 @@ declare global { } } -// TODO: [Auth-enablement] Uncomment once auth is enabled +// TODO: [Global auth] Uncomment once auth is enabled // Cypress.Commands.add('visitWithLogin', (url, user = TEST_USER) => { // if (Cypress.env('MOCK')) { // cy.visit(url); diff --git a/clients/ui/frontend/src/app/App.tsx b/clients/ui/frontend/src/app/App.tsx index 7510d690..66fc6967 100644 --- a/clients/ui/frontend/src/app/App.tsx +++ b/clients/ui/frontend/src/app/App.tsx @@ -13,7 +13,8 @@ import { } from '@patternfly/react-core'; import ToastNotifications from '~/shared/components/ToastNotifications'; import { useSettings } from '~/shared/hooks/useSettings'; -import { isMUITheme, Theme, USER_ID } from '~/shared/utilities/const'; +import { isMUITheme, Theme, AUTH_HEADER, DEV_MODE } from '~/shared/utilities/const'; +import { logout } from '~/shared/utilities/appUtils'; import NavSidebar from './NavSidebar'; import AppRoutes from './AppRoutes'; import { AppContext } from './AppContext'; @@ -40,12 +41,10 @@ const App: React.FC = () => { }, []); React.useEffect(() => { - // Add the user to localStorage if in PoC - // TODO: [Env Handling] Just add this logic in PoC mode - if (username) { - localStorage.setItem(USER_ID, username); + if (DEV_MODE && username) { + localStorage.setItem(AUTH_HEADER, username); } else { - localStorage.removeItem(USER_ID); + localStorage.removeItem(AUTH_HEADER); } }, [username]); @@ -76,9 +75,7 @@ const App: React.FC = () => { @@ -104,7 +101,7 @@ const App: React.FC = () => { { - //TODO: [Auth-enablement] Logout when auth is enabled + logout().then(() => window.location.reload()); }} /> } diff --git a/clients/ui/frontend/src/app/AppRoutes.tsx b/clients/ui/frontend/src/app/AppRoutes.tsx index 2e8b5f71..9c858d56 100644 --- a/clients/ui/frontend/src/app/AppRoutes.tsx +++ b/clients/ui/frontend/src/app/AppRoutes.tsx @@ -3,7 +3,7 @@ import { Navigate, Route, Routes } from 'react-router-dom'; import { NotFound } from './pages/notFound/NotFound'; import ModelRegistrySettingsRoutes from './pages/settings/ModelRegistrySettingsRoutes'; import ModelRegistryRoutes from './pages/modelRegistry/ModelRegistryRoutes'; -import { useAppContext } from './AppContext'; +import useUser from './hooks/useUser'; export const isNavDataGroup = (navItem: NavDataItem): navItem is NavDataGroup => 'children' in navItem; @@ -23,7 +23,7 @@ export type NavDataGroup = NavDataCommon & { type NavDataItem = NavDataHref | NavDataGroup; export const useAdminSettings = (): NavDataItem[] => { - const { clusterAdmin } = useAppContext().user; + const { clusterAdmin } = useUser(); if (!clusterAdmin) { return []; @@ -46,7 +46,7 @@ export const useNavData = (): NavDataItem[] => [ ]; const AppRoutes: React.FC = () => { - const { clusterAdmin } = useAppContext().user; + const { clusterAdmin } = useUser(); return ( diff --git a/clients/ui/frontend/src/app/NavSidebar.tsx b/clients/ui/frontend/src/app/NavSidebar.tsx index 80ed9c43..8af64a6f 100644 --- a/clients/ui/frontend/src/app/NavSidebar.tsx +++ b/clients/ui/frontend/src/app/NavSidebar.tsx @@ -9,6 +9,7 @@ import { PageSidebar, PageSidebarBody, } from '@patternfly/react-core'; +import { LOGO_LIGHT } from '~/shared/utilities/const'; import { useNavData, isNavDataGroup, NavDataHref, NavDataGroup } from './AppRoutes'; const NavHref: React.FC<{ item: NavDataHref }> = ({ item }) => ( @@ -50,7 +51,7 @@ const NavSidebar: React.FC = () => { diff --git a/clients/ui/frontend/src/app/hooks/useUser.ts b/clients/ui/frontend/src/app/hooks/useUser.ts new file mode 100644 index 00000000..5f4f60f7 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useUser.ts @@ -0,0 +1,10 @@ +import { useContext } from 'react'; +import { UserSettings } from '~/shared/types'; +import { AppContext } from '~/app/AppContext'; + +const useUser = (): UserSettings => { + const { user } = useContext(AppContext); + return user; +}; + +export default useUser; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts index 993616b2..fadde80c 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts @@ -1,6 +1,7 @@ import React from 'react'; import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; import { ModelRegistryAPIState } from '~/app/context/useModelRegistryAPIState'; +import useUser from '~/app/hooks/useUser'; type RegistrationCommonState = { isSubmitting: boolean; @@ -17,7 +18,7 @@ export const useRegistrationCommonState = (): RegistrationCommonState => { const [submitError, setSubmitError] = React.useState(undefined); const { apiState } = React.useContext(ModelRegistryContext); - const author = 'kubeflow-user'; // TODO: [Auth-enablement] Enable this once we have users ---> useAppSelector((state) => state.user || ''); + const { userId } = useUser(); const handleSubmit = (doSubmit: () => Promise) => { setIsSubmitting(true); @@ -35,6 +36,6 @@ export const useRegistrationCommonState = (): RegistrationCommonState => { setSubmitError, handleSubmit, apiState, - author, + author: userId, }; }; diff --git a/clients/ui/frontend/src/shared/api/apiUtils.ts b/clients/ui/frontend/src/shared/api/apiUtils.ts index f6cfb7fa..01d03161 100644 --- a/clients/ui/frontend/src/shared/api/apiUtils.ts +++ b/clients/ui/frontend/src/shared/api/apiUtils.ts @@ -1,7 +1,7 @@ import { APIOptions } from '~/shared/api/types'; import { EitherOrNone } from '~/shared/typeHelpers'; import { ModelRegistryBody } from '~/app/types'; -import { USER_ID } from '~/shared/utilities/const'; +import { DEV_MODE, AUTH_HEADER } from '~/shared/utilities/const'; export const mergeRequestInit = ( opts: APIOptions = {}, @@ -65,17 +65,11 @@ const callRestJSON = ( requestData = JSON.stringify(data); } - // Get from the browser storage the value from the key USER_ID - // and set it as a header value for the request. - // THIS IS ONLY INTENEDED FOR THE POC WHEN YOU CANNOT INJECT IT WITH A PROXY - // TODO: [Env Handling] Just add it when in PoC - const userID = localStorage.getItem(USER_ID); - return fetch(`${host}${path}${searchParams ? `?${searchParams}` : ''}`, { ...otherOptions, headers: { ...otherOptions.headers, - ...(userID && { [USER_ID]: userID }), // TODO: [Env Handling] Just add it when in PoC + ...(DEV_MODE && { [AUTH_HEADER]: localStorage.getItem(AUTH_HEADER) }), ...(contentType && { 'Content-Type': contentType }), }, method, diff --git a/clients/ui/frontend/src/shared/hooks/useSettings.tsx b/clients/ui/frontend/src/shared/hooks/useSettings.tsx index c5d45ee9..d81488c6 100644 --- a/clients/ui/frontend/src/shared/hooks/useSettings.tsx +++ b/clients/ui/frontend/src/shared/hooks/useSettings.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { mockedUsername, POLL_INTERVAL, USER_ID } from '~/shared/utilities/const'; +import { USERNAME, POLL_INTERVAL, AUTH_HEADER, DEV_MODE } from '~/shared/utilities/const'; import { useDeepCompareMemoize } from '~/shared/utilities/useDeepCompareMemoize'; import { ConfigSettings, UserSettings } from '~/shared/types'; import useTimeBasedRefresh from '~/shared/hooks/useTimeBasedRefresh'; @@ -22,9 +22,7 @@ export const useSettings = (): { let watchHandle: ReturnType; let cancelled = false; const watchConfig = () => { - // TODO: [Env Handling] Add mocked mode for frontend in dev - // const headers = process.env.mocked === 'true' ? { [USER_ID]: mockedUsername } : undefined; - const headers = { [USER_ID]: mockedUsername }; + const headers = DEV_MODE ? { [AUTH_HEADER]: USERNAME } : undefined; Promise.all([fetchConfig(), userSettings({ headers })]) .then(([fetchedConfig, fetchedUser]) => { if (cancelled) { diff --git a/clients/ui/frontend/src/shared/hooks/useTimeBasedRefresh.ts b/clients/ui/frontend/src/shared/hooks/useTimeBasedRefresh.ts index 3ec4b296..f51a66a7 100644 --- a/clients/ui/frontend/src/shared/hooks/useTimeBasedRefresh.ts +++ b/clients/ui/frontend/src/shared/hooks/useTimeBasedRefresh.ts @@ -1,5 +1,6 @@ import * as React from 'react'; import { useBrowserStorage } from '~/shared/components/browserStorage'; +import { logout } from '~/shared/utilities/appUtils'; export type SetTime = (refreshDateMarker: Date) => void; @@ -33,8 +34,7 @@ const useTimeBasedRefresh = (): SetTime => { if (lastDate < refreshDateMarker) { setNewDateString(refreshDateMarker.toString()); console.log('Logging out and refreshing'); - // TODO: [Auth-enablement] Replace with actual logout function - //logout().then(() => window.location.reload()); + logout().then(() => window.location.reload()); } else { console.error( `We should have refreshed but it appears the last time we auto-refreshed was less than an hour ago. '${KEY_NAME}' session storage setting can be cleared for this to refresh again within the hour from the last refresh.`, diff --git a/clients/ui/frontend/src/shared/types.ts b/clients/ui/frontend/src/shared/types.ts index 394c1720..31658843 100644 --- a/clients/ui/frontend/src/shared/types.ts +++ b/clients/ui/frontend/src/shared/types.ts @@ -1,22 +1,18 @@ import { ValueOf } from '~/shared/typeHelpers'; -// TODO: [Data Flow] Get the status config params export type UserSettings = { userId: string; clusterAdmin?: boolean; }; -// TODO: [Data Flow] Add more config parameters export type ConfigSettings = { common: CommonConfig; }; -// TODO: [Data Flow] Add more config parameters export type CommonConfig = { featureFlags: FeatureFlag; }; -// TODO: [Data Flow] Add more config parameters export type FeatureFlag = { modelRegistry: boolean; }; diff --git a/clients/ui/frontend/src/shared/utilities/appUtils.ts b/clients/ui/frontend/src/shared/utilities/appUtils.ts new file mode 100644 index 00000000..84a622a0 --- /dev/null +++ b/clients/ui/frontend/src/shared/utilities/appUtils.ts @@ -0,0 +1,3 @@ +export const logout = (): Promise => + /* eslint-disable-next-line no-console */ + fetch('/oauth/sign_out').catch((err) => console.error('Error logging out', err)); diff --git a/clients/ui/frontend/src/shared/utilities/const.ts b/clients/ui/frontend/src/shared/utilities/const.ts index 5cc0a8a8..5fcb7e79 100644 --- a/clients/ui/frontend/src/shared/utilities/const.ts +++ b/clients/ui/frontend/src/shared/utilities/const.ts @@ -1,6 +1,3 @@ -// TODO: [Env Handling] Fetch the .env variable here. -const POLL_INTERVAL = 30000; - export enum Theme { Default = 'default-theme', MUI = 'mui-theme', @@ -9,9 +6,12 @@ export enum Theme { export const isMUITheme = (): boolean => STYLE_THEME === Theme.MUI; -export const STYLE_THEME = process.env.STYLE_THEME || Theme.MUI; - -export const USER_ID = 'kubeflow-userid'; -export const mockedUsername = 'user@example.com'; +const STYLE_THEME = process.env.STYLE_THEME || Theme.MUI; +const DEV_MODE = process.env.APP_ENV === 'development'; +const POLL_INTERVAL = process.env.POLL_INTERVAL ? parseInt(process.env.POLL_INTERVAL) : 30000; +const AUTH_HEADER = process.env.AUTH_HEADER || 'kubeflow-userid'; +const USERNAME = process.env.USERNAME || 'user@example.com'; +const IMAGE_DIR = process.env.IMAGE_DIR || 'images'; +const LOGO_LIGHT = process.env.LOGO || 'logo-light-theme.svg'; -export { POLL_INTERVAL }; +export { POLL_INTERVAL, DEV_MODE, AUTH_HEADER, USERNAME, IMAGE_DIR, LOGO_LIGHT };