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: add editor project restricted role and testing [DET-10428] #9796

Merged
merged 2 commits into from
Aug 6, 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
11 changes: 11 additions & 0 deletions docs/release-notes/editor-project-restricted.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
:orphan:

**New Features**

- RBAC: Add a pre-canned role called ``EditorProjectRestricted`` which supersedes the ``Viewer``
role and precedes the ``Editor`` role.

- Like the ``Editor`` role, the ``EditorProjectRestricted`` role grants the permissions to
create, edit, or delete experiments and NSC (Notebook, Shell or Command) type workloads within
its designated scope. However, the ``EditorProjectRestricted`` role lacks the permissions to
create or update projects.
110 changes: 99 additions & 11 deletions master/internal/db/postgres_rbac_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,29 @@ var (
)

var (
userModelViewer model.User
userModelEditor model.User
userModelEditorRestricted model.User
userModelClusterAdmin model.User
userModelViewer model.User
userModelEditor model.User
userModelEditorRestricted model.User
userModelEditorProjectRestricted model.User
userModelClusterAdmin model.User
)

var roles = map[string]int{
"ClusterAdmin": 1,
"WorkspaceAdmin": 2,
"WorkspaceCreator": 3,
"Viewer": 4,
"Editor": 5,
"ModelRegistryViewer": 6,
"EditorRestricted": 7,
"ClusterAdmin": 1,
"WorkspaceAdmin": 2,
"WorkspaceCreator": 3,
"Viewer": 4,
"Editor": 5,
"ModelRegistryViewer": 6,
"EditorRestricted": 7,
"EditorProjectRestricted": 9,
}

var wsIDs,
viewerGroupIDs,
editorGroupIDs,
editorRestrictedGroupIDs,
editorProjectRestrictedGroupIDs,
clusterAdminGroupIDs []int

func setup(t *testing.T, pgDB *PgDB) {
Expand All @@ -62,6 +65,8 @@ func setup(t *testing.T, pgDB *PgDB) {
editorGroupName := fmt.Sprintf("test_group_editor_%s", nameExt)
editorRestrictedGroupName := fmt.Sprintf("test_group_editor_restricted_%s",
nameExt)
editorProjectRestrictedGroupName := fmt.Sprintf("test_group_editor_project_restricted_%s",
nameExt)
clusterAdminGroupName := fmt.Sprintf("test_group_cluster_admin_%s", nameExt)

wsName := fmt.Sprintf("test_workspace_permissions_%s", nameExt)
Expand Down Expand Up @@ -109,6 +114,17 @@ func setup(t *testing.T, pgDB *PgDB) {
_, err = Bun().NewInsert().Model(&raData).Table("role_assignments").Exec(ctx)
require.NoError(t, err, "error inserting editor-restricted role assignment")

grp = &model.Group{Name: editorProjectRestrictedGroupName}
_, err = Bun().NewInsert().Model(grp).Returning("id").Exec(ctx)
require.NoError(t, err, "error inserting editor-project-restricted group")
editorProjectRestrictedGroupIDs = append(editorProjectRestrictedGroupIDs, grp.ID)

raData["group_id"] = grp.ID
raData["role_id"] = roles["EditorProjectRestricted"]
raData["scope_id"] = scopeID
_, err = Bun().NewInsert().Model(&raData).Table("role_assignments").Exec(ctx)
require.NoError(t, err, "error inserting editor-project-restricted role assignment")

grp = &model.Group{Name: clusterAdminGroupName}
_, err = Bun().NewInsert().Model(grp).Returning("id").Exec(ctx)
require.NoError(t, err, "error inserting cluster admin group")
Expand All @@ -135,6 +151,10 @@ func setup(t *testing.T, pgDB *PgDB) {
_, err = HackAddUser(context.TODO(), &userModelEditorRestricted)
require.NoError(t, err)

userModelEditorProjectRestricted = model.User{Username: uuid.New().String(), Active: true}
_, err = HackAddUser(context.TODO(), &userModelEditorProjectRestricted)
require.NoError(t, err)

userModelClusterAdmin = model.User{Username: uuid.New().String(), Active: true}
_, err = HackAddUser(context.TODO(), &userModelClusterAdmin)
require.NoError(t, err)
Expand Down Expand Up @@ -168,6 +188,15 @@ func setup(t *testing.T, pgDB *PgDB) {
require.NoError(t, err, "error inserting user group membership "+
strconv.Itoa(editorRestrictedGID))

editorProjectRestrictedGID := editorProjectRestrictedGroupIDs[i]
groupMembership["user_id"] = userModelEditorProjectRestricted.ID
groupMembership["group_id"] = editorProjectRestrictedGID

_, err = Bun().NewInsert().Model(&groupMembership).Table("user_group_membership").
Exec(ctx)
require.NoError(t, err, "error inserting user group membership "+
strconv.Itoa(editorProjectRestrictedGID))

clusterAdminGID := clusterAdminGroupIDs[i]
groupMembership["user_id"] = userModelClusterAdmin.ID
groupMembership["group_id"] = clusterAdminGID
Expand All @@ -193,6 +222,10 @@ func cleanUp(t *testing.T) {
Exec(ctx)
require.NoError(t, err)

_, err = Bun().NewDelete().Table("users").Where("id = ?", userModelEditorProjectRestricted.ID).
Exec(ctx)
require.NoError(t, err)

_, err = Bun().NewDelete().Table("users").Where("id = ?", userModelClusterAdmin.ID).
Exec(ctx)
require.NoError(t, err)
Expand All @@ -212,6 +245,10 @@ func cleanUp(t *testing.T) {
Exec(ctx)
require.NoError(t, err, "error deleting editor-restricted groups")

_, err = Bun().NewDelete().Table("groups").Where("id IN (?)", bun.In(editorProjectRestrictedGroupIDs)).
Exec(ctx)
require.NoError(t, err, "error deleting editor-project-restricted groups")

_, err = Bun().NewDelete().Table("groups").Where("id IN (?)", bun.In(clusterAdminGroupIDs)).
Exec(ctx)
require.NoError(t, err, "error deleting cluster admin groups")
Expand All @@ -230,6 +267,7 @@ func TestPermissionMatch(t *testing.T) {
userIDViewer := userModelViewer.ID
userIDEditor := userModelEditor.ID
userIDEditorRestricted := userModelEditorRestricted.ID
userIDEditorProjectRestricted := userModelEditorProjectRestricted.ID
userIDClusterAdmin := userModelClusterAdmin.ID

badWorkspaceID := int32(maxWsID) + 10
Expand Down Expand Up @@ -262,6 +300,18 @@ func TestPermissionMatch(t *testing.T) {
require.IsType(t, authz.PermissionDeniedError{}, err,
"user should not have permission to create NSC tasks")

// Verify that the user assigned to the groups with EditorProjectRestricted privileges within the
// given workspace cannot create or update Projects.
err = DoesPermissionMatch(ctx, userIDEditorProjectRestricted, &workspaceID,
rbacv1.PermissionType_PERMISSION_TYPE_UPDATE_PROJECT)
require.IsType(t, authz.PermissionDeniedError{}, err,
"user should not have permission to update Projects")

err = DoesPermissionMatch(ctx, userIDEditorProjectRestricted, &workspaceID,
rbacv1.PermissionType_PERMISSION_TYPE_CREATE_PROJECT)
require.IsType(t, authz.PermissionDeniedError{}, err,
"user should not have permission to create Projects")

// Verify that a user who has ClusterAdmin privileges can add workspace-namespace bindings,
// while a user with Editor privileges and below cannot.
err = DoesPermissionMatch(ctx, userIDViewer, &workspaceID,
Expand Down Expand Up @@ -325,6 +375,16 @@ func TestPermissionMatch(t *testing.T) {
require.IsType(t, authz.PermissionDeniedError{}, err,
"user should not have permission to update experiments")

err = DoesPermissionMatchAll(ctx, userIDEditorProjectRestricted,
rbacv1.PermissionType_PERMISSION_TYPE_UPDATE_PROJECT, workspaceID)
require.IsType(t, authz.PermissionDeniedError{}, err,
"user should not have permission to update Projects")

err = DoesPermissionMatchAll(ctx, userIDEditorProjectRestricted,
rbacv1.PermissionType_PERMISSION_TYPE_CREATE_PROJECT, workspaceID)
require.IsType(t, authz.PermissionDeniedError{}, err,
"user should not have permission to create Projects")

err = DoesPermissionMatchAll(ctx, userIDEditor,
rbacv1.PermissionType_PERMISSION_TYPE_SET_WORKSPACE_NAMESPACE_BINDINGS, workspaceID)
require.IsType(t, authz.PermissionDeniedError{}, err,
Expand Down Expand Up @@ -520,3 +580,31 @@ func TestEditorVSEditorRestricted(t *testing.T) {

require.Zero(t, num)
}

func TestEditorVSEditorProjectRestricted(t *testing.T) {
// Verify that the EditorProjectRestricted role only has two less permissions than Editor and that
// it does not have the create or update projects permissions.
ctx := context.Background()
pgDB, closeDB := MustResolveTestPostgres(t)
defer closeDB()
MustMigrateTestPostgres(t, pgDB, MigrationsFromDB)

numEditorProjectRestrictedPermissions, err := Bun().NewSelect().Table("permission_assignments").
Where("role_id = ?", roles["EditorProjectRestricted"]).Count(ctx)
require.NoError(t, err)

numEditorPermissions, err := Bun().NewSelect().Table("permission_assignments").
Where("role_id = ?", roles["Editor"]).Count(ctx)
require.NoError(t, err)

require.Equal(t, numEditorPermissions-2, numEditorProjectRestrictedPermissions)

// Verify that EditorProjectRestricted role does not have create/update project permissions
num, err := Bun().NewSelect().Table("permission_assignments").
Where("role_id = ?", roles["EditorProjectRestricted"]).
Where("permission_id = 4001 OR permission_id = 4003").
Count(ctx)
require.NoError(t, err)

require.Zero(t, num)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
INSERT INTO roles(id, role_name) VALUES
(9, 'EditorProjectRestricted');

INSERT INTO permission_assignments(permission_id, role_id)
SELECT id AS permission_id, 9 FROM permissions WHERE name IN (
'create experiment',
'update experiment',
'view project',
'update experiment metadata',
'view experiment artifacts',
'view experiment metadata',
'delete experiment',
'view workspace',
'view model registry',
'edit model registry',
'create model registry',
'create notebooks/shells/commands',
'view notebooks/shells/commands',
'update notebooks/shells/commands',
'delete model registry',
'view templates',
'update templates',
'create templates',
'delete templates',
'delete model version'
);
Loading