From d77801353fb033dea603199395760e35df82f1cb Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 13:38:47 -0400 Subject: [PATCH 1/8] changed passwords_needed_to_start to take into acount if a credential is deleted --- awx/api/serializers.py | 9 +++++---- awx/main/models/ad_hoc_commands.py | 2 +- awx/main/models/jobs.py | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 2881b52b94..6d2d0d60f9 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1775,13 +1775,14 @@ class JobLaunchSerializer(BaseSerializer): passwords = self.context.get('passwords') data = self.context.get('data') + credential = attrs.get('credential', None) or obj.credential # fill passwords dict with request data passwords - if obj.passwords_needed_to_start: + if credential.passwords_needed: try: - for p in obj.passwords_needed_to_start: - passwords[p] = data.get(p) + for p in credential.passwords_needed: + passwords[p] = data[p] except KeyError: - raise serializers.ValidationError(obj.passwords_needed_to_start) + raise serializers.ValidationError(credential.passwords_needed) return attrs def validate(self, attrs): diff --git a/awx/main/models/ad_hoc_commands.py b/awx/main/models/ad_hoc_commands.py index d3dce392d2..ffeb90522c 100644 --- a/awx/main/models/ad_hoc_commands.py +++ b/awx/main/models/ad_hoc_commands.py @@ -114,7 +114,7 @@ class AdHocCommand(UnifiedJob): @property def passwords_needed_to_start(self): '''Return list of password field names needed to start the job.''' - if self.credential: + if self.credential and self.credential.active: return self.credential.passwords_needed else: return [] diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 0ab8d0ee67..df7b96eeef 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -141,7 +141,7 @@ class JobOptions(BaseModel): @property def passwords_needed_to_start(self): '''Return list of password field names needed to start the job.''' - if self.credential: + if self.credential and self.credential.active: return self.credential.passwords_needed else: return [] From f08d8cb5b96d7824288a78d7a3038ac1859a7682 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 13:39:30 -0400 Subject: [PATCH 2/8] consider a password specified if it equals '' --- awx/main/models/unified_jobs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 0a34e18bca..f74d6f8622 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -764,9 +764,9 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique # Get any passwords or other data that are prerequisites to running # the job. needed = self.get_passwords_needed_to_start() - opts = dict([(field, kwargs.get(field, '')) for field in needed]) - if not all(opts.values()): - return False + for field in needed: + if field not in kwargs: + return False if 'extra_vars' in kwargs: self.handle_extra_data(kwargs['extra_vars']) From 9e67169cb9b2cc7733383fc4de963cf5eccefb0f Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 15:26:22 -0400 Subject: [PATCH 3/8] ensure credentials exist before we go looking at them --- awx/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 6d2d0d60f9..20810acaff 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1777,7 +1777,7 @@ class JobLaunchSerializer(BaseSerializer): credential = attrs.get('credential', None) or obj.credential # fill passwords dict with request data passwords - if credential.passwords_needed: + if credential and credential.passwords_needed: try: for p in credential.passwords_needed: passwords[p] = data[p] From 8d13cafd7b0f6dd09c4abd30de7d9f9609fbc2e5 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 16:57:03 -0400 Subject: [PATCH 4/8] added tests for explicit and deleted credential In doing so, moved tests from jobs_monolithic --- awx/main/tests/jobs/__init__.py | 1 + awx/main/tests/jobs/base.py | 15 ++ awx/main/tests/jobs/job_launch.py | 187 +++++++++++++++++++++++++ awx/main/tests/jobs/jobs_monolithic.py | 99 ------------- 4 files changed, 203 insertions(+), 99 deletions(-) create mode 100644 awx/main/tests/jobs/job_launch.py diff --git a/awx/main/tests/jobs/__init__.py b/awx/main/tests/jobs/__init__.py index a4b24cdc16..bf5eedafe7 100644 --- a/awx/main/tests/jobs/__init__.py +++ b/awx/main/tests/jobs/__init__.py @@ -4,6 +4,7 @@ from __future__ import absolute_import from .jobs_monolithic import * # noqa +from .job_launch import * # noqa from .survey_password import * # noqa from .start_cancel import * # noqa from .base import * # noqa diff --git a/awx/main/tests/jobs/base.py b/awx/main/tests/jobs/base.py index 44cd059ab9..cc4a169a8a 100644 --- a/awx/main/tests/jobs/base.py +++ b/awx/main/tests/jobs/base.py @@ -264,6 +264,21 @@ class BaseJobTestMixin(BaseTestMixin): password=TEST_SSH_KEY_DATA, created_by=self.user_sue, ) + self.cred_sue_ask = self.user_sue.credentials.create( + username='sue', + password='ASK', + created_by=self.user_sue, + ) + self.cred_sue_ask_many = self.user_sue.credentials.create( + username='sue', + password='ASK', + become_method='sudo', + become_username='root', + become_password='ASK', + ssh_key_data=TEST_SSH_KEY_DATA_LOCKED, + ssh_key_unlock='ASK', + created_by=self.user_sue, + ) self.cred_bob = self.user_bob.credentials.create( username='bob', password='ASK', diff --git a/awx/main/tests/jobs/job_launch.py b/awx/main/tests/jobs/job_launch.py new file mode 100644 index 0000000000..1979a0c571 --- /dev/null +++ b/awx/main/tests/jobs/job_launch.py @@ -0,0 +1,187 @@ +# Copyright (c) 2015 Ansible, Inc. +# All Rights Reserved + +# Python +from __future__ import absolute_import + +# Django +import django +from django.core.urlresolvers import reverse +from django.conf import settings + +# AWX +from awx.main.models import * # noqa +from awx.main.tests.base import BaseLiveServerTest +from .base import BaseJobTestMixin + +__all__ = ['JobTemplateLaunchTest', 'JobTemplateLaunchPasswordsTest'] + +class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TestCase): + def setUp(self): + super(JobTemplateLaunchTest, self).setUp() + + self.url = reverse('api:job_template_list') + self.data = dict( + name = 'launched job template', + job_type = PERM_INVENTORY_DEPLOY, + inventory = self.inv_eng.pk, + project = self.proj_dev.pk, + credential = self.cred_sue.pk, + playbook = self.proj_dev.playbooks[0], + ) + self.data_no_cred = dict( + name = 'launched job template no credential', + job_type = PERM_INVENTORY_DEPLOY, + inventory = self.inv_eng.pk, + project = self.proj_dev.pk, + playbook = self.proj_dev.playbooks[0], + ) + self.data_cred_ask = dict(self.data) + self.data_cred_ask['name'] = 'launched job templated with ask passwords' + self.data_cred_ask['credential'] = self.cred_sue_ask.pk + + with self.current_user(self.user_sue): + response = self.post(self.url, self.data, expect=201) + self.launch_url = reverse('api:job_template_launch', + args=(response['id'],)) + + def test_launch_job_template(self): + with self.current_user(self.user_sue): + self.data['name'] = 'something different' + response = self.post(self.url, self.data, expect=201) + detail_url = reverse('api:job_template_detail', + args=(response['id'],)) + self.assertEquals(response['url'], detail_url) + + def test_no_cred_update_template(self): + # You can still post the job template without a credential, just can't launch it without one + with self.current_user(self.user_sue): + response = self.post(self.url, self.data_no_cred, expect=201) + detail_url = reverse('api:job_template_detail', + args=(response['id'],)) + self.assertEquals(response['url'], detail_url) + + def test_invalid_auth_unauthorized(self): + # Invalid auth can't trigger the launch endpoint + self.check_invalid_auth(self.launch_url, {}, methods=('post',)) + + def test_credential_implicit(self): + # Implicit, attached credentials + with self.current_user(self.user_sue): + response = self.post(self.launch_url, {}, expect=202) + j = Job.objects.get(pk=response['job']) + self.assertTrue(j.status == 'new') + + def test_credential_explicit(self): + # Explicit, credential + with self.current_user(self.user_sue): + self.cred_sue.mark_inactive() + response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=202) + j = Job.objects.get(pk=response['job']) + self.assertEqual(j.status, 'new') + self.assertEqual(j.credential.pk, self.cred_doug.pk) + + def test_credential_explicit_via_credential_id(self): + # Explicit, credential + with self.current_user(self.user_sue): + self.cred_sue.mark_inactive() + response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=202) + j = Job.objects.get(pk=response['job']) + self.assertEqual(j.status, 'new') + self.assertEqual(j.credential.pk, self.cred_doug.pk) + + def test_credential_override(self): + # Explicit, credential + with self.current_user(self.user_sue): + response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=202) + j = Job.objects.get(pk=response['job']) + self.assertEqual(j.status, 'new') + self.assertEqual(j.credential.pk, self.cred_doug.pk) + + def test_credential_override_via_credential_id(self): + # Explicit, credential + with self.current_user(self.user_sue): + response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=202) + j = Job.objects.get(pk=response['job']) + self.assertEqual(j.status, 'new') + self.assertEqual(j.credential.pk, self.cred_doug.pk) + + def test_bad_credential_launch_fail(self): + # Can't launch a job template without a credential defined (or if we + # pass an invalid/inactive credential value). + with self.current_user(self.user_sue): + self.cred_sue.mark_inactive() + response = self.post(self.launch_url, {}, expect=400) + response = self.post(self.launch_url, {'credential': 0}, expect=400) + response = self.post(self.launch_url, {'credential_id': 0}, expect=400) + response = self.post(self.launch_url, {'credential': 'one'}, expect=400) + response = self.post(self.launch_url, {'credential_id': 'one'}, expect=400) + self.cred_doug.mark_inactive() + response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=400) + response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=400) + + def test_no_project_fail(self): + # Job Templates without projects can not be launched + with self.current_user(self.user_sue): + self.data['name'] = "missing proj" + response = self.post(self.url, self.data, expect=201) + jt = JobTemplate.objects.get(pk=response['id']) + jt.project = None + jt.save() + launch_url2 = reverse('api:job_template_launch', + args=(response['id'],)) + self.post(launch_url2, {}, expect=400) + + def test_no_inventory_fail(self): + # Job Templates without inventory can not be launched + with self.current_user(self.user_sue): + self.data['name'] = "missing inv" + response = self.post(self.url, self.data, expect=201) + jt = JobTemplate.objects.get(pk=response['id']) + jt.inventory = None + jt.save() + launch_url3 = reverse('api:job_template_launch', + args=(response['id'],)) + self.post(launch_url3, {}, expect=400) + + def test_deleted_credential_fail(self): + # Job Templates with deleted credentials cannot be launched. + self.cred_sue.mark_inactive() + with self.current_user(self.user_sue): + response = self.post(self.launch_url, {}, expect=400) + +class JobTemplateLaunchPasswordsTest(BaseJobTestMixin, django.test.TestCase): + def setUp(self): + super(JobTemplateLaunchPasswordsTest, self).setUp() + + self.url = reverse('api:job_template_list') + self.data = dict( + name = 'launched job template', + job_type = PERM_INVENTORY_DEPLOY, + inventory = self.inv_eng.pk, + project = self.proj_dev.pk, + credential = self.cred_sue_ask.pk, + playbook = self.proj_dev.playbooks[0], + ) + + with self.current_user(self.user_sue): + response = self.post(self.url, self.data, expect=201) + self.launch_url = reverse('api:job_template_launch', + args=(response['id'],)) + + # should return explicit credentials required passwords + def test_explicit_cred_with_ask_passwords_fail(self): + passwords_required = ['ssh_password', 'become_password', 'ssh_key_unlock'] + # Job Templates with deleted credentials cannot be launched. + with self.current_user(self.user_sue): + self.cred_sue_ask.mark_inactive() + response = self.post(self.launch_url, {'credential_id': self.cred_sue_ask_many.pk}, expect=400) + for p in passwords_required: + self.assertIn(p, response['passwords_needed_to_start']) + self.assertEqual(len(passwords_required), len(response['passwords_needed_to_start'])) + + def test_explicit_cred_with_ask_password(self): + with self.current_user(self.user_sue): + response = self.post(self.launch_url, {'ssh_password': 'whatever'}, expect=202) + j = Job.objects.get(pk=response['job']) + self.assertEqual(j.status, 'new') diff --git a/awx/main/tests/jobs/jobs_monolithic.py b/awx/main/tests/jobs/jobs_monolithic.py index cb251be088..f5851c5dbf 100644 --- a/awx/main/tests/jobs/jobs_monolithic.py +++ b/awx/main/tests/jobs/jobs_monolithic.py @@ -443,105 +443,6 @@ class JobTemplateTest(BaseJobTestMixin, django.test.TestCase): with self.current_user(self.user_doug): self.get(detail_url, expect=403) - def test_launch_job_template(self): - url = reverse('api:job_template_list') - data = dict( - name = 'launched job template', - job_type = PERM_INVENTORY_DEPLOY, - inventory = self.inv_eng.pk, - project = self.proj_dev.pk, - credential = self.cred_sue.pk, - playbook = self.proj_dev.playbooks[0], - ) - data_no_cred = dict( - name = 'launched job template no credential', - job_type = PERM_INVENTORY_DEPLOY, - inventory = self.inv_eng.pk, - project = self.proj_dev.pk, - playbook = self.proj_dev.playbooks[0], - ) - - with self.current_user(self.user_sue): - response = self.post(url, data, expect=201) - detail_url = reverse('api:job_template_detail', - args=(response['id'],)) - self.assertEquals(response['url'], detail_url) - - launch_url = reverse('api:job_template_launch', - args=(response['id'],)) - - # You can still post the job template without a credential, just can't launch it without one - with self.current_user(self.user_sue): - response = self.post(url, data_no_cred, expect=201) - detail_url = reverse('api:job_template_detail', - args=(response['id'],)) - self.assertEquals(response['url'], detail_url) - - no_launch_url = reverse('api:job_template_launch', - args=(response['id'],)) - # Invalid auth can't trigger the launch endpoint - self.check_invalid_auth(launch_url, {}, methods=('post',)) - - # Implicit, attached credentials - with self.current_user(self.user_sue): - response = self.post(launch_url, {}, expect=202) - j = Job.objects.get(pk=response['job']) - self.assertTrue(j.status == 'new') - - # Explicit, override credentials - with self.current_user(self.user_sue): - response = self.post(launch_url, {'credential': self.cred_doug.pk}, expect=202) - j = Job.objects.get(pk=response['job']) - self.assertTrue(j.status == 'new') - self.assertEqual(j.credential.pk, self.cred_doug.pk) - - # Explicit, override credentials - with self.current_user(self.user_sue): - response = self.post(launch_url, {'credential_id': self.cred_doug.pk}, expect=202) - j = Job.objects.get(pk=response['job']) - self.assertTrue(j.status == 'new') - self.assertEqual(j.credential.pk, self.cred_doug.pk) - - # Can't launch a job template without a credential defined (or if we - # pass an invalid/inactive credential value). - with self.current_user(self.user_sue): - response = self.post(no_launch_url, {}, expect=400) - response = self.post(no_launch_url, {'credential': 0}, expect=400) - response = self.post(no_launch_url, {'credential_id': 0}, expect=400) - response = self.post(no_launch_url, {'credential': 'one'}, expect=400) - response = self.post(no_launch_url, {'credential_id': 'one'}, expect=400) - self.cred_doug.mark_inactive() - response = self.post(no_launch_url, {'credential': self.cred_doug.pk}, expect=400) - response = self.post(no_launch_url, {'credential_id': self.cred_doug.pk}, expect=400) - - # Job Templates without projects can not be launched - with self.current_user(self.user_sue): - data['name'] = "missing proj" - response = self.post(url, data, expect=201) - jt = JobTemplate.objects.get(pk=response['id']) - jt.project = None - jt.save() - launch_url2 = reverse('api:job_template_launch', - args=(response['id'],)) - self.post(launch_url2, {}, expect=400) - - # Job Templates without inventory can not be launched - with self.current_user(self.user_sue): - data['name'] = "missing inv" - response = self.post(url, data, expect=201) - jt = JobTemplate.objects.get(pk=response['id']) - jt.inventory = None - jt.save() - launch_url3 = reverse('api:job_template_launch', - args=(response['id'],)) - self.post(launch_url3, {}, expect=400) - - # Job Templates with deleted credentials cannot be launched. - self.cred_sue.mark_inactive() - with self.current_user(self.user_sue): - response = self.post(launch_url, {}, expect=400) - - class JobTest(BaseJobTestMixin, django.test.TestCase): def test_get_job_list(self): From f3a644a418a24039a25aa8dc19ba5dec0445ff1c Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 17:15:20 -0400 Subject: [PATCH 5/8] added password blank test --- awx/main/tests/jobs/job_launch.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/awx/main/tests/jobs/job_launch.py b/awx/main/tests/jobs/job_launch.py index 1979a0c571..32b0e62287 100644 --- a/awx/main/tests/jobs/job_launch.py +++ b/awx/main/tests/jobs/job_launch.py @@ -185,3 +185,9 @@ class JobTemplateLaunchPasswordsTest(BaseJobTestMixin, django.test.TestCase): response = self.post(self.launch_url, {'ssh_password': 'whatever'}, expect=202) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') + + def test_explicit_cred_with_ask_password_empty_string_fail(self): + with self.current_user(self.user_sue): + response = self.post(self.launch_url, {'ssh_password': ''}, expect=400) + j = Job.objects.get(pk=response['job']) + self.assertEqual(j.status, 'new') From 257dc733b9bbefb7597989a4d6fc76b4143e2e34 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 17:15:48 -0400 Subject: [PATCH 6/8] Revert "consider a password specified if it equals ''" This reverts commit 22987c4a122c5f784be62a0951ffd3081e8d2f12. --- awx/main/models/unified_jobs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index f74d6f8622..0a34e18bca 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -764,9 +764,9 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique # Get any passwords or other data that are prerequisites to running # the job. needed = self.get_passwords_needed_to_start() - for field in needed: - if field not in kwargs: - return False + opts = dict([(field, kwargs.get(field, '')) for field in needed]) + if not all(opts.values()): + return False if 'extra_vars' in kwargs: self.handle_extra_data(kwargs['extra_vars']) From e36a80f5092ee9e2e278555810eb896b3a1861a7 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 17:25:44 -0400 Subject: [PATCH 7/8] changed test case logic to know that the server doesn't like passwords like '' --- awx/main/tests/jobs/job_launch.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/main/tests/jobs/job_launch.py b/awx/main/tests/jobs/job_launch.py index 32b0e62287..3fd5e8452b 100644 --- a/awx/main/tests/jobs/job_launch.py +++ b/awx/main/tests/jobs/job_launch.py @@ -189,5 +189,5 @@ class JobTemplateLaunchPasswordsTest(BaseJobTestMixin, django.test.TestCase): def test_explicit_cred_with_ask_password_empty_string_fail(self): with self.current_user(self.user_sue): response = self.post(self.launch_url, {'ssh_password': ''}, expect=400) - j = Job.objects.get(pk=response['job']) - self.assertEqual(j.status, 'new') + self.assertIn('ssh_password', response['passwords_needed_to_start']) + From 4fc100ac8c2555e3fc3a39e821339cb4861cbb26 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Tue, 28 Apr 2015 18:04:12 -0400 Subject: [PATCH 8/8] flake8 --- awx/main/tests/jobs/job_launch.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/awx/main/tests/jobs/job_launch.py b/awx/main/tests/jobs/job_launch.py index 3fd5e8452b..26124b1f61 100644 --- a/awx/main/tests/jobs/job_launch.py +++ b/awx/main/tests/jobs/job_launch.py @@ -7,11 +7,9 @@ from __future__ import absolute_import # Django import django from django.core.urlresolvers import reverse -from django.conf import settings # AWX from awx.main.models import * # noqa -from awx.main.tests.base import BaseLiveServerTest from .base import BaseJobTestMixin __all__ = ['JobTemplateLaunchTest', 'JobTemplateLaunchPasswordsTest'] @@ -111,14 +109,14 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TestCase): # pass an invalid/inactive credential value). with self.current_user(self.user_sue): self.cred_sue.mark_inactive() - response = self.post(self.launch_url, {}, expect=400) - response = self.post(self.launch_url, {'credential': 0}, expect=400) - response = self.post(self.launch_url, {'credential_id': 0}, expect=400) - response = self.post(self.launch_url, {'credential': 'one'}, expect=400) - response = self.post(self.launch_url, {'credential_id': 'one'}, expect=400) + self.post(self.launch_url, {}, expect=400) + self.post(self.launch_url, {'credential': 0}, expect=400) + self.post(self.launch_url, {'credential_id': 0}, expect=400) + self.post(self.launch_url, {'credential': 'one'}, expect=400) + self.post(self.launch_url, {'credential_id': 'one'}, expect=400) self.cred_doug.mark_inactive() - response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=400) - response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=400) + self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=400) + self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=400) def test_no_project_fail(self): # Job Templates without projects can not be launched @@ -148,7 +146,7 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TestCase): # Job Templates with deleted credentials cannot be launched. self.cred_sue.mark_inactive() with self.current_user(self.user_sue): - response = self.post(self.launch_url, {}, expect=400) + self.post(self.launch_url, {}, expect=400) class JobTemplateLaunchPasswordsTest(BaseJobTestMixin, django.test.TestCase): def setUp(self):