fix: rootCoord decide the builtin role cannot be deleted (#29248)

issue: #29243

only rootCoord read the configuration item `builtinRoles`, so proxy
never know whether the role to be deleted is builtin.

Signed-off-by: PowderLi <[email protected]>

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)

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,

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

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 {

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))

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())

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
+}