Skip to content

fix(auth-center): restrict role assignment in user create and update - #40

Merged
danilovid merged 5 commits into
mainfrom
hotfix/auth-center-role-escalation
Aug 28, 2026
Merged

fix(auth-center): restrict role assignment in user create and update#40
danilovid merged 5 commits into
mainfrom
hotfix/auth-center-role-escalation

Conversation

@danilovid

Copy link
Copy Markdown
Collaborator

No description provided.

return status.Errorf(codes.Unauthenticated, "can't get token: %v", err)
}

if token.Role.ID == model.AdminRoleID {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last admin can be demoted here. need to additionally check isn't it a last admin user

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed!

Comment thread auth-center/pkg/service/user_generic.go Outdated
"can't modify an administrator account").Err()
}

if newRoleID != token.Role.ID {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

role preserving requests will be failed too, i think it should be smth like

if newRoleID != token.Role.ID && newRoleID != currentRoleID {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed!

Comment thread auth-center/pkg/service/user_generic.go Outdated

if newRoleID != token.Role.ID && newRoleID != currentRoleID {
return errcommon.StatusWithReason(codes.PermissionDenied, RoleAssignmentRestricted,
"can't assign a role other than your own").Err()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of such limitation? shouldn't admin be able to create users with other roles?

p.s currently, admin and security engineer are allowed to create users. i suggest removing this permission from security engineer, since it doesn't make sense imo. after we do it, we can get rid of this check safely if i'm not mistaken. what do you think?

@danilovid
danilovid merged commit dae3fbb into main Aug 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants