diff --git a/clients/ui/api/openapi/mod-arch.yaml b/clients/ui/api/openapi/mod-arch.yaml index d1749468..931e7cc0 100644 --- a/clients/ui/api/openapi/mod-arch.yaml +++ b/clients/ui/api/openapi/mod-arch.yaml @@ -25,13 +25,15 @@ paths: operationId: healthcheck summary: HealthCheck description: HealthCheck endpoint. - /api/v1/config: - summary: Path used to manage Configuration information regarding the UI. + /api/v1/user: + summary: Path used to Retrieve a user based on the header. description: >- The REST endpoint/path used pass all the config information needed for the UI. get: tags: - K8SOperation + parameters: + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/ConfigResponse" @@ -49,6 +51,8 @@ paths: get: tags: - K8SOperation + parameters: + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/ModelRegistryRespone" @@ -68,6 +72,7 @@ paths: - ModelRegistryService parameters: - $ref: "#/components/parameters/modelRegistryName" + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/ModelVersionResponse" @@ -112,6 +117,7 @@ paths: description: Updates an existing `ModelVersion`. parameters: - $ref: "#/components/parameters/modelRegistryName" + - $ref: "#/components/parameters/kubeflowUserId" - name: modelversionId description: A unique identifier for a `ModelVersion`. schema: @@ -131,6 +137,7 @@ paths: - $ref: "#/components/parameters/orderBy" - $ref: "#/components/parameters/sortOrder" - $ref: "#/components/parameters/nextPageToken" + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/RegisteredModelListResponse" @@ -159,6 +166,7 @@ paths: - ModelRegistryService parameters: - $ref: "#/components/parameters/modelRegistryName" + - $ref: "#/components/parameters/kubeflowUserId" responses: "201": $ref: "#/components/responses/RegisteredModelResponse" @@ -180,6 +188,7 @@ paths: - ModelRegistryService parameters: - $ref: "#/components/parameters/modelRegistryName" + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/RegisteredModelResponse" @@ -224,6 +233,7 @@ paths: description: Updates an existing `RegisteredModel`. parameters: - $ref: "#/components/parameters/modelRegistryName" + - $ref: "#/components/parameters/kubeflowUserId" - name: registeredmodelId description: A unique identifier for a `RegisteredModel`. schema: @@ -245,6 +255,7 @@ paths: - $ref: "#/components/parameters/orderBy" - $ref: "#/components/parameters/sortOrder" - $ref: "#/components/parameters/nextPageToken" + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/ArtifactListResponse" @@ -274,6 +285,7 @@ paths: - ModelRegistryService parameters: - $ref: "#/components/parameters/modelRegistryName" + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/ArtifactResponse" @@ -311,6 +323,7 @@ paths: - $ref: "#/components/parameters/orderBy" - $ref: "#/components/parameters/sortOrder" - $ref: "#/components/parameters/nextPageToken" + - $ref: "#/components/parameters/kubeflowUserId" responses: "200": $ref: "#/components/responses/ModelVersionListResponse" @@ -355,6 +368,7 @@ paths: description: Creates a new instance of a `ModelVersion`. parameters: - $ref: "#/components/parameters/modelRegistryName" + - $ref: "#/components/parameters/kubeflowUserId" - name: registeredmodelId description: A unique identifier for a `RegisteredModel`. schema: @@ -365,12 +379,16 @@ components: schemas: Config: required: - - username + - userId + - clusterAdmin type: object properties: - username: + userId: type: string - example: username-1 + example: user@example.com + clusterAdmin: + type: boolean + example: true ModelRegistry: required: - name @@ -1193,6 +1211,12 @@ components: $ref: "#/components/schemas/ServeModel" description: A response containing a `ServeModel` entity. parameters: + kubeflowUserId: + in: header + name: kubeflow-userid + schema: + type: string + required: true modelRegistryName: name: modelRegistryName description: Name of the Model Registry selected. diff --git a/clients/ui/bff/internal/models/health_check.go b/clients/ui/bff/internal/models/health_check.go index cfee33ac..530fe1d4 100644 --- a/clients/ui/bff/internal/models/health_check.go +++ b/clients/ui/bff/internal/models/health_check.go @@ -7,5 +7,5 @@ type SystemInfo struct { type HealthCheckModel struct { Status string `json:"status"` SystemInfo SystemInfo `json:"system_info"` - UserID string `json:"user-id"` + UserID string `json:"userId"` } diff --git a/clients/ui/bff/internal/models/user.go b/clients/ui/bff/internal/models/user.go index c719bced..3cb49fa9 100644 --- a/clients/ui/bff/internal/models/user.go +++ b/clients/ui/bff/internal/models/user.go @@ -1,6 +1,6 @@ package models type User struct { - UserID string `json:"user-id"` - ClusterAdmin bool `json:"cluster-admin"` + UserID string `json:"userId"` + ClusterAdmin bool `json:"clusterAdmin"` } diff --git a/clients/ui/frontend/src/__mocks__/mockUserSettings.ts b/clients/ui/frontend/src/__mocks__/mockUserSettings.ts new file mode 100644 index 00000000..926b0c0f --- /dev/null +++ b/clients/ui/frontend/src/__mocks__/mockUserSettings.ts @@ -0,0 +1,14 @@ +import { UserSettings } from '~/shared/types'; + +type MockUserSettingsType = { + userId?: string; + clusterAdmin?: boolean; +}; + +export const mockUserSettings = ({ + userId = 'user@example.com', + clusterAdmin = true, +}: MockUserSettingsType): UserSettings => ({ + userId, + clusterAdmin, +}); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts index 08423f51..54cbb27e 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts @@ -9,6 +9,7 @@ import type { RegisteredModel, RegisteredModelList, } from '~/app/types'; +import type { UserSettings } from '~/shared/types'; const MODEL_REGISTRY_API_VERSION = 'v1'; export { MODEL_REGISTRY_API_VERSION }; @@ -102,6 +103,11 @@ declare global { type: 'GET /api/:apiVersion/model_registry', options: { path: { apiVersion: string } }, response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'GET /api/:apiVersion/user', + options: { path: { apiVersion: string } }, + response: ApiResponse, ) => Cypress.Chainable); } } diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/support/e2e.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/support/e2e.ts index 4a6016e9..5b196a70 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/support/e2e.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/support/e2e.ts @@ -15,8 +15,10 @@ import chaiSubset from 'chai-subset'; import '@cypress/code-coverage/support'; +import { mockUserSettings } from '~/__mocks__/mockUserSettings'; import 'cypress-mochawesome-reporter/register'; import './commands'; +import { MODEL_REGISTRY_API_VERSION } from './commands/api'; chai.use(chaiSubset); @@ -28,5 +30,15 @@ beforeEach(() => { if (Cypress.env('MOCK')) { // fallback: return 404 for all api requests cy.intercept({ pathname: '/api/**' }, { statusCode: 404 }); + + cy.interceptApi( + 'GET /api/:apiVersion/user', + { + path: { + apiVersion: MODEL_REGISTRY_API_VERSION, + }, + }, + mockUserSettings({}), + ); } }); diff --git a/clients/ui/frontend/src/app/App.tsx b/clients/ui/frontend/src/app/App.tsx index d4c7f091..7510d690 100644 --- a/clients/ui/frontend/src/app/App.tsx +++ b/clients/ui/frontend/src/app/App.tsx @@ -5,9 +5,6 @@ import { Alert, Bullseye, Button, - Masthead, - MastheadContent, - MastheadMain, Page, PageSection, Spinner, @@ -16,11 +13,12 @@ import { } from '@patternfly/react-core'; import ToastNotifications from '~/shared/components/ToastNotifications'; import { useSettings } from '~/shared/hooks/useSettings'; -import { isMUITheme, Theme } from '~/shared/utilities/const'; +import { isMUITheme, Theme, USER_ID } from '~/shared/utilities/const'; import NavSidebar from './NavSidebar'; import AppRoutes from './AppRoutes'; import { AppContext } from './AppContext'; import { ModelRegistrySelectorContextProvider } from './context/ModelRegistrySelectorContext'; +import NavBar from './NavBar'; const App: React.FC = () => { const { @@ -30,6 +28,8 @@ const App: React.FC = () => { loadError: configError, } = useSettings(); + const username = userSettings?.userId; + React.useEffect(() => { // Apply the theme based on the value of STYLE_THEME if (isMUITheme()) { @@ -39,6 +39,16 @@ 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); + } else { + localStorage.removeItem(USER_ID); + } + }, [username]); + const contextValue = React.useMemo( () => configSettings && userSettings @@ -82,15 +92,6 @@ const App: React.FC = () => { // Waiting on the API to finish const loading = !configLoaded || !userSettings || !configSettings || !contextValue; - const masthead = ( - - - - {/* TODO: [Auth-enablement] Add logout and user status once we enable itNavigates to register page from table toolbar */} - - - ); - return loading ? ( @@ -99,7 +100,14 @@ const App: React.FC = () => { { + //TODO: [Auth-enablement] Logout when auth is enabled + }} + /> + } isManagedSidebar sidebar={} > diff --git a/clients/ui/frontend/src/app/AppRoutes.tsx b/clients/ui/frontend/src/app/AppRoutes.tsx index 7dbf30b9..2e8b5f71 100644 --- a/clients/ui/frontend/src/app/AppRoutes.tsx +++ b/clients/ui/frontend/src/app/AppRoutes.tsx @@ -3,6 +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'; export const isNavDataGroup = (navItem: NavDataItem): navItem is NavDataGroup => 'children' in navItem; @@ -22,12 +23,9 @@ export type NavDataGroup = NavDataCommon & { type NavDataItem = NavDataHref | NavDataGroup; export const useAdminSettings = (): NavDataItem[] => { - // get auth access for example set admin as true - const isAdmin = true; //this should be a call to getting auth / role access + const { clusterAdmin } = useAppContext().user; - // TODO: [Auth-enablement] Remove the linter skip when we implement authentication - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - if (!isAdmin) { + if (!clusterAdmin) { return []; } @@ -48,20 +46,16 @@ export const useNavData = (): NavDataItem[] => [ ]; const AppRoutes: React.FC = () => { - const isAdmin = true; + const { clusterAdmin } = useAppContext().user; return ( } /> } /> } /> - { - // TODO: [Auth-enablement] Remove the linter skip when we implement authentication - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - isAdmin && ( - } /> - ) - } + {clusterAdmin && ( + } /> + )} ); }; diff --git a/clients/ui/frontend/src/app/NavBar.tsx b/clients/ui/frontend/src/app/NavBar.tsx new file mode 100644 index 00000000..62876c85 --- /dev/null +++ b/clients/ui/frontend/src/app/NavBar.tsx @@ -0,0 +1,91 @@ +import React from 'react'; +import { + Dropdown, + DropdownItem, + DropdownList, + Masthead, + MastheadContent, + MastheadMain, + MenuToggle, + MenuToggleElement, + Toolbar, + ToolbarContent, + ToolbarGroup, + ToolbarItem, +} from '@patternfly/react-core'; +import { SimpleSelect, SimpleSelectOption } from '@patternfly/react-templates'; + +interface NavBarProps { + username?: string; + onLogout: () => void; +} + +const Options: SimpleSelectOption[] = [{ content: 'All Namespaces', value: 'All' }]; + +const NavBar: React.FC = ({ username, onLogout }) => { + const [selected, setSelected] = React.useState('All'); + const [userMenuOpen, setUserMenuOpen] = React.useState(false); + + const initialOptions = React.useMemo( + () => Options.map((o) => ({ ...o, selected: o.value === selected })), + [selected], + ); + + const handleLogout = () => { + setUserMenuOpen(false); + onLogout(); + }; + + const userMenuItems = [ + + Log out + , + ]; + + return ( + + + + + + + + setSelected(String(selection))} + /> + + + {username && ( + + + setUserMenuOpen(isOpen)} + toggle={(toggleRef: React.Ref) => ( + setUserMenuOpen(!userMenuOpen)} + isExpanded={userMenuOpen} + > + {username} + + )} + isOpen={userMenuOpen} + > + {userMenuItems} + + + + )} + + + + + ); +}; + +export default NavBar; diff --git a/clients/ui/frontend/src/shared/api/apiUtils.ts b/clients/ui/frontend/src/shared/api/apiUtils.ts index b5980718..f6cfb7fa 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_ACCESS_TOKEN } from '~/shared/utilities/const'; +import { USER_ID } from '~/shared/utilities/const'; export const mergeRequestInit = ( opts: APIOptions = {}, @@ -9,6 +9,10 @@ export const mergeRequestInit = ( ): RequestInit => ({ ...specificOpts, ...(opts.signal && { signal: opts.signal }), + headers: { + ...(opts.headers ?? {}), + ...(specificOpts.headers ?? {}), + }, }); type CallRestJSONOptions = { @@ -61,23 +65,17 @@ const callRestJSON = ( requestData = JSON.stringify(data); } - // Get from the browser storage the value from the key USER_ACCESS_TOKEN - // and set it as the value for the header key 'x-forwarded-access-token' - // This is a security measure to ensure that the user is authenticated - // before making any API calls. Local Storage is not secure, but it is - // enough for this PoC. - const token = localStorage.getItem(USER_ACCESS_TOKEN); - if (token) { - otherOptions.headers = { - ...otherOptions.headers, - [USER_ACCESS_TOKEN]: token, - }; - } + // 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 ...(contentType && { 'Content-Type': contentType }), }, method, diff --git a/clients/ui/frontend/src/shared/api/k8s.ts b/clients/ui/frontend/src/shared/api/k8s.ts index 61b83d76..6c483eae 100644 --- a/clients/ui/frontend/src/shared/api/k8s.ts +++ b/clients/ui/frontend/src/shared/api/k8s.ts @@ -3,6 +3,7 @@ import { handleRestFailures } from '~/shared/api/errorUtils'; import { isModelRegistryResponse, restGET } from '~/shared/api/apiUtils'; import { ModelRegistry } from '~/app/types'; import { BFF_API_VERSION } from '~/app/const'; +import { UserSettings } from '~/shared/types'; export const getListModelRegistries = (hostPath: string) => @@ -15,3 +16,15 @@ export const getListModelRegistries = throw new Error('Invalid response format'); }, ); + +export const getUser = + (hostPath: string) => + (opts: APIOptions): Promise => + handleRestFailures(restGET(hostPath, `/api/${BFF_API_VERSION}/user`, {}, opts)).then( + (response) => { + if (isModelRegistryResponse(response)) { + return response.data; + } + throw new Error('Invalid response format'); + }, + ); diff --git a/clients/ui/frontend/src/shared/api/types.ts b/clients/ui/frontend/src/shared/api/types.ts index e7335512..bb0055bc 100644 --- a/clients/ui/frontend/src/shared/api/types.ts +++ b/clients/ui/frontend/src/shared/api/types.ts @@ -2,6 +2,7 @@ export type APIOptions = { dryRun?: boolean; signal?: AbortSignal; parseJSON?: boolean; + headers?: Record; }; export type APIError = { diff --git a/clients/ui/frontend/src/shared/hooks/useSettings.tsx b/clients/ui/frontend/src/shared/hooks/useSettings.tsx index 4aad461b..c5d45ee9 100644 --- a/clients/ui/frontend/src/shared/hooks/useSettings.tsx +++ b/clients/ui/frontend/src/shared/hooks/useSettings.tsx @@ -1,8 +1,9 @@ import * as React from 'react'; -import { POLL_INTERVAL } from '~/shared/utilities/const'; +import { mockedUsername, POLL_INTERVAL, USER_ID } from '~/shared/utilities/const'; import { useDeepCompareMemoize } from '~/shared/utilities/useDeepCompareMemoize'; import { ConfigSettings, UserSettings } from '~/shared/types'; import useTimeBasedRefresh from '~/shared/hooks/useTimeBasedRefresh'; +import { getUser } from '~/shared/api/k8s'; export const useSettings = (): { configSettings: ConfigSettings | null; @@ -14,13 +15,17 @@ export const useSettings = (): { const [loadError, setLoadError] = React.useState(); const [config, setConfig] = React.useState(null); const [user, setUser] = React.useState(null); + const userSettings = React.useMemo(() => getUser(''), []); const setRefreshMarker = useTimeBasedRefresh(); React.useEffect(() => { let watchHandle: ReturnType; let cancelled = false; const watchConfig = () => { - Promise.all([fetchConfig(), fetchUser()]) + // 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 }; + Promise.all([fetchConfig(), userSettings({ headers })]) .then(([fetchedConfig, fetchedUser]) => { if (cancelled) { return; @@ -53,7 +58,7 @@ export const useSettings = (): { cancelled = true; clearTimeout(watchHandle); }; - }, [setRefreshMarker]); + }, [setRefreshMarker, userSettings]); const retConfig = useDeepCompareMemoize(config); const retUser = useDeepCompareMemoize(user); @@ -62,7 +67,7 @@ export const useSettings = (): { }; // Mock a settings config call -// TODO: [Data Flow] replace with thea actual call once we have the endpoint +// TODO: [Data Flow] replace with the actual call once we have the endpoint export const fetchConfig = async (): Promise => ({ common: { featureFlags: { @@ -70,11 +75,3 @@ export const fetchConfig = async (): Promise => ({ }, }, }); - -// Mock a settings user call -// TODO: [Auth-enablement] replace with thea actual call once we have the endpoint -export const fetchUser = async (): Promise => ({ - username: 'admin', - isAdmin: true, - isAllowed: true, -}); diff --git a/clients/ui/frontend/src/shared/style/MUI-theme.scss b/clients/ui/frontend/src/shared/style/MUI-theme.scss index 8896ec5b..5596c0c2 100644 --- a/clients/ui/frontend/src/shared/style/MUI-theme.scss +++ b/clients/ui/frontend/src/shared/style/MUI-theme.scss @@ -747,6 +747,7 @@ --pf-v6-c-masthead--BackgroundColor: var(--mui-palette-common-white); box-shadow: var(--mui-shadows-1); min-height: var(--kf-central-app-bar-height); + margin-left: var(--kf-central-app-drawer-width); } .mui-theme .pf-v6-c-modal-box { @@ -795,7 +796,5 @@ } .mui-theme .pf-v6-c-page__main-container { - margin-left: var(--kf-central-app-drawer-width); /* Move content area to right of the sidebar */ margin-top: var(--kf-central-app-bar-height); /* Move content area below the app bar */ } - diff --git a/clients/ui/frontend/src/shared/types.ts b/clients/ui/frontend/src/shared/types.ts index 2b540f17..394c1720 100644 --- a/clients/ui/frontend/src/shared/types.ts +++ b/clients/ui/frontend/src/shared/types.ts @@ -2,9 +2,8 @@ import { ValueOf } from '~/shared/typeHelpers'; // TODO: [Data Flow] Get the status config params export type UserSettings = { - username: string; - isAdmin: boolean; - isAllowed: boolean; + userId: string; + clusterAdmin?: boolean; }; // TODO: [Data Flow] Add more config parameters diff --git a/clients/ui/frontend/src/shared/utilities/const.ts b/clients/ui/frontend/src/shared/utilities/const.ts index 359a6677..5cc0a8a8 100644 --- a/clients/ui/frontend/src/shared/utilities/const.ts +++ b/clients/ui/frontend/src/shared/utilities/const.ts @@ -11,6 +11,7 @@ export const isMUITheme = (): boolean => STYLE_THEME === Theme.MUI; export const STYLE_THEME = process.env.STYLE_THEME || Theme.MUI; -export const USER_ACCESS_TOKEN = 'x-forwarded-access-token'; +export const USER_ID = 'kubeflow-userid'; +export const mockedUsername = 'user@example.com'; export { POLL_INTERVAL };