Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix rbac v2 grantor and error messages #38004

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/proxy/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -5345,7 +5345,7 @@ func (node *Proxy) validOperatePrivilegeV2Params(req *milvuspb.OperatePrivilegeV
if req.Type != milvuspb.OperatePrivilegeType_Grant && req.Type != milvuspb.OperatePrivilegeType_Revoke {
return fmt.Errorf("the type in the request not grant or revoke")
}
if err := ValidateObjectName(req.DbName); err != nil {
if err := ValidateParamDatabaseName(req.DbName); err != nil {
return err
}
if err := ValidateObjectName(req.CollectionName); err != nil {
Expand Down
24 changes: 16 additions & 8 deletions internal/proxy/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,26 +157,27 @@ func validateCollectionNameOrAlias(entity, entityType string) error {
}

func ValidatePrivilegeGroupName(groupName string) error {
groupName = strings.TrimSpace(groupName)

if groupName == "" {
return merr.WrapErrDatabaseNameInvalid(groupName, "privilege group name couldn't be empty")
return merr.WrapErrParameterInvalidMsg("privilege group name should not be empty")
}

invalidMsg := fmt.Sprintf("Invalid privilege group %s. ", groupName)
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.WrapErrParameterInvalidMsg("%s the length of a privilege group %s must be less than %s characters", invalidMsg, 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.WrapErrParameterInvalidMsg("%s the first character of a privilege group %s must be an underscore or letter", invalidMsg, 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("%s privielge group %s can only contain numbers, letters and underscores", invalidMsg, groupName)
}
}
return nil
Expand Down Expand Up @@ -207,6 +208,13 @@ func ValidateResourceGroupName(entity string) error {
return nil
}

func ValidateParamDatabaseName(dbName string) error {
if util.IsAnyWord(dbName) {
return nil
}
return ValidateDatabaseName(dbName)
}

func ValidateDatabaseName(dbName string) error {
if dbName == "" {
return merr.WrapErrDatabaseNameInvalid(dbName, "database name couldn't be empty")
Expand Down Expand Up @@ -1099,7 +1107,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 {
Expand Down
26 changes: 13 additions & 13 deletions internal/rootcoord/root_coord.go
Original file line number Diff line number Diff line change
Expand Up @@ -3099,12 +3099,11 @@
ctxLog.Debug(method)

if err := merr.CheckHealthy(c.GetStateCode()); err != nil {
return merr.Status(err), nil
return merr.StatusWithErrorCode(err, commonpb.ErrorCode_CreatePrivilegeGroupFailure), nil

Check warning on line 3102 in internal/rootcoord/root_coord.go

View check run for this annotation

Codecov / codecov/patch

internal/rootcoord/root_coord.go#L3102

Added line #L3102 was not covered by tests
}

if err := c.meta.CreatePrivilegeGroup(ctx, 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")
Expand All @@ -3122,12 +3121,11 @@
ctxLog.Debug(method)

if err := merr.CheckHealthy(c.GetStateCode()); err != nil {
return merr.Status(err), nil
return merr.StatusWithErrorCode(err, commonpb.ErrorCode_DropPrivilegeGroupFailure), nil

Check warning on line 3124 in internal/rootcoord/root_coord.go

View check run for this annotation

Codecov / codecov/patch

internal/rootcoord/root_coord.go#L3124

Added line #L3124 was not covered by tests
}

if err := c.meta.DropPrivilegeGroup(ctx, 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")
Expand Down Expand Up @@ -3312,10 +3310,7 @@

err := redoTask.Execute(ctx)
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")
Expand All @@ -3327,16 +3322,21 @@
func (c *Core) expandPrivilegeGroups(ctx context.Context, grants []*milvuspb.GrantEntity, groups map[string][]*milvuspb.PrivilegeEntity) ([]*milvuspb.GrantEntity, error) {
newGrants := []*milvuspb.GrantEntity{}
createGrantEntity := func(grant *milvuspb.GrantEntity, privilegeName string) (*milvuspb.GrantEntity, error) {
metaName, err := c.getMetastorePrivilegeName(ctx, privilegeName)
metaName, err := c.getMetastorePrivilegeName(c.ctx, privilegeName)
if err != nil {
return nil, err
}
var newObject *milvuspb.ObjectEntity
if objectType := util.GetObjectType(privilegeName); objectType != "" {
grant.Object.Name = objectType
newObject = &milvuspb.ObjectEntity{
Name: objectType,
}
} else {
newObject = grant.Object

Check warning on line 3335 in internal/rootcoord/root_coord.go

View check run for this annotation

Codecov / codecov/patch

internal/rootcoord/root_coord.go#L3335

Added line #L3335 was not covered by tests
}
return &milvuspb.GrantEntity{
Role: grant.Role,
Object: grant.Object,
Object: newObject,
ObjectName: grant.ObjectName,
Grantor: &milvuspb.GrantorEntity{
User: grant.Grantor.User,
Expand Down
Loading