diff --git a/internal/proxy/impl.go b/internal/proxy/impl.go index 8219c70f48b54..e503b7fa380da 100644 --- a/internal/proxy/impl.go +++ b/internal/proxy/impl.go @@ -4235,8 +4235,8 @@ func (node *Proxy) DropRole(ctx context.Context, req *milvuspb.DropRoleRequest) if err := ValidateRoleName(req.RoleName); err != nil { return merr.Status(err), nil } - if IsBuiltinRole(req.RoleName) { - err := merr.WrapErrPrivilegeNotPermitted("the role[%s] is a default role, which can't be droped", req.GetRoleName()) + if IsDefaultRole(req.RoleName) { + err := merr.WrapErrPrivilegeNotPermitted("the role[%s] is a default role, which can't be dropped", req.GetRoleName()) return merr.Status(err), nil } result, err := node.rootCoord.DropRole(ctx, req) diff --git a/internal/proxy/privilege_interceptor.go b/internal/proxy/privilege_interceptor.go index acd386ded2fee..9eb3e8d77f4f9 100644 --- a/internal/proxy/privilege_interceptor.go +++ b/internal/proxy/privilege_interceptor.go @@ -168,7 +168,7 @@ func PrivilegeInterceptor(ctx context.Context, req interface{}) (context.Context } log.Info("permission deny", zap.Strings("roles", roleNames)) - return ctx, status.Error(codes.PermissionDenied, fmt.Sprintf("%s: permission deny", objectPrivilege)) + return ctx, status.Error(codes.PermissionDenied, fmt.Sprintf("%s: permission deny to %s", objectPrivilege, username)) } // isCurUserObject Determine whether it is an Object of type User that operates on its own user information, diff --git a/internal/proxy/util.go b/internal/proxy/util.go index e58a2faaabf68..e54f30b0d7283 100644 --- a/internal/proxy/util.go +++ b/internal/proxy/util.go @@ -835,18 +835,6 @@ func IsDefaultRole(roleName string) bool { return false } -func IsBuiltinRole(roleName string) bool { - if IsDefaultRole(roleName) { - return true - } - for _, builtinRole := range util.BuiltinRoles { - if builtinRole == roleName { - return true - } - } - return false -} - func ValidateObjectName(entity string) error { if util.IsAnyWord(entity) { return nil diff --git a/internal/proxy/util_test.go b/internal/proxy/util_test.go index 91b93605989ef..1bbc147f4da4e 100644 --- a/internal/proxy/util_test.go +++ b/internal/proxy/util_test.go @@ -816,13 +816,8 @@ func TestValidateName(t *testing.T) { func TestIsDefaultRole(t *testing.T) { assert.Equal(t, true, IsDefaultRole(util.RoleAdmin)) - assert.Equal(t, true, IsBuiltinRole(util.RoleAdmin)) assert.Equal(t, true, IsDefaultRole(util.RolePublic)) - assert.Equal(t, true, IsBuiltinRole(util.RolePublic)) assert.Equal(t, false, IsDefaultRole("manager")) - assert.Equal(t, false, IsBuiltinRole("manager")) - util.BuiltinRoles = append(util.BuiltinRoles, "manager") - assert.Equal(t, true, IsBuiltinRole("manager")) } func GetContext(ctx context.Context, originValue string) context.Context { diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index bc74a61a747b9..5da64031340bd 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -2308,6 +2308,10 @@ func (c *Core) DropRole(ctx context.Context, in *milvuspb.DropRoleRequest) (*com if err := merr.CheckHealthy(c.GetStateCode()); err != nil { return merr.Status(err), nil } + for util.IsBuiltinRole(in.GetRoleName()) { + err := merr.WrapErrPrivilegeNotPermitted("the role[%s] is a builtin role, which can't be dropped", in.GetRoleName()) + return merr.Status(err), nil + } if _, err := c.meta.SelectRole(util.DefaultTenant, &milvuspb.RoleEntity{Name: in.RoleName}, false); err != nil { errMsg := "not found the role, maybe the role isn't existed or internal system error" ctxLog.Warn(errMsg, zap.Error(err)) diff --git a/internal/rootcoord/root_coord_test.go b/internal/rootcoord/root_coord_test.go index 3e855c48f07d9..08a1d998c1ee0 100644 --- a/internal/rootcoord/root_coord_test.go +++ b/internal/rootcoord/root_coord_test.go @@ -49,6 +49,7 @@ import ( "github.com/milvus-io/milvus/internal/util/importutil" "github.com/milvus-io/milvus/internal/util/sessionutil" "github.com/milvus-io/milvus/pkg/common" + "github.com/milvus-io/milvus/pkg/util" "github.com/milvus-io/milvus/pkg/util/etcd" "github.com/milvus-io/milvus/pkg/util/funcutil" "github.com/milvus-io/milvus/pkg/util/merr" @@ -2088,9 +2089,10 @@ func TestRootCoord_RBACError(t *testing.T) { } func TestRootCoord_BuiltinRoles(t *testing.T) { + roleDbAdmin := "db_admin" paramtable.Init() paramtable.Get().Save(paramtable.Get().RoleCfg.Enabled.Key, "true") - paramtable.Get().Save(paramtable.Get().RoleCfg.Roles.Key, `{"db_admin": {"privileges": [{"object_type": "Global", "object_name": "*", "privilege": "CreateCollection", "db_name": "*"}]}}`) + paramtable.Get().Save(paramtable.Get().RoleCfg.Roles.Key, `{"`+roleDbAdmin+`": {"privileges": [{"object_type": "Global", "object_name": "*", "privilege": "CreateCollection", "db_name": "*"}]}}`) t.Run("init builtin roles success", func(t *testing.T) { c := newTestCore(withHealthyCode(), withInvalidMeta()) mockMeta := c.meta.(*mockMetaTable) @@ -2102,6 +2104,11 @@ func TestRootCoord_BuiltinRoles(t *testing.T) { } err := c.initBuiltinRoles() assert.Equal(t, nil, err) + assert.True(t, util.IsBuiltinRole(roleDbAdmin)) + assert.False(t, util.IsBuiltinRole(util.RoleAdmin)) + resp, err := c.DropRole(context.Background(), &milvuspb.DropRoleRequest{RoleName: roleDbAdmin}) + assert.Equal(t, nil, err) + assert.Equal(t, int32(1401), resp.Code) // merr.ErrPrivilegeNotPermitted }) t.Run("init builtin roles fail to create role", func(t *testing.T) { c := newTestCore(withHealthyCode(), withInvalidMeta()) diff --git a/pkg/util/constant.go b/pkg/util/constant.go index 28fa62e6ac201..1c6e5bf3ffa6d 100644 --- a/pkg/util/constant.go +++ b/pkg/util/constant.go @@ -182,3 +182,12 @@ func PrivilegeNameForMetastore(name string) string { func IsAnyWord(word string) bool { return word == AnyWord } + +func IsBuiltinRole(roleName string) bool { + for _, builtinRole := range BuiltinRoles { + if builtinRole == roleName { + return true + } + } + return false +}