Merge pull request #3126 from wwitzel3/3.0.1

Restrict what roles you can assign to teams.

awx/main/access.py

@@ -720,18 +720,25 @@ class TeamAccess(BaseAccess):
     def can_attach(self, obj, sub_obj, relationship, *args, **kwargs):
         """Reverse obj and sub_obj, defer to RoleAccess if this is an assignment
         of a resource role to the team."""
-        if isinstance(sub_obj, Role) and isinstance(sub_obj.content_object, ResourceMixin):
-            role_access = RoleAccess(self.user)
-            return role_access.can_attach(sub_obj, obj, 'member_role.parents',
-                                          *args, **kwargs)
+        if isinstance(sub_obj, Role):
+            if sub_obj.content_object is None:
+                raise PermissionDenied("The {} role cannot be assigned to a team".format(sub_obj.name))
+            elif isinstance(sub_obj.content_object, User):
+                raise PermissionDenied("The admin_role for a User cannot be assigned to a team")
+
+            if isinstance(sub_obj.content_object, ResourceMixin):
+                role_access = RoleAccess(self.user)
+                return role_access.can_attach(sub_obj, obj, 'member_role.parents',
+                                              *args, **kwargs)
         return super(TeamAccess, self).can_attach(obj, sub_obj, relationship,
                                                   *args, **kwargs)
 
     def can_unattach(self, obj, sub_obj, relationship, *args, **kwargs):
-        if isinstance(sub_obj, Role) and isinstance(sub_obj.content_object, ResourceMixin):
-            role_access = RoleAccess(self.user)
-            return role_access.can_unattach(sub_obj, obj, 'member_role.parents',
-                                            *args, **kwargs)
+        if isinstance(sub_obj, Role):
+            if isinstance(sub_obj.content_object, ResourceMixin):
+                role_access = RoleAccess(self.user)
+                return role_access.can_unattach(sub_obj, obj, 'member_role.parents',
+                                                *args, **kwargs)
         return super(TeamAccess, self).can_unattach(obj, sub_obj, relationship,
                                                     *args, **kwargs)
 
@@ -1681,8 +1688,7 @@ class RoleAccess(BaseAccess):
             if not check_user_access(self.user, sub_obj.__class__, 'read', sub_obj):
                 return False
 
-        if obj.object_id and \
-           isinstance(obj.content_object, ResourceMixin) and \
+        if isinstance(obj.content_object, ResourceMixin) and \
            self.user in obj.content_object.admin_role:
             return True
         return False

awx/main/notifications/webhook_backend.py

@@ -3,7 +3,6 @@
 
 import logging
 import requests
-import json
 
 from django.utils.encoding import smart_text
 

awx/main/tests/functional/test_rbac_api.py

@@ -12,7 +12,7 @@ def mock_feature_enabled(feature, bypass_database=None):
 
 @pytest.fixture
 def role():
-    return Role.objects.create()
+    return Role.objects.create(role_field='admin_role')
 
 
 #
@@ -210,33 +210,33 @@ def test_get_teams_roles_list(get, team, organization, admin):
 
 
 @pytest.mark.django_db
-def test_add_role_to_teams(team, role, post, admin):
-    assert team.member_role.children.filter(id=role.id).count() == 0
+def test_add_role_to_teams(team, post, admin):
+    assert team.member_role.children.filter(id=team.member_role.id).count() == 0
     url = reverse('api:team_roles_list', args=(team.id,))
 
-    response = post(url, {'id': role.id}, admin)
+    response = post(url, {'id': team.member_role.id}, admin)
     assert response.status_code == 204
-    assert team.member_role.children.filter(id=role.id).count() == 1
+    assert team.member_role.children.filter(id=team.member_role.id).count() == 1
 
-    response = post(url, {'id': role.id}, admin)
+    response = post(url, {'id': team.member_role.id}, admin)
     assert response.status_code == 204
-    assert team.member_role.children.filter(id=role.id).count() == 1
+    assert team.member_role.children.filter(id=team.member_role.id).count() == 1
 
     response = post(url, {}, admin)
     assert response.status_code == 400
-    assert team.member_role.children.filter(id=role.id).count() == 1
+    assert team.member_role.children.filter(id=team.member_role.id).count() == 1
 
 @pytest.mark.django_db
-def test_remove_role_from_teams(team, role, post, admin):
-    assert team.member_role.children.filter(id=role.id).count() == 0
+def test_remove_role_from_teams(team, post, admin):
+    assert team.member_role.children.filter(id=team.member_role.id).count() == 0
     url = reverse('api:team_roles_list', args=(team.id,))
-    response = post(url, {'id': role.id}, admin)
+    response = post(url, {'id': team.member_role.id}, admin)
     assert response.status_code == 204
-    assert team.member_role.children.filter(id=role.id).count() == 1
+    assert team.member_role.children.filter(id=team.member_role.id).count() == 1
 
-    response = post(url, {'disassociate': role.id, 'id': role.id}, admin)
+    response = post(url, {'disassociate': team.member_role.id, 'id': team.member_role.id}, admin)
     assert response.status_code == 204
-    assert team.member_role.children.filter(id=role.id).count() == 0
+    assert team.member_role.children.filter(id=team.member_role.id).count() == 0
 
 
 

awx/main/tests/functional/test_rbac_team.py

@@ -10,17 +10,17 @@ def test_team_attach_unattach(team, user):
     access = TeamAccess(u)
 
     team.member_role.members.add(u)
-    assert not access.can_attach(team, u.admin_role, 'member_role.children', None)
-    assert not access.can_unattach(team, u.admin_role, 'member_role.children')
+    assert not access.can_attach(team, team.member_role, 'member_role.children', None)
+    assert not access.can_unattach(team, team.member_role, 'member_role.children')
 
     team.admin_role.members.add(u)
-    assert access.can_attach(team, u.admin_role, 'member_role.children', None)
-    assert access.can_unattach(team, u.admin_role, 'member_role.children')
+    assert access.can_attach(team, team.member_role, 'member_role.children', None)
+    assert access.can_unattach(team, team.member_role, 'member_role.children')
 
     u2 = user('non-member', False)
     access = TeamAccess(u2)
-    assert not access.can_attach(team, u2.admin_role, 'member_role.children', None)
-    assert not access.can_unattach(team, u2.admin_role, 'member_role.chidlren')
+    assert not access.can_attach(team, team.member_role, 'member_role.children', None)
+    assert not access.can_unattach(team, team.member_role, 'member_role.chidlren')
 
 @pytest.mark.django_db
 def test_team_access_superuser(team, user):

tools/data_generators/rbac_dummy_data_generator.py

@@ -3,11 +3,6 @@
 # All Rights Reserved
 import os
 import sys
-os.environ.setdefault("DJANGO_SETTINGS_MODULE", "awx.settings.development") # noqa
-
-import django
-django.setup() # noqa
-
 
 # Python
 from collections import defaultdict
@@ -15,7 +10,7 @@ from optparse import make_option, OptionParser
 
 
 # Django
-
+import django
 from django.utils.timezone import now
 from django.contrib.auth.models import User
 from django.db import transaction
@@ -23,7 +18,8 @@ from django.db import transaction
 # awx
 from awx.main.models import * # noqa
 
-
+os.environ.setdefault("DJANGO_SETTINGS_MODULE", "awx.settings.development") # noqa
+django.setup() # noqa
 
 
 option_list = [