From 5821b4d72a9ace9f42ae350b13dac35db274af44 Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Wed, 14 Aug 2024 14:41:55 -0700 Subject: [PATCH 1/8] feat: allow users with role Viewer and above to view resource quotas --- harness/determined/common/api/bindings.py | 2 + master/internal/workspace/authz_basic_impl.go | 9 ++ master/internal/workspace/authz_iface.go | 2 +- master/internal/workspace/authz_permissive.go | 8 ++ master/internal/workspace/authz_rbac.go | 15 +++ ...view-resource-quotas-permissions.tx.up.sql | 13 ++ proto/pkg/rbacv1/rbac.pb.go | 18 ++- proto/src/determined/rbac/v1/rbac.proto | 3 + .../src/components/WorkspaceCreateModal.tsx | 119 +++++++++++++----- webui/react/src/hooks/usePermissions.ts | 15 +++ .../WorkspaceList/WorkspaceActionDropdown.tsx | 4 + webui/react/src/services/api-ts-sdk/api.ts | 3 +- 12 files changed, 175 insertions(+), 36 deletions(-) create mode 100644 master/static/migrations/20240814123944_add-view-resource-quotas-permissions.tx.up.sql diff --git a/harness/determined/common/api/bindings.py b/harness/determined/common/api/bindings.py index c468b0a6dab..5c4695910bb 100644 --- a/harness/determined/common/api/bindings.py +++ b/harness/determined/common/api/bindings.py @@ -10778,6 +10778,7 @@ class v1PermissionType(DetEnum): - PERMISSION_TYPE_MODIFY_RP_WORKSPACE_BINDINGS: Ability to bind, unbind or overwrite resource pool workspace bindings. - PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS: Ability to bind, unbind, or overwrite namespace workspace bindings. - PERMISSION_TYPE_SET_RESOURCE_QUOTAS: Ability to set resource quotas on workspaces. + - PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS: Ability to view resource quotas on workspaces. """ UNSPECIFIED = "PERMISSION_TYPE_UNSPECIFIED" ADMINISTRATE_USER = "PERMISSION_TYPE_ADMINISTRATE_USER" @@ -10828,6 +10829,7 @@ class v1PermissionType(DetEnum): MODIFY_RP_WORKSPACE_BINDINGS = "PERMISSION_TYPE_MODIFY_RP_WORKSPACE_BINDINGS" SET_WORKSPACE_NAMESPACE_BINDINGS = "PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS" SET_RESOURCE_QUOTAS = "PERMISSION_TYPE_SET_RESOURCE_QUOTAS" + VIEW_RESOURCE_QUOTAS = "PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS" class v1PolymorphicFilter(Printable): doubleRange: "typing.Optional[v1DoubleFieldFilter]" = None diff --git a/master/internal/workspace/authz_basic_impl.go b/master/internal/workspace/authz_basic_impl.go index c7179b4481b..ba1d3a8d13a 100644 --- a/master/internal/workspace/authz_basic_impl.go +++ b/master/internal/workspace/authz_basic_impl.go @@ -177,6 +177,15 @@ func (a *WorkspaceAuthZBasic) CanSetResourceQuotas(ctx context.Context, curUser return nil } +// CanViewResourceQuotas returns an error if the user is not a cluster admin. +func (a *WorkspaceAuthZBasic) CanViewResourceQuotas(ctx context.Context, curUser model.User, +) error { + if !curUser.Admin { + return fmt.Errorf("only admins may set resource quotas") + } + return nil +} + // CanSetWorkspacesDefaultPools returns a nil error. func (a *WorkspaceAuthZBasic) CanSetWorkspacesDefaultPools( ctx context.Context, curUser model.User, workspace *workspacev1.Workspace, diff --git a/master/internal/workspace/authz_iface.go b/master/internal/workspace/authz_iface.go index f9f798933ed..cb4b77aa401 100644 --- a/master/internal/workspace/authz_iface.go +++ b/master/internal/workspace/authz_iface.go @@ -46,7 +46,7 @@ type WorkspaceAuthZ interface { CanCreateWorkspaceWithCheckpointStorageConfig(ctx context.Context, curUser model.User) error CanSetWorkspaceNamespaceBindings(ctx context.Context, curUser model.User) error CanSetResourceQuotas(ctx context.Context, curUser model.User) error - + CanViewResourceQuotas(ctx context.Context, curUser model.User) error // PATCH /api/v1/workspaces/:workspace_id CanSetWorkspacesName( ctx context.Context, curUser model.User, workspace *workspacev1.Workspace, diff --git a/master/internal/workspace/authz_permissive.go b/master/internal/workspace/authz_permissive.go index 50e8817216d..df7bfffe93a 100644 --- a/master/internal/workspace/authz_permissive.go +++ b/master/internal/workspace/authz_permissive.go @@ -171,6 +171,14 @@ func (p *WorkspaceAuthZPermissive) CanSetResourceQuotas( return (&WorkspaceAuthZBasic{}).CanSetResourceQuotas(ctx, curUser) } +// CanViewResourceQuotas calls RBAC authz but enforces basic authz. +func (p *WorkspaceAuthZPermissive) CanViewResourceQuotas( + ctx context.Context, curUser model.User, +) error { + _ = (&WorkspaceAuthZRBAC{}).CanViewResourceQuotas(ctx, curUser) + return (&WorkspaceAuthZBasic{}).CanViewResourceQuotas(ctx, curUser) +} + func init() { AuthZProvider.Register("permissive", &WorkspaceAuthZPermissive{}) } diff --git a/master/internal/workspace/authz_rbac.go b/master/internal/workspace/authz_rbac.go index 934e2a01ae4..36c530ec598 100644 --- a/master/internal/workspace/authz_rbac.go +++ b/master/internal/workspace/authz_rbac.go @@ -397,6 +397,21 @@ func (r *WorkspaceAuthZRBAC) CanSetResourceQuotas(ctx context.Context, rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS) } +// CanViewResourceQuotas determines whether a user can view resource quotas on a workspace. +func (r *WorkspaceAuthZRBAC) CanViewResourceQuotas(ctx context.Context, + curUser model.User, +) (err error) { + fields := audit.ExtractLogFields(ctx) + addInfoWithoutWorkspace(curUser, fields, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + defer func() { + audit.LogFromErr(fields, err) + }() + + return db.DoesPermissionMatch(ctx, curUser.ID, nil, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) +} + func hasPermissionOnWorkspace(ctx context.Context, uid model.UserID, workspace *workspacev1.Workspace, permID rbacv1.PermissionType, ) error { diff --git a/master/static/migrations/20240814123944_add-view-resource-quotas-permissions.tx.up.sql b/master/static/migrations/20240814123944_add-view-resource-quotas-permissions.tx.up.sql new file mode 100644 index 00000000000..5c58c3ffcfe --- /dev/null +++ b/master/static/migrations/20240814123944_add-view-resource-quotas-permissions.tx.up.sql @@ -0,0 +1,13 @@ +/* Add RBAC permissions for creating/updating/deleting namespace-workspace bindings and +resource quotas. */ +INSERT into permissions(id, name, global_only) VALUES + (11003, 'view resource quotas', false); + +INSERT INTO permission_assignments(permission_id, role_id) VALUES + (11003, 1), + (11003, 2), + (11003, 4), + (11003, 5), + (11003, 7), + (11003, 8), + (11003, 9); diff --git a/proto/pkg/rbacv1/rbac.pb.go b/proto/pkg/rbacv1/rbac.pb.go index 9d99f0beede..2d540be0fe4 100644 --- a/proto/pkg/rbacv1/rbac.pb.go +++ b/proto/pkg/rbacv1/rbac.pb.go @@ -128,6 +128,8 @@ const ( PermissionType_PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS PermissionType = 11001 // Ability to set resource quotas on workspaces. PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS PermissionType = 11002 + // Ability to view resource quotas on workspaces. + PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS PermissionType = 11003 ) // Enum value maps for PermissionType. @@ -182,6 +184,7 @@ var ( 10001: "PERMISSION_TYPE_MODIFY_RP_WORKSPACE_BINDINGS", 11001: "PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS", 11002: "PERMISSION_TYPE_SET_RESOURCE_QUOTAS", + 11003: "PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS", } PermissionType_value = map[string]int32{ "PERMISSION_TYPE_UNSPECIFIED": 0, @@ -233,6 +236,7 @@ var ( "PERMISSION_TYPE_MODIFY_RP_WORKSPACE_BINDINGS": 10001, "PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS": 11001, "PERMISSION_TYPE_SET_RESOURCE_QUOTAS": 11002, + "PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS": 11003, } ) @@ -889,7 +893,7 @@ var file_determined_rbac_v1_rbac_proto_rawDesc = []byte{ 0x74, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x65, 0x64, 0x2e, 0x72, 0x62, 0x61, 0x63, 0x2e, 0x76, 0x31, 0x2e, 0x55, 0x73, 0x65, 0x72, 0x52, 0x6f, 0x6c, 0x65, 0x41, 0x73, 0x73, 0x69, 0x67, 0x6e, 0x6d, 0x65, 0x6e, 0x74, 0x52, 0x13, 0x75, 0x73, 0x65, 0x72, 0x52, 0x6f, 0x6c, 0x65, 0x41, 0x73, 0x73, - 0x69, 0x67, 0x6e, 0x6d, 0x65, 0x6e, 0x74, 0x73, 0x2a, 0x99, 0x10, 0x0a, 0x0e, 0x50, 0x65, 0x72, + 0x69, 0x67, 0x6e, 0x6d, 0x65, 0x6e, 0x74, 0x73, 0x2a, 0xc4, 0x10, 0x0a, 0x0e, 0x50, 0x65, 0x72, 0x6d, 0x69, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x54, 0x79, 0x70, 0x65, 0x12, 0x1f, 0x0a, 0x1b, 0x50, 0x45, 0x52, 0x4d, 0x49, 0x53, 0x53, 0x49, 0x4f, 0x4e, 0x5f, 0x54, 0x59, 0x50, 0x45, 0x5f, 0x55, 0x4e, 0x53, 0x50, 0x45, 0x43, 0x49, 0x46, 0x49, 0x45, 0x44, 0x10, 0x00, 0x12, 0x27, 0x0a, 0x21, @@ -1019,11 +1023,13 @@ var file_determined_rbac_v1_rbac_proto_rawDesc = []byte{ 0x49, 0x4e, 0x44, 0x49, 0x4e, 0x47, 0x53, 0x10, 0xf9, 0x55, 0x12, 0x28, 0x0a, 0x23, 0x50, 0x45, 0x52, 0x4d, 0x49, 0x53, 0x53, 0x49, 0x4f, 0x4e, 0x5f, 0x54, 0x59, 0x50, 0x45, 0x5f, 0x53, 0x45, 0x54, 0x5f, 0x52, 0x45, 0x53, 0x4f, 0x55, 0x52, 0x43, 0x45, 0x5f, 0x51, 0x55, 0x4f, 0x54, 0x41, - 0x53, 0x10, 0xfa, 0x55, 0x42, 0x36, 0x5a, 0x34, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, - 0x6f, 0x6d, 0x2f, 0x64, 0x65, 0x74, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x65, 0x64, 0x2d, 0x61, 0x69, - 0x2f, 0x64, 0x65, 0x74, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x65, 0x64, 0x2f, 0x70, 0x72, 0x6f, 0x74, - 0x6f, 0x2f, 0x70, 0x6b, 0x67, 0x2f, 0x72, 0x62, 0x61, 0x63, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, - 0x6f, 0x74, 0x6f, 0x33, + 0x53, 0x10, 0xfa, 0x55, 0x12, 0x29, 0x0a, 0x24, 0x50, 0x45, 0x52, 0x4d, 0x49, 0x53, 0x53, 0x49, + 0x4f, 0x4e, 0x5f, 0x54, 0x59, 0x50, 0x45, 0x5f, 0x56, 0x49, 0x45, 0x57, 0x5f, 0x52, 0x45, 0x53, + 0x4f, 0x55, 0x52, 0x43, 0x45, 0x5f, 0x51, 0x55, 0x4f, 0x54, 0x41, 0x53, 0x10, 0xfb, 0x55, 0x42, + 0x36, 0x5a, 0x34, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x64, 0x65, + 0x74, 0x65, 0x72, 0x6d, 0x69, 0x6e, 0x65, 0x64, 0x2d, 0x61, 0x69, 0x2f, 0x64, 0x65, 0x74, 0x65, + 0x72, 0x6d, 0x69, 0x6e, 0x65, 0x64, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x70, 0x6b, 0x67, + 0x2f, 0x72, 0x62, 0x61, 0x63, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/proto/src/determined/rbac/v1/rbac.proto b/proto/src/determined/rbac/v1/rbac.proto index 0c10cbbddba..dad5ff25415 100644 --- a/proto/src/determined/rbac/v1/rbac.proto +++ b/proto/src/determined/rbac/v1/rbac.proto @@ -174,6 +174,9 @@ enum PermissionType { // Ability to set resource quotas on workspaces. PERMISSION_TYPE_SET_RESOURCE_QUOTAS = 11002; + + // Ability to view resource quotas on workspaces. + PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS = 11003; } // RoleAssignmentSummary is used to describe permissions a user has. diff --git a/webui/react/src/components/WorkspaceCreateModal.tsx b/webui/react/src/components/WorkspaceCreateModal.tsx index 40e89bcb86b..4d213f73a5f 100644 --- a/webui/react/src/components/WorkspaceCreateModal.tsx +++ b/webui/react/src/components/WorkspaceCreateModal.tsx @@ -63,6 +63,8 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } canModifyWorkspaceCheckpointStorage, canSetWorkspaceNamespaceBindings, canSetResourceQuotas, + canViewResourceQuotas, + canModifyWorkspace, } = usePermissions(); const info = useObservable(determinedStore.info); const [form] = Form.useForm(); @@ -119,6 +121,8 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } [resourceManagers, workspaceId], ); + const loadedResourceQuotaList = Loadable.getOrElse({}, resourceQuotasList); + const initFields = useCallback( (ws?: Workspace) => { if (ws) { @@ -143,12 +147,12 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } namespaceBindingsList.forEach((bindingsList) => { form.setFieldValue('bindings', { ...bindingsList }); }); - resourceQuotasList.forEach((quotasList) => { - form.setFieldValue('resourceQuotas', { ...quotasList }); - }); + for (const key in loadedResourceQuotaList) { + form.setFieldValue(['resourceQuotas', key], loadedResourceQuotaList[key]); + } } }, - [form, namespaceBindingsList, resourceQuotasList], + [form, loadedResourceQuotaList, namespaceBindingsList], ); const loadableWorkspace = useObservable(workspaceStore.getWorkspace(workspaceId || 0)); @@ -185,7 +189,10 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } required: true, }, ]}> - + {canSetWorkspaceNamespaceBindings && resourceManagers.length > 0 && ( <> @@ -211,7 +218,12 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } name={['bindings', name, 'autoCreateNamespace']} valuePropName="checked"> form.setFieldValue(['resourceQuotas', name], undefined)} + onChange={() => { + form.setFieldValue(['resourceQuotas', name], undefined); + if (loadedResourceQuotaList[name]) { + delete loadedResourceQuotaList[name]; + } + }} /> {canSetResourceQuotas && ( @@ -238,6 +250,25 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } )} + {!canSetWorkspaceNamespaceBindings && + canViewResourceQuotas && + workspaceId && + info.branding === BrandingType.HPE && + resourceManagers.length > 0 && ( + <> + + Resource Quotas + <> + {resourceManagers.map((name) => ( + + + + + + ))} + + + )} {canModifyAUG && ( <> @@ -342,16 +373,20 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } loadableWorkspace, form, idPrefix, + workspace, + canModifyWorkspace, canSetWorkspaceNamespaceBindings, resourceManagers, + canViewResourceQuotas, + info.branding, canModifyAUG, useAgentUser, useAgentGroup, canModifyCPS, useCheckpointStorage, watchBindings, - info.branding, canSetResourceQuotas, + loadedResourceQuotaList, ]); const handleSubmit = useCallback(async () => { @@ -430,8 +465,22 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } resourceQuotaBody['id'] = response.id; } - if (values.resourceQuotas) { - resourceQuotaBody['clusterQuotaPairs'] = pick(values.resourceQuotas, resourceManagers); + let submittedRQ = values.resourceQuotas; + if (loadedResourceQuotaList) { + submittedRQ = Object.keys(values.resourceQuotas ?? {}).reduce( + (memo: Record, name) => { + if (values.resourceQuotas) { + if (values.resourceQuotas[name] !== loadedResourceQuotaList[name]) { + memo[name] = values.resourceQuotas[name]; + } + } + return memo; + }, + {}, + ); + } + if (submittedRQ && Object.keys(submittedRQ).length !== 0) { + resourceQuotaBody['clusterQuotaPairs'] = pick(submittedRQ, resourceManagers); await setResourceQuotas(resourceQuotaBody); } form.resetFields(); @@ -455,26 +504,40 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } }); } } - }, [form, workspaceId, canModifyAUG, canModifyCPS, resourceManagers]); + }, [form, canModifyAUG, canModifyCPS, workspaceId, loadedResourceQuotaList, resourceManagers]); - return ( - { - initFields(undefined); - onClose?.(); - }}> - {modalContent} - - ); + if ((workspace && !canModifyWorkspace({ workspace })) ?? false) { + return ( + { + initFields(undefined); + onClose?.(); + }}> + {modalContent} + + ); + } else { + return ( + { + initFields(undefined); + onClose?.(); + }}> + {modalContent} + + ); + } }; export default WorkspaceCreateModalComponent; diff --git a/webui/react/src/hooks/usePermissions.ts b/webui/react/src/hooks/usePermissions.ts index 2ab66d3ad12..aae97a16da3 100644 --- a/webui/react/src/hooks/usePermissions.ts +++ b/webui/react/src/hooks/usePermissions.ts @@ -106,6 +106,7 @@ export interface PermissionsHook { canViewModelRegistry: (arg0: WorkspacePermissionsArgs) => boolean; canViewWorkspace: (arg0: WorkspacePermissionsArgs) => boolean; canViewWorkspaces: boolean; + canViewResourceQuotas: boolean; loading: boolean; } @@ -208,6 +209,7 @@ const usePermissions = (): PermissionsHook => { canViewGroups: canViewGroups(rbacOpts), canViewModelRegistry: (args: WorkspacePermissionsArgs) => canViewModelRegistry(rbacOpts, args.workspace), + canViewResourceQuotas: canViewResourceQuotas(rbacOpts), canViewWorkspace: (args: WorkspacePermissionsArgs) => canViewWorkspace(rbacOpts, args.workspace), canViewWorkspaces: canViewWorkspaces(rbacOpts), @@ -626,6 +628,19 @@ const canSetResourceQuotas = ({ ); }; +const canViewResourceQuotas = ({ + currentUser, + rbacEnabled, + userAssignments, + userRoles, +}: RbacOptsProps): boolean => { + const permitted = relevantPermissions(userAssignments, userRoles); + return ( + !!currentUser && + (rbacEnabled ? permitted.has(V1PermissionType.VIEWRESOURCEQUOTAS) : currentUser.isAdmin) + ); +}; + const canViewWorkspace = ( { rbacEnabled, userAssignments, userRoles }: RbacOptsProps, workspace?: PermissionWorkspace, diff --git a/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx b/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx index 98812825181..70ce216c7fa 100644 --- a/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx +++ b/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx @@ -153,6 +153,10 @@ export const useWorkspaceActionMenu: (props: WorkspaceMenuPropsIn) => WorkspaceM key: MenuKey.SwitchArchived, label: workspace.archived ? 'Unarchive' : 'Archive', }); + } else { + if (!workspace.archived) { + menuItems.push({ key: MenuKey.Edit, label: 'Config' }); + } } if (canDeleteWorkspace({ workspace }) && workspace.numExperiments === 0) { menuItems.push({ type: 'divider' }); diff --git a/webui/react/src/services/api-ts-sdk/api.ts b/webui/react/src/services/api-ts-sdk/api.ts index a68fa0544ec..732b5397f95 100644 --- a/webui/react/src/services/api-ts-sdk/api.ts +++ b/webui/react/src/services/api-ts-sdk/api.ts @@ -7830,7 +7830,7 @@ export interface V1Permission { scopeTypeMask?: V1ScopeTypeMask; } /** - * List of permissions types. Value of the enum has 9xxxx for global only permissions. Permissions on the same object share the thousands place value like 2001 and 2002. - PERMISSION_TYPE_UNSPECIFIED: The permission type is unknown. - PERMISSION_TYPE_ADMINISTRATE_USER: Can create and update other users. Allows updating other users passwords making this permission give all other permissions effectively. - PERMISSION_TYPE_ADMINISTRATE_OAUTH: Ability to manage OAuth clients and settings. - PERMISSION_TYPE_CREATE_EXPERIMENT: Ability to create experiments. - PERMISSION_TYPE_VIEW_EXPERIMENT_ARTIFACTS: Ability to view experiment's model code, checkpoints, trials. - PERMISSION_TYPE_VIEW_EXPERIMENT_METADATA: Ability to view experiment's metadata such as experiment config, progress. - PERMISSION_TYPE_UPDATE_EXPERIMENT: Ability to update experiment and experiment's lifecycle. - PERMISSION_TYPE_UPDATE_EXPERIMENT_METADATA: Ability to update experiment's metadata. - PERMISSION_TYPE_DELETE_EXPERIMENT: Ability to delete experiment. - PERMISSION_TYPE_CREATE_NSC: Ability to create Notebooks, Shells, and Commands. - PERMISSION_TYPE_VIEW_NSC: Ability to view Notebooks, Shells, and Commands. - PERMISSION_TYPE_UPDATE_NSC: Ability to terminate Notebooks, Shells, and Commands. - PERMISSION_TYPE_UPDATE_GROUP: Ability to create, update, and add / remove users from groups. - PERMISSION_TYPE_CREATE_WORKSPACE: Ability to create workspaces. - PERMISSION_TYPE_VIEW_WORKSPACE: Ability to view workspace. - PERMISSION_TYPE_UPDATE_WORKSPACE: Ability to update workspace. - PERMISSION_TYPE_DELETE_WORKSPACE: Ability to delete workspace. - PERMISSION_TYPE_SET_WORKSPACE_AGENT_USER_GROUP: Ability to set workspace agent user group config. - PERMISSION_TYPE_SET_WORKSPACE_CHECKPOINT_STORAGE_CONFIG: Ability to set workspace checkpoint storage config. - PERMISSION_TYPE_SET_WORKSPACE_DEFAULT_RESOURCE_POOL: Ability to set workspace default resource pool. - PERMISSION_TYPE_CREATE_PROJECT: Ability to create projects. - PERMISSION_TYPE_VIEW_PROJECT: Ability to view projects. - PERMISSION_TYPE_UPDATE_PROJECT: Ability to update projects. - PERMISSION_TYPE_DELETE_PROJECT: Ability to delete projects. - PERMISSION_TYPE_ASSIGN_ROLES: Ability to assign roles to groups / users. If assigned at a workspace scope, can only assign roles to that workspace scope. - PERMISSION_TYPE_VIEW_MODEL_REGISTRY: Ability to view model registry. - PERMISSION_TYPE_EDIT_MODEL_REGISTRY: Ability to edit model registry. - PERMISSION_TYPE_CREATE_MODEL_REGISTRY: Ability to create model registry. - PERMISSION_TYPE_DELETE_MODEL_REGISTRY: Ability to delete model registry. - PERMISSION_TYPE_DELETE_MODEL_VERSION: Ability to delete model version. - PERMISSION_TYPE_DELETE_OTHER_USER_MODEL_REGISTRY: Ability to delete another user's model registry. - PERMISSION_TYPE_DELETE_OTHER_USER_MODEL_VERSION: Ability to delete another user's model version. - PERMISSION_TYPE_VIEW_MASTER_LOGS: Ability to view master logs. - PERMISSION_TYPE_VIEW_CLUSTER_USAGE: Ability to view detailed cluster usage info. - PERMISSION_TYPE_UPDATE_AGENTS: Ability to update agents. - PERMISSION_TYPE_VIEW_SENSITIVE_AGENT_INFO: Ability to view sensitive subset of agent info. - PERMISSION_TYPE_VIEW_MASTER_CONFIG: Ability to view master configs. - PERMISSION_TYPE_UPDATE_MASTER_CONFIG: Ability to update master configs. - PERMISSION_TYPE_VIEW_EXTERNAL_JOBS: Ability to view external jobs. - PERMISSION_TYPE_CONTROL_STRICT_JOB_QUEUE: Ability to control strict job queue. - PERMISSION_TYPE_VIEW_TEMPLATES: Ability to view templates. - PERMISSION_TYPE_UPDATE_TEMPLATES: Ability to update templates. - PERMISSION_TYPE_CREATE_TEMPLATES: Ability to create templates. - PERMISSION_TYPE_DELETE_TEMPLATES: Ability to delete templates. - PERMISSION_TYPE_UPDATE_ROLES: Ability to create and update role definitions. - PERMISSION_TYPE_EDIT_WEBHOOKS: Ability to create and delete webhooks. - PERMISSION_TYPE_MODIFY_RP_WORKSPACE_BINDINGS: Ability to bind, unbind or overwrite resource pool workspace bindings. - PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS: Ability to bind, unbind, or overwrite namespace workspace bindings. - PERMISSION_TYPE_SET_RESOURCE_QUOTAS: Ability to set resource quotas on workspaces. + * List of permissions types. Value of the enum has 9xxxx for global only permissions. Permissions on the same object share the thousands place value like 2001 and 2002. - PERMISSION_TYPE_UNSPECIFIED: The permission type is unknown. - PERMISSION_TYPE_ADMINISTRATE_USER: Can create and update other users. Allows updating other users passwords making this permission give all other permissions effectively. - PERMISSION_TYPE_ADMINISTRATE_OAUTH: Ability to manage OAuth clients and settings. - PERMISSION_TYPE_CREATE_EXPERIMENT: Ability to create experiments. - PERMISSION_TYPE_VIEW_EXPERIMENT_ARTIFACTS: Ability to view experiment's model code, checkpoints, trials. - PERMISSION_TYPE_VIEW_EXPERIMENT_METADATA: Ability to view experiment's metadata such as experiment config, progress. - PERMISSION_TYPE_UPDATE_EXPERIMENT: Ability to update experiment and experiment's lifecycle. - PERMISSION_TYPE_UPDATE_EXPERIMENT_METADATA: Ability to update experiment's metadata. - PERMISSION_TYPE_DELETE_EXPERIMENT: Ability to delete experiment. - PERMISSION_TYPE_CREATE_NSC: Ability to create Notebooks, Shells, and Commands. - PERMISSION_TYPE_VIEW_NSC: Ability to view Notebooks, Shells, and Commands. - PERMISSION_TYPE_UPDATE_NSC: Ability to terminate Notebooks, Shells, and Commands. - PERMISSION_TYPE_UPDATE_GROUP: Ability to create, update, and add / remove users from groups. - PERMISSION_TYPE_CREATE_WORKSPACE: Ability to create workspaces. - PERMISSION_TYPE_VIEW_WORKSPACE: Ability to view workspace. - PERMISSION_TYPE_UPDATE_WORKSPACE: Ability to update workspace. - PERMISSION_TYPE_DELETE_WORKSPACE: Ability to delete workspace. - PERMISSION_TYPE_SET_WORKSPACE_AGENT_USER_GROUP: Ability to set workspace agent user group config. - PERMISSION_TYPE_SET_WORKSPACE_CHECKPOINT_STORAGE_CONFIG: Ability to set workspace checkpoint storage config. - PERMISSION_TYPE_SET_WORKSPACE_DEFAULT_RESOURCE_POOL: Ability to set workspace default resource pool. - PERMISSION_TYPE_CREATE_PROJECT: Ability to create projects. - PERMISSION_TYPE_VIEW_PROJECT: Ability to view projects. - PERMISSION_TYPE_UPDATE_PROJECT: Ability to update projects. - PERMISSION_TYPE_DELETE_PROJECT: Ability to delete projects. - PERMISSION_TYPE_ASSIGN_ROLES: Ability to assign roles to groups / users. If assigned at a workspace scope, can only assign roles to that workspace scope. - PERMISSION_TYPE_VIEW_MODEL_REGISTRY: Ability to view model registry. - PERMISSION_TYPE_EDIT_MODEL_REGISTRY: Ability to edit model registry. - PERMISSION_TYPE_CREATE_MODEL_REGISTRY: Ability to create model registry. - PERMISSION_TYPE_DELETE_MODEL_REGISTRY: Ability to delete model registry. - PERMISSION_TYPE_DELETE_MODEL_VERSION: Ability to delete model version. - PERMISSION_TYPE_DELETE_OTHER_USER_MODEL_REGISTRY: Ability to delete another user's model registry. - PERMISSION_TYPE_DELETE_OTHER_USER_MODEL_VERSION: Ability to delete another user's model version. - PERMISSION_TYPE_VIEW_MASTER_LOGS: Ability to view master logs. - PERMISSION_TYPE_VIEW_CLUSTER_USAGE: Ability to view detailed cluster usage info. - PERMISSION_TYPE_UPDATE_AGENTS: Ability to update agents. - PERMISSION_TYPE_VIEW_SENSITIVE_AGENT_INFO: Ability to view sensitive subset of agent info. - PERMISSION_TYPE_VIEW_MASTER_CONFIG: Ability to view master configs. - PERMISSION_TYPE_UPDATE_MASTER_CONFIG: Ability to update master configs. - PERMISSION_TYPE_VIEW_EXTERNAL_JOBS: Ability to view external jobs. - PERMISSION_TYPE_CONTROL_STRICT_JOB_QUEUE: Ability to control strict job queue. - PERMISSION_TYPE_VIEW_TEMPLATES: Ability to view templates. - PERMISSION_TYPE_UPDATE_TEMPLATES: Ability to update templates. - PERMISSION_TYPE_CREATE_TEMPLATES: Ability to create templates. - PERMISSION_TYPE_DELETE_TEMPLATES: Ability to delete templates. - PERMISSION_TYPE_UPDATE_ROLES: Ability to create and update role definitions. - PERMISSION_TYPE_EDIT_WEBHOOKS: Ability to create and delete webhooks. - PERMISSION_TYPE_MODIFY_RP_WORKSPACE_BINDINGS: Ability to bind, unbind or overwrite resource pool workspace bindings. - PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS: Ability to bind, unbind, or overwrite namespace workspace bindings. - PERMISSION_TYPE_SET_RESOURCE_QUOTAS: Ability to set resource quotas on workspaces. - PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS: Ability to view resource quotas on workspaces. * @export * @enum {string} */ @@ -7884,6 +7884,7 @@ export const V1PermissionType = { MODIFYRPWORKSPACEBINDINGS: 'PERMISSION_TYPE_MODIFY_RP_WORKSPACE_BINDINGS', SETWORKSPACENAMESPACEBINDINGS: 'PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS', SETRESOURCEQUOTAS: 'PERMISSION_TYPE_SET_RESOURCE_QUOTAS', + VIEWRESOURCEQUOTAS: 'PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS', } as const export type V1PermissionType = ValueOf /** From 627e42be900b4c0a8ee8669c306810c3bf2fcc38 Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Wed, 14 Aug 2024 17:45:54 -0700 Subject: [PATCH 2/8] allow non admin user to view quotas when RBAC is not enabled --- master/internal/api_workspace.go | 4 ++-- master/internal/workspace/authz_basic_impl.go | 5 +---- webui/react/src/hooks/usePermissions.ts | 5 +---- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/master/internal/api_workspace.go b/master/internal/api_workspace.go index d65c9532d87..2388f844b9b 100644 --- a/master/internal/api_workspace.go +++ b/master/internal/api_workspace.go @@ -1558,8 +1558,8 @@ func (a *apiServer) GetKubernetesResourceQuotas( if err != nil { return nil, err } - err = workspace.AuthZProvider.Get().CanGetWorkspaceID( - ctx, *curUser, req.Id, + err = workspace.AuthZProvider.Get().CanViewResourceQuotas( + ctx, *curUser, ) if err != nil { return nil, err diff --git a/master/internal/workspace/authz_basic_impl.go b/master/internal/workspace/authz_basic_impl.go index ba1d3a8d13a..fea3692da41 100644 --- a/master/internal/workspace/authz_basic_impl.go +++ b/master/internal/workspace/authz_basic_impl.go @@ -177,12 +177,9 @@ func (a *WorkspaceAuthZBasic) CanSetResourceQuotas(ctx context.Context, curUser return nil } -// CanViewResourceQuotas returns an error if the user is not a cluster admin. +// CanViewResourceQuotas returns a nil error. func (a *WorkspaceAuthZBasic) CanViewResourceQuotas(ctx context.Context, curUser model.User, ) error { - if !curUser.Admin { - return fmt.Errorf("only admins may set resource quotas") - } return nil } diff --git a/webui/react/src/hooks/usePermissions.ts b/webui/react/src/hooks/usePermissions.ts index aae97a16da3..e27f1cb54b2 100644 --- a/webui/react/src/hooks/usePermissions.ts +++ b/webui/react/src/hooks/usePermissions.ts @@ -635,10 +635,7 @@ const canViewResourceQuotas = ({ userRoles, }: RbacOptsProps): boolean => { const permitted = relevantPermissions(userAssignments, userRoles); - return ( - !!currentUser && - (rbacEnabled ? permitted.has(V1PermissionType.VIEWRESOURCEQUOTAS) : currentUser.isAdmin) - ); + return !!currentUser && (!rbacEnabled || permitted.has(V1PermissionType.VIEWRESOURCEQUOTAS)); }; const canViewWorkspace = ( From 4b0d1cdfe3f10d4fe247666f0209abdbf807c8a6 Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Thu, 15 Aug 2024 12:59:09 -0700 Subject: [PATCH 3/8] revert some earlier changes and use permissions to avoid sending set rq API req --- .../src/components/WorkspaceCreateModal.tsx | 126 +++++++----------- 1 file changed, 51 insertions(+), 75 deletions(-) diff --git a/webui/react/src/components/WorkspaceCreateModal.tsx b/webui/react/src/components/WorkspaceCreateModal.tsx index 4d213f73a5f..9d6664306f3 100644 --- a/webui/react/src/components/WorkspaceCreateModal.tsx +++ b/webui/react/src/components/WorkspaceCreateModal.tsx @@ -121,8 +121,6 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } [resourceManagers, workspaceId], ); - const loadedResourceQuotaList = Loadable.getOrElse({}, resourceQuotasList); - const initFields = useCallback( (ws?: Workspace) => { if (ws) { @@ -147,16 +145,18 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } namespaceBindingsList.forEach((bindingsList) => { form.setFieldValue('bindings', { ...bindingsList }); }); - for (const key in loadedResourceQuotaList) { - form.setFieldValue(['resourceQuotas', key], loadedResourceQuotaList[key]); - } + resourceQuotasList.forEach((quotasList) => { + form.setFieldValue('resourceQuotas', { ...quotasList }); + }); } }, - [form, loadedResourceQuotaList, namespaceBindingsList], + [form, namespaceBindingsList, resourceQuotasList], ); const loadableWorkspace = useObservable(workspaceStore.getWorkspace(workspaceId || 0)); const workspace = Loadable.getOrElse(undefined, loadableWorkspace); + const isViewing = !!workspace && !canModifyWorkspace({ workspace }); + const isEditing = workspaceId !== undefined; useEffect(() => { initFields(workspace || undefined); @@ -171,7 +171,7 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } }, [canModifyWorkspaceAgentUserGroup, canModifyWorkspaceCheckpointStorage, workspaceId]); const modalContent = useMemo(() => { - if (workspaceId && loadableWorkspace === NotLoaded) return ; + if (isEditing && loadableWorkspace === NotLoaded) return ; return (
= ({ onClose, workspaceId } required: true, }, ]}> - + {canSetWorkspaceNamespaceBindings && resourceManagers.length > 0 && ( <> @@ -218,12 +215,7 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } name={['bindings', name, 'autoCreateNamespace']} valuePropName="checked"> { - form.setFieldValue(['resourceQuotas', name], undefined); - if (loadedResourceQuotaList[name]) { - delete loadedResourceQuotaList[name]; - } - }} + onChange={() => form.setFieldValue(['resourceQuotas', name], undefined)} /> {canSetResourceQuotas && ( @@ -252,7 +244,7 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } )} {!canSetWorkspaceNamespaceBindings && canViewResourceQuotas && - workspaceId && + isEditing && info.branding === BrandingType.HPE && resourceManagers.length > 0 && ( <> @@ -261,9 +253,7 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } <> {resourceManagers.map((name) => ( - - - + ))} @@ -369,12 +359,11 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } ); }, [ - workspaceId, loadableWorkspace, form, idPrefix, - workspace, - canModifyWorkspace, + isEditing, + isViewing, canSetWorkspaceNamespaceBindings, resourceManagers, canViewResourceQuotas, @@ -386,7 +375,6 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } useCheckpointStorage, watchBindings, canSetResourceQuotas, - loadedResourceQuotaList, ]); const handleSubmit = useCallback(async () => { @@ -455,7 +443,7 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } } } - if (workspaceId) { + if (isEditing) { await patchWorkspace({ id: workspaceId, ...body }); workspaceStore.fetch(undefined, true); resourceQuotaBody['id'] = workspaceId; @@ -465,22 +453,12 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } resourceQuotaBody['id'] = response.id; } - let submittedRQ = values.resourceQuotas; - if (loadedResourceQuotaList) { - submittedRQ = Object.keys(values.resourceQuotas ?? {}).reduce( - (memo: Record, name) => { - if (values.resourceQuotas) { - if (values.resourceQuotas[name] !== loadedResourceQuotaList[name]) { - memo[name] = values.resourceQuotas[name]; - } - } - return memo; - }, - {}, - ); - } - if (submittedRQ && Object.keys(submittedRQ).length !== 0) { - resourceQuotaBody['clusterQuotaPairs'] = pick(submittedRQ, resourceManagers); + if ( + canSetResourceQuotas && + values.resourceQuotas && + Object.keys(values.resourceQuotas).length !== 0 + ) { + resourceQuotaBody['clusterQuotaPairs'] = pick(values.resourceQuotas, resourceManagers); await setResourceQuotas(resourceQuotaBody); } form.resetFields(); @@ -504,40 +482,38 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } }); } } - }, [form, canModifyAUG, canModifyCPS, workspaceId, loadedResourceQuotaList, resourceManagers]); + }, [ + form, + canModifyAUG, + canModifyCPS, + canSetResourceQuotas, + isEditing, + workspaceId, + resourceManagers, + ]); - if ((workspace && !canModifyWorkspace({ workspace })) ?? false) { - return ( - { - initFields(undefined); - onClose?.(); - }}> - {modalContent} - - ); - } else { - return ( - { - initFields(undefined); - onClose?.(); - }}> - {modalContent} - - ); - } + return ( + { + initFields(undefined); + onClose?.(); + }}> + {modalContent} + + ); }; export default WorkspaceCreateModalComponent; From e81eab47cbcb6e13cea9c65c87f0f2624a5f4ef3 Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Thu, 15 Aug 2024 14:02:39 -0700 Subject: [PATCH 4/8] added postgres test --- master/internal/api_workspace.go | 7 +- master/internal/db/postgres_rbac_intg_test.go | 79 ++++++++++++++++++- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/master/internal/api_workspace.go b/master/internal/api_workspace.go index 2388f844b9b..63ffb331e0c 100644 --- a/master/internal/api_workspace.go +++ b/master/internal/api_workspace.go @@ -1554,12 +1554,15 @@ func (a *apiServer) ListRPsBoundToWorkspace( func (a *apiServer) GetKubernetesResourceQuotas( ctx context.Context, req *apiv1.GetKubernetesResourceQuotasRequest, ) (*apiv1.GetKubernetesResourceQuotasResponse, error) { - curUser, _, err := grpcutil.GetUser(ctx) + _, curUser, err := a.getWorkspaceAndCheckCanDoActions( + ctx, req.Id, false, workspace.AuthZProvider.Get().CanGetWorkspace, + ) if err != nil { return nil, err } + err = workspace.AuthZProvider.Get().CanViewResourceQuotas( - ctx, *curUser, + ctx, curUser, ) if err != nil { return nil, err diff --git a/master/internal/db/postgres_rbac_intg_test.go b/master/internal/db/postgres_rbac_intg_test.go index 198b99d2fb9..99f878727e6 100644 --- a/master/internal/db/postgres_rbac_intg_test.go +++ b/master/internal/db/postgres_rbac_intg_test.go @@ -334,6 +334,26 @@ func TestPermissionMatch(t *testing.T) { require.IsType(t, authz.PermissionDeniedError{}, err, "user should not have permission to set resource quotas") + err = DoesPermissionMatch(ctx, userIDClusterAdmin, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDEditor, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDViewer, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDEditorProjectRestricted, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDEditorRestricted, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + err = DoesPermissionMatch(ctx, userIDClusterAdmin, &workspaceID, rbacv1.PermissionType_PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS) require.NoError(t, err) @@ -402,6 +422,26 @@ func TestPermissionMatch(t *testing.T) { err = DoesPermissionMatchAll(ctx, userIDClusterAdmin, rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS, workspaceID) require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDClusterAdmin, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDEditor, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDViewer, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDEditorProjectRestricted, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoesPermissionMatch(ctx, userIDEditorRestricted, &workspaceID, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) }) t.Run("test DoesPermissionMatchAll multiple inputs no failure", func(t *testing.T) { @@ -421,6 +461,26 @@ func TestPermissionMatch(t *testing.T) { err = DoesPermissionMatchAll(ctx, userIDClusterAdmin, rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS, workspaceIDs...) require.NoError(t, err, "error when searching for permissions") + + err = DoesPermissionMatchAll(ctx, userIDClusterAdmin, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceIDs...) + require.NoError(t, err) + + err = DoesPermissionMatchAll(ctx, userIDEditor, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceIDs...) + require.NoError(t, err) + + err = DoesPermissionMatchAll(ctx, userIDViewer, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceIDs...) + require.NoError(t, err) + + err = DoesPermissionMatchAll(ctx, userIDEditorProjectRestricted, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceIDs...) + require.NoError(t, err) + + err = DoesPermissionMatchAll(ctx, userIDEditorRestricted, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceIDs...) + require.NoError(t, err) }) t.Run("test DoesPermissionMatchAll multiple inputs single failure", func(t *testing.T) { @@ -497,6 +557,22 @@ func TestPermissionMatch(t *testing.T) { rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS) require.NoError(t, err) + err = DoPermissionsExist(ctx, userIDEditor, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoPermissionsExist(ctx, userIDEditorRestricted, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoPermissionsExist(ctx, userIDEditorProjectRestricted, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + + err = DoPermissionsExist(ctx, userIDViewer, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + require.NoError(t, err) + err = DoPermissionsExist(ctx, userIDEditorRestricted, rbacv1.PermissionType_PERMISSION_TYPE_UPDATE_NSC) require.IsType(t, authz.PermissionDeniedError{}, err, @@ -546,10 +622,9 @@ func TestPermissionMatch(t *testing.T) { require.NoError(t, err, "error when searching for permissions") require.Equal(t, noWorkspaces, workspaces) - workspaces, err = GetNonGlobalWorkspacesWithPermission(ctx, userIDEditorRestricted, + _, err = GetNonGlobalWorkspacesWithPermission(ctx, userIDEditorRestricted, rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS) require.NoError(t, err, "error when searching for permissions") - require.Equal(t, noWorkspaces, workspaces) }) } From c4de4a9a70ed928223b012f214e96821e20a92bd Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Thu, 15 Aug 2024 14:12:50 -0700 Subject: [PATCH 5/8] addressing comments --- master/internal/db/postgres_rbac_intg_test.go | 20 +++++++++---------- .../WorkspaceList/WorkspaceActionDropdown.tsx | 6 ++---- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/master/internal/db/postgres_rbac_intg_test.go b/master/internal/db/postgres_rbac_intg_test.go index 99f878727e6..838d57ed3f4 100644 --- a/master/internal/db/postgres_rbac_intg_test.go +++ b/master/internal/db/postgres_rbac_intg_test.go @@ -423,24 +423,24 @@ func TestPermissionMatch(t *testing.T) { rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS, workspaceID) require.NoError(t, err) - err = DoesPermissionMatch(ctx, userIDClusterAdmin, &workspaceID, - rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + err = DoesPermissionMatchAll(ctx, userIDClusterAdmin, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID) require.NoError(t, err) - err = DoesPermissionMatch(ctx, userIDEditor, &workspaceID, - rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + err = DoesPermissionMatchAll(ctx, userIDEditor, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID) require.NoError(t, err) - err = DoesPermissionMatch(ctx, userIDViewer, &workspaceID, - rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + err = DoesPermissionMatchAll(ctx, userIDViewer, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID) require.NoError(t, err) - err = DoesPermissionMatch(ctx, userIDEditorProjectRestricted, &workspaceID, - rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + err = DoesPermissionMatchAll(ctx, userIDEditorProjectRestricted, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID) require.NoError(t, err) - err = DoesPermissionMatch(ctx, userIDEditorRestricted, &workspaceID, - rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS) + err = DoesPermissionMatchAll(ctx, userIDEditorRestricted, + rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID) require.NoError(t, err) }) diff --git a/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx b/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx index 70ce216c7fa..99784d5adbd 100644 --- a/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx +++ b/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx @@ -153,10 +153,8 @@ export const useWorkspaceActionMenu: (props: WorkspaceMenuPropsIn) => WorkspaceM key: MenuKey.SwitchArchived, label: workspace.archived ? 'Unarchive' : 'Archive', }); - } else { - if (!workspace.archived) { - menuItems.push({ key: MenuKey.Edit, label: 'Config' }); - } + } else if (!workspace.archived) { + menuItems.push({ key: MenuKey.Edit, label: 'View Config' }); } if (canDeleteWorkspace({ workspace }) && workspace.numExperiments === 0) { menuItems.push({ type: 'divider' }); From 8690ffb1807c4f0b93ad29f4b17002c340eaac29 Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Thu, 15 Aug 2024 14:20:04 -0700 Subject: [PATCH 6/8] addressing more comments --- master/internal/db/postgres_rbac_intg_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/master/internal/db/postgres_rbac_intg_test.go b/master/internal/db/postgres_rbac_intg_test.go index 838d57ed3f4..d74c03fae88 100644 --- a/master/internal/db/postgres_rbac_intg_test.go +++ b/master/internal/db/postgres_rbac_intg_test.go @@ -622,9 +622,10 @@ func TestPermissionMatch(t *testing.T) { require.NoError(t, err, "error when searching for permissions") require.Equal(t, noWorkspaces, workspaces) - _, err = GetNonGlobalWorkspacesWithPermission(ctx, userIDEditorRestricted, + workspaces, err = GetNonGlobalWorkspacesWithPermission(ctx, userIDEditorRestricted, rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS) require.NoError(t, err, "error when searching for permissions") + require.Equal(t, noWorkspaces, workspaces) }) } From 1105cdfdcab6213c93fdde163ce4ebe9384ec07b Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Thu, 15 Aug 2024 14:34:26 -0700 Subject: [PATCH 7/8] add back disabled={true} --- webui/react/src/components/WorkspaceCreateModal.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/webui/react/src/components/WorkspaceCreateModal.tsx b/webui/react/src/components/WorkspaceCreateModal.tsx index 9d6664306f3..402c1c8afdf 100644 --- a/webui/react/src/components/WorkspaceCreateModal.tsx +++ b/webui/react/src/components/WorkspaceCreateModal.tsx @@ -253,7 +253,9 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } <> {resourceManagers.map((name) => ( - + + + ))} From 19813959df55e76042c37e6618d6a1410de20c9f Mon Sep 17 00:00:00 2001 From: Saloni Gupta Date: Thu, 15 Aug 2024 15:20:08 -0700 Subject: [PATCH 8/8] addressed nits --- webui/react/src/components/WorkspaceCreateModal.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/webui/react/src/components/WorkspaceCreateModal.tsx b/webui/react/src/components/WorkspaceCreateModal.tsx index 402c1c8afdf..43699454381 100644 --- a/webui/react/src/components/WorkspaceCreateModal.tsx +++ b/webui/react/src/components/WorkspaceCreateModal.tsx @@ -252,11 +252,12 @@ const WorkspaceCreateModalComponent: React.FC = ({ onClose, workspaceId } Resource Quotas <> {resourceManagers.map((name) => ( - - - - - + + + ))}