Merge pull request #1270 from AlanCoding/user_editability

User editing permission changes (3.2.4 backport)

awx/main/access.py

@@ -491,10 +491,35 @@ class UserAccess(BaseAccess):
         # that a user should be able to edit for themselves.
         return bool(self.user == obj or self.can_admin(obj, data))
 
+    def user_membership_roles(self, u):
+        return Role.objects.filter(
+            content_type=ContentType.objects.get_for_model(Organization),
+            role_field__in=['admin_role', 'member_role'],
+            members=u
+        )
+
+    def is_all_org_admin(self, u):
+        return not self.user_membership_roles(u).exclude(
+            ancestors__in=self.user.roles.filter(role_field='admin_role')
+        ).exists()
+
+    def user_is_orphaned(self, u):
+        return not self.user_membership_roles(u).exists()
+
     @check_superuser
-    def can_admin(self, obj, data):
-        return Organization.objects.filter(Q(member_role__members=obj) | Q(admin_role__members=obj),
-                                           Q(admin_role__members=self.user)).exists()
+    def can_admin(self, obj, data, allow_orphans=False):
+        if obj.is_superuser or obj.is_system_auditor:
+            # must be superuser to admin users with system roles
+            return False
+        if self.user_is_orphaned(obj):
+            if not allow_orphans:
+                # in these cases only superusers can modify orphan users
+                return False
+            return not obj.roles.all().exclude(
+                content_type=ContentType.objects.get_for_model(User)
+            ).filter(ancestors__in=self.user.roles.all()).exists()
+        else:
+            return self.is_all_org_admin(obj)
 
     def can_delete(self, obj):
         if obj == self.user:
@@ -2401,7 +2426,7 @@ class RoleAccess(BaseAccess):
         # unwanted escalations lets ensure that the Organization administartor has the abilty
         # to admin the user being added to the role.
         if isinstance(obj.content_object, Organization) and obj.role_field in ['member_role', 'admin_role']:
-            if not UserAccess(self.user).can_admin(sub_obj, None):
+            if not UserAccess(self.user).can_admin(sub_obj, None, allow_orphans=True):
                 return False
 
         if isinstance(obj.content_object, ResourceMixin) and \

awx/main/tests/functional/test_rbac_role.py

@@ -4,6 +4,7 @@ from awx.main.access import (
     RoleAccess,
     UserAccess,
     TeamAccess)
+from awx.main.models import Role, Organization
 
 
 @pytest.mark.django_db
@@ -35,12 +36,99 @@ def test_role_access_attach(rando, inventory):
 
 
 @pytest.mark.django_db
-def test_org_user_role_attach(user, organization):
+def test_visible_roles(admin_user, system_auditor, rando, organization, project):
+    '''
+    system admin & system auditor fixtures needed to create system roles
+    '''
+    organization.auditor_role.members.add(rando)
+    access = RoleAccess(rando)
+
+    assert rando not in organization.admin_role
+    assert access.can_read(organization.admin_role)
+    assert organization.admin_role in Role.visible_roles(rando)
+
+    assert rando not in project.admin_role
+    assert access.can_read(project.admin_role)
+    assert project.admin_role in Role.visible_roles(rando)
+
+
+# Permissions when adding users to org member/admin
+@pytest.mark.django_db
+def test_org_user_role_attach(user, organization, inventory):
+    '''
+    Org admins must not be able to add arbitrary users to their
+    organization, because that would give them admin permission to that user
+    '''
     admin = user('admin')
     nonmember = user('nonmember')
+    inventory.admin_role.members.add(nonmember)
 
     organization.admin_role.members.add(admin)
 
-    access = RoleAccess(admin)
-    assert not access.can_attach(organization.member_role, nonmember, 'members', None)
-    assert not access.can_attach(organization.admin_role, nonmember, 'members', None)
+    role_access = RoleAccess(admin)
+    assert not role_access.can_attach(organization.member_role, nonmember, 'members', None)
+    assert not role_access.can_attach(organization.admin_role, nonmember, 'members', None)
+
+
+# Singleton user editing restrictions
+@pytest.mark.django_db
+def test_org_superuser_role_attach(admin_user, org_admin, organization):
+    '''
+    Ideally, you would not add superusers to roles (particularly member_role)
+    but it has historically been possible
+    this checks that the situation does not grant unexpected permissions
+    '''
+    organization.member_role.members.add(admin_user)
+
+    role_access = RoleAccess(org_admin)
+    assert not role_access.can_attach(organization.member_role, admin_user, 'members', None)
+    assert not role_access.can_attach(organization.admin_role, admin_user, 'members', None)
+    user_access = UserAccess(org_admin)
+    assert not user_access.can_change(admin_user, {'last_name': 'Witzel'})
+
+
+# Org admin user editing permission ANY to ALL change
+@pytest.mark.django_db
+def test_need_all_orgs_to_admin_user(user):
+    '''
+    Old behavior - org admin to ANY organization that a user is member of
+        grants permission to admin that user
+    New behavior enforced here - org admin to ALL organizations that a
+        user is member of grants permission to admin that user
+    '''
+    org1 = Organization.objects.create(name='org1')
+    org2 = Organization.objects.create(name='org2')
+
+    org1_admin = user('org1-admin')
+    org1.admin_role.members.add(org1_admin)
+
+    org12_member = user('org12-member')
+    org1.member_role.members.add(org12_member)
+    org2.member_role.members.add(org12_member)
+
+    user_access = UserAccess(org1_admin)
+    assert not user_access.can_change(org12_member, {'last_name': 'Witzel'})
+
+    role_access = RoleAccess(org1_admin)
+    assert not role_access.can_attach(org1.admin_role, org12_member, 'members', None)
+    assert not role_access.can_attach(org1.member_role, org12_member, 'members', None)
+
+    org2.admin_role.members.add(org1_admin)
+    assert role_access.can_attach(org1.admin_role, org12_member, 'members', None)
+    assert role_access.can_attach(org1.member_role, org12_member, 'members', None)
+
+
+# Orphaned user can be added to member role, only in special cases
+@pytest.mark.django_db
+def test_orphaned_user_allowed(org_admin, rando, organization):
+    '''
+    We still allow adoption of orphaned* users by assigning them to
+    organization member role, but only in the situation where the
+    org admin already posesses indirect access to all of the user's roles
+    *orphaned means user is not a member of any organization
+    '''
+    role_access = RoleAccess(org_admin)
+    assert role_access.can_attach(organization.member_role, rando, 'members', None)
+    # Cannot edit the user directly without adding to org first
+    user_access = UserAccess(org_admin)
+    assert not user_access.can_change(rando, {'last_name': 'Witzel'})