From 48c155dfb0ee81be185c3f67b9a214eb1e3e3d74 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 10 Oct 2016 10:49:58 -0400 Subject: [PATCH 1/3] schedule capability set to False for manual groups --- awx/main/access.py | 4 ++-- awx/main/tests/functional/api/test_rbac_displays.py | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 9059b4c7d0..002d61bedc 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -305,13 +305,13 @@ class BaseAccess(object): if display_method not in method_list: continue - # Validation consistency checks + # Actions not possible for reason unrelated to RBAC: validation, etc. if display_method == 'copy' and isinstance(obj, JobTemplate): validation_errors, resources_needed_to_start = obj.resource_validation_data() if validation_errors: user_capabilities[display_method] = False continue - elif display_method == 'start' and isinstance(obj, Group): + elif display_method in ['start', 'schedule'] and isinstance(obj, Group): if obj.inventory_source and not obj.inventory_source._can_update(): user_capabilities[display_method] = False continue diff --git a/awx/main/tests/functional/api/test_rbac_displays.py b/awx/main/tests/functional/api/test_rbac_displays.py index cccc51482b..cd336c66c8 100644 --- a/awx/main/tests/functional/api/test_rbac_displays.py +++ b/awx/main/tests/functional/api/test_rbac_displays.py @@ -333,13 +333,15 @@ def test_group_update_capabilities_possible(group, inventory_source, admin_user) @pytest.mark.django_db def test_group_update_capabilities_impossible(group, inventory_source, admin_user): + "Manual groups can not be updated or scheduled" inventory_source.source = "" inventory_source.save() group.inventory_source = inventory_source group.save() - capabilities = get_user_capabilities(admin_user, group, method_list=['start']) + capabilities = get_user_capabilities(admin_user, group, method_list=['edit', 'start', 'schedule']) assert not capabilities['start'] + assert not capabilities['schedule'] @pytest.mark.django_db From 42cf74b085cf48eaa01afb40ae5faa4ac4203495 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 10 Oct 2016 11:40:36 -0400 Subject: [PATCH 2/3] block user from shooting themselves in the foot by scheduling a manual project --- awx/api/serializers.py | 2 ++ awx/main/access.py | 11 ++++++----- awx/main/tests/functional/api/test_rbac_displays.py | 1 + awx/main/tests/functional/test_projects.py | 9 +++++++++ 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index e8b0706789..38c3253b13 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2826,6 +2826,8 @@ class ScheduleSerializer(BaseSerializer): def validate_unified_job_template(self, value): if type(value) == InventorySource and value.source not in SCHEDULEABLE_PROVIDERS: raise serializers.ValidationError(_('Inventory Source must be a cloud resource.')) + elif type(value) == Project and value.scm_type == '': + raise serializers.ValidationError(_('Manual Project can not have a schedule set.')) return value # We reject rrules if: diff --git a/awx/main/access.py b/awx/main/access.py index 002d61bedc..375f286d9d 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -305,7 +305,8 @@ class BaseAccess(object): if display_method not in method_list: continue - # Actions not possible for reason unrelated to RBAC: validation, etc. + # Actions not possible for reason unrelated to RBAC + # Cannot copy with validation errors, or update a manual group/project if display_method == 'copy' and isinstance(obj, JobTemplate): validation_errors, resources_needed_to_start = obj.resource_validation_data() if validation_errors: @@ -315,6 +316,10 @@ class BaseAccess(object): if obj.inventory_source and not obj.inventory_source._can_update(): user_capabilities[display_method] = False continue + elif display_method in ['start', 'schedule'] and isinstance(obj, (Project)): + if obj.scm_type == '': + user_capabilities[display_method] = False + continue # Grab the answer from the cache, if available if hasattr(obj, 'capabilities_cache') and display_method in obj.capabilities_cache: @@ -341,10 +346,6 @@ class BaseAccess(object): elif display_method == 'copy' and isinstance(obj, (Group, Host)): user_capabilities['copy'] = user_capabilities['edit'] continue - elif display_method == 'start' and isinstance(obj, (Project)) and obj.scm_type == '': - # Special case to return False for a manual project - user_capabilities['start'] = False - continue # Preprocessing before the access method is called data = {} diff --git a/awx/main/tests/functional/api/test_rbac_displays.py b/awx/main/tests/functional/api/test_rbac_displays.py index cd336c66c8..e1255ffea9 100644 --- a/awx/main/tests/functional/api/test_rbac_displays.py +++ b/awx/main/tests/functional/api/test_rbac_displays.py @@ -320,6 +320,7 @@ def test_prefetch_jt_copy_capability(job_template, project, inventory, machine_c def test_manual_projects_no_update(project, get, admin_user): response = get(reverse('api:project_detail', args=[project.pk]), admin_user, expect=200) assert not response.data['summary_fields']['user_capabilities']['start'] + assert not response.data['summary_fields']['user_capabilities']['schedule'] @pytest.mark.django_db diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py index cd3f6e57ff..6d66c4e346 100644 --- a/awx/main/tests/functional/test_projects.py +++ b/awx/main/tests/functional/test_projects.py @@ -136,3 +136,12 @@ def test_patch_project_null_organization(patch, organization, project, admin): @pytest.mark.django_db() def test_patch_project_null_organization_xfail(patch, project, org_admin): patch(reverse('api:project_detail', args=(project.id,)), { 'name': 't', 'organization': None}, org_admin, expect=400) + +@pytest.mark.django_db +def test_cannot_schedule_manual_project(project, admin_user, post): + response = post( + reverse('api:project_schedules_list', args=(project.pk,)), + {"name": "foo", "description": "", "enabled": True, + "rrule": "DTSTART:20160926T040000Z RRULE:FREQ=HOURLY;INTERVAL=1", + "extra_data": {}}, admin_user, expect=400) + assert 'Manual' in response.data['unified_job_template'][0] From fa766c3657c15e1f0808ccc17178f3a9a3fbd2f8 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Sat, 15 Oct 2016 11:22:59 -0400 Subject: [PATCH 3/3] update old test to new scheme --- awx/main/tests/functional/test_projects.py | 1 + awx/main/tests/old/projects.py | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py index 6d66c4e346..8d93fcf6d9 100644 --- a/awx/main/tests/functional/test_projects.py +++ b/awx/main/tests/functional/test_projects.py @@ -137,6 +137,7 @@ def test_patch_project_null_organization(patch, organization, project, admin): def test_patch_project_null_organization_xfail(patch, project, org_admin): patch(reverse('api:project_detail', args=(project.id,)), { 'name': 't', 'organization': None}, org_admin, expect=400) + @pytest.mark.django_db def test_cannot_schedule_manual_project(project, admin_user, post): response = post( diff --git a/awx/main/tests/old/projects.py b/awx/main/tests/old/projects.py index 3b894e18cc..10510c5e24 100644 --- a/awx/main/tests/old/projects.py +++ b/awx/main/tests/old/projects.py @@ -233,7 +233,8 @@ class ProjectsTest(BaseTransactionTest): 'name': 'My Test Project', 'description': 'Does amazing things', 'local_path': os.path.basename(project_dir), - 'scm_type': None, + 'scm_type': 'git', # must not be manual in order to schedule + 'scm_url': 'http://192.168.100.128.git', 'scm_update_on_launch': '', 'scm_delete_on_update': None, 'scm_clean': False, @@ -244,7 +245,7 @@ class ProjectsTest(BaseTransactionTest): # or an empty string for False, but save the value as a boolean. response = self.post(projects, project_data, expect=201, auth=self.get_super_credentials()) - self.assertEqual(response['scm_type'], u'') + self.assertEqual(response['scm_type'], u'git') self.assertEqual(response['scm_update_on_launch'], False) self.assertEqual(response['scm_delete_on_update'], False) self.assertEqual(response['scm_clean'], False)