feat: added RBAC for DeleteModel/Version [DET-9048] (determined-ai#755)

master/internal/model/authz_basic_impl.go

@@ -46,10 +46,9 @@ func (a *ModelAuthZBasic) CanCreateModel(ctx context.Context,
 func (a *ModelAuthZBasic) CanDeleteModel(ctx context.Context, curUser model.User,
 	m *modelv1.Model, workspaceID int32,
 ) error {
-	// TODO: Modify model UserID to use UserID
 	curUserIsOwner := m.UserId == int32(curUser.ID)
 	if !curUser.Admin && !curUserIsOwner {
-		return fmt.Errorf("non admin users may not delete another user's models")
+		return fmt.Errorf("non admin users may not delete other users' models")
 	}
 	return nil
 }

master/internal/model/authz_permissive.go

@@ -44,6 +44,14 @@ func (a *ModelAuthZPermissive) CanCreateModel(ctx context.Context,
 	return (&ModelAuthZBasic{}).CanCreateModel(ctx, curUser, workspaceID)
 }
 
+// CanDeleteModel calls RBAC authz but enforces basic authz..
+func (a *ModelAuthZPermissive) CanDeleteModel(ctx context.Context, curUser model.User,
+	m *modelv1.Model, workspaceID int32,
+) error {
+	_ = (&ModelAuthZRBAC{}).CanDeleteModel(ctx, curUser, m, workspaceID)
+	return (&ModelAuthZBasic{}).CanDeleteModel(ctx, curUser, m, workspaceID)
+}
+
 // CanMoveModel always returns true.
 func (a *ModelAuthZPermissive) CanMoveModel(ctx context.Context,
 	curUser model.User, m *modelv1.Model, origin int32, destination int32,
@@ -60,13 +68,6 @@ func (a *ModelAuthZPermissive) FilterReadableModelsQuery(
 	return (&ModelAuthZBasic{}).FilterReadableModelsQuery(ctx, curUser, query)
 }
 
-// CanDeleteModel implements ModelAuthZ.
-func (a *ModelAuthZPermissive) CanDeleteModel(ctx context.Context, curUser model.User,
-	m *modelv1.Model, workspaceID int32,
-) error {
-	panic("unimplemented")
-}
-
 func init() {
 	AuthZProvider.Register("permissive", &ModelAuthZPermissive{})
 }

master/internal/model/authz_rbac.go

@@ -144,6 +144,25 @@ func (a *ModelAuthZRBAC) CanCreateModel(ctx context.Context,
 		rbacv1.PermissionType_PERMISSION_TYPE_CREATE_MODEL_REGISTRY)
 }
 
+// CanDeleteModel checks if users has permission to delete models.
+func (a *ModelAuthZRBAC) CanDeleteModel(ctx context.Context, curUser model.User,
+	m *modelv1.Model, workspaceID int32,
+) (err error) {
+	fields := audit.ExtractLogFields(ctx)
+	addExpInfo(curUser, fields, string(m.Id),
+		[]rbacv1.PermissionType{rbacv1.PermissionType_PERMISSION_TYPE_DELETE_MODEL_REGISTRY})
+	defer func() {
+		audit.LogFromErr(fields, err)
+	}()
+
+	return db.DoesPermissionMatch(ctx, curUser.ID, &workspaceID,
+		rbacv1.PermissionType_PERMISSION_TYPE_DELETE_MODEL_REGISTRY)
+}
+
+func init() {
+	AuthZProvider.Register("rbac", &ModelAuthZRBAC{})
+}
+
 // CanMoveModel checks for edit permission in origin and create permission in destination.
 func (a *ModelAuthZRBAC) CanMoveModel(ctx context.Context,
 	curUser model.User, _ *modelv1.Model, origin int32, destination int32,
@@ -220,14 +239,3 @@ func (a *ModelAuthZRBAC) FilterReadableModelsQuery(
 
 	return query, nil
 }
-
-// CanDeleteModel implements ModelAuthZ.
-func (a *ModelAuthZRBAC) CanDeleteModel(ctx context.Context, curUser model.User,
-	m *modelv1.Model, workspaceID int32,
-) error {
-	panic("unimplemented")
-}
-
-func init() {
-	AuthZProvider.Register("rbac", &ModelAuthZRBAC{})
-}

master/static/migrations/20230404144709_add-rbac-delete-model.tx.down.sql

@@ -0,0 +1,7 @@
+DELETE FROM permissions_assignments WHERE permission_id IN (
+    7004
+);
+
+DELETE FROM permissions WHERE id IN (
+    7004
+);

master/static/migrations/20230404144709_add-rbac-delete-model.tx.up.sql

@@ -0,0 +1,9 @@
+INSERT INTO permissions (
+    id, name, global_only
+) VALUES
+(7004, 'delete model registry', false);
+-- ClusterAdmin, WorkspaceAdmin, Editor roles
+INSERT INTO permission_assignments (permission_id, role_id) VALUES
+(7004, 1),
+(7004, 2),
+(7004, 5);

proto/src/determined/rbac/v1/rbac.proto

@@ -119,7 +119,7 @@ enum PermissionType {
   PERMISSION_TYPE_EDIT_MODEL_REGISTRY = 7002;
   // Ability to create model registry.
   PERMISSION_TYPE_CREATE_MODEL_REGISTRY = 7003;
-  // Ability to delete model registry
+  // Ability to delete model registry.
   PERMISSION_TYPE_DELETE_MODEL_REGISTRY = 7004;
 
   // Ability to view master logs.