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

Disable ability to add user to admin user group, unless user himself has admin rights #11208

Open
vierkantemeter opened this issue Mar 14, 2014 · 15 comments
Labels
area-security proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. urgent The issue requires attention and has higher priority over others.

Comments

@vierkantemeter
Copy link

Disable ability to add a user to the admin user group, unless the user who is trying to do this has admin rights himself. (Or; restrict users to add new users with higher permissions than they have themselves)

Now, content-editors with permissions to add new users can also create admin users, or give themselves admin rights.

This is a huge security flaw in my view, becasue I have content-editors who would like to.. experiment. ;)

@exside
Copy link
Contributor

exside commented Mar 14, 2014

+1

@ReSpawN
Copy link

ReSpawN commented Apr 2, 2014

+1, with an additional option to restrict it to SUDO (some of our customers are experienced MODX-ers themselfs but we don't want them to create new Admins, that's restricted to SUDO users.

@thomasd
Copy link
Contributor

thomasd commented Apr 2, 2014

👍

@rthrash rthrash modified the milestones: Revolution-2.3.0-beta-1, Revolution-2.2.14-pl Apr 5, 2014
@wuuti
Copy link
Contributor

wuuti commented Apr 17, 2014

+1

1 similar comment
@pixelchutes
Copy link
Contributor

👍

@jpdevries
Copy link
Contributor

Or herself ;)—
Sent from Mailbox for iPhone

On Thu, Apr 17, 2014 at 2:11 AM, Jens Külzer notifications@github.com
wrote:

+1

Reply to this email directly or view it on GitHub:
#11208 (comment)

@ReSpawN
Copy link

ReSpawN commented Apr 17, 2014

... yes, that too ...

@pixelchutes
Copy link
Contributor

I am currently looking into this and am curious how others would prefer to see this approached?

For example:

  • Only allow Manager users to assign Roles lesser than (or equal to) the max Authority level of their currently assigned Roles, e.g.:
    • Prevent adding others (or themselves) to Administrator group with a lower Authority e.g. Super User Role
    • Allow assigning to any possible User Groups OR somehow provide a way to limit to certain Groups (similar to aforementioned Role restrictions?)
    • Prevent enabling the sudo flag
  • Preventing Edits (and Viewing Profiles?) of higher ranked users (determined by Role > Authority).
  • Sudo handling:
    • Only a sudo user can access / edit / assign Roles to another sudo user.
    • Administrators with Super User Role: Yes/No?
  • Can any changes to privilege escalation handling be applied without the introduction of additional Permissions / Policies?
  • Other?

Also, this issue appears related to #7547 and #10443

@exside
Copy link
Contributor

exside commented May 12, 2014

I think

  • Prevent adding others (or themselves) to Administrator group with a lower Authority e.g. Super User Role

and

  • Sudo handling:
    Only a sudo user can access / edit / assign Roles to another sudo user.
    Administrators with Super User Role: Yes

would be fine for me

@pixelchutes
Copy link
Contributor

Thanks for that feedback, @exside.

I guess another item worth discussing is the current ability to Deactivate and Block higher authority Manager Users.

Currently, you can simply block / deactivate an Administrator (Super User) or sudo user with minimal permissions yourself.

@rthrash
Copy link
Member

rthrash commented May 12, 2014

Good catches Mike and @exside definitely should be able to do that either IMO.

@exside
Copy link
Contributor

exside commented May 13, 2014

definitely a good catch! that should not be possible (lower permission deactivate higher)!

@SebastienDaniel
Copy link

+1

@theboxer theboxer removed this from the v2.3.2-pl milestone Oct 21, 2014
@theboxer theboxer removed this from the v2.3.3-pl milestone Jan 7, 2015
@victorbanev
Copy link

So... problem is solved? or forgotten? :)))))
or what?
why it was removed from milestone?

@pixelchutes
Copy link
Contributor

Problem is not solved. Milestone was removed because a solution has yet to be provided in time for that release.

@alroniks alroniks added urgent The issue requires attention and has higher priority over others. and removed priority-2-high labels Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-security proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. urgent The issue requires attention and has higher priority over others.
Projects
None yet
Development

No branches or pull requests