From 0154d80f196e5e75dc5875340c53b23e21fd07fd Mon Sep 17 00:00:00 2001
From: Vismay Golwala <vgolwala@redhat.com>
Date: Thu, 2 May 2019 13:17:46 -0400
Subject: [PATCH] Raise meaningful error when permission denied to copy JT

When a user doesn't have access to all the credentials of a job
template, they cannot copy the JT. However, currently we raise a
default `PermissionDenied`, which doesn't give the user insight
into what's wrong. So, this PR just adds a custom message indicating
that access to credentials is missing.

Signed-off-by: Vismay Golwala <vgolwala@redhat.com>
---
 awx/api/generics.py                    |  7 ++--
 awx/main/access.py                     |  5 ++-
 awx/main/tests/functional/test_copy.py | 45 ++++++++++++++++++--------
 3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/awx/api/generics.py b/awx/api/generics.py
index 2f2eca791b..c3f7d22700 100644
--- a/awx/api/generics.py
+++ b/awx/api/generics.py
@@ -959,8 +959,11 @@ class CopyAPIView(GenericAPIView):
         create_kwargs = self._build_create_dict(obj)
         for key in create_kwargs:
             create_kwargs[key] = getattr(create_kwargs[key], 'pk', None) or create_kwargs[key]
-        can_copy = request.user.can_access(self.model, 'add', create_kwargs) and \
-            request.user.can_access(self.model, 'copy_related', obj)
+        try:
+            can_copy = request.user.can_access(self.model, 'add', create_kwargs) and \
+                request.user.can_access(self.model, 'copy_related', obj)
+        except PermissionDenied:
+            return Response({'can_copy': False})
         return Response({'can_copy': can_copy})
 
     def post(self, request, *args, **kwargs):
diff --git a/awx/main/access.py b/awx/main/access.py
index 21848a92f0..12de86b73f 100644
--- a/awx/main/access.py
+++ b/awx/main/access.py
@@ -1507,7 +1507,10 @@ class JobTemplateAccess(NotificationAttachMixin, BaseAccess):
 
         # obj.credentials.all() is accessible ONLY when object is saved (has valid id)
         credential_manager = getattr(obj, 'credentials', None) if getattr(obj, 'id', False) else Credential.objects.none()
-        return reduce(lambda prev, cred: prev and self.user in cred.use_role, credential_manager.all(), True)
+        user_can_copy = reduce(lambda prev, cred: prev and self.user in cred.use_role, credential_manager.all(), True)
+        if not user_can_copy:
+            raise PermissionDenied(_('Insufficient access to Job Template credentials.'))
+        return user_can_copy
 
     def can_start(self, obj, validate_license=True):
         # Check license.
diff --git a/awx/main/tests/functional/test_copy.py b/awx/main/tests/functional/test_copy.py
index ed03e0f61b..a4d2859110 100644
--- a/awx/main/tests/functional/test_copy.py
+++ b/awx/main/tests/functional/test_copy.py
@@ -28,25 +28,44 @@ def test_job_template_copy(post, get, project, inventory, machine_credential, va
         reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
         admin, expect=200
     ).data['can_copy'] is True
-    post(
+    assert post(
         reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
         {'name': 'new jt name'}, alice, expect=403
-    )
+    ).data['detail'] == 'Insufficient access to Job Template credentials.'
     jt_copy_pk = post(
         reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
         {'name': 'new jt name'}, admin, expect=201
     ).data['id']
-    jt_copy = type(job_template_with_survey_passwords).objects.get(pk=jt_copy_pk)
-    assert jt_copy.created_by == admin
-    assert jt_copy.name == 'new jt name'
-    assert jt_copy.project == project
-    assert jt_copy.inventory == inventory
-    assert jt_copy.playbook == job_template_with_survey_passwords.playbook
-    assert jt_copy.credentials.count() == 3
-    assert credential in jt_copy.credentials.all()
-    assert vault_credential in jt_copy.credentials.all()
-    assert machine_credential in jt_copy.credentials.all()
-    assert job_template_with_survey_passwords.survey_spec == jt_copy.survey_spec
+
+    # give credential access to user 'alice'
+    for c in (credential, machine_credential, vault_credential):
+        c.use_role.members.add(alice)
+        c.save()
+    assert get(
+        reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
+        alice, expect=200
+    ).data['can_copy'] is True
+    jt_copy_pk_alice = post(
+        reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
+        {'name': 'new jt name'}, alice, expect=201
+    ).data['id']
+
+    jt_copy_admin = type(job_template_with_survey_passwords).objects.get(pk=jt_copy_pk)
+    jt_copy_alice = type(job_template_with_survey_passwords).objects.get(pk=jt_copy_pk_alice)
+
+    assert jt_copy_admin.created_by == admin
+    assert jt_copy_alice.created_by == alice
+
+    for jt_copy in (jt_copy_admin, jt_copy_alice):
+        assert jt_copy.name == 'new jt name'
+        assert jt_copy.project == project
+        assert jt_copy.inventory == inventory
+        assert jt_copy.playbook == job_template_with_survey_passwords.playbook
+        assert jt_copy.credentials.count() == 3
+        assert credential in jt_copy.credentials.all()
+        assert vault_credential in jt_copy.credentials.all()
+        assert machine_credential in jt_copy.credentials.all()
+        assert job_template_with_survey_passwords.survey_spec == jt_copy.survey_spec
 
 
 @pytest.mark.django_db