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

Tenant admin can create a super admin #13291

Closed
evertmulder opened this issue Dec 21, 2016 · 5 comments
Closed

Tenant admin can create a super admin #13291

evertmulder opened this issue Dec 21, 2016 · 5 comments

Comments

@evertmulder
Copy link
Contributor

Steps to reproduce:

  • Create child tenant: "My tenant"
  • Create group with:
    • "My Tenant admin group" with role: "EvmRole-tenant_administrator"
    • Tenant: "My tenant"
  • Create tenant admin user "tenantadmin" with group "My Tenant admin group"
  • login as tenant admin
  • I can now create a new user with the group "EvmRole-super_administrator"

This escalates the privileges of a tenant admin to global admin, defeating the tenant separation.

2016-12-21 13_34_52- 189747e9ba44__var_www_miq_vmdb

2016-12-21 13_40_23-manageiq_ configuration

Tested both in darga and euwe

@evertmulder
Copy link
Contributor Author

This issue should be moved to the classic-ui repo now.

The issue is in manageiq-ui-classic/app/controllers/ops_controller/ops_rbac.rb, line 988

 @edit[:groups] = MiqGroup.non_tenant_groups_in_my_region.sort_by { |g| g.description.downcase }.collect { |g| [g.description, g.id] }

should be

 @edit[:groups] = Rbac.filtered(MiqGroup.non_tenant_groups_in_my_region).sort_by { |g| g.description.downcase }.collect { |g| [g.description, g.id] }

and when saving the user the input should be checked again in rbac_user_validate?

I'll try a PR for this.

@martinpovolny
Copy link
Member

Fixed via ManageIQ/manageiq-ui-classic#127

@evertmulder : thanks a lot!

@evertmulder
Copy link
Contributor Author

Thanks for all the help!
There is still an issue where an Tenant admin can create a group with, role "super admin", and assign it to his own tenant, but this group is not selectable when creating users. I will open a issue in de classic-UI-repo for this.
Note. The API possible also has this issue. I will check this.

@martinpovolny
Copy link
Member

@evertmulder : please, ping me in the issue that you create. I'd like to make sure we resolve this quick.

Thx!

@evertmulder
Copy link
Contributor Author

evertmulder commented Jan 11, 2017

@martinpovolny. every working is as expected using the API. The UI still has an issue: ManageIQ/manageiq-ui-classic#134 and ManageIQ/manageiq-ui-classic#135

simaishi pushed a commit that referenced this issue Jan 13, 2017
isimluk pushed a commit to isimluk/manageiq that referenced this issue Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants