fix privilege group list and list collections

Signed-off-by: shaoting-huang <[email protected]>

configs/milvus.yaml

@@ -801,7 +801,7 @@ common:
     superUsers: 
     defaultRootPassword: "Milvus" # default password for root user. The maximum length is 72 characters, and double quotes are required.
     rbac:
-      overrideBuiltInPrivilgeGroups:
+      overrideBuiltInPrivilegeGroups:
         enabled: false # Whether to override build-in privilege groups
       cluster:
         readonly:

internal/proxy/privilege_interceptor_test.go

@@ -576,9 +576,9 @@ func TestBuiltinPrivilegeGroup(t *testing.T) {
 		mgr := newShardClientMgr()
 
 		policies := []string{}
-		for _, priv := range util.BuiltinPrivilegeGroups["ClusterReadOnly"] {
-			objectType := util.GetObjectType(priv)
-			policies = append(policies, funcutil.PolicyForPrivilege("role1", objectType, "*", util.PrivilegeNameForMetastore(priv), "default"))
+		for _, priv := range Params.RbacConfig.GetDefaultPrivilegeGroup("ClusterReadOnly").Privileges {
+			objectType := util.GetObjectType(priv.Name)
+			policies = append(policies, funcutil.PolicyForPrivilege("role1", objectType, "*", util.PrivilegeNameForMetastore(priv.Name), "default"))
 		}
 		client.listPolicy = func(ctx context.Context, in *internalpb.ListPolicyRequest) (*internalpb.ListPolicyResponse, error) {
 			return &internalpb.ListPolicyResponse{

internal/rootcoord/root_coord.go

@@ -611,50 +611,6 @@ func (c *Core) initPublicRolePrivilege() error {
 	return nil
 }
 
-func (c *Core) initBuiltinPrivilegeGroups() []*milvuspb.PrivilegeGroupInfo {
-	// init built in privilege groups, override by config if rbac config enabled
-	builtinGroups := make([]*milvuspb.PrivilegeGroupInfo, 0)
-	for groupName, privileges := range util.BuiltinPrivilegeGroups {
-		if Params.RbacConfig.Enabled.GetAsBool() {
-			var confPrivs []string
-			switch groupName {
-			case "ClusterReadOnly":
-				confPrivs = Params.RbacConfig.ClusterReadOnlyPrivileges.GetAsStrings()
-			case "ClusterReadWrite":
-				confPrivs = Params.RbacConfig.ClusterReadWritePrivileges.GetAsStrings()
-			case "ClusterAdmin":
-				confPrivs = Params.RbacConfig.ClusterAdminPrivileges.GetAsStrings()
-			case "DatabaseReadOnly":
-				confPrivs = Params.RbacConfig.DBReadOnlyPrivileges.GetAsStrings()
-			case "DatabaseReadWrite":
-				confPrivs = Params.RbacConfig.DBReadWritePrivileges.GetAsStrings()
-			case "DatabaseAdmin":
-				confPrivs = Params.RbacConfig.DBAdminPrivileges.GetAsStrings()
-			case "CollectionReadOnly":
-				confPrivs = Params.RbacConfig.CollectionReadOnlyPrivileges.GetAsStrings()
-			case "CollectionReadWrite":
-				confPrivs = Params.RbacConfig.CollectionReadWritePrivileges.GetAsStrings()
-			case "CollectionAdmin":
-				confPrivs = Params.RbacConfig.CollectionAdminPrivileges.GetAsStrings()
-			default:
-				return nil
-			}
-			if len(confPrivs) > 0 {
-				privileges = confPrivs
-			}
-		}
-
-		privs := lo.Map(privileges, func(name string, _ int) *milvuspb.PrivilegeEntity {
-			return &milvuspb.PrivilegeEntity{Name: name}
-		})
-		builtinGroups = append(builtinGroups, &milvuspb.PrivilegeGroupInfo{
-			GroupName:  groupName,
-			Privileges: privs,
-		})
-	}
-	return builtinGroups
-}
-
 func (c *Core) initBuiltinRoles() error {
 	rolePrivilegesMap := Params.RoleCfg.Roles.GetAsRoleDetails()
 	for role, privilegesJSON := range rolePrivilegesMap {
@@ -2622,7 +2578,7 @@ func (c *Core) isValidPrivilege(ctx context.Context, privilegeName string, objec
 	if customPrivGroup {
 		return fmt.Errorf("can not operate the custom privilege group [%s]", privilegeName)
 	}
-	if lo.Contains(lo.Keys(util.BuiltinPrivilegeGroups), privilegeName) {
+	if lo.Contains(Params.RbacConfig.GetDefaultPrivilegeGroupNames(), privilegeName) {
 		return fmt.Errorf("can not operate the built-in privilege group [%s]", privilegeName)
 	}
 	// check object privileges for built-in privileges
@@ -2731,7 +2687,7 @@ func (c *Core) OperatePrivilege(ctx context.Context, in *milvuspb.OperatePrivile
 		grants := []*milvuspb.GrantEntity{in.Entity}
 
 		allGroups, err := c.meta.ListPrivilegeGroups()
-		allGroups = append(allGroups, c.initBuiltinPrivilegeGroups()...)
+		allGroups = append(allGroups, Params.RbacConfig.GetDefaultPrivilegeGroups()...)
 		if err != nil {
 			return nil, err
 		}
@@ -3204,16 +3160,7 @@ func (c *Core) ListPrivilegeGroups(ctx context.Context, in *milvuspb.ListPrivile
 	metrics.RootCoordDDLReqLatency.WithLabelValues(method).Observe(float64(tr.ElapseSpan().Milliseconds()))
 
 	// append built in privilege groups
-	for groupName, privileges := range util.BuiltinPrivilegeGroups {
-		privGroups = append(privGroups, &milvuspb.PrivilegeGroupInfo{
-			GroupName: groupName,
-			Privileges: lo.Map(privileges, func(p string, _ int) *milvuspb.PrivilegeEntity {
-				return &milvuspb.PrivilegeEntity{
-					Name: p,
-				}
-			}),
-		})
-	}
+	privGroups = append(privGroups, Params.RbacConfig.GetDefaultPrivilegeGroups()...)
 	return &milvuspb.ListPrivilegeGroupsResponse{
 		Status:          merr.Success(),
 		PrivilegeGroups: privGroups,

internal/rootcoord/root_coord_test.go

@@ -1898,29 +1898,6 @@ func TestCore_InitRBAC(t *testing.T) {
 		err := c.initRbac()
 		assert.NoError(t, err)
 	})
-
-	t.Run("init default privilege groups", func(t *testing.T) {
-		clusterReadWrite := `SelectOwnership,SelectUser,DescribeResourceGroup`
-		meta := mockrootcoord.NewIMetaTable(t)
-		c := newTestCore(withHealthyCode(), withMeta(meta))
-
-		Params.Save(Params.RbacConfig.Enabled.Key, "true")
-		Params.Save(Params.RbacConfig.ClusterReadWritePrivileges.Key, clusterReadWrite)
-
-		defer func() {
-			Params.Reset(Params.RbacConfig.Enabled.Key)
-			Params.Reset(Params.RbacConfig.ClusterReadWritePrivileges.Key)
-		}()
-
-		builtinGroups := c.initBuiltinPrivilegeGroups()
-		fmt.Println(builtinGroups)
-		assert.Equal(t, len(util.BuiltinPrivilegeGroups), len(builtinGroups))
-		for _, group := range builtinGroups {
-			if group.GroupName == "ClusterReadWrite" {
-				assert.Equal(t, len(group.Privileges), 3)
-			}
-		}
-	})
 }
 
 func TestCore_BackupRBAC(t *testing.T) {

internal/rootcoord/show_collection_task.go

@@ -88,12 +88,15 @@ func (t *showCollectionTask) Execute(ctx context.Context) error {
 			}
 			for _, entity := range entities {
 				objectType := entity.GetObject().GetName()
+				priv := entity.GetGrantor().GetPrivilege().GetName()
 				if objectType == commonpb.ObjectType_Global.String() &&
-					entity.GetGrantor().GetPrivilege().GetName() == util.PrivilegeNameForAPI(commonpb.ObjectPrivilege_PrivilegeAll.String()) {
+					priv == util.PrivilegeNameForAPI(commonpb.ObjectPrivilege_PrivilegeAll.String()) {
 					privilegeColls.Insert(util.AnyWord)
 					return privilegeColls, nil
 				}
-				if objectType != commonpb.ObjectType_Collection.String() {
+				// should list collection level built-in privilege group objects
+				if objectType != commonpb.ObjectType_Collection.String() &&
+					!Params.RbacConfig.IsCollectionPrivilegeGroup(priv) {
 					continue
 				}
 				collectionName := entity.GetObjectName()

internal/rootcoord/show_collection_task_test.go

@@ -325,6 +325,57 @@ func TestShowCollectionsAuth(t *testing.T) {
 		assert.Equal(t, "foo", task.Rsp.GetCollectionNames()[0])
 	})
 
+	t.Run("collection level privilege group", func(t *testing.T) {
+		Params.Save(Params.CommonCfg.AuthorizationEnabled.Key, "true")
+		defer Params.Reset(Params.CommonCfg.AuthorizationEnabled.Key)
+		meta := mockrootcoord.NewIMetaTable(t)
+		core := newTestCore(withMeta(meta))
+
+		meta.EXPECT().SelectUser(mock.Anything, mock.Anything, mock.Anything).
+			Return([]*milvuspb.UserResult{
+				{
+					User: &milvuspb.UserEntity{
+						Name: "foo",
+					},
+					Roles: []*milvuspb.RoleEntity{
+						{
+							Name: "hoooo",
+						},
+					},
+				},
+			}, nil).Once()
+		meta.EXPECT().SelectGrant(mock.Anything, mock.Anything).Return([]*milvuspb.GrantEntity{
+			{
+				Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Global.String()},
+				Grantor: &milvuspb.GrantorEntity{
+					Privilege: &milvuspb.PrivilegeEntity{
+						Name: util.PrivilegeNameForAPI(commonpb.ObjectPrivilege_PrivilegeGroupCollectionReadOnly.String()),
+					},
+				},
+				ObjectName: util.AnyWord,
+			},
+		}, nil).Once()
+		meta.EXPECT().ListCollections(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Collection{
+			{
+				DBID:         1,
+				CollectionID: 100,
+				Name:         "foo",
+				CreateTime:   tsoutil.GetCurrentTime(),
+			},
+		}, nil).Once()
+
+		task := &showCollectionTask{
+			baseTask: newBaseTask(context.Background(), core),
+			Req:      &milvuspb.ShowCollectionsRequest{DbName: "default"},
+			Rsp:      &milvuspb.ShowCollectionsResponse{},
+		}
+		ctx := GetContext(context.Background(), "foo:root")
+		err := task.Execute(ctx)
+		assert.NoError(t, err)
+		assert.Equal(t, 1, len(task.Rsp.GetCollectionNames()))
+		assert.Equal(t, "foo", task.Rsp.GetCollectionNames()[0])
+	})
+
 	t.Run("all collection", func(t *testing.T) {
 		Params.Save(Params.CommonCfg.AuthorizationEnabled.Key, "true")
 		defer Params.Reset(Params.CommonCfg.AuthorizationEnabled.Key)

pkg/util/constant.go

@@ -290,101 +290,6 @@ var (
 		commonpb.ObjectPrivilege_PrivilegeAlterDatabase.String(),
 		commonpb.ObjectPrivilege_PrivilegeFlush.String(),
 	}
-
-	BuiltinPrivilegeGroups = map[string][]string{
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupCollectionReadOnly.String()):  CollectionReadOnlyPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupCollectionReadWrite.String()): CollectionReadWritePrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupCollectionAdmin.String()):     CollectionAdminPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupDatabaseReadOnly.String()):    DatabaseReadOnlyPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupDatabaseReadWrite.String()):   DatabaseReadWritePrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupDatabaseAdmin.String()):       DatabaseAdminPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupClusterReadOnly.String()):     ClusterReadOnlyPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupClusterReadWrite.String()):    ClusterReadWritePrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGroupClusterAdmin.String()):        ClusterAdminPrivilegeGroup,
-	}
-
-	CollectionReadOnlyPrivilegeGroup = []string{
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeQuery.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeSearch.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeIndexDetail.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGetFlushState.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGetLoadState.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGetLoadingProgress.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeHasPartition.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeShowPartitions.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDescribeCollection.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDescribeAlias.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeGetStatistics.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeListAliases.String()),
-	}
-
-	CollectionReadWritePrivilegeGroup = append(CollectionReadOnlyPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeLoad.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeRelease.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeInsert.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDelete.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeUpsert.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeImport.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeFlush.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCompaction.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeLoadBalance.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateIndex.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropIndex.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreatePartition.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropPartition.String()),
-	)
-
-	CollectionAdminPrivilegeGroup = append(CollectionReadWritePrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateAlias.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropAlias.String()),
-	)
-
-	DatabaseReadOnlyPrivilegeGroup = []string{
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeShowCollections.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDescribeDatabase.String()),
-	}
-
-	DatabaseReadWritePrivilegeGroup = append(DatabaseReadOnlyPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeAlterDatabase.String()),
-	)
-
-	DatabaseAdminPrivilegeGroup = append(DatabaseReadWritePrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateCollection.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropCollection.String()),
-	)
-
-	ClusterReadOnlyPrivilegeGroup = []string{
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeListDatabases.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeSelectOwnership.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeSelectUser.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDescribeResourceGroup.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeListResourceGroups.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeListPrivilegeGroups.String()),
-	}
-
-	ClusterReadWritePrivilegeGroup = append(ClusterReadOnlyPrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeFlushAll.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeTransferNode.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeTransferReplica.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeUpdateResourceGroups.String()),
-	)
-
-	ClusterAdminPrivilegeGroup = append(ClusterReadWritePrivilegeGroup,
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeBackupRBAC.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeRestoreRBAC.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateDatabase.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropDatabase.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateOwnership.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropOwnership.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeManageOwnership.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateResourceGroup.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropResourceGroup.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeUpdateUser.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeRenameCollection.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreatePrivilegeGroup.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropPrivilegeGroup.String()),
-		MetaStore2API(commonpb.ObjectPrivilege_PrivilegeOperatePrivilegeGroup.String()),
-	)
 )
 
 // StringSet convert array to map for conveniently check if the array contains an element

pkg/util/paramtable/rbac_config_test.go

@@ -20,22 +20,41 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
-	"github.com/milvus-io/milvus/pkg/util"
 )
 
 func TestRbacConfig_Init(t *testing.T) {
 	params := ComponentParam{}
 	params.Init(NewBaseTable(SkipRemote(true)))
 	cfg := &params.RbacConfig
+	assert.Equal(t, len(cfg.GetDefaultPrivilegeGroupNames()), 9)
+	assert.True(t, cfg.IsCollectionPrivilegeGroup("CollectionReadOnly"))
+	assert.False(t, cfg.IsCollectionPrivilegeGroup("DatabaseReadOnly"))
 	assert.Equal(t, cfg.Enabled.GetAsBool(), false)
-	assert.Equal(t, cfg.ClusterReadOnlyPrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["ClusterReadOnly"])
-	assert.Equal(t, cfg.ClusterReadWritePrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["ClusterReadWrite"])
-	assert.Equal(t, cfg.ClusterAdminPrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["ClusterAdmin"])
-	assert.Equal(t, cfg.DBReadOnlyPrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["DatabaseReadOnly"])
-	assert.Equal(t, cfg.DBReadWritePrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["DatabaseReadWrite"])
-	assert.Equal(t, cfg.DBAdminPrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["DatabaseAdmin"])
-	assert.Equal(t, cfg.CollectionReadOnlyPrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["CollectionReadOnly"])
-	assert.Equal(t, cfg.CollectionReadWritePrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["CollectionReadWrite"])
-	assert.Equal(t, cfg.CollectionAdminPrivileges.GetAsStrings(), util.BuiltinPrivilegeGroups["CollectionAdmin"])
+	assert.Equal(t, cfg.ClusterReadOnlyPrivileges.GetAsStrings(), builtinPrivilegeGroups["ClusterReadOnly"])
+	assert.Equal(t, cfg.ClusterReadWritePrivileges.GetAsStrings(), builtinPrivilegeGroups["ClusterReadWrite"])
+	assert.Equal(t, cfg.ClusterAdminPrivileges.GetAsStrings(), builtinPrivilegeGroups["ClusterAdmin"])
+	assert.Equal(t, cfg.DBReadOnlyPrivileges.GetAsStrings(), builtinPrivilegeGroups["DatabaseReadOnly"])
+	assert.Equal(t, cfg.DBReadWritePrivileges.GetAsStrings(), builtinPrivilegeGroups["DatabaseReadWrite"])
+	assert.Equal(t, cfg.DBAdminPrivileges.GetAsStrings(), builtinPrivilegeGroups["DatabaseAdmin"])
+	assert.Equal(t, cfg.CollectionReadOnlyPrivileges.GetAsStrings(), builtinPrivilegeGroups["CollectionReadOnly"])
+	assert.Equal(t, cfg.CollectionReadWritePrivileges.GetAsStrings(), builtinPrivilegeGroups["CollectionReadWrite"])
+	assert.Equal(t, cfg.CollectionAdminPrivileges.GetAsStrings(), builtinPrivilegeGroups["CollectionAdmin"])
+}
+
+func TestRbacConfig_Override(t *testing.T) {
+	params := ComponentParam{}
+	params.Init(NewBaseTable(SkipRemote(true)))
+
+	clusterReadWrite := `SelectOwnership,SelectUser,DescribeResourceGroup`
+
+	params.Save(params.RbacConfig.Enabled.Key, "true")
+	params.Save(params.RbacConfig.ClusterReadWritePrivileges.Key, clusterReadWrite)
+
+	defer func() {
+		params.Reset(params.RbacConfig.Enabled.Key)
+		params.Reset(params.RbacConfig.ClusterReadWritePrivileges.Key)
+	}()
+
+	group := params.RbacConfig.GetDefaultPrivilegeGroup("ClusterReadWrite")
+	assert.Equal(t, len(group.Privileges), 3)
 }