From 11033a169403d6a131b1cef6bb4ec4ccfb1d5648 Mon Sep 17 00:00:00 2001 From: shaoting-huang Date: Fri, 29 Nov 2024 18:06:35 +0800 Subject: [PATCH] fix grant/revoke v2 meta and unclear error messages Signed-off-by: shaoting-huang --- configs/milvus.yaml | 6 +- internal/proxy/impl.go | 10 +- internal/proxy/util.go | 16 +-- internal/rootcoord/meta_table.go | 4 + internal/rootcoord/root_coord.go | 52 ++++---- pkg/util/constant.go | 4 +- pkg/util/merr/errors.go | 3 +- pkg/util/merr/utils.go | 8 ++ .../integration/rbac/privilege_group_test.go | 126 ++++++++++++------ 9 files changed, 143 insertions(+), 86 deletions(-) diff --git a/configs/milvus.yaml b/configs/milvus.yaml index 4dd8e8a873b74..7ff8b898ae1aa 100644 --- a/configs/milvus.yaml +++ b/configs/milvus.yaml @@ -830,7 +830,7 @@ common: readwrite: privileges: ListDatabases,SelectOwnership,SelectUser,DescribeResourceGroup,ListResourceGroups,FlushAll,TransferNode,TransferReplica,UpdateResourceGroups # Cluster level readwrite privileges admin: - privileges: ListDatabases,SelectOwnership,SelectUser,DescribeResourceGroup,ListResourceGroups,FlushAll,TransferNode,TransferReplica,UpdateResourceGroups,BackupRBAC,RestoreRBAC,CreateDatabase,DropDatabase,CreateOwnership,DropOwnership,ManageOwnership,CreateResourceGroup,DropResourceGroup,UpdateUser # Cluster level admin privileges + privileges: ListDatabases,SelectOwnership,SelectUser,DescribeResourceGroup,ListResourceGroups,FlushAll,TransferNode,TransferReplica,UpdateResourceGroups,BackupRBAC,RestoreRBAC,CreateDatabase,DropDatabase,CreateOwnership,DropOwnership,ManageOwnership,CreateResourceGroup,DropResourceGroup,UpdateUser,RenameCollection # Cluster level admin privileges database: readonly: privileges: ShowCollections,DescribeDatabase # Database level readonly privileges @@ -842,9 +842,9 @@ common: readonly: privileges: Query,Search,IndexDetail,GetFlushState,GetLoadState,GetLoadingProgress,HasPartition,ShowPartitions,DescribeCollection,DescribeAlias,GetStatistics,ListAliases # Collection level readonly privileges readwrite: - privileges: Query,Search,IndexDetail,GetFlushState,GetLoadState,GetLoadingProgress,HasPartition,ShowPartitions,DescribeCollection,DescribeAlias,GetStatistics,ListAliases,Load,Release,Insert,Delete,Upsert,Import,Flush,Compaction,LoadBalance,RenameCollection,CreateIndex,DropIndex,CreatePartition,DropPartition # Collection level readwrite privileges + privileges: Query,Search,IndexDetail,GetFlushState,GetLoadState,GetLoadingProgress,HasPartition,ShowPartitions,DescribeCollection,DescribeAlias,GetStatistics,ListAliases,Load,Release,Insert,Delete,Upsert,Import,Flush,Compaction,LoadBalance,CreateIndex,DropIndex,CreatePartition,DropPartition # Collection level readwrite privileges admin: - privileges: Query,Search,IndexDetail,GetFlushState,GetLoadState,GetLoadingProgress,HasPartition,ShowPartitions,DescribeCollection,DescribeAlias,GetStatistics,ListAliases,Load,Release,Insert,Delete,Upsert,Import,Flush,Compaction,LoadBalance,RenameCollection,CreateIndex,DropIndex,CreatePartition,DropPartition,CreateAlias,DropAlias # Collection level admin privileges + privileges: Query,Search,IndexDetail,GetFlushState,GetLoadState,GetLoadingProgress,HasPartition,ShowPartitions,DescribeCollection,DescribeAlias,GetStatistics,ListAliases,Load,Release,Insert,Delete,Upsert,Import,Flush,Compaction,LoadBalance,CreateIndex,DropIndex,CreatePartition,DropPartition,CreateAlias,DropAlias # Collection level admin privileges internaltlsEnabled: false tlsMode: 0 session: diff --git a/internal/proxy/impl.go b/internal/proxy/impl.go index 3a1ab22c1046e..708852708395e 100644 --- a/internal/proxy/impl.go +++ b/internal/proxy/impl.go @@ -5332,7 +5332,7 @@ func (node *Proxy) validPrivilegeParams(req *milvuspb.OperatePrivilegeRequest) e return nil } -func (node *Proxy) validOperatePrivilegeV2Params(req *milvuspb.OperatePrivilegeV2Request) error { +func (node *Proxy) validateOperatePrivilegeV2Params(req *milvuspb.OperatePrivilegeV2Request) error { if req.Role == nil { return fmt.Errorf("the role in the request is nil") } @@ -5345,8 +5345,10 @@ func (node *Proxy) validOperatePrivilegeV2Params(req *milvuspb.OperatePrivilegeV if req.Type != milvuspb.OperatePrivilegeType_Grant && req.Type != milvuspb.OperatePrivilegeType_Revoke { return fmt.Errorf("the type in the request not grant or revoke") } - if err := ValidateObjectName(req.DbName); err != nil { - return err + if req.DbName != "" && !util.IsAnyWord(req.DbName) { + if err := ValidateDatabaseName(req.DbName); err != nil { + return err + } } if err := ValidateObjectName(req.CollectionName); err != nil { return err @@ -5365,7 +5367,7 @@ func (node *Proxy) OperatePrivilegeV2(ctx context.Context, req *milvuspb.Operate if err := merr.CheckHealthy(node.GetStateCode()); err != nil { return merr.Status(err), nil } - if err := node.validOperatePrivilegeV2Params(req); err != nil { + if err := node.validateOperatePrivilegeV2Params(req); err != nil { return merr.Status(err), nil } curUser, err := GetCurUserFromContext(ctx) diff --git a/internal/proxy/util.go b/internal/proxy/util.go index 1468a283fbf7f..05c683324cd39 100644 --- a/internal/proxy/util.go +++ b/internal/proxy/util.go @@ -158,25 +158,25 @@ func validateCollectionNameOrAlias(entity, entityType string) error { func ValidatePrivilegeGroupName(groupName string) error { if groupName == "" { - return merr.WrapErrDatabaseNameInvalid(groupName, "privilege group name couldn't be empty") + return merr.WrapErrPrivilegeGroupNameInvalid("privilege group name should not be empty") } if len(groupName) > Params.ProxyCfg.MaxNameLength.GetAsInt() { - return merr.WrapErrDatabaseNameInvalid(groupName, - fmt.Sprintf("the length of a privilege group name must be less than %d characters", Params.ProxyCfg.MaxNameLength.GetAsInt())) + return merr.WrapErrPrivilegeGroupNameInvalid( + "the length of a privilege group name %s must be less than %s characters", groupName, Params.ProxyCfg.MaxNameLength.GetValue()) } firstChar := groupName[0] if firstChar != '_' && !isAlpha(firstChar) { - return merr.WrapErrDatabaseNameInvalid(groupName, - "the first character of a privilege group name must be an underscore or letter") + return merr.WrapErrPrivilegeGroupNameInvalid( + "the first character of a privilege group name %s must be an underscore or letter", groupName) } for i := 1; i < len(groupName); i++ { c := groupName[i] if c != '_' && !isAlpha(c) && !isNumber(c) { - return merr.WrapErrDatabaseNameInvalid(groupName, - "privilege group name can only contain numbers, letters and underscores") + return merr.WrapErrParameterInvalidMsg( + "privilege group name %s can only contain numbers, letters and underscores", groupName) } } return nil @@ -1099,7 +1099,7 @@ func ValidateObjectName(entity string) error { if util.IsAnyWord(entity) { return nil } - return validateName(entity, "role name") + return validateName(entity, "object name") } func ValidateObjectType(entity string) error { diff --git a/internal/rootcoord/meta_table.go b/internal/rootcoord/meta_table.go index 7574f729296c6..bca7f7e6e0fd5 100644 --- a/internal/rootcoord/meta_table.go +++ b/internal/rootcoord/meta_table.go @@ -1558,6 +1558,10 @@ func (mt *MetaTable) OperatePrivilegeGroup(ctx context.Context, groupName string mt.permissionLock.Lock() defer mt.permissionLock.Unlock() + if util.IsBuiltinPrivilegeGroup(groupName) { + return merr.WrapErrParameterInvalidMsg("the privilege group name [%s] is defined by built in privilege groups in system", groupName) + } + // validate input params definedByUsers, err := mt.IsCustomPrivilegeGroup(ctx, groupName) if err != nil { diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index 0222d0caa52a6..4d3c9f93c3592 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -2645,26 +2645,27 @@ func (c *Core) OperatePrivilege(ctx context.Context, in *milvuspb.OperatePrivile return merr.StatusWithErrorCode(err, commonpb.ErrorCode_OperatePrivilegeFailure), nil } + privName := in.Entity.Grantor.Privilege.Name switch in.Version { case "v2": - if err := c.isValidPrivilegeV2(ctx, in.Entity.Grantor.Privilege.Name, - in.Entity.DbName, in.Entity.ObjectName); err != nil { + if err := c.isValidPrivilegeV2(ctx, privName, in.Entity.DbName, in.Entity.ObjectName); err != nil { ctxLog.Error("", zap.Error(err)) return merr.StatusWithErrorCode(err, commonpb.ErrorCode_OperatePrivilegeFailure), nil } + // set up object type for metastore, to be compatible with v1 version + in.Entity.Object.Name = util.GetObjectType(privName) default: - if err := c.isValidPrivilege(ctx, in.Entity.Grantor.Privilege.Name, in.Entity.Object.Name); err != nil { + if err := c.isValidPrivilege(ctx, privName, in.Entity.Object.Name); err != nil { ctxLog.Error("", zap.Error(err)) return merr.StatusWithErrorCode(err, commonpb.ErrorCode_OperatePrivilegeFailure), nil } // set up object name if it is global object type and not built in privilege group - if in.Entity.Object.Name == commonpb.ObjectType_Global.String() && !lo.Contains(lo.Keys(util.BuiltinPrivilegeGroups), in.Entity.Grantor.Privilege.Name) { + if in.Entity.Object.Name == commonpb.ObjectType_Global.String() && !util.IsBuiltinPrivilegeGroup(in.Entity.Grantor.Privilege.Name) { in.Entity.ObjectName = util.AnyWord } } - // set up privilege name for metastore - privName := in.Entity.Grantor.Privilege.Name + privName = in.Entity.Grantor.Privilege.Name redoTask := newBaseRedoTask(c.stepExecutor) redoTask.AddSyncStep(NewSimpleStep("operate privilege meta data", func(ctx context.Context) ([]nestedStep, error) { @@ -3103,12 +3104,12 @@ func (c *Core) CreatePrivilegeGroup(ctx context.Context, in *milvuspb.CreatePriv ctxLog.Debug(method) if err := merr.CheckHealthy(c.GetStateCode()); err != nil { - return merr.Status(err), nil + return merr.StatusWithErrorCode(err, commonpb.ErrorCode_CreatePrivilegeGroupFailure), nil } if err := c.meta.CreatePrivilegeGroup(ctx, in.GroupName); err != nil { ctxLog.Warn("fail to create privilege group", zap.Error(err)) - return merr.Status(err), nil + return merr.StatusWithErrorCode(err, commonpb.ErrorCode_CreatePrivilegeGroupFailure), nil } ctxLog.Debug(method + " success") @@ -3126,12 +3127,12 @@ func (c *Core) DropPrivilegeGroup(ctx context.Context, in *milvuspb.DropPrivileg ctxLog.Debug(method) if err := merr.CheckHealthy(c.GetStateCode()); err != nil { - return merr.Status(err), nil + return merr.StatusWithErrorCode(err, commonpb.ErrorCode_DropPrivilegeGroupFailure), nil } if err := c.meta.DropPrivilegeGroup(ctx, in.GroupName); err != nil { ctxLog.Warn("fail to drop privilege group", zap.Error(err)) - return merr.Status(err), nil + return merr.StatusWithErrorCode(err, commonpb.ErrorCode_DropPrivilegeGroupFailure), nil } ctxLog.Debug(method + " success") @@ -3318,8 +3319,7 @@ func (c *Core) OperatePrivilegeGroup(ctx context.Context, in *milvuspb.OperatePr if err != nil { errMsg := "fail to execute task when operate privilege group" ctxLog.Warn(errMsg, zap.Error(err)) - status := merr.StatusWithErrorCode(errors.New(errMsg), commonpb.ErrorCode_OperatePrivilegeGroupFailure) - return status, nil + return merr.StatusWithErrorCode(err, commonpb.ErrorCode_OperatePrivilegeGroupFailure), nil } ctxLog.Debug(method + " success") @@ -3335,13 +3335,17 @@ func (c *Core) expandPrivilegeGroups(ctx context.Context, grants []*milvuspb.Gra if err != nil { return nil, err } - if objectType := util.GetObjectType(privilegeName); objectType != "" { - grant.Object.Name = objectType + objectType := &milvuspb.ObjectEntity{ + Name: util.GetObjectType(privilegeName), + } + objectName := grant.ObjectName + if objectType.Name == commonpb.ObjectType_Global.String() { + objectName = util.AnyWord } return &milvuspb.GrantEntity{ Role: grant.Role, - Object: grant.Object, - ObjectName: grant.ObjectName, + Object: objectType, + ObjectName: objectName, Grantor: &milvuspb.GrantorEntity{ User: grant.Grantor.User, Privilege: &milvuspb.PrivilegeEntity{ @@ -3354,20 +3358,16 @@ func (c *Core) expandPrivilegeGroups(ctx context.Context, grants []*milvuspb.Gra for _, grant := range grants { privName := grant.Grantor.Privilege.Name - if privGroup, exists := groups[privName]; !exists { - newGrant, err := createGrantEntity(grant, privName) + privGroup, exists := groups[privName] + if !exists { + privGroup = []*milvuspb.PrivilegeEntity{{Name: privName}} + } + for _, priv := range privGroup { + newGrant, err := createGrantEntity(grant, priv.Name) if err != nil { return nil, err } newGrants = append(newGrants, newGrant) - } else { - for _, priv := range privGroup { - newGrant, err := createGrantEntity(grant, priv.Name) - if err != nil { - return nil, err - } - newGrants = append(newGrants, newGrant) - } } } // uniq by role + object + object name + grantor user + privilege name + db name diff --git a/pkg/util/constant.go b/pkg/util/constant.go index a1323f7ab1e77..c762009d51fd9 100644 --- a/pkg/util/constant.go +++ b/pkg/util/constant.go @@ -332,7 +332,6 @@ var ( MetaStore2API(commonpb.ObjectPrivilege_PrivilegeFlush.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCompaction.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeLoadBalance.String()), - MetaStore2API(commonpb.ObjectPrivilege_PrivilegeRenameCollection.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateIndex.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropIndex.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreatePartition.String()), @@ -384,6 +383,7 @@ var ( MetaStore2API(commonpb.ObjectPrivilege_PrivilegeCreateResourceGroup.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeDropResourceGroup.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeUpdateUser.String()), + MetaStore2API(commonpb.ObjectPrivilege_PrivilegeRenameCollection.String()), ) ) @@ -475,5 +475,5 @@ func GetObjectType(privName string) string { return objectType } } - return "" + return commonpb.ObjectType_Global.String() } diff --git a/pkg/util/merr/errors.go b/pkg/util/merr/errors.go index a0cec3408096d..7d81d8a4da70d 100644 --- a/pkg/util/merr/errors.go +++ b/pkg/util/merr/errors.go @@ -148,7 +148,8 @@ var ( // this operation is denied because the user not authorized, user need to login in first ErrPrivilegeNotAuthenticated = newMilvusError("not authenticated", 1400, false) // this operation is denied because the user has no permission to do this, user need higher privilege - ErrPrivilegeNotPermitted = newMilvusError("privilege not permitted", 1401, false) + ErrPrivilegeNotPermitted = newMilvusError("privilege not permitted", 1401, false) + ErrPrivilegeGroupInvalidName = newMilvusError("invalid privilege group name", 1402, false) // Alias related ErrAliasNotFound = newMilvusError("alias not found", 1600, false) diff --git a/pkg/util/merr/utils.go b/pkg/util/merr/utils.go index 3ded3f7029dc5..bf3caad93e6e4 100644 --- a/pkg/util/merr/utils.go +++ b/pkg/util/merr/utils.go @@ -460,6 +460,14 @@ func WrapErrDatabaseNameInvalid(database any, msg ...string) error { return err } +func WrapErrPrivilegeGroupNameInvalid(privilegeGroup any, msg ...string) error { + err := wrapFields(ErrPrivilegeGroupInvalidName, value("privielgeGroup", privilegeGroup)) + if len(msg) > 0 { + err = errors.Wrap(err, strings.Join(msg, "->")) + } + return err +} + // Collection related func WrapErrCollectionNotFound(collection any, msg ...string) error { err := wrapFields(ErrCollectionNotFound, value("collection", collection)) diff --git a/tests/integration/rbac/privilege_group_test.go b/tests/integration/rbac/privilege_group_test.go index 1f1b13d41b2f4..068a803505c9f 100644 --- a/tests/integration/rbac/privilege_group_test.go +++ b/tests/integration/rbac/privilege_group_test.go @@ -69,13 +69,9 @@ func (s *PrivilegeGroupTestSuite) TestBuiltinPrivilegeGroup() { s.True(merr.Ok(resp)) for _, builtinGroup := range lo.Keys(util.BuiltinPrivilegeGroups) { - fmt.Println("!!! builtinGroup: ", builtinGroup) resp, _ = s.operatePrivilege(ctx, roleName, builtinGroup, commonpb.ObjectType_Global.String(), milvuspb.OperatePrivilegeType_Grant) s.False(merr.Ok(resp)) } - - s.validateGrants(ctx, roleName, commonpb.ObjectType_Global.String(), 1) - s.validateGrants(ctx, roleName, commonpb.ObjectType_Collection.String(), 2) } func (s *PrivilegeGroupTestSuite) TestInvalidPrivilegeGroup() { @@ -145,26 +141,12 @@ func (s *PrivilegeGroupTestSuite) TestGrantV2BuiltinPrivilegeGroup() { s.NoError(err) s.True(merr.Ok(createRoleResp)) - resp, _ := s.operatePrivilegeV2(ctx, roleName, "ClusterReadOnly", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "ClusterReadWrite", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "ClusterAdmin", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "DatabaseReadOnly", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "DatabaseReadWrite", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "DatabaseAdmin", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "CollectionReadOnly", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "CollectionReadWrite", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) - resp, _ = s.operatePrivilegeV2(ctx, roleName, "CollectionAdmin", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) + for _, builtinGroup := range lo.Keys(util.BuiltinPrivilegeGroups) { + resp, _ := s.operatePrivilegeV2(ctx, roleName, builtinGroup, util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) + s.True(merr.Ok(resp)) + } - resp, _ = s.operatePrivilegeV2(ctx, roleName, "ClusterAdmin", "db1", util.AnyWord, milvuspb.OperatePrivilegeType_Grant) + resp, _ := s.operatePrivilegeV2(ctx, roleName, "ClusterAdmin", "db1", util.AnyWord, milvuspb.OperatePrivilegeType_Grant) s.False(merr.Ok(resp)) resp, _ = s.operatePrivilegeV2(ctx, roleName, "ClusterAdmin", "db1", "col1", milvuspb.OperatePrivilegeType_Grant) s.False(merr.Ok(resp)) @@ -233,12 +215,14 @@ func (s *PrivilegeGroupTestSuite) TestGrantV2CustomPrivilegeGroup() { s.True(merr.Ok(createRoleResp)) resp, _ := s.operatePrivilegeV2(ctx, role, "Insert", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) s.True(merr.Ok(resp)) - s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), 1) + selectResp, _ := s.validateGrants(ctx, role, commonpb.ObjectType_Collection.String(), util.AnyWord, util.AnyWord) + s.Len(selectResp.GetEntities(), 1) // grant group1 to role -> role: insert, group1(query, search) resp, _ = s.operatePrivilegeV2(ctx, role, "group1", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) s.True(merr.Ok(resp)) - s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), 2) + selectResp, _ = s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), util.AnyWord, util.AnyWord) + s.Len(selectResp.GetEntities(), 1) // create group2: query, delete createResp2, err := s.Cluster.Proxy.CreatePrivilegeGroup(ctx, &milvuspb.CreatePrivilegeGroupRequest{ @@ -256,7 +240,8 @@ func (s *PrivilegeGroupTestSuite) TestGrantV2CustomPrivilegeGroup() { // grant group2 to role -> role: insert, group1(query, search), group2(query, delete) resp, _ = s.operatePrivilegeV2(ctx, role, "group2", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType_Grant) s.True(merr.Ok(resp)) - s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), 3) + selectResp, _ = s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), util.AnyWord, util.AnyWord) + s.Len(selectResp.GetEntities(), 2) // add query, load to group1 -> group1: query, search, load -> role: insert, group1(query, search, load), group2(query, delete) operatePrivilegeGroup("group1", milvuspb.OperatePrivilegeGroupType_AddPrivilegesToGroup, []*milvuspb.PrivilegeEntity{ @@ -264,7 +249,8 @@ func (s *PrivilegeGroupTestSuite) TestGrantV2CustomPrivilegeGroup() { {Name: "Load"}, }) validatePrivilegeGroup("group1", 3) - s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), 3) + selectResp, _ = s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), util.AnyWord, util.AnyWord) + s.Len(selectResp.GetEntities(), 2) // add different object type privileges to group1 is not allowed resp, _ = s.Cluster.Proxy.OperatePrivilegeGroup(ctx, &milvuspb.OperatePrivilegeGroupRequest{ @@ -279,7 +265,8 @@ func (s *PrivilegeGroupTestSuite) TestGrantV2CustomPrivilegeGroup() { {Name: "Query"}, }) validatePrivilegeGroup("group1", 2) - s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), 3) + selectResp, _ = s.validateGrants(ctx, role, commonpb.ObjectType_Global.String(), util.AnyWord, util.AnyWord) + s.Len(selectResp.GetEntities(), 2) // Drop the group during any role usage will cause error dropResp, _ := s.Cluster.Proxy.DropPrivilegeGroup(ctx, &milvuspb.DropPrivilegeGroupRequest{ @@ -334,42 +321,91 @@ func (s *PrivilegeGroupTestSuite) TestGrantV2CustomPrivilegeGroup() { s.True(merr.Ok(dropRoleResp)) } -func (s *PrivilegeGroupTestSuite) operatePrivilege(ctx context.Context, role, privilege, objectType string, operateType milvuspb.OperatePrivilegeType) (*commonpb.Status, error) { +func (s *PrivilegeGroupTestSuite) TestVersionCrossed() { + ctx := GetContext(context.Background(), "root:123456") + + role := "role1" + createRoleResp, err := s.Cluster.Proxy.CreateRole(ctx, &milvuspb.CreateRoleRequest{ + Entity: &milvuspb.RoleEntity{Name: role}, + }) + s.NoError(err) + s.True(merr.Ok(createRoleResp)) resp, err := s.Cluster.Proxy.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{ - Type: operateType, + Type: milvuspb.OperatePrivilegeType_Grant, Entity: &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: role}, - Object: &milvuspb.ObjectEntity{Name: objectType}, - ObjectName: util.AnyWord, - DbName: util.AnyWord, + Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Collection.String()}, + ObjectName: "collection1", + DbName: "", Grantor: &milvuspb.GrantorEntity{ User: &milvuspb.UserEntity{Name: util.UserRoot}, - Privilege: &milvuspb.PrivilegeEntity{Name: privilege}, + Privilege: &milvuspb.PrivilegeEntity{Name: "Insert"}, }, }, }) - return resp, err + s.NoError(err) + s.True(merr.Ok(resp)) + selectResp, err := s.Cluster.Proxy.SelectGrant(ctx, &milvuspb.SelectGrantRequest{ + Entity: &milvuspb.GrantEntity{ + Role: &milvuspb.RoleEntity{Name: role}, + Object: nil, + ObjectName: "", + DbName: "", + }, + }) + s.NoError(err) + s.True(merr.Ok(selectResp.GetStatus())) + s.Len(selectResp.GetEntities(), 1) + + revoke, err := s.operatePrivilegeV2(ctx, role, "Insert", "default", "collection1", milvuspb.OperatePrivilegeType_Revoke) + s.NoError(err) + s.True(merr.Ok(revoke)) + + selectResp, err = s.Cluster.Proxy.SelectGrant(ctx, &milvuspb.SelectGrantRequest{ + Entity: &milvuspb.GrantEntity{ + Role: &milvuspb.RoleEntity{Name: role}, + Object: nil, + ObjectName: "", + DbName: "", + }, + }) + s.NoError(err) + s.True(merr.Ok(selectResp.GetStatus())) + s.Len(selectResp.GetEntities(), 0) } -func (s *PrivilegeGroupTestSuite) operatePrivilegeV2(ctx context.Context, role, privilege, dbName, collectionName string, operateType milvuspb.OperatePrivilegeType) (*commonpb.Status, error) { +func (s *PrivilegeGroupTestSuite) operatePrivilege(ctx context.Context, role, privilege, objectType string, operateType milvuspb.OperatePrivilegeType) (*commonpb.Status, error) { resp, err := s.Cluster.Proxy.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{ Type: operateType, Entity: &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: role}, - Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Global.String()}, - ObjectName: collectionName, - DbName: dbName, + Object: &milvuspb.ObjectEntity{Name: objectType}, + ObjectName: util.AnyWord, + DbName: util.AnyWord, Grantor: &milvuspb.GrantorEntity{ User: &milvuspb.UserEntity{Name: util.UserRoot}, Privilege: &milvuspb.PrivilegeEntity{Name: privilege}, }, }, - Version: "v2", }) return resp, err } -func (s *PrivilegeGroupTestSuite) validateGrants(ctx context.Context, roleName, objectType string, expectedCount int) { +func (s *PrivilegeGroupTestSuite) operatePrivilegeV2(ctx context.Context, role, privilege, dbName, collectionName string, operateType milvuspb.OperatePrivilegeType) (*commonpb.Status, error) { + resp, err := s.Cluster.Proxy.OperatePrivilegeV2(ctx, &milvuspb.OperatePrivilegeV2Request{ + Role: &milvuspb.RoleEntity{Name: role}, + Grantor: &milvuspb.GrantorEntity{ + User: &milvuspb.UserEntity{Name: util.UserRoot}, + Privilege: &milvuspb.PrivilegeEntity{Name: privilege}, + }, + Type: operateType, + DbName: dbName, + CollectionName: collectionName, + }) + return resp, err +} + +func (s *PrivilegeGroupTestSuite) validateGrants(ctx context.Context, roleName, objectType, database, resource string) (*milvuspb.SelectGrantResponse, error) { resp, err := s.Cluster.Proxy.SelectGrant(ctx, &milvuspb.SelectGrantRequest{ Entity: &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: roleName}, @@ -380,7 +416,13 @@ func (s *PrivilegeGroupTestSuite) validateGrants(ctx context.Context, roleName, }) s.NoError(err) s.True(merr.Ok(resp.GetStatus())) - s.Len(resp.GetEntities(), expectedCount) + return resp, err +} + +func (s *PrivilegeGroupTestSuite) marshalGrants(selectResp *milvuspb.SelectGrantResponse) map[string]*milvuspb.GrantEntity { + return lo.SliceToMap(selectResp.GetEntities(), func(e *milvuspb.GrantEntity) (string, *milvuspb.GrantEntity) { + return fmt.Sprintf("%s-%s-%s-%s", e.Object.Name, e.Grantor.Privilege.Name, e.DbName, e.ObjectName), e + }) } func TestPrivilegeGroup(t *testing.T) {