diff --git a/internal/metastore/kv/kv_catalog.go b/internal/metastore/kv/kv_catalog.go index dca1cf4396..59e2a21c0b 100644 --- a/internal/metastore/kv/kv_catalog.go +++ b/internal/metastore/kv/kv_catalog.go @@ -1049,13 +1049,18 @@ func (kc *Catalog) SelectGrant(ctx context.Context, tenant string, entity *milvu return err } for _, grantorEntity := range grantPrivilegeEntity.Entities { + privilegeName := util.PrivilegeNameForAPI(grantorEntity.Privilege.Name) + if grantorEntity.Privilege.Name == util.AnyWord { + privilegeName = util.AnyWord + } + entities = append(entities, &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: entity.Role.Name}, Object: &milvuspb.ObjectEntity{Name: object}, ObjectName: objectName, Grantor: &milvuspb.GrantorEntity{ User: &milvuspb.UserEntity{Name: grantorEntity.User.Name}, - Privilege: &milvuspb.PrivilegeEntity{Name: util.PrivilegeNameForAPI(grantorEntity.Privilege.Name)}, + Privilege: &milvuspb.PrivilegeEntity{Name: privilegeName}, }, }) } diff --git a/internal/proxy/impl.go b/internal/proxy/impl.go index 6d96006130..cda4912fc8 100644 --- a/internal/proxy/impl.go +++ b/internal/proxy/impl.go @@ -4095,6 +4095,13 @@ func (node *Proxy) DropRole(ctx context.Context, req *milvuspb.DropRoleRequest) Reason: err.Error(), }, err } + if IsDefaultRole(req.RoleName) { + errMsg := fmt.Sprintf("the role[%s] is a default role, which can't be droped", req.RoleName) + return &commonpb.Status{ + ErrorCode: commonpb.ErrorCode_IllegalArgument, + Reason: errMsg, + }, errors.New(errMsg) + } result, err := node.rootCoord.DropRole(ctx, req) if err != nil { logger.Error("fail to drop role", zap.String("role_name", req.RoleName), zap.Error(err)) diff --git a/internal/proxy/privilege_interceptor.go b/internal/proxy/privilege_interceptor.go index e60a9c5c5c..e1f818e5c2 100644 --- a/internal/proxy/privilege_interceptor.go +++ b/internal/proxy/privilege_interceptor.go @@ -36,7 +36,7 @@ p = sub, obj, act e = some(where (p.eft == allow)) [matchers] -m = r.sub == p.sub && globMatch(p.obj, r.obj) && globMatch(p.act, r.act) || r.sub == "admin" || r.act == "All" +m = r.sub == p.sub && globMatch(r.obj, p.obj) && globMatch(r.act, p.act) || r.sub == "admin" || r.act == "All" ` ModelKey = "casbin" ) @@ -83,6 +83,9 @@ func PrivilegeInterceptor(ctx context.Context, req interface{}) (context.Context log.Error("GetCurUserFromContext fail", zap.Error(err)) return ctx, err } + if username == util.UserRoot { + return ctx, nil + } roleNames, err := GetRole(username) if err != nil { log.Error("GetRole fail", zap.String("username", username), zap.Error(err)) diff --git a/internal/proxy/privilege_interceptor_test.go b/internal/proxy/privilege_interceptor_test.go index f506a0c874..e045b46b41 100644 --- a/internal/proxy/privilege_interceptor_test.go +++ b/internal/proxy/privilege_interceptor_test.go @@ -65,6 +65,12 @@ func TestPrivilegeInterceptor(t *testing.T) { }) assert.NotNil(t, err) + _, err = PrivilegeInterceptor(GetContext(context.Background(), "root:123456"), &milvuspb.LoadCollectionRequest{ + DbName: "db_test", + CollectionName: "col1", + }) + assert.Nil(t, err) + err = InitMetaCache(ctx, client, queryCoord, mgr) assert.Nil(t, err) _, err = PrivilegeInterceptor(ctx, &milvuspb.HasCollectionRequest{ diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 6f5f4e8218..73b372c105 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -3197,6 +3197,9 @@ func testProxyRole(ctx context.Context, t *testing.T, proxy *Proxy) { resp, _ := proxy.DropRole(ctx, &milvuspb.DropRoleRequest{RoleName: " "}) assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + resp, _ = proxy.DropRole(ctx, &milvuspb.DropRoleRequest{RoleName: "public"}) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + username := "root" roleName := "unit_test" @@ -3294,10 +3297,16 @@ func testProxyRole(ctx context.Context, t *testing.T, proxy *Proxy) { roleResp, _ = proxy.DropRole(ctx, &milvuspb.DropRoleRequest{RoleName: roleName}) assert.Equal(t, commonpb.ErrorCode_Success, roleResp.ErrorCode) + opResp, _ := proxy.OperateUserRole(ctx, &milvuspb.OperateUserRoleRequest{Username: "root", RoleName: "admin"}) + assert.Equal(t, commonpb.ErrorCode_Success, opResp.ErrorCode) + resp, _ = proxy.SelectRole(ctx, &milvuspb.SelectRoleRequest{Role: &milvuspb.RoleEntity{Name: "admin"}, IncludeUserInfo: true}) assert.Equal(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) assert.NotEqual(t, 0, len(resp.Results)) assert.NotEqual(t, 0, len(resp.Results[0].Users)) + + opResp, _ = proxy.OperateUserRole(ctx, &milvuspb.OperateUserRoleRequest{Username: "root", RoleName: "admin", Type: milvuspb.OperateUserRoleType_RemoveUserFromRole}) + assert.Equal(t, commonpb.ErrorCode_Success, opResp.ErrorCode) }) wg.Add(1) @@ -3318,10 +3327,16 @@ func testProxyRole(ctx context.Context, t *testing.T, proxy *Proxy) { assert.Equal(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) assert.NotEqual(t, 0, len(resp.Results)) + opResp, _ := proxy.OperateUserRole(ctx, &milvuspb.OperateUserRoleRequest{Username: "root", RoleName: "admin"}) + assert.Equal(t, commonpb.ErrorCode_Success, opResp.ErrorCode) + resp, _ = proxy.SelectUser(ctx, &milvuspb.SelectUserRequest{User: entity, IncludeRoleInfo: true}) assert.Equal(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) assert.NotEqual(t, 0, len(resp.Results)) assert.NotEqual(t, 0, len(resp.Results[0].Roles)) + + opResp, _ = proxy.OperateUserRole(ctx, &milvuspb.OperateUserRoleRequest{Username: "root", RoleName: "admin", Type: milvuspb.OperateUserRoleType_RemoveUserFromRole}) + assert.Equal(t, commonpb.ErrorCode_Success, opResp.ErrorCode) }) wg.Wait() @@ -3509,6 +3524,38 @@ func testProxyPrivilege(ctx context.Context, t *testing.T, proxy *Proxy) { roleReq.Type = milvuspb.OperatePrivilegeType_Revoke resp, _ = proxy.OperatePrivilege(ctx, roleReq) assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + + roleReq = &milvuspb.OperatePrivilegeRequest{ + Entity: &milvuspb.GrantEntity{ + Role: &milvuspb.RoleEntity{Name: "public"}, + Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Collection.String()}, + ObjectName: "col1", + Grantor: &milvuspb.GrantorEntity{Privilege: &milvuspb.PrivilegeEntity{Name: util.AnyWord}}, + }, + Type: milvuspb.OperatePrivilegeType_Grant, + } + resp, _ = proxy.OperatePrivilege(ctx, roleReq) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + + roleReq.Type = milvuspb.OperatePrivilegeType_Revoke + resp, _ = proxy.OperatePrivilege(ctx, roleReq) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + + roleReq = &milvuspb.OperatePrivilegeRequest{ + Entity: &milvuspb.GrantEntity{ + Role: &milvuspb.RoleEntity{Name: "public"}, + Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Global.String()}, + ObjectName: "col1", + Grantor: &milvuspb.GrantorEntity{Privilege: &milvuspb.PrivilegeEntity{Name: util.MetaStore2API(commonpb.ObjectPrivilege_PrivilegeManageOwnership.String())}}, + }, + Type: milvuspb.OperatePrivilegeType_Grant, + } + resp, _ = proxy.OperatePrivilege(ctx, roleReq) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + + roleReq.Type = milvuspb.OperatePrivilegeType_Revoke + resp, _ = proxy.OperatePrivilege(ctx, roleReq) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) }) wg.Wait() diff --git a/internal/proxy/util.go b/internal/proxy/util.go index a361365abe..13bf3ddf26 100644 --- a/internal/proxy/util.go +++ b/internal/proxy/util.go @@ -638,18 +638,20 @@ func ValidateRoleName(entity string) error { return validateName(entity, "role name") } -func ValidateObjectName(entity string) error { - entity = strings.TrimSpace(entity) - if entity == "" { - return fmt.Errorf("objectName should not be empty") +func IsDefaultRole(roleName string) bool { + for _, defaultRole := range util.DefaultRoles { + if defaultRole == roleName { + return true + } } + return false +} - if int64(len(entity)) > Params.ProxyCfg.MaxNameLength { - msg := fmt.Sprintf("invalid object name: %s. The length of resource name must be less than %s characters", - entity, strconv.FormatInt(Params.ProxyCfg.MaxNameLength, 10)) - return errors.New(msg) +func ValidateObjectName(entity string) error { + if util.IsAnyWord(entity) { + return nil } - return nil + return validateName(entity, "role name") } func ValidateObjectType(entity string) error { @@ -665,6 +667,9 @@ func ValidatePrincipalType(entity string) error { } func ValidatePrivilege(entity string) error { + if util.IsAnyWord(entity) { + return nil + } return validateName(entity, "Privilege") } diff --git a/internal/proxy/util_test.go b/internal/proxy/util_test.go index 5db55d9a6c..c3d013eec4 100644 --- a/internal/proxy/util_test.go +++ b/internal/proxy/util_test.go @@ -665,6 +665,13 @@ func TestValidateName(t *testing.T) { } assert.NotNil(t, ValidateObjectName(" ")) assert.NotNil(t, ValidateObjectName(string(longName))) + assert.Nil(t, ValidateObjectName("*")) +} + +func TestIsDefaultRole(t *testing.T) { + assert.Equal(t, true, IsDefaultRole(util.RoleAdmin)) + assert.Equal(t, true, IsDefaultRole(util.RolePublic)) + assert.Equal(t, false, IsDefaultRole("manager")) } func GetContext(ctx context.Context, originValue string) context.Context { diff --git a/internal/rootcoord/meta_table_test.go b/internal/rootcoord/meta_table_test.go index e6ef40a1a1..677de25336 100644 --- a/internal/rootcoord/meta_table_test.go +++ b/internal/rootcoord/meta_table_test.go @@ -1434,6 +1434,7 @@ func TestRbacSelectGrant(t *testing.T) { grantPrivilegeEntity := &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ {User: &milvuspb.UserEntity{Name: "aaa"}, Privilege: &milvuspb.PrivilegeEntity{Name: "111"}}, {User: &milvuspb.UserEntity{Name: "bbb"}, Privilege: &milvuspb.PrivilegeEntity{Name: "222"}}, + {User: &milvuspb.UserEntity{Name: "ccc"}, Privilege: &milvuspb.PrivilegeEntity{Name: "*"}}, }} grantPrivilegeEntityByte, _ := proto.Marshal(grantPrivilegeEntity) @@ -1448,7 +1449,7 @@ func TestRbacSelectGrant(t *testing.T) { } results, err := mt.SelectGrant(util.DefaultTenant, entity) assert.Nil(t, err) - assert.Equal(t, 2, len(results)) + assert.Equal(t, 3, len(results)) mockTxnKV.load = func(key string) (string, error) { return "", fmt.Errorf("load error") @@ -1468,7 +1469,7 @@ func TestRbacSelectGrant(t *testing.T) { entity.Object = nil results, err = mt.SelectGrant(util.DefaultTenant, entity) assert.Nil(t, err) - assert.Equal(t, 3, len(results)) + assert.Equal(t, 4, len(results)) mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { return nil, nil, fmt.Errorf("load with prefix error") diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index c3bd34909f..6fec935b39 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -1298,14 +1298,6 @@ func (c *Core) initRbac() (initError error) { return } - // create default rolemapping, root -> admin - if initError = c.MetaTable.OperateUserRole(util.DefaultTenant, - &milvuspb.UserEntity{Name: util.UserRoot}, - &milvuspb.RoleEntity{Name: util.RoleAdmin}, - milvuspb.OperateUserRoleType_AddUserToRole); initError != nil { - return - } - // grant privileges for the public role globalPrivileges := []string{ commonpb.ObjectPrivilege_PrivilegeDescribeCollection.String(), @@ -1319,7 +1311,7 @@ func (c *Core) initRbac() (initError error) { if initError = c.MetaTable.OperatePrivilege(util.DefaultTenant, &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: util.RolePublic}, Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Global.String()}, - ObjectName: funcutil.AnyObjectName, + ObjectName: util.AnyWord, Grantor: &milvuspb.GrantorEntity{ User: &milvuspb.UserEntity{Name: util.RoleAdmin}, Privilege: &milvuspb.PrivilegeEntity{Name: globalPrivilege}, @@ -1332,7 +1324,7 @@ func (c *Core) initRbac() (initError error) { if initError = c.MetaTable.OperatePrivilege(util.DefaultTenant, &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: util.RolePublic}, Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Collection.String()}, - ObjectName: funcutil.AnyObjectName, + ObjectName: util.AnyWord, Grantor: &milvuspb.GrantorEntity{ User: &milvuspb.UserEntity{Name: util.RoleAdmin}, Privilege: &milvuspb.PrivilegeEntity{Name: collectionPrivilege}, @@ -3253,10 +3245,13 @@ func (c *Core) isValidGrantor(entity *milvuspb.GrantorEntity, object string) err if entity.Privilege == nil { return fmt.Errorf("the privilege entity in the grantor entity is nil") } + if util.IsAnyWord(entity.Privilege.Name) { + return nil + } if privilegeName := util.PrivilegeNameForMetastore(entity.Privilege.Name); privilegeName == "" { return fmt.Errorf("the privilege name in the privilege entity is invalid, current value: %s", entity.Privilege.Name) } - privileges, ok := util.GetObjectPrivileges()[object] + privileges, ok := util.ObjectPrivileges[object] if !ok { return fmt.Errorf("the object type is invalid, current value: %s", object) } @@ -3303,10 +3298,12 @@ func (c *Core) OperatePrivilege(ctx context.Context, in *milvuspb.OperatePrivile } logger.Debug("before PrivilegeNameForMetastore", zap.String("privilege", in.Entity.Grantor.Privilege.Name)) - in.Entity.Grantor.Privilege.Name = util.PrivilegeNameForMetastore(in.Entity.Grantor.Privilege.Name) + if !util.IsAnyWord(in.Entity.Grantor.Privilege.Name) { + in.Entity.Grantor.Privilege.Name = util.PrivilegeNameForMetastore(in.Entity.Grantor.Privilege.Name) + } logger.Debug("after PrivilegeNameForMetastore", zap.String("privilege", in.Entity.Grantor.Privilege.Name)) if in.Entity.Object.Name == commonpb.ObjectType_Global.String() { - in.Entity.ObjectName = funcutil.AnyObjectName + in.Entity.ObjectName = util.AnyWord } if err := c.MetaTable.OperatePrivilege(util.DefaultTenant, in.Entity, in.Type); err != nil { errMsg := "fail to operate the privilege" diff --git a/internal/util/constant.go b/internal/util/constant.go index 9ef6014e3a..fbc4f32bc4 100644 --- a/internal/util/constant.go +++ b/internal/util/constant.go @@ -43,28 +43,13 @@ const ( RolePublic = "public" PrivilegeWord = "Privilege" + AnyWord = "*" ) -// StringSet convert array to map for conveniently check if the array contains an element -func StringSet(strings []string) map[string]struct{} { - stringsMap := make(map[string]struct{}) - for _, str := range strings { - stringsMap[str] = struct{}{} - } - return stringsMap -} +var ( + DefaultRoles = []string{RoleAdmin, RolePublic} -func StringList(stringMap map[string]struct{}) []string { - strs := make([]string, 0, len(stringMap)) - for k := range stringMap { - strs = append(strs, k) - } - return strs -} - -// GetObjectPrivileges get the mapping between object types and privileges. This kind of data is constant and doesn't support to CRUD -func GetObjectPrivileges() map[string][]string { - return map[string][]string{ + ObjectPrivileges = map[string][]string{ commonpb.ObjectType_Collection.String(): { MetaStore2API(commonpb.ObjectPrivilege_PrivilegeLoad.String()), MetaStore2API(commonpb.ObjectPrivilege_PrivilegeRelease.String()), @@ -99,6 +84,23 @@ func GetObjectPrivileges() map[string][]string { MetaStore2API(commonpb.ObjectPrivilege_PrivilegeSelectUser.String()), }, } +) + +// StringSet convert array to map for conveniently check if the array contains an element +func StringSet(strings []string) map[string]struct{} { + stringsMap := make(map[string]struct{}) + for _, str := range strings { + stringsMap[str] = struct{}{} + } + return stringsMap +} + +func StringList(stringMap map[string]struct{}) []string { + strs := make([]string, 0, len(stringMap)) + for k := range stringMap { + strs = append(strs, k) + } + return strs } // MetaStore2API convert meta-store's privilege name to api's @@ -123,3 +125,7 @@ func PrivilegeNameForMetastore(name string) string { } return dbPrivilege } + +func IsAnyWord(word string) bool { + return word == AnyWord +} diff --git a/internal/util/funcutil/policy.go b/internal/util/funcutil/policy.go index 81349c1b9f..7669bad32f 100644 --- a/internal/util/funcutil/policy.go +++ b/internal/util/funcutil/policy.go @@ -3,6 +3,8 @@ package funcutil import ( "fmt" + "github.com/milvus-io/milvus/internal/util" + "github.com/golang/protobuf/descriptor" "github.com/golang/protobuf/proto" "github.com/milvus-io/milvus/internal/log" @@ -12,10 +14,6 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" ) -const ( - AnyObjectName = "*" -) - func GetVersion(m proto.GeneratedMessage) (string, error) { md, _ := descriptor.MessageDescriptorProto(m) if md == nil { @@ -56,7 +54,7 @@ func GetPrivilegeExtObj(m proto.GeneratedMessage) (commonpb.PrivilegeExt, error) // GetObjectName get object name from the grpc message according to the filed index. The filed is a string. func GetObjectName(m proto.GeneratedMessage, index int32) string { if index <= 0 { - return AnyObjectName + return util.AnyWord } msg := proto.MessageReflect(proto.MessageV1(m)) msgDesc := msg.Descriptor() @@ -66,7 +64,7 @@ func GetObjectName(m proto.GeneratedMessage, index int32) string { userDesc := user.Descriptor() value = user.Get(userDesc.Fields().ByNumber(protoreflect.FieldNumber(1))) if value.String() == "" { - return AnyObjectName + return util.AnyWord } } return value.String()