ansible / django-ansible-base

Apache License 2.0
11 stars 43 forks source link

Require superuser permission to revoke global roles #497

Closed AlanCoding closed 1 month ago

AlanCoding commented 2 months ago

Policy edit to be what AWX prior is_system_auditor policy was.

I gave more detail in a followup comment, but here are the key changes:

AAP-26308

AlanCoding commented 2 months ago

This is flawed. Because it would prevent a system auditor from demoting themselves from being a system auditor. We did support that in the old AWX logic, at least for a user editing event.

AlanCoding commented 2 months ago

Walking back through from the beginning:

Original story of concern:

However, the obvious fix for this would open up another user story (test_remove_global_assignment_yourself) that is likely to regress:

The reason this needs to be stated is that our prior criteria ("you can remove a system role assignment if you have all its permissions") covered that case implicitly. So this needs to add an explicit criteria that you can remove assignments for a given user, regardless of the associated object, or regardless of it being a system role.

There is another object-role variant of this scenario (test_remove_object_assignment_yourself).

I saw another fairly nuanced problem with system roles (test_auditor_for_external_models). This is overlapping with recent merge https://github.com/ansible/django-ansible-base/pull/501, which added an exception for "view" permission to objects, but this adds an exception-to-the-exception for system roles.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

AlanCoding commented 1 month ago

What I'm not sure, how user can delete own global permission

Code-wise, on assignment DELETE requests check_can_remove_assignment is called for the permission check, and (given a user assignment) if they can make changes to the user, this unconditionally returns without error. You can make changes to a user if that user is yourself.