Deprecate & Swap for Auth over DashboardConfig for group configs (#3577)

* Deprecate & Swap for Auth over DashboardConfig for group configs * Added tests and adjusted logic to meet missing error logic * Fix useGroups & update ssar check to patch * Update where to look for the dropdown values in cypress * Switch to GET over SSAR to not lock cluster-admins * Fix tests
opendatahub-io · Dec 19, 2024 · 075914a · 075914a
1 parent 4260e3e
commit 075914a
Show file tree

Hide file tree

Showing 23 changed files with 491 additions and 127 deletions.
diff --git a/backend/src/routes/api/groups-config/groupsConfigUtil.ts b/backend/src/routes/api/groups-config/groupsConfigUtil.ts
@@ -1,3 +1,9 @@
+/**
+ * @fileOverview
+ * @deprecated
+ * The Whole file is deprecated. The exposed functions of the module are deprecated to help.
+ * We are moving to direct k8s auth control and doing away with the OdhDashboardConfig group info.
+ */
 import { FastifyRequest } from 'fastify';
 import createError from 'http-errors';
 import {
@@ -13,6 +19,7 @@ import { isUserAdmin } from '../../../utils/adminUtils';
 
 const SYSTEM_AUTHENTICATED = 'system:authenticated';
 
+/** @deprecated - see RHOAIENG-16988 */
 export const getGroupsConfig = async (fastify: KubeFastifyInstance): Promise<GroupsConfig> => {
   const { customObjectsApi } = fastify.kube;
 
@@ -27,6 +34,7 @@ export const getGroupsConfig = async (fastify: KubeFastifyInstance): Promise<Gro
 const transformGroupsConfig = (groupStatus: GroupStatus[]): string[] =>
   groupStatus.filter((group) => group.enabled).map((group) => group.name);
 
+/** @deprecated - see RHOAIENG-16988 */
 export const updateGroupsConfig = async (
   fastify: KubeFastifyInstance,
   request: FastifyRequest<{ Body: GroupsConfig }>,
@@ -151,6 +159,7 @@ const getError = (
 };
 
 /**
+ * @deprecated - see RHOAIENG-16988
  * Check if any selected groups has been deleted and update the configuration if so
  * @param fastify Fastify instance
  * @param groupsData Custom Resource Data for group configuration

diff --git a/backend/src/routes/api/groups-config/index.ts b/backend/src/routes/api/groups-config/index.ts
@@ -1,9 +1,14 @@
+/**
+ * @fileOverview
+ * @deprecated see RHOAIENG-16988
+ */
 import { FastifyRequest } from 'fastify';
 import { GroupsConfig, KubeFastifyInstance } from '../../../types';
 import { getGroupsConfig, updateGroupsConfig } from './groupsConfigUtil';
 import { secureAdminRoute } from '../../../utils/route-security';
 
 export default async (fastify: KubeFastifyInstance): Promise<void> => {
+  /** @deprecated - see RHOAIENG-16988 */
   fastify.get(
     '/',
     secureAdminRoute(fastify)(async (request, reply) => {
@@ -16,6 +21,7 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
     }),
   );
 
+  /** @deprecated - see RHOAIENG-16988 */
   fastify.put(
     '/',
     secureAdminRoute(fastify)(async (request: FastifyRequest<{ Body: GroupsConfig }>, reply) => {

diff --git a/backend/src/types.ts b/backend/src/types.ts
@@ -46,8 +46,11 @@ export type DashboardConfig = K8sResourceCommon & {
       disableStorageClasses: boolean;
       disableNIMModelServing: boolean;
     };
+    /** @deprecated -- replacing this with Platform Auth resource -- remove when this is no longer in the CRD */
     groupsConfig?: {
+      /** @deprecated -- see above */
       adminGroups: string;
+      /** @deprecated -- see above */
       allowedGroups: string;
     };
     notebookSizes?: NotebookSize[];
@@ -1271,3 +1274,14 @@ export type ListConfigSecretsResponse = {
   secrets: ConfigSecretItem[];
   configMaps: ConfigSecretItem[];
 };
+
+export type AuthKind = K8sResourceCommon & {
+  metadata: {
+    name: 'auth'; // singleton, immutable name
+    namespace?: never; // Cluster resource
+  };
+  spec: {
+    adminGroups: string[];
+    allowedGroups: string[];
+  };
+};
diff --git a/backend/src/utils/adminUtils.ts b/backend/src/utils/adminUtils.ts
@@ -7,6 +7,7 @@ import { KubeFastifyInstance, ResourceAccessReviewResponse } from '../types';
 import { getAdminGroups, getAllGroupsByUser, getAllowedGroups, getGroup } from './groupsUtils';
 import { flatten, uniq } from 'lodash';
 import { getNamespaces } from '../utils/notebookUtils';
+import { getAuth } from './resourceUtils';
 
 const SYSTEM_AUTHENTICATED = 'system:authenticated';
 /** Usernames with invalid characters can start with `b64:` to keep their unwanted characters */
@@ -24,6 +25,12 @@ const getGroupUserList = async (
 };
 
 export const getAdminUserList = async (fastify: KubeFastifyInstance): Promise<string[]> => {
+  const auth = getAuth();
+  if (auth) {
+    return getGroupUserList(fastify, auth.spec.adminGroups);
+  }
+
+  // FIXME: see RHOAIENG-16988
   const adminGroups = getAdminGroups();
   const adminGroupsList = adminGroups
     .split(',')
@@ -61,6 +68,12 @@ export const getClusterAdminUserList = async (fastify: KubeFastifyInstance): Pro
 };
 
 export const getAllowedUserList = async (fastify: KubeFastifyInstance): Promise<string[]> => {
+  const auth = getAuth();
+  if (auth) {
+    return getGroupUserList(fastify, auth.spec.allowedGroups);
+  }
+
+  // FIXME: see RHOAIENG-16988
   const allowedGroups = getAllowedGroups();
   const allowedGroupList = allowedGroups
     .split(',')
@@ -74,8 +87,16 @@ export const getGroupsConfig = async (
   username: string,
 ): Promise<boolean> => {
   try {
-    const adminGroups = getAdminGroups();
-    const adminGroupsList = adminGroups.split(',');
+    const auth = getAuth();
+
+    let adminGroupsList: string[];
+    if (auth) {
+      adminGroupsList = auth.spec.adminGroups;
+    } else {
+      // FIXME: see RHOAIENG-16988
+      const adminGroups = getAdminGroups();
+      adminGroupsList = adminGroups.split(',');
+    }
 
     if (adminGroupsList.includes(SYSTEM_AUTHENTICATED)) {
       throw new Error('It is not allowed to set system:authenticated as admin group.');
@@ -102,8 +123,17 @@ export const isUserAllowed = async (
   username: string,
 ): Promise<boolean> => {
   try {
-    const allowedGroups = getAllowedGroups();
-    const allowedGroupsList = allowedGroups.split(',');
+    const auth = getAuth();
+
+    let allowedGroupsList: string[];
+    if (auth) {
+      allowedGroupsList = auth.spec.allowedGroups;
+    } else {
+      // FIXME: see RHOAIENG-16988
+      const allowedGroups = getAllowedGroups();
+      allowedGroupsList = allowedGroups.split(',');
+    }
+
     if (allowedGroupsList.includes(SYSTEM_AUTHENTICATED)) {
       return true;
     } else {

diff --git a/backend/src/utils/groupsUtils.ts b/backend/src/utils/groupsUtils.ts
@@ -15,13 +15,15 @@ export class MissingGroupError extends Error {
   }
 }
 
+/** @deprecated - see RHOAIENG-16988 */
 export const getGroupsCR = (): GroupsConfigBody => {
   if (getDashboardConfig().spec.groupsConfig) {
     return getDashboardConfig().spec.groupsConfig;
   }
   throw new Error(`Failed to retrieve Dashboard CR groups configuration`);
 };
 
+/** @deprecated - see RHOAIENG-16988 */
 export const updateGroupsCR = async (
   fastify: KubeFastifyInstance,
   groupsConfigBody: GroupsConfigBody,
@@ -38,6 +40,7 @@ export const updateGroupsCR = async (
   }
 };
 
+/** @deprecated - see RHOAIENG-16988 */
 export const getAdminGroups = (): string => {
   try {
     return getGroupsCR().adminGroups;
@@ -46,6 +49,7 @@ export const getAdminGroups = (): string => {
   }
 };
 
+/** @deprecated - see RHOAIENG-16988 */
 export const getAllowedGroups = (): string => {
   try {
     return getGroupsCR().allowedGroups;
@@ -71,6 +75,7 @@ export const getGroup = async (
   }
 };
 
+/** @deprecated - see RHOAIENG-16988 */
 export const getAllGroups = async (customObjectsApi: CustomObjectsApi): Promise<string[]> => {
   try {
     const adminGroupResponse = await customObjectsApi.listClusterCustomObject(

diff --git a/backend/src/utils/resourceUtils.ts b/backend/src/utils/resourceUtils.ts
@@ -22,6 +22,7 @@ import {
   TolerationOperator,
   DataScienceClusterKindStatus,
   KnownLabels,
+  AuthKind,
 } from '../types';
 import {
   DEFAULT_ACTIVE_TIMEOUT,
@@ -50,6 +51,7 @@ const quickStartsVersion = 'v1';
 const quickStartsPlural = 'odhquickstarts';
 
 let dashboardConfigWatcher: ResourceWatcher<DashboardConfig>;
+let authWatcher: ResourceWatcher<AuthKind>;
 let clusterStatusWatcher: ResourceWatcher<DataScienceClusterKindStatus>;
 let subscriptionWatcher: ResourceWatcher<SubscriptionStatusData>;
 let appWatcher: ResourceWatcher<OdhApplication>;
@@ -549,8 +551,16 @@ const fetchConsoleLinks = async (fastify: KubeFastifyInstance) => {
     });
 };
 
+const fetchAuthKind = (fastify: KubeFastifyInstance): Promise<AuthKind[]> => {
+  return fastify.kube.customObjectsApi
+    .getClusterCustomObject('services.platform.opendatahub.io', 'v1alpha1', 'auths', 'auth')
+    .then((response) => response.body as AuthKind)
+    .then((auth) => [auth]);
+};
+
 export const initializeWatchedResources = (fastify: KubeFastifyInstance): void => {
   dashboardConfigWatcher = new ResourceWatcher<DashboardConfig>(fastify, fetchDashboardCR);
+  authWatcher = new ResourceWatcher<AuthKind>(fastify, fetchAuthKind);
   clusterStatusWatcher = new ResourceWatcher<DataScienceClusterKindStatus>(
     fastify,
     fetchWatchedClusterStatus,
@@ -627,6 +637,10 @@ export const getApplication = (appName: string): OdhApplication => {
   return apps.find((app) => app.metadata.name === appName);
 };
 
+export const getAuth = (): AuthKind => {
+  return authWatcher.getResources()[0];
+};
+
 export const getDocs = (): OdhDocument[] => {
   return docWatcher.getResources();
 };

diff --git a/frontend/src/__mocks__/mockAuth.ts b/frontend/src/__mocks__/mockAuth.ts
@@ -0,0 +1,21 @@
+import { AuthKind } from '~/k8sTypes';
+
+type MockAuthData = {
+  adminGroups?: string[];
+  allowedGroups?: string[];
+};
+
+export const mockAuth = ({
+  adminGroups = ['odh-admins'],
+  allowedGroups = ['system:authenticated'],
+}: MockAuthData = {}): AuthKind => ({
+  apiVersion: 'services.platform.opendatahub.io/v1alpha1',
+  kind: 'Auth',
+  metadata: {
+    name: 'auth',
+  },
+  spec: {
+    adminGroups,
+    allowedGroups,
+  },
+});
diff --git a/frontend/src/__mocks__/mockGroupConfig.ts b/frontend/src/__mocks__/mockGroupConfig.ts
@@ -1,4 +1,4 @@
-import { GroupsConfig } from '~/pages/groupSettings/groupTypes';
+import { GroupsConfig } from '~/concepts/userConfigs/groupTypes';
 
 export const mockGroupSettings = (): GroupsConfig => ({
   adminGroups: [

diff --git a/frontend/src/__tests__/cypress/cypress/pages/userManagement.ts b/frontend/src/__tests__/cypress/cypress/pages/userManagement.ts
@@ -16,7 +16,7 @@ class GroupSettingSection extends Contextual<HTMLElement> {
   }
 
   findMultiGroupOptions(name: string) {
-    return this.find().findByRole('option', { name });
+    return this.find().document().findByRole('option', { name });
   }
 
   private findChipGroup() {

diff --git a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts
@@ -24,7 +24,7 @@ import type {
 
 import type { StartNotebookData } from '~/pages/projects/types';
 import type { AllowedUser } from '~/pages/notebookController/screens/admin/types';
-import type { GroupsConfig } from '~/pages/groupSettings/groupTypes';
+import type { GroupsConfig } from '~/concepts/userConfigs/groupTypes';
 import type { StatusResponse } from '~/redux/types';
 import type {
   BYONImage,

diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts
@@ -1,5 +1,6 @@
 // Normal SDK/pass-through network API calls
 export * from './k8s/nimAccounts';
+export * from './k8s/auth';
 export * from './k8s/builds';
 export * from './k8s/configMaps';
 export * from './k8s/events';

diff --git a/frontend/src/api/k8s/auth.ts b/frontend/src/api/k8s/auth.ts
@@ -0,0 +1,34 @@
+import { k8sGetResource, k8sPatchResource, Patch } from '@openshift/dynamic-plugin-sdk-utils';
+import { AuthModel } from '~/api';
+import { AuthKind } from '~/k8sTypes';
+
+export const AUTH_SINGLETON_NAME = 'auth';
+
+export const getAuth = (): Promise<AuthKind> =>
+  k8sGetResource<AuthKind>({ model: AuthModel, queryOptions: { name: AUTH_SINGLETON_NAME } });
+
+export type GroupData = { adminGroups?: string[]; allowedGroups?: string[] };
+export const patchAuth = (groupData: GroupData): Promise<AuthKind> => {
+  const { adminGroups, allowedGroups } = groupData;
+  const patches: Patch[] = [];
+  if (adminGroups) {
+    patches.push({
+      value: adminGroups,
+      op: 'replace',
+      path: '/spec/adminGroups',
+    });
+  }
+  if (allowedGroups) {
+    patches.push({
+      value: allowedGroups,
+      op: 'replace',
+      path: '/spec/allowedGroups',
+    });
+  }
+
+  return k8sPatchResource<AuthKind>({
+    model: AuthModel,
+    queryOptions: { name: AUTH_SINGLETON_NAME },
+    patches,
+  });
+};
diff --git a/frontend/src/api/k8s/groups.ts b/frontend/src/api/k8s/groups.ts
@@ -26,9 +26,12 @@ export const useGroups = (): CustomWatchK8sResult<GroupKind[]> => {
   const [groupData, loaded, error] = useK8sWatchResourceList<GroupKind[]>(initResource, GroupModel);
 
   return React.useMemo(() => {
+    if (!accessReviewLoaded) {
+      return [[], false, undefined];
+    }
     if (!allowList) {
       return [[], true, undefined];
     }
     return [groupData, loaded, error];
-  }, [error, groupData, loaded, allowList]);
+  }, [accessReviewLoaded, allowList, groupData, loaded, error]);
 };
diff --git a/frontend/src/api/models/odh.ts b/frontend/src/api/models/odh.ts
@@ -48,3 +48,10 @@ export const NIMAccountModel: K8sModelCommon = {
   kind: 'Account',
   plural: 'accounts',
 };
+
+export const AuthModel: K8sModelCommon = {
+  apiVersion: 'v1alpha1',
+  apiGroup: 'services.platform.opendatahub.io',
+  kind: 'Auth',
+  plural: 'auths',
+};