From 9bad20cee34f21a32937f82a56c20c584e9529b0 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 26 Jul 2016 12:09:36 -0400 Subject: [PATCH 1/4] do not allow assignment of system roles or user.admin_role to teams --- awx/main/access.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 592359a031..2c01b62065 100644 --- a/awx/main/access.py +++ b/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) From 74b013aad0905120410317b4f084deadb578dc7a Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 26 Jul 2016 12:09:58 -0400 Subject: [PATCH 2/4] driveby cleanup --- awx/main/access.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 2c01b62065..c109e5d449 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1688,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 From df1464d4bf47c4fbf93827c247db20b97a8d9a46 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 26 Jul 2016 12:10:19 -0400 Subject: [PATCH 3/4] fixing tests for new team role assignment restrictions --- awx/main/tests/functional/test_rbac_api.py | 28 ++++++++++----------- awx/main/tests/functional/test_rbac_team.py | 12 ++++----- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/awx/main/tests/functional/test_rbac_api.py b/awx/main/tests/functional/test_rbac_api.py index d5ccbdf5d0..54dcc8deb5 100644 --- a/awx/main/tests/functional/test_rbac_api.py +++ b/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 diff --git a/awx/main/tests/functional/test_rbac_team.py b/awx/main/tests/functional/test_rbac_team.py index 0c16ba9f6f..6907589462 100644 --- a/awx/main/tests/functional/test_rbac_team.py +++ b/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): From a0c563e831f3a729513e87b1f479ec9dd804cd58 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 26 Jul 2016 14:50:48 -0400 Subject: [PATCH 4/4] flake8 fixups --- awx/main/notifications/webhook_backend.py | 1 - tools/data_generators/rbac_dummy_data_generator.py | 10 +++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/awx/main/notifications/webhook_backend.py b/awx/main/notifications/webhook_backend.py index fe887efc68..e74f39f654 100644 --- a/awx/main/notifications/webhook_backend.py +++ b/awx/main/notifications/webhook_backend.py @@ -3,7 +3,6 @@ import logging import requests -import json from django.utils.encoding import smart_text diff --git a/tools/data_generators/rbac_dummy_data_generator.py b/tools/data_generators/rbac_dummy_data_generator.py index ad90dc74a9..f9e583c3a3 100755 --- a/tools/data_generators/rbac_dummy_data_generator.py +++ b/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 = [