Merge pull request #649 from ryanpetrello/multicred

fix a permissions bug for credentials specified at JT launch time

awx/api/views.py

@@ -2797,10 +2797,14 @@ class JobTemplateLaunch(RetrieveAPIView):
             if request.user not in use_role:
                 raise PermissionDenied()
 
-        for cred in prompted_fields.get('credentials', []):
-            new_credential = get_object_or_400(Credential, pk=cred)
-            if request.user not in new_credential.use_role:
-                raise PermissionDenied()
+        # For credentials that are _added_ via launch parameters, ensure the
+        # launching user has access
+        current_credentials = set(obj.credentials.values_list('id', flat=True))
+        for new_cred in Credential.objects.filter(id__in=prompted_fields.get('credentials', [])):
+            if new_cred.pk not in current_credentials and request.user not in new_cred.use_role:
+                raise PermissionDenied(_(
+                    "You do not have access to credential {}".format(new_cred.name)
+                ))
 
         new_job = obj.create_unified_job(**prompted_fields)
         result = new_job.signal_start(**passwords)
@@ -2994,7 +2998,7 @@ class JobTemplateExtraCredentialsList(JobTemplateCredentialsList):
 
     def get_queryset(self):
         sublist_qs = super(JobTemplateExtraCredentialsList, self).get_queryset()
-        sublist_qs = sublist_qs.filter(**{'credential_type__kind__in': ['cloud', 'net']})
+        sublist_qs = sublist_qs.filter(credential_type__kind__in=['cloud', 'net'])
         return sublist_qs
 
     def is_valid_relation(self, parent, sub, created=False):
@@ -3790,7 +3794,7 @@ class JobExtraCredentialsList(JobCredentialsList):
 
     def get_queryset(self):
         sublist_qs = super(JobExtraCredentialsList, self).get_queryset()
-        sublist_qs = sublist_qs.filter(**{'credential_type__kind__in': ['cloud', 'net']})
+        sublist_qs = sublist_qs.filter(credential_type__kind__in=['cloud', 'net'])
         return sublist_qs
 
 

awx/main/models/jobs.py

@@ -166,11 +166,11 @@ class JobOptions(BaseModel):
 
     @property
     def network_credentials(self):
-        return [cred for cred in self.credentials.all() if cred.credential_type.kind == 'net']
+        return list(self.credentials.filter(credential_type__kind='net'))
 
     @property
     def cloud_credentials(self):
-        return [cred for cred in self.credentials.all() if cred.credential_type.kind == 'cloud']
+        return list(self.credentials.filter(credential_type__kind='cloud'))
 
     @property
     def credential(self):
@@ -186,7 +186,7 @@ class JobOptions(BaseModel):
 
     def get_deprecated_credential(self, kind):
         try:
-            return [cred for cred in self.credentials.all() if cred.credential_type.kind == kind][0]
+            return self.credentials.filter(credential_type__kind=kind).first()
         except IndexError:
             return None
 

awx/main/tests/functional/api/test_deprecated_credential_assignment.py

@@ -338,3 +338,30 @@ def test_extra_creds_prompted_at_launch(get, post, job_template, admin, net_cred
 def test_invalid_mixed_credentials_specification(get, post, job_template, admin, net_credential):
     url = reverse('api:job_template_launch', kwargs={'pk': job_template.pk})
     post(url, {'credentials': [net_credential.pk], 'extra_credentials': [net_credential.pk]}, admin, expect=400)
+
+
+@pytest.mark.django_db
+def test_rbac_default_credential_usage(get, post, job_template, alice, machine_credential):
+    job_template.credentials.add(machine_credential)
+    job_template.execute_role.members.add(alice)
+    job_template.save()
+
+    # alice can launch; she's not adding any _new_ credentials, and she has
+    # execute access to the JT
+    url = reverse('api:job_template_launch', kwargs={'pk': job_template.pk})
+    post(url, {'credential': machine_credential.pk}, alice, expect=201)
+
+    # make (copy) a _new_ SSH cred
+    new_cred = machine_credential
+    new_cred.pk = None
+    new_cred.save()
+
+    # alice is attempting to launch with a *different* SSH cred, but
+    # she does not have access to it, so she cannot launch
+    url = reverse('api:job_template_launch', kwargs={'pk': job_template.pk})
+    post(url, {'credential': new_cred.pk}, alice, expect=403)
+
+    # if alice has gains access to the credential, she *can* launch
+    new_cred.use_role.members.add(alice)
+    url = reverse('api:job_template_launch', kwargs={'pk': job_template.pk})
+    post(url, {'credential': new_cred.pk}, alice, expect=201)

awx/main/tests/functional/api/test_job_runtime_params.py

@@ -237,7 +237,7 @@ def test_job_launch_fails_without_credential_access(job_template_prompts, runtim
     response = post(reverse('api:job_template_launch', kwargs={'pk':job_template.pk}),
                     dict(credentials=runtime_data['credentials']), rando, expect=403)
 
-    assert response.data['detail'] == u'You do not have permission to perform this action.'
+    assert response.data['detail'] == u'You do not have access to credential runtime-cred'
 
 
 @pytest.mark.django_db

awx/main/tests/unit/test_tasks.py

@@ -245,12 +245,15 @@ class TestJobExecution:
 
         # mock the job.credentials M2M relation so we can avoid DB access
         job._credentials = []
-        patch = mock.patch.object(UnifiedJob, 'credentials', mock.Mock(
-            all=lambda: job._credentials,
-            add=job._credentials.append,
-            filter=mock.Mock(return_value=job._credentials),
-            spec_set=['all', 'add', 'filter']
-        ))
+        patch = mock.patch.object(UnifiedJob, 'credentials', mock.Mock(**{
+            'all': lambda: job._credentials,
+            'add': job._credentials.append,
+            'filter.return_value': mock.Mock(
+                __iter__ = lambda *args: iter(job._credentials),
+                first = lambda: job._credentials[0]
+            ),
+            'spec_set': ['all', 'add', 'filter']
+        }))
         self.patches.append(patch)
         patch.start()