Skip to content

Commit 5c5948c

Browse files
shaoting-huangshaoting-huang
authored
fix: rbac custom group privilege level check (#39164)
related: #39086 Signed-off-by: shaoting-huang <[email protected]>
1 parent 5f94954 commit 5c5948c

File tree

11 files changed

+235
-185
lines changed

11 files changed

+235
-185
lines changed

‎internal/proxy/impl.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5439,10 +5439,6 @@ func (node *Proxy) validateOperatePrivilegeV2Params(req *milvuspb.OperatePrivile
54395439
if err := ValidatePrivilege(req.Grantor.Privilege.Name); err != nil {
54405440
return err
54415441
}
5442-
// validate built-in privilege group params
5443-
if err := ValidateBuiltInPrivilegeGroup(req.Grantor.Privilege.Name, req.DbName, req.CollectionName); err != nil {
5444-
return err
5445-
}
54465442
if req.Type != milvuspb.OperatePrivilegeType_Grant && req.Type != milvuspb.OperatePrivilegeType_Revoke {
54475443
return merr.WrapErrParameterInvalidMsg("the type in the request not grant or revoke")
54485444
}

‎internal/proxy/util.go‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,31 +1120,6 @@ func ValidatePrivilege(entity string) error {
11201120
return validateName(entity, "Privilege")
11211121
}
11221122

1123-
func ValidateBuiltInPrivilegeGroup(entity string, dbName string, collectionName string) error {
1124-
if !util.IsBuiltinPrivilegeGroup(entity) {
1125-
return nil
1126-
}
1127-
switch {
1128-
case strings.HasPrefix(entity, milvuspb.PrivilegeLevel_Cluster.String()):
1129-
if !util.IsAnyWord(dbName) || !util.IsAnyWord(collectionName) {
1130-
return merr.WrapErrParameterInvalidMsg("dbName and collectionName should be * for the cluster level privilege: %s", entity)
1131-
}
1132-
return nil
1133-
case strings.HasPrefix(entity, milvuspb.PrivilegeLevel_Database.String()):
1134-
if collectionName != "" && collectionName != util.AnyWord {
1135-
return merr.WrapErrParameterInvalidMsg("collectionName should be * for the database level privilege: %s", entity)
1136-
}
1137-
return nil
1138-
case strings.HasPrefix(entity, milvuspb.PrivilegeLevel_Collection.String()):
1139-
if util.IsAnyWord(dbName) && !util.IsAnyWord(collectionName) && collectionName != "" {
1140-
return merr.WrapErrParameterInvalidMsg("please specify database name for the collection level privilege: %s", entity)
1141-
}
1142-
return nil
1143-
default:
1144-
return nil
1145-
}
1146-
}
1147-
11481123
func GetCurUserFromContext(ctx context.Context) (string, error) {
11491124
return contextutil.GetCurUserFromContext(ctx)
11501125
}

‎internal/rootcoord/meta_table.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,7 +1525,7 @@ func (mt *MetaTable) RestoreRBAC(ctx context.Context, tenant string, meta *milvu
15251525
return mt.catalog.RestoreRBAC(ctx, tenant, meta)
15261526
}
15271527

1528-
// check if the privielge group name is defined by users
1528+
// check if the privilege group name is defined by users
15291529
func (mt *MetaTable) IsCustomPrivilegeGroup(ctx context.Context, groupName string) (bool, error) {
15301530
privGroups, err := mt.catalog.ListPrivilegeGroups(ctx)
15311531
if err != nil {
@@ -1641,7 +1641,7 @@ func (mt *MetaTable) OperatePrivilegeGroup(ctx context.Context, groupName string
16411641
if group.GroupName == p.Name {
16421642
privileges = append(privileges, group.Privileges...)
16431643
} else {
1644-
return merr.WrapErrParameterInvalidMsg("there is no privilege name or privielge group name [%s] defined in system to operate", p.Name)
1644+
return merr.WrapErrParameterInvalidMsg("there is no privilege name or privilege group name [%s] defined in system to operate", p.Name)
16451645
}
16461646
}
16471647
}

‎internal/rootcoord/rbac_task.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func executeOperatePrivilegeTaskSteps(ctx context.Context, core *Core, in *milvu
179179
}
180180
grants := []*milvuspb.GrantEntity{in.Entity}
181181

182-
allGroups, err := core.getPrivilegeGroups(ctx)
182+
allGroups, err := core.getDefaultAndCustomPrivilegeGroups(ctx)
183183
if err != nil {
184184
return nil, err
185185
}
@@ -275,12 +275,12 @@ func executeOperatePrivilegeGroupTaskSteps(ctx context.Context, core *Core, in *
275275
return p.Name
276276
})
277277

278-
// check if privileges are the same object type
279-
objectTypes := lo.SliceToMap(newPrivs, func(p *milvuspb.PrivilegeEntity) (string, struct{}) {
280-
return util.GetObjectType(p.Name), struct{}{}
278+
// check if privileges are the same privilege level
279+
privilegeLevels := lo.SliceToMap(newPrivs, func(p *milvuspb.PrivilegeEntity) (string, struct{}) {
280+
return util.GetPrivilegeLevel(p.Name), struct{}{}
281281
})
282-
if len(objectTypes) > 1 {
283-
return nil, errors.New("privileges are not the same object type")
282+
if len(privilegeLevels) > 1 {
283+
return nil, errors.New("privileges are not the same privilege level")
284284
}
285285
case milvuspb.OperatePrivilegeGroupType_RemovePrivilegesFromGroup:
286286
newPrivs, _ := lo.Difference(v, in.Privileges)

‎internal/rootcoord/root_coord.go‎

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2595,6 +2595,10 @@ func (c *Core) OperatePrivilege(ctx context.Context, in *milvuspb.OperatePrivile
25952595
ctxLog.Error("", zap.Error(err))
25962596
return merr.StatusWithErrorCode(err, commonpb.ErrorCode_OperatePrivilegeFailure), nil
25972597
}
2598+
if err := c.validatePrivilegeGroupParams(ctx, privName, in.Entity.DbName, in.Entity.ObjectName); err != nil {
2599+
ctxLog.Error("", zap.Error(err))
2600+
return merr.StatusWithErrorCode(err, commonpb.ErrorCode_OperatePrivilegeFailure), nil
2601+
}
25982602
// set up object type for metastore, to be compatible with v1 version
25992603
in.Entity.Object.Name = util.GetObjectType(privName)
26002604
default:
@@ -2656,6 +2660,42 @@ func (c *Core) operatePrivilegeCommonCheck(ctx context.Context, in *milvuspb.Ope
26562660
return nil
26572661
}
26582662

2663+
func (c *Core) validatePrivilegeGroupParams(ctx context.Context, entity string, dbName string, collectionName string) error {
2664+
allGroups, err := c.getDefaultAndCustomPrivilegeGroups(ctx)
2665+
if err != nil {
2666+
return err
2667+
}
2668+
groups := lo.SliceToMap(allGroups, func(group *milvuspb.PrivilegeGroupInfo) (string, []*milvuspb.PrivilegeEntity) {
2669+
return group.GroupName, group.Privileges
2670+
})
2671+
privs, exists := groups[entity]
2672+
if !exists || len(privs) == 0 {
2673+
// it is a privilege, no need to check with other params
2674+
return nil
2675+
}
2676+
// since all privileges are same level in a group, just check the first privilege
2677+
level := util.GetPrivilegeLevel(privs[0].GetName())
2678+
switch level {
2679+
case milvuspb.PrivilegeLevel_Cluster.String():
2680+
if !util.IsAnyWord(dbName) || !util.IsAnyWord(collectionName) {
2681+
return merr.WrapErrParameterInvalidMsg("dbName and collectionName should be * for the cluster level privilege: %s", entity)
2682+
}
2683+
return nil
2684+
case milvuspb.PrivilegeLevel_Database.String():
2685+
if collectionName != "" && collectionName != util.AnyWord {
2686+
return merr.WrapErrParameterInvalidMsg("collectionName should be * for the database level privilege: %s", entity)
2687+
}
2688+
return nil
2689+
case milvuspb.PrivilegeLevel_Collection.String():
2690+
if util.IsAnyWord(dbName) && !util.IsAnyWord(collectionName) && collectionName != "" {
2691+
return merr.WrapErrParameterInvalidMsg("please specify database name for the collection level privilege: %s", entity)
2692+
}
2693+
return nil
2694+
default:
2695+
return errors.New("not found the privilege level")
2696+
}
2697+
}
2698+
26592699
func (c *Core) getMetastorePrivilegeName(ctx context.Context, privName string) (string, error) {
26602700
// if it is built-in privilege, return the privilege name directly
26612701
if util.IsPrivilegeNameDefined(privName) {
@@ -2757,7 +2797,7 @@ func (c *Core) ListPolicy(ctx context.Context, in *internalpb.ListPolicyRequest)
27572797
}, nil
27582798
}
27592799
// expand privilege groups and turn to policies
2760-
allGroups, err := c.getPrivilegeGroups(ctx)
2800+
allGroups, err := c.getDefaultAndCustomPrivilegeGroups(ctx)
27612801
if err != nil {
27622802
errMsg := "fail to get privilege groups"
27632803
ctxLog.Warn(errMsg, zap.Error(err))
@@ -3131,8 +3171,8 @@ func (c *Core) expandPrivilegeGroups(ctx context.Context, grants []*milvuspb.Gra
31313171
}), nil
31323172
}
31333173

3134-
// getPrivilegeGroups returns default privilege groups and user-defined privilege groups.
3135-
func (c *Core) getPrivilegeGroups(ctx context.Context) ([]*milvuspb.PrivilegeGroupInfo, error) {
3174+
// getDefaultAndCustomPrivilegeGroups returns default privilege groups and user-defined privilege groups.
3175+
func (c *Core) getDefaultAndCustomPrivilegeGroups(ctx context.Context) ([]*milvuspb.PrivilegeGroupInfo, error) {
31363176
allGroups, err := c.meta.ListPrivilegeGroups(ctx)
31373177
allGroups = append(allGroups, Params.RbacConfig.GetDefaultPrivilegeGroups()...)
31383178
if err != nil {

‎pkg/util/constant.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/samber/lo"
2323

2424
"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
25+
"github.com/milvus-io/milvus-proto/go-api/v2/milvuspb"
2526
"github.com/milvus-io/milvus/pkg/common"
2627
"github.com/milvus-io/milvus/pkg/util/typeutil"
2728
)
@@ -292,6 +293,124 @@ var (
292293
}
293294
)
294295

296+
// rbac v2 uses privilege level to group privileges rather than object type
297+
var (
298+
CollectionReadOnlyPrivileges = ConvertPrivileges([]string{
299+
commonpb.ObjectPrivilege_PrivilegeQuery.String(),
300+
commonpb.ObjectPrivilege_PrivilegeSearch.String(),
301+
commonpb.ObjectPrivilege_PrivilegeIndexDetail.String(),
302+
commonpb.ObjectPrivilege_PrivilegeGetFlushState.String(),
303+
commonpb.ObjectPrivilege_PrivilegeGetLoadState.String(),
304+
commonpb.ObjectPrivilege_PrivilegeGetLoadingProgress.String(),
305+
commonpb.ObjectPrivilege_PrivilegeHasPartition.String(),
306+
commonpb.ObjectPrivilege_PrivilegeShowPartitions.String(),
307+
commonpb.ObjectPrivilege_PrivilegeDescribeCollection.String(),
308+
commonpb.ObjectPrivilege_PrivilegeDescribeAlias.String(),
309+
commonpb.ObjectPrivilege_PrivilegeGetStatistics.String(),
310+
commonpb.ObjectPrivilege_PrivilegeListAliases.String(),
311+
})
312+
313+
CollectionReadWritePrivileges = append(CollectionReadOnlyPrivileges,
314+
ConvertPrivileges([]string{
315+
commonpb.ObjectPrivilege_PrivilegeLoad.String(),
316+
commonpb.ObjectPrivilege_PrivilegeRelease.String(),
317+
commonpb.ObjectPrivilege_PrivilegeInsert.String(),
318+
commonpb.ObjectPrivilege_PrivilegeDelete.String(),
319+
commonpb.ObjectPrivilege_PrivilegeUpsert.String(),
320+
commonpb.ObjectPrivilege_PrivilegeImport.String(),
321+
commonpb.ObjectPrivilege_PrivilegeFlush.String(),
322+
commonpb.ObjectPrivilege_PrivilegeCompaction.String(),
323+
commonpb.ObjectPrivilege_PrivilegeLoadBalance.String(),
324+
commonpb.ObjectPrivilege_PrivilegeCreateIndex.String(),
325+
commonpb.ObjectPrivilege_PrivilegeDropIndex.String(),
326+
commonpb.ObjectPrivilege_PrivilegeCreatePartition.String(),
327+
commonpb.ObjectPrivilege_PrivilegeDropPartition.String(),
328+
})...,
329+
)
330+
331+
CollectionAdminPrivileges = append(CollectionReadWritePrivileges,
332+
ConvertPrivileges([]string{
333+
commonpb.ObjectPrivilege_PrivilegeCreateAlias.String(),
334+
commonpb.ObjectPrivilege_PrivilegeDropAlias.String(),
335+
})...,
336+
)
337+
338+
DatabaseReadOnlyPrivileges = ConvertPrivileges([]string{
339+
commonpb.ObjectPrivilege_PrivilegeShowCollections.String(),
340+
commonpb.ObjectPrivilege_PrivilegeDescribeDatabase.String(),
341+
})
342+
343+
DatabaseReadWritePrivileges = append(DatabaseReadOnlyPrivileges,
344+
ConvertPrivileges([]string{
345+
commonpb.ObjectPrivilege_PrivilegeAlterDatabase.String(),
346+
})...,
347+
)
348+
349+
DatabaseAdminPrivileges = append(DatabaseReadWritePrivileges,
350+
ConvertPrivileges([]string{
351+
commonpb.ObjectPrivilege_PrivilegeCreateCollection.String(),
352+
commonpb.ObjectPrivilege_PrivilegeDropCollection.String(),
353+
})...,
354+
)
355+
356+
ClusterReadOnlyPrivileges = ConvertPrivileges([]string{
357+
commonpb.ObjectPrivilege_PrivilegeListDatabases.String(),
358+
commonpb.ObjectPrivilege_PrivilegeSelectOwnership.String(),
359+
commonpb.ObjectPrivilege_PrivilegeSelectUser.String(),
360+
commonpb.ObjectPrivilege_PrivilegeDescribeResourceGroup.String(),
361+
commonpb.ObjectPrivilege_PrivilegeListResourceGroups.String(),
362+
commonpb.ObjectPrivilege_PrivilegeListPrivilegeGroups.String(),
363+
})
364+
365+
ClusterReadWritePrivileges = append(ClusterReadOnlyPrivileges,
366+
ConvertPrivileges([]string{
367+
commonpb.ObjectPrivilege_PrivilegeFlushAll.String(),
368+
commonpb.ObjectPrivilege_PrivilegeTransferNode.String(),
369+
commonpb.ObjectPrivilege_PrivilegeTransferReplica.String(),
370+
commonpb.ObjectPrivilege_PrivilegeUpdateResourceGroups.String(),
371+
})...,
372+
)
373+
374+
ClusterAdminPrivileges = append(ClusterReadWritePrivileges,
375+
ConvertPrivileges([]string{
376+
commonpb.ObjectPrivilege_PrivilegeBackupRBAC.String(),
377+
commonpb.ObjectPrivilege_PrivilegeRestoreRBAC.String(),
378+
commonpb.ObjectPrivilege_PrivilegeCreateDatabase.String(),
379+
commonpb.ObjectPrivilege_PrivilegeDropDatabase.String(),
380+
commonpb.ObjectPrivilege_PrivilegeCreateOwnership.String(),
381+
commonpb.ObjectPrivilege_PrivilegeDropOwnership.String(),
382+
commonpb.ObjectPrivilege_PrivilegeManageOwnership.String(),
383+
commonpb.ObjectPrivilege_PrivilegeCreateResourceGroup.String(),
384+
commonpb.ObjectPrivilege_PrivilegeDropResourceGroup.String(),
385+
commonpb.ObjectPrivilege_PrivilegeUpdateUser.String(),
386+
commonpb.ObjectPrivilege_PrivilegeRenameCollection.String(),
387+
commonpb.ObjectPrivilege_PrivilegeCreatePrivilegeGroup.String(),
388+
commonpb.ObjectPrivilege_PrivilegeDropPrivilegeGroup.String(),
389+
commonpb.ObjectPrivilege_PrivilegeOperatePrivilegeGroup.String(),
390+
})...,
391+
)
392+
)
393+
394+
// ConvertPrivileges converts each privilege from metastore format to API format.
395+
func ConvertPrivileges(privileges []string) []string {
396+
return lo.Map(privileges, func(name string, _ int) string {
397+
return MetaStore2API(name)
398+
})
399+
}
400+
401+
func GetPrivilegeLevel(privilege string) string {
402+
if lo.Contains(ClusterAdminPrivileges, privilege) {
403+
return milvuspb.PrivilegeLevel_Cluster.String()
404+
}
405+
if lo.Contains(DatabaseAdminPrivileges, privilege) {
406+
return milvuspb.PrivilegeLevel_Database.String()
407+
}
408+
if lo.Contains(CollectionAdminPrivileges, privilege) {
409+
return milvuspb.PrivilegeLevel_Collection.String()
410+
}
411+
return ""
412+
}
413+
295414
// StringSet convert array to map for conveniently check if the array contains an element
296415
func StringSet(strings []string) map[string]struct{} {
297416
stringsMap := make(map[string]struct{})

‎pkg/util/paramtable/rbac_config_test.go‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,24 @@ import (
2222
"github.com/stretchr/testify/assert"
2323
)
2424

25-
func TestRbacConfig_Init(t *testing.T) {
25+
func TestRbacConfig_DefaultPrivileges(t *testing.T) {
2626
params := ComponentParam{}
2727
params.Init(NewBaseTable(SkipRemote(true)))
2828
cfg := &params.RbacConfig
2929
assert.Equal(t, len(cfg.GetDefaultPrivilegeGroupNames()), 9)
30-
assert.True(t, cfg.IsCollectionPrivilegeGroup("CollectionReadOnly"))
31-
assert.False(t, cfg.IsCollectionPrivilegeGroup("DatabaseReadOnly"))
3230
assert.Equal(t, cfg.Enabled.GetAsBool(), false)
33-
assert.Equal(t, cfg.ClusterReadOnlyPrivileges.GetAsStrings(), builtinPrivilegeGroups["ClusterReadOnly"])
34-
assert.Equal(t, cfg.ClusterReadWritePrivileges.GetAsStrings(), builtinPrivilegeGroups["ClusterReadWrite"])
35-
assert.Equal(t, cfg.ClusterAdminPrivileges.GetAsStrings(), builtinPrivilegeGroups["ClusterAdmin"])
36-
assert.Equal(t, cfg.DBReadOnlyPrivileges.GetAsStrings(), builtinPrivilegeGroups["DatabaseReadOnly"])
37-
assert.Equal(t, cfg.DBReadWritePrivileges.GetAsStrings(), builtinPrivilegeGroups["DatabaseReadWrite"])
38-
assert.Equal(t, cfg.DBAdminPrivileges.GetAsStrings(), builtinPrivilegeGroups["DatabaseAdmin"])
39-
assert.Equal(t, cfg.CollectionReadOnlyPrivileges.GetAsStrings(), builtinPrivilegeGroups["CollectionReadOnly"])
40-
assert.Equal(t, cfg.CollectionReadWritePrivileges.GetAsStrings(), builtinPrivilegeGroups["CollectionReadWrite"])
41-
assert.Equal(t, cfg.CollectionAdminPrivileges.GetAsStrings(), builtinPrivilegeGroups["CollectionAdmin"])
31+
assert.Equal(t, cfg.ClusterReadOnlyPrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("ClusterReadOnly"))
32+
assert.Equal(t, cfg.ClusterReadWritePrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("ClusterReadWrite"))
33+
assert.Equal(t, cfg.ClusterAdminPrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("ClusterAdmin"))
34+
assert.Equal(t, cfg.DBReadOnlyPrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("DatabaseReadOnly"))
35+
assert.Equal(t, cfg.DBReadWritePrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("DatabaseReadWrite"))
36+
assert.Equal(t, cfg.DBAdminPrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("DatabaseAdmin"))
37+
assert.Equal(t, cfg.CollectionReadOnlyPrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("CollectionReadOnly"))
38+
assert.Equal(t, cfg.CollectionReadWritePrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("CollectionReadWrite"))
39+
assert.Equal(t, cfg.CollectionAdminPrivileges.GetAsStrings(), cfg.GetDefaultPrivilegeGroupPrivileges("CollectionAdmin"))
4240
}
4341

44-
func TestRbacConfig_Override(t *testing.T) {
42+
func TestRbacConfig_OverridePrivileges(t *testing.T) {
4543
params := ComponentParam{}
4644
params.Init(NewBaseTable(SkipRemote(true)))
4745

0 commit comments

Comments
 (0)