diff --git a/awx/api/views.py b/awx/api/views.py index e5647791c5..fb5e54565b 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2637,23 +2637,31 @@ class InventorySourceUpdateView(RetrieveAPIView): is_job_start = True new_in_14 = True - def _build_update_response(self, update, request): - if not update: + def _update_dependent_project(self, obj, request): + if not self.request.user or not self.request.user.can_access(Project, 'start', obj.source_project): + raise PermissionDenied(detail=_( + 'You do not have permission to update project `{}`.'.format(obj.source_project.name))) + project_update = obj.source_project.update() + if not project_update: return Response({}, status=status.HTTP_400_BAD_REQUEST) else: - headers = {'Location': update.get_absolute_url(request=request)} - return Response(dict(inventory_update=update.id), - status=status.HTTP_202_ACCEPTED, headers=headers) + headers = {'Location': project_update.get_absolute_url(request=request)} + return Response(dict( + detail=_('Request to update dependent project has been accepted.'), inventory_update=None), + status=status.HTTP_202_ACCEPTED, headers=headers) def post(self, request, *args, **kwargs): obj = self.get_object() if obj.can_update: if obj.source == 'scm' and obj.update_on_project_update: - if not self.request.user or not self.request.user.can_access(Project, 'start', obj.source_project): - raise PermissionDenied(detail=_( - 'You do not have permission to update project `{}`.'.format(obj.source_project.name))) - return self._build_update_response(obj.source_project.update(), request) - return self._build_update_response(obj.update(), request) + return self._update_dependent_project(obj, request) + update = obj.update() + if not update: + return Response({}, status=status.HTTP_400_BAD_REQUEST) + else: + headers = {'Location': update.get_absolute_url(request=request)} + return Response(dict(inventory_update=update.id), + status=status.HTTP_202_ACCEPTED, headers=headers) else: return self.http_method_not_allowed(request, *args, **kwargs) diff --git a/awx/main/access.py b/awx/main/access.py index 220609a7c4..6ce7600026 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -95,7 +95,7 @@ def check_user_access(user, model_class, action, *args, **kwargs): continue result = access_method(*args, **kwargs) logger.debug('%s.%s %r returned %r', access_instance.__class__.__name__, - access_method.__name__, args, result) + getattr(access_method, '__name__', 'unknown'), args, result) if result: return result return False diff --git a/awx/main/tests/conftest.py b/awx/main/tests/conftest.py index de1f2bdd97..35f77c1f1d 100644 --- a/awx/main/tests/conftest.py +++ b/awx/main/tests/conftest.py @@ -2,6 +2,8 @@ # Python import time import pytest +import mock +from contextlib import contextmanager from awx.main.tests.factories import ( create_organization, @@ -14,6 +16,22 @@ from awx.main.tests.factories import ( ) +@pytest.fixture +def mock_access(): + @contextmanager + def access_given_class(TowerClass): + try: + mock_instance = mock.MagicMock(__name__='foobar') + MockAccess = mock.MagicMock(return_value=mock_instance) + the_patch = mock.patch.dict('awx.main.access.access_registry', + {TowerClass: [MockAccess]}, clear=False) + the_patch.__enter__() + yield mock_instance + finally: + the_patch.__exit__() + return access_given_class + + @pytest.fixture def job_template_factory(): return create_job_template diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index 1cd93182f9..831c7afc2d 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -3,7 +3,16 @@ import mock from awx.api.versioning import reverse -from awx.main.models import InventorySource +from awx.main.models import InventorySource, Project, ProjectUpdate + + +@pytest.fixture +def scm_inventory(inventory, project): + with mock.patch.object(project, 'update'): + inventory.inventory_sources.create( + name='foobar', update_on_project_update=True, source='scm', + source_project=project, scm_last_revision=project.scm_revision) + return inventory @pytest.mark.django_db @@ -210,18 +219,43 @@ def test_delete_inventory_host(delete, host, alice, role_field, expected_status_ delete(reverse('api:host_detail', kwargs={'pk': host.id}), alice, expect=expected_status_code) -@pytest.mark.parametrize("role_field,expected_status_code", [ - (None, 403), - ('admin_role', 202), - ('update_role', 202), - ('adhoc_role', 403), - ('use_role', 403) +# See companion test in tests/functional/test_rbac_inventory.py::test_inventory_source_update +@pytest.mark.parametrize("start_access,expected_status_code", [ + (True, 202), + (False, 403) ]) @pytest.mark.django_db -def test_inventory_source_update(post, inventory_source, alice, role_field, expected_status_code): - if role_field: - getattr(inventory_source.inventory, role_field).members.add(alice) - post(reverse('api:inventory_source_update_view', kwargs={'pk': inventory_source.id}), {}, alice, expect=expected_status_code) +def test_inventory_update_access_called(post, inventory_source, alice, mock_access, start_access, expected_status_code): + with mock_access(InventorySource) as mock_instance: + mock_instance.can_start = mock.MagicMock(return_value=start_access) + post(reverse('api:inventory_source_update_view', kwargs={'pk': inventory_source.id}), + {}, alice, expect=expected_status_code) + mock_instance.can_start.assert_called_once_with(inventory_source) + + +@pytest.mark.django_db +class TestUpdateOnProjUpdate: + + def test_no_access_update_denied(self, admin_user, scm_inventory, mock_access, post): + inv_src = scm_inventory.inventory_sources.first() + with mock_access(Project) as mock_access: + mock_access.can_start = mock.MagicMock(return_value=False) + r = post(reverse('api:inventory_source_update_view', kwargs={'pk': inv_src.id}), + {}, admin_user, expect=403) + assert 'You do not have permission to update project' in r.data['detail'] + + def test_no_access_update_allowed(self, admin_user, scm_inventory, mock_access, post): + inv_src = scm_inventory.inventory_sources.first() + inv_src.source_project.scm_type = 'git' + inv_src.source_project.save() + with mock.patch('awx.api.views.InventorySourceUpdateView.get_object') as get_object: + get_object.return_value = inv_src + with mock.patch.object(inv_src.source_project, 'update') as mock_update: + mock_update.return_value = ProjectUpdate(pk=48, id=48) + r = post(reverse('api:inventory_source_update_view', kwargs={'pk': inv_src.id}), + {}, admin_user, expect=202) + assert 'dependent project' in r.data['detail'] + assert not r.data['inventory_update'] @pytest.mark.django_db @@ -235,15 +269,6 @@ def test_inventory_source_vars_prohibition(post, inventory, admin_user): assert 'FOOBAR' in r.data['source_vars'][0] -@pytest.fixture -def scm_inventory(inventory, project): - with mock.patch.object(project, 'update'): - inventory.inventory_sources.create( - name='foobar', update_on_project_update=True, source='scm', - source_project=project, scm_last_revision=project.scm_revision) - return inventory - - @pytest.mark.django_db class TestControlledBySCM: ''' diff --git a/awx/main/tests/functional/test_rbac_inventory.py b/awx/main/tests/functional/test_rbac_inventory.py index 7b85e1d44a..99937899a8 100644 --- a/awx/main/tests/functional/test_rbac_inventory.py +++ b/awx/main/tests/functional/test_rbac_inventory.py @@ -93,6 +93,21 @@ def test_inventory_update_org_admin(inventory_update, org_admin): assert access.can_delete(inventory_update) +# See companion test in tests/functional/api/test_inventory.py::test_inventory_update_access_called +@pytest.mark.parametrize("role_field,allowed", [ + (None, False), + ('admin_role', True), + ('update_role', True), + ('adhoc_role', False), + ('use_role', False) +]) +@pytest.mark.django_db +def test_inventory_source_update(inventory_source, alice, role_field, allowed): + if role_field: + getattr(inventory_source.inventory, role_field).members.add(alice) + assert allowed == InventorySourceAccess(alice).can_start(inventory_source), '{} test failed'.format(role_field) + + @pytest.mark.django_db def test_host_access(organization, inventory, group, user, group_factory): other_inventory = organization.inventories.create(name='other-inventory')