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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from
Aug 15, 2024
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5821b4d
feat: allow users with role Viewer and above to view resource quotas
salonig23 Aug 14, 2024
627e42b
allow non admin user to view quotas when RBAC is not enabled
salonig23 Aug 15, 2024
4b0d1cd
revert some earlier changes and use permissions to avoid sending set …
salonig23 Aug 15, 2024
e81eab4
added postgres test
salonig23 Aug 15, 2024
c4de4a9
addressing comments
salonig23 Aug 15, 2024
8690ffb
addressing more comments
salonig23 Aug 15, 2024
1105cdf
add back disabled={true}
salonig23 Aug 15, 2024
1981395
addressed nits
salonig23 Aug 15, 2024
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
View file
Open in desktop

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
View file
Open in desktop
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
79 changes: 77 additions & 2 deletions master/internal/db/postgres_rbac_intg_test.go
View file
Open in desktop
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 = DoesPermissionMatch(ctx, userIDClusterAdmin, &workspaceID,
amandavialva01 marked this conversation as resolved.
Show resolved Hide resolved
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) {
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 Expand Up @@ -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,
salonig23 marked this conversation as resolved.
Show resolved Hide resolved
rbacv1.PermissionType_PERMISSION_TYPE_SET_RESOURCE_QUOTAS)
require.NoError(t, err, "error when searching for permissions")
require.Equal(t, noWorkspaces, workspaces)
})
}

Expand Down
6 changes: 6 additions & 0 deletions master/internal/workspace/authz_basic_impl.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
13 changes: 13 additions & 0 deletions master/static/migrations/20240814123944_add-view-resource-quotas-permissions.tx.up.sql
View file
Open in desktop
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
View file
Open in desktop

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
View file
Open in desktop
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