From 0ae3016910c3a5951a41de35b1afce1ef0257148 Mon Sep 17 00:00:00 2001 From: sthuang <167743503+shaoting-huang@users.noreply.github.com> Date: Mon, 2 Dec 2024 11:36:39 +0800 Subject: [PATCH] fix grant/revoke v2 meta and unclear error messages [#38110] [#38130] Signed-off-by: shaoting-huang --- configs/milvus.yaml | 6 +- internal/proxy/impl.go | 18 ++- internal/proxy/util.go | 41 ++++- internal/rootcoord/meta_table.go | 4 + internal/rootcoord/meta_table_test.go | 2 + internal/rootcoord/root_coord.go | 91 +++++------ pkg/util/constant.go | 4 +- pkg/util/merr/errors.go | 3 +- pkg/util/merr/utils.go | 8 + .../integration/rbac/privilege_group_test.go | 144 ++++++++++++------ 10 files changed, 201 insertions(+), 120 deletions(-) diff --git a/configs/milvus.yaml b/configs/milvus.yaml index 42555652f26ca..4e3294feed906 100644 --- a/configs/milvus.yaml +++ b/configs/milvus.yaml @@ -806,7 +806,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 @@ -818,9 +818,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 tlsMode: 0 session: ttl: 30 # ttl value when session granting a lease to register service diff --git a/internal/proxy/impl.go b/internal/proxy/impl.go index fc0f3a91d4028..be2a32fa441f0 100644 --- a/internal/proxy/impl.go +++ b/internal/proxy/impl.go @@ -5254,9 +5254,9 @@ 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") + return merr.WrapErrParameterInvalidMsg("the role in the request is nil") } if err := ValidateRoleName(req.Role.Name); err != nil { return err @@ -5264,11 +5264,17 @@ func (node *Proxy) validOperatePrivilegeV2Params(req *milvuspb.OperatePrivilegeV if err := ValidatePrivilege(req.Grantor.Privilege.Name); err != nil { return err } + // validate built-in privilege group params + if err := ValidateBuiltInPrivilegeGroup(req.Grantor.Privilege.Name, req.DbName, req.CollectionName); err != nil { + return err + } if req.Type != milvuspb.OperatePrivilegeType_Grant && req.Type != milvuspb.OperatePrivilegeType_Revoke { - return fmt.Errorf("the type in the request not grant or revoke") + return merr.WrapErrParameterInvalidMsg("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 @@ -5287,7 +5293,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 3817a643d3c1a..b5732a1a6dc1f 100644 --- a/internal/proxy/util.go +++ b/internal/proxy/util.go @@ -157,25 +157,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 @@ -925,7 +925,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 { @@ -939,6 +939,31 @@ func ValidatePrivilege(entity string) error { return validateName(entity, "Privilege") } +func ValidateBuiltInPrivilegeGroup(entity string, dbName string, collectionName string) error { + if !util.IsBuiltinPrivilegeGroup(entity) { + return nil + } + switch { + case strings.HasPrefix(entity, milvuspb.PrivilegeLevel_Cluster.String()): + if !util.IsAnyWord(dbName) || !util.IsAnyWord(collectionName) { + return merr.WrapErrParameterInvalidMsg("dbName and collectionName should be * for the cluster level privilege: %s", entity) + } + return nil + case strings.HasPrefix(entity, milvuspb.PrivilegeLevel_Database.String()): + if collectionName != "" && collectionName != util.AnyWord { + return merr.WrapErrParameterInvalidMsg("collectionName should be * for the database level privilege: %s", entity) + } + return nil + case strings.HasPrefix(entity, milvuspb.PrivilegeLevel_Collection.String()): + if util.IsAnyWord(dbName) && !util.IsAnyWord(collectionName) && collectionName != "" { + return merr.WrapErrParameterInvalidMsg("please specify database name for the collection level privilege: %s", entity) + } + return nil + default: + return nil + } +} + func GetCurUserFromContext(ctx context.Context) (string, error) { return contextutil.GetCurUserFromContext(ctx) } diff --git a/internal/rootcoord/meta_table.go b/internal/rootcoord/meta_table.go index 31364f431b055..09821dadae6c7 100644 --- a/internal/rootcoord/meta_table.go +++ b/internal/rootcoord/meta_table.go @@ -1524,6 +1524,10 @@ func (mt *MetaTable) OperatePrivilegeGroup(groupName string, privileges []*milvu 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(groupName) if err != nil { diff --git a/internal/rootcoord/meta_table_test.go b/internal/rootcoord/meta_table_test.go index 4192b655c03be..3122eb4554318 100644 --- a/internal/rootcoord/meta_table_test.go +++ b/internal/rootcoord/meta_table_test.go @@ -2105,6 +2105,8 @@ func TestMetaTable_PrivilegeGroup(t *testing.T) { assert.NoError(t, err) err = mt.OperatePrivilegeGroup("", []*milvuspb.PrivilegeEntity{}, milvuspb.OperatePrivilegeGroupType_AddPrivilegesToGroup) assert.Error(t, err) + err = mt.OperatePrivilegeGroup("ClusterReadOnly", []*milvuspb.PrivilegeEntity{}, milvuspb.OperatePrivilegeGroupType_AddPrivilegesToGroup) + assert.Error(t, err) err = mt.OperatePrivilegeGroup("pg3", []*milvuspb.PrivilegeEntity{}, milvuspb.OperatePrivilegeGroupType_AddPrivilegesToGroup) assert.Error(t, err) _, err = mt.GetPrivilegeGroupRoles("") diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index 798eb1b2ec38a..26c5a8596f599 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -22,7 +22,6 @@ import ( "math/rand" "os" "strconv" - "strings" "sync" "time" @@ -2572,43 +2571,21 @@ func (c *Core) isValidPrivilege(ctx context.Context, privilegeName string, objec return fmt.Errorf("not found the privilege name[%s] in object[%s]", privilegeName, object) } -func (c *Core) isValidPrivilegeV2(ctx context.Context, privilegeName, dbName, collectionName string) error { +func (c *Core) isValidPrivilegeV2(ctx context.Context, privilegeName string) error { if util.IsAnyWord(privilegeName) { return nil } - var privilegeLevel string - for group, privileges := range util.BuiltinPrivilegeGroups { - if privilegeName == group || lo.Contains(privileges, privilegeName) { - privilegeLevel = group - break - } - } - if privilegeLevel == "" { - customPrivGroup, err := c.meta.IsCustomPrivilegeGroup(privilegeName) - if err != nil { - return err - } - if customPrivGroup { - return nil - } - return fmt.Errorf("not found the privilege name[%s] in the custom privilege groups", privilegeName) + customPrivGroup, err := c.meta.IsCustomPrivilegeGroup(privilegeName) + if err != nil { + return err } - switch { - case strings.HasPrefix(privilegeLevel, milvuspb.PrivilegeLevel_Cluster.String()): - if !util.IsAnyWord(dbName) || !util.IsAnyWord(collectionName) { - return fmt.Errorf("dbName and collectionName should be * for the cluster level privilege: %s", privilegeName) - } - return nil - case strings.HasPrefix(privilegeLevel, milvuspb.PrivilegeLevel_Database.String()): - if collectionName != "" && collectionName != util.AnyWord { - return fmt.Errorf("collectionName should be empty or * for the database level privilege: %s", privilegeName) - } - return nil - case strings.HasPrefix(privilegeLevel, milvuspb.PrivilegeLevel_Collection.String()): + if customPrivGroup { return nil - default: + } + if util.IsPrivilegeNameDefined(privilegeName) { return nil } + return fmt.Errorf("not found the privilege name[%s]", privilegeName) } // OperatePrivilege operate the privilege, including grant and revoke @@ -2629,26 +2606,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); 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) { @@ -3087,12 +3065,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(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") @@ -3110,12 +3088,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(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") @@ -3302,8 +3280,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") @@ -3319,13 +3296,17 @@ func (c *Core) expandPrivilegeGroups(grants []*milvuspb.GrantEntity, groups map[ 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{ @@ -3338,20 +3319,16 @@ func (c *Core) expandPrivilegeGroups(grants []*milvuspb.GrantEntity, groups map[ 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 872ddf19b5eb7..d47fe7399aaaf 100644 --- a/pkg/util/merr/errors.go +++ b/pkg/util/merr/errors.go @@ -147,7 +147,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 c5041aeaf6e0c..68ce7bc80132e 100644 --- a/pkg/util/merr/utils.go +++ b/pkg/util/merr/utils.go @@ -455,6 +455,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..7e44c5301e119 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() { @@ -135,6 +131,22 @@ func (s *PrivilegeGroupTestSuite) TestInvalidPrivilegeGroup() { s.False(merr.Ok(operateResp)) } +func (s *PrivilegeGroupTestSuite) TestInvalidGrantV2() { + ctx := GetContext(context.Background(), "root:123456") + + // invalid operate privilege type + resp, _ := s.operatePrivilegeV2(ctx, "role", "Insert", util.AnyWord, util.AnyWord, milvuspb.OperatePrivilegeType(-1)) + s.False(merr.Ok(resp)) + + // invlaid database name + resp, _ = s.operatePrivilegeV2(ctx, "role", "Insert", "%$", util.AnyWord, milvuspb.OperatePrivilegeType_Grant) + s.False(merr.Ok(resp)) + + // invalid collection name + resp, _ = s.operatePrivilegeV2(ctx, "role", "Insert", util.AnyWord, "%$", milvuspb.OperatePrivilegeType_Grant) + s.False(merr.Ok(resp)) +} + func (s *PrivilegeGroupTestSuite) TestGrantV2BuiltinPrivilegeGroup() { ctx := GetContext(context.Background(), "root:123456") @@ -145,26 +157,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)) @@ -181,7 +179,7 @@ func (s *PrivilegeGroupTestSuite) TestGrantV2BuiltinPrivilegeGroup() { resp, _ = s.operatePrivilegeV2(ctx, roleName, "CollectionAdmin", "db1", "col1", milvuspb.OperatePrivilegeType_Grant) s.True(merr.Ok(resp)) resp, _ = s.operatePrivilegeV2(ctx, roleName, "CollectionAdmin", util.AnyWord, "col1", milvuspb.OperatePrivilegeType_Grant) - s.True(merr.Ok(resp)) + s.False(merr.Ok(resp)) } func (s *PrivilegeGroupTestSuite) TestGrantV2CustomPrivilegeGroup() { @@ -233,12 +231,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 +256,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 +265,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 +281,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 +337,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 +432,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) {