Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow users with role Viewer and above to view resource quotas #9822

Merged
merged 8 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions harness/determined/common/api/bindings.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions master/internal/api_workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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().CanGetWorkspaceID(
ctx, *curUser, req.Id,

err = workspace.AuthZProvider.Get().CanViewResourceQuotas(
ctx, curUser,
)
if err != nil {
return nil, err
Expand Down
76 changes: 76 additions & 0 deletions master/internal/db/postgres_rbac_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 = DoesPermissionMatchAll(ctx, userIDClusterAdmin,
rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID)
require.NoError(t, err)

err = DoesPermissionMatchAll(ctx, userIDEditor,
rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID)
require.NoError(t, err)

err = DoesPermissionMatchAll(ctx, userIDViewer,
rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID)
require.NoError(t, err)

err = DoesPermissionMatchAll(ctx, userIDEditorProjectRestricted,
rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID)
require.NoError(t, err)

err = DoesPermissionMatchAll(ctx, userIDEditorRestricted,
rbacv1.PermissionType_PERMISSION_TYPE_VIEW_RESOURCE_QUOTAS, workspaceID)
require.NoError(t, err)
})

t.Run("test DoesPermissionMatchAll multiple inputs no failure", func(t *testing.T) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions master/internal/workspace/authz_basic_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ func (a *WorkspaceAuthZBasic) CanSetResourceQuotas(ctx context.Context, curUser
return nil
}

// CanViewResourceQuotas returns a nil error.
func (a *WorkspaceAuthZBasic) CanViewResourceQuotas(ctx context.Context, curUser model.User,
) error {
return nil
}

// CanSetWorkspacesDefaultPools returns a nil error.
func (a *WorkspaceAuthZBasic) CanSetWorkspacesDefaultPools(
ctx context.Context, curUser model.User, workspace *workspacev1.Workspace,
Expand Down
2 changes: 1 addition & 1 deletion master/internal/workspace/authz_iface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions master/internal/workspace/authz_permissive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
}
15 changes: 15 additions & 0 deletions master/internal/workspace/authz_rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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),
salonig23 marked this conversation as resolved.
Show resolved Hide resolved
(11003, 5),
(11003, 7),
(11003, 8),
(11003, 9);
18 changes: 12 additions & 6 deletions proto/pkg/rbacv1/rbac.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions proto/src/determined/rbac/v1/rbac.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading
Loading