From 3e97bdae7f2917d313d485c965835416f7810d11 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 10 Jun 2016 17:12:26 -0400 Subject: [PATCH 1/5] add reverse attach access checks pointing toward roles --- awx/main/access.py | 6 +++ awx/main/tests/functional/test_rbac_role.py | 46 +++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 awx/main/tests/functional/test_rbac_role.py diff --git a/awx/main/access.py b/awx/main/access.py index 231ece8042..eecaf69253 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -270,6 +270,12 @@ class UserAccess(BaseAccess): return True return False + def can_attach(self, obj, sub_obj, relationship, data, skip_sub_obj_read_check=False): + if relationship == 'roles': + role_access = RoleAccess(self.user) + return role_access.can_attach(sub_obj, obj, 'members', data, skip_sub_obj_read_check=False) + return super(UserAccess, self).can_attach(obj, sub_obj, relationship, data, skip_sub_obj_read_check=False) + class OrganizationAccess(BaseAccess): ''' diff --git a/awx/main/tests/functional/test_rbac_role.py b/awx/main/tests/functional/test_rbac_role.py new file mode 100644 index 0000000000..bb61262fd5 --- /dev/null +++ b/awx/main/tests/functional/test_rbac_role.py @@ -0,0 +1,46 @@ +import mock +import pytest + +from awx.main.access import ( + RoleAccess, + UserAccess +) + +from django.core.urlresolvers import reverse +from django.contrib.auth.models import User + + +@pytest.mark.django_db +def test_inventory_read_role_access_functional(rando, inventory, mocker, post): + inventory.read_role.members.add(rando) + role_pk = inventory.admin_role.pk + + mock_access = mocker.MagicMock(spec=RoleAccess, id=968) + with mocker.patch('awx.main.access.RoleAccess', return_value=mock_access): + response = post(url=reverse('api:user_roles_list', args=(rando.pk,)), + data={'id': role_pk}, user=rando) + mock_access.can_attach.assert_called_once_with( + inventory.admin_role, rando, 'members', {"id": role_pk}, + skip_sub_obj_read_check=False) + +@pytest.mark.django_db +def test_inventory_read_role_user_can_access(rando, inventory): + inventory.read_role.members.add(rando) + access = RoleAccess(rando) + assert not rando.can_access( + User, 'attach', rando, inventory.admin_role, 'roles', + {'id': inventory.admin_role.pk}, False) + +@pytest.mark.django_db +def test_inventory_read_role_user_access(rando, inventory): + inventory.read_role.members.add(rando) + access = UserAccess(rando) + data = {'id': inventory.admin_role.pk} + assert not access.can_attach(rando, inventory.admin_role, 'roles', data, False) + +@pytest.mark.django_db +def test_inventory_read_role_access(rando, inventory): + inventory.read_role.members.add(rando) + access = RoleAccess(rando) + assert not access.can_attach(inventory.admin_role, rando, 'members', None) + From 7f38227e1158dd5e0808ead346301d66efba92f8 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Sat, 11 Jun 2016 18:04:05 -0400 Subject: [PATCH 2/5] fix bug in RoleTeamsList --- awx/api/views.py | 5 +++++ awx/main/tests/functional/test_rbac_role.py | 25 ++++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index 6bd917cab2..1cb44f3c60 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -3786,6 +3786,11 @@ class RoleTeamsList(ListAPIView): # XXX: Need to pull in can_attach and can_unattach kinda code from SubListCreateAttachDetachAPIView role = Role.objects.get(pk=self.kwargs['pk']) team = Team.objects.get(pk=sub_id) + from awx.main.access import RoleAccess + access = RoleAccess(request.user) + if access.can_attach(role, team, 'members', {"id": role.pk}, skip_sub_obj_read_check=False): + raise PermissionDenied() + if request.data.get('disassociate', None): team.member_role.children.remove(role) else: diff --git a/awx/main/tests/functional/test_rbac_role.py b/awx/main/tests/functional/test_rbac_role.py index bb61262fd5..20ab8ed6ba 100644 --- a/awx/main/tests/functional/test_rbac_role.py +++ b/awx/main/tests/functional/test_rbac_role.py @@ -11,17 +11,36 @@ from django.contrib.auth.models import User @pytest.mark.django_db -def test_inventory_read_role_access_functional(rando, inventory, mocker, post): +def test_user_role_access_view(rando, inventory, mocker, post): + # rando has read access for the inventory inventory.read_role.members.add(rando) - role_pk = inventory.admin_role.pk - mock_access = mocker.MagicMock(spec=RoleAccess, id=968) + role_pk = inventory.admin_role.pk + mock_access = mocker.MagicMock(spec=RoleAccess, can_attach=mock.MagicMock(return_value=False)) with mocker.patch('awx.main.access.RoleAccess', return_value=mock_access): response = post(url=reverse('api:user_roles_list', args=(rando.pk,)), data={'id': role_pk}, user=rando) mock_access.can_attach.assert_called_once_with( inventory.admin_role, rando, 'members', {"id": role_pk}, skip_sub_obj_read_check=False) + assert rando not in inventory.admin_role + +@pytest.mark.django_db +def test_role_team_access_view(rando, team, inventory, mocker, post): + # rando is admin of the team + team.admin_role.members.add(rando) + # team has read_role for the inventory + team.member_role.children.add(inventory.read_role) + + role_pk = inventory.admin_role.pk + mock_access = mocker.MagicMock(spec=RoleAccess) + with mocker.patch('awx.main.access.RoleAccess', return_value=mock_access): + response = post(url=reverse('api:role_teams_list', args=(role_pk,)), + data={'id': team.pk}, user=rando) + mock_access.can_attach.assert_called_once_with( + inventory.admin_role, team, 'members', {"id": role_pk}, + skip_sub_obj_read_check=False) + assert team not in inventory.admin_role @pytest.mark.django_db def test_inventory_read_role_user_can_access(rando, inventory): From b485b85076ee32ca0356811724701f08ef687b20 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 13 Jun 2016 09:16:03 -0400 Subject: [PATCH 3/5] TeamRolesList permission tests and fix, organize tests --- awx/api/views.py | 14 ++--- awx/main/access.py | 30 +++++++++-- .../api/test_create_attach_views.py | 48 +++++++++++++++++ awx/main/tests/functional/test_rbac_role.py | 52 ++++--------------- 4 files changed, 92 insertions(+), 52 deletions(-) create mode 100644 awx/main/tests/functional/api/test_create_attach_views.py diff --git a/awx/api/views.py b/awx/api/views.py index 1cb44f3c60..3fd803369c 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -3783,22 +3783,22 @@ class RoleTeamsList(ListAPIView): if not sub_id: data = dict(msg="Role 'id' field is missing.") return Response(data, status=status.HTTP_400_BAD_REQUEST) - # XXX: Need to pull in can_attach and can_unattach kinda code from SubListCreateAttachDetachAPIView + role = Role.objects.get(pk=self.kwargs['pk']) team = Team.objects.get(pk=sub_id) - from awx.main.access import RoleAccess - access = RoleAccess(request.user) - if access.can_attach(role, team, 'members', {"id": role.pk}, skip_sub_obj_read_check=False): + action = 'attach' + if request.data.get('disassociate', None): + action = 'unattach' + if not request.user.can_access(self.parent_model, action, role, team, + self.relationship, request.data, + skip_sub_obj_read_check=False): raise PermissionDenied() - if request.data.get('disassociate', None): team.member_role.children.remove(role) else: team.member_role.children.add(role) return Response(status=status.HTTP_204_NO_CONTENT) - # XXX attach/detach needs to ensure we have the appropriate perms - class RoleParentsList(SubListAPIView): diff --git a/awx/main/access.py b/awx/main/access.py index eecaf69253..2a4f4746b0 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -270,11 +270,18 @@ class UserAccess(BaseAccess): return True return False - def can_attach(self, obj, sub_obj, relationship, data, skip_sub_obj_read_check=False): + def can_attach(self, obj, sub_obj, relationship, *args, **kwargs): + "Reverse obj and sub_obj, defer to RoleAccess if this is a role assignment." if relationship == 'roles': role_access = RoleAccess(self.user) - return role_access.can_attach(sub_obj, obj, 'members', data, skip_sub_obj_read_check=False) - return super(UserAccess, self).can_attach(obj, sub_obj, relationship, data, skip_sub_obj_read_check=False) + return role_access.can_attach(sub_obj, obj, 'members', *args, **kwargs) + return super(UserAccess, self).can_attach(obj, sub_obj, relationship, *args, **kwargs) + + def can_unattach(self, obj, sub_obj, relationship, *args, **kwargs): + if relationship == 'roles': + role_access = RoleAccess(self.user) + return role_access.can_unattach(sub_obj, obj, 'members', *args, **kwargs) + return super(UserAccess, self).can_unattach(obj, sub_obj, relationship, *args, **kwargs) class OrganizationAccess(BaseAccess): @@ -652,6 +659,23 @@ class TeamAccess(BaseAccess): def can_delete(self, obj): return self.can_change(obj, None) + def can_attach(self, obj, sub_obj, relationship, *args, **kwargs): + "Reverse obj and sub_obj, defer to RoleAccess if this is a role assignment." + if relationship == 'member_role.children': + 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 relationship == 'member_role.children': + 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) + class ProjectAccess(BaseAccess): ''' I can see projects when: diff --git a/awx/main/tests/functional/api/test_create_attach_views.py b/awx/main/tests/functional/api/test_create_attach_views.py new file mode 100644 index 0000000000..4882b8563a --- /dev/null +++ b/awx/main/tests/functional/api/test_create_attach_views.py @@ -0,0 +1,48 @@ +import pytest + +from django.core.urlresolvers import reverse + + +@pytest.mark.django_db +def test_user_role_view_access(rando, inventory, mocker, post): + "Assure correct access method is called when assigning users new roles" + role_pk = inventory.admin_role.pk + data = {"id": role_pk} + mock_access = mocker.MagicMock(can_attach=mocker.MagicMock(return_value=False)) + with mocker.patch('awx.main.access.RoleAccess', return_value=mock_access): + post(url=reverse('api:user_roles_list', args=(rando.pk,)), + data=data, user=rando, expect=403) + mock_access.can_attach.assert_called_once_with( + inventory.admin_role, rando, 'members', data, + skip_sub_obj_read_check=False) + assert rando not in inventory.admin_role + +@pytest.mark.django_db +def test_team_role_view_access(rando, team, inventory, mocker, post): + "Assure correct access method is called when assigning teams new roles" + team.admin_role.members.add(rando) + role_pk = inventory.admin_role.pk + data = {"id": role_pk} + mock_access = mocker.MagicMock(can_attach=mocker.MagicMock(return_value=False)) + with mocker.patch('awx.main.access.RoleAccess', return_value=mock_access): + post(url=reverse('api:team_roles_list', args=(team.pk,)), + data=data, user=rando, expect=403) + mock_access.can_attach.assert_called_once_with( + inventory.admin_role, team, 'member_role.parents', data, + skip_sub_obj_read_check=False) + assert team not in inventory.admin_role + +@pytest.mark.django_db +def test_role_team_view_access(rando, team, inventory, mocker, post): + """Assure that /role/N/teams/ enforces the same permission restrictions + that /teams/N/roles/ does when assigning teams new roles""" + role_pk = inventory.admin_role.pk + data = {"id": team.pk} + mock_access = mocker.MagicMock(return_value=False, __name__='mocked') + with mocker.patch('awx.main.access.RoleAccess.can_attach', mock_access): + post(url=reverse('api:role_teams_list', args=(role_pk,)), + data=data, user=rando, expect=403) + mock_access.assert_called_once_with( + inventory.admin_role, team, 'member_role.parents', data, + skip_sub_obj_read_check=False) + assert team not in inventory.admin_role diff --git a/awx/main/tests/functional/test_rbac_role.py b/awx/main/tests/functional/test_rbac_role.py index 20ab8ed6ba..c180efc198 100644 --- a/awx/main/tests/functional/test_rbac_role.py +++ b/awx/main/tests/functional/test_rbac_role.py @@ -1,64 +1,32 @@ -import mock import pytest from awx.main.access import ( RoleAccess, - UserAccess -) - -from django.core.urlresolvers import reverse -from django.contrib.auth.models import User + UserAccess, + TeamAccess) @pytest.mark.django_db -def test_user_role_access_view(rando, inventory, mocker, post): - # rando has read access for the inventory - inventory.read_role.members.add(rando) - - role_pk = inventory.admin_role.pk - mock_access = mocker.MagicMock(spec=RoleAccess, can_attach=mock.MagicMock(return_value=False)) - with mocker.patch('awx.main.access.RoleAccess', return_value=mock_access): - response = post(url=reverse('api:user_roles_list', args=(rando.pk,)), - data={'id': role_pk}, user=rando) - mock_access.can_attach.assert_called_once_with( - inventory.admin_role, rando, 'members', {"id": role_pk}, - skip_sub_obj_read_check=False) - assert rando not in inventory.admin_role - -@pytest.mark.django_db -def test_role_team_access_view(rando, team, inventory, mocker, post): +def test_team_access_attach(rando, team, inventory): # rando is admin of the team team.admin_role.members.add(rando) + inventory.read_role.members.add(rando) # team has read_role for the inventory team.member_role.children.add(inventory.read_role) - - role_pk = inventory.admin_role.pk - mock_access = mocker.MagicMock(spec=RoleAccess) - with mocker.patch('awx.main.access.RoleAccess', return_value=mock_access): - response = post(url=reverse('api:role_teams_list', args=(role_pk,)), - data={'id': team.pk}, user=rando) - mock_access.can_attach.assert_called_once_with( - inventory.admin_role, team, 'members', {"id": role_pk}, - skip_sub_obj_read_check=False) - assert team not in inventory.admin_role + + access = TeamAccess(rando) + data = {'id': inventory.admin_role.pk} + assert not access.can_attach(team, inventory.admin_role, 'member_role.children', data, False) @pytest.mark.django_db -def test_inventory_read_role_user_can_access(rando, inventory): - inventory.read_role.members.add(rando) - access = RoleAccess(rando) - assert not rando.can_access( - User, 'attach', rando, inventory.admin_role, 'roles', - {'id': inventory.admin_role.pk}, False) - -@pytest.mark.django_db -def test_inventory_read_role_user_access(rando, inventory): +def test_user_access_attach(rando, inventory): inventory.read_role.members.add(rando) access = UserAccess(rando) data = {'id': inventory.admin_role.pk} assert not access.can_attach(rando, inventory.admin_role, 'roles', data, False) @pytest.mark.django_db -def test_inventory_read_role_access(rando, inventory): +def test_role_access_attach(rando, inventory): inventory.read_role.members.add(rando) access = RoleAccess(rando) assert not access.can_attach(inventory.admin_role, rando, 'members', None) From c63176109129ff399f61f0fa54bef3e6c6042a08 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 13 Jun 2016 12:34:10 -0400 Subject: [PATCH 4/5] check team permissions if attaching user roles --- awx/main/access.py | 7 ++++--- awx/main/tests/functional/api/test_create_attach_views.py | 3 --- awx/main/tests/functional/test_rbac_role.py | 1 - 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 2a4f4746b0..17b787ea60 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -660,8 +660,9 @@ class TeamAccess(BaseAccess): return self.can_change(obj, None) def can_attach(self, obj, sub_obj, relationship, *args, **kwargs): - "Reverse obj and sub_obj, defer to RoleAccess if this is a role assignment." - if relationship == 'member_role.children': + """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) @@ -669,7 +670,7 @@ class TeamAccess(BaseAccess): *args, **kwargs) def can_unattach(self, obj, sub_obj, relationship, *args, **kwargs): - if relationship == 'member_role.children': + 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) diff --git a/awx/main/tests/functional/api/test_create_attach_views.py b/awx/main/tests/functional/api/test_create_attach_views.py index 4882b8563a..5399356a21 100644 --- a/awx/main/tests/functional/api/test_create_attach_views.py +++ b/awx/main/tests/functional/api/test_create_attach_views.py @@ -15,7 +15,6 @@ def test_user_role_view_access(rando, inventory, mocker, post): mock_access.can_attach.assert_called_once_with( inventory.admin_role, rando, 'members', data, skip_sub_obj_read_check=False) - assert rando not in inventory.admin_role @pytest.mark.django_db def test_team_role_view_access(rando, team, inventory, mocker, post): @@ -30,7 +29,6 @@ def test_team_role_view_access(rando, team, inventory, mocker, post): mock_access.can_attach.assert_called_once_with( inventory.admin_role, team, 'member_role.parents', data, skip_sub_obj_read_check=False) - assert team not in inventory.admin_role @pytest.mark.django_db def test_role_team_view_access(rando, team, inventory, mocker, post): @@ -45,4 +43,3 @@ def test_role_team_view_access(rando, team, inventory, mocker, post): mock_access.assert_called_once_with( inventory.admin_role, team, 'member_role.parents', data, skip_sub_obj_read_check=False) - assert team not in inventory.admin_role diff --git a/awx/main/tests/functional/test_rbac_role.py b/awx/main/tests/functional/test_rbac_role.py index c180efc198..613051e395 100644 --- a/awx/main/tests/functional/test_rbac_role.py +++ b/awx/main/tests/functional/test_rbac_role.py @@ -30,4 +30,3 @@ def test_role_access_attach(rando, inventory): inventory.read_role.members.add(rando) access = RoleAccess(rando) assert not access.can_attach(inventory.admin_role, rando, 'members', None) - From 9d86fb0a092fa0f874d1aef48fe7df8262912678 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 13 Jun 2016 14:11:02 -0400 Subject: [PATCH 5/5] change wording to reflect team sublist endpoint --- awx/api/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index 3fd803369c..9a0c77eb1c 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -3778,10 +3778,10 @@ class RoleTeamsList(ListAPIView): return Team.objects.filter(member_role__children=role) def post(self, request, pk, *args, **kwargs): - # Forbid implicit role creation here + # Forbid implicit team creation here sub_id = request.data.get('id', None) if not sub_id: - data = dict(msg="Role 'id' field is missing.") + data = dict(msg="Team 'id' field is missing.") return Response(data, status=status.HTTP_400_BAD_REQUEST) role = Role.objects.get(pk=self.kwargs['pk'])