Fix credential leak when copying Job Templates.

Signed-off-by: Yunfan Zhang <yz322@duke.edu>

awx/api/generics.py

@@ -93,7 +93,7 @@ class LoggedLoginView(auth_views.LoginView):
             current_user = JSONRenderer().render(current_user.data)
             current_user = urllib.quote('%s' % current_user, '')
             ret.set_cookie('current_user', current_user)
-            
+
             return ret
         else:
             ret.status_code = 401
@@ -755,6 +755,7 @@ class DeleteLastUnattachLabelMixin(object):
     when the last disassociate is called should inherit from this class. Further,
     the model should implement is_detached()
     '''
+
     def unattach(self, request, *args, **kwargs):
         (sub_id, res) = super(DeleteLastUnattachLabelMixin, self).unattach_validate(request)
         if res:
@@ -945,7 +946,9 @@ 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]
-        return Response({'can_copy': request.user.can_access(self.model, 'add', create_kwargs)})
+        can_copy = request.user.can_access(self.model, 'add', create_kwargs) and \
+            request.user.can_access(self.model, 'copy_related', obj)
+        return Response({'can_copy': can_copy})
 
     def post(self, request, *args, **kwargs):
         if get_request_version(request) < 2:
@@ -957,6 +960,8 @@ class CopyAPIView(GenericAPIView):
             create_kwargs_check[key] = getattr(create_kwargs[key], 'pk', None) or create_kwargs[key]
         if not request.user.can_access(self.model, 'add', create_kwargs_check):
             raise PermissionDenied()
+        if not request.user.can_access(self.model, 'copy_related', obj):
+            raise PermissionDenied()
         serializer = self.get_serializer(data=request.data)
         if not serializer.is_valid():
             return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)

awx/main/access.py

@@ -6,6 +6,7 @@ import os
 import sys
 import logging
 import six
+from functools import reduce
 
 # Django
 from django.conf import settings
@@ -218,6 +219,15 @@ class BaseAccess(object):
     def can_copy(self, obj):
         return self.can_add({'reference_obj': obj})
 
+    def can_copy_related(self, obj):
+        '''
+        can_copy_related() should only be used to check if the user have access to related
+        many to many credentials in when copying the object. It does not check if the user
+        has permission for any other related objects. Therefore, when checking if the user
+        can copy an object, it should always be used in conjunction with can_add()
+        '''
+        return True
+
     def can_attach(self, obj, sub_obj, relationship, data,
                    skip_sub_obj_read_check=False):
         if skip_sub_obj_read_check:
@@ -1328,6 +1338,17 @@ class JobTemplateAccess(BaseAccess):
             return self.user in project.use_role
         else:
             return False
+    
+    @check_superuser
+    def can_copy_related(self, obj):
+        '''
+        Check if we have access to all the credentials related to Job Templates.
+        Does not verify the user's permission for any other related fields (projects, inventories, etc).
+        '''
+
+        # 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 Credentials.objects.none()
+        return reduce(lambda prev, cred: prev and self.user in cred.use_role, credential_manager.all(), True)
 
     def can_start(self, obj, validate_license=True):
         # Check license.

awx/main/tests/functional/test_copy.py

@@ -18,6 +18,8 @@ def test_job_template_copy(post, get, project, inventory, machine_credential, va
     job_template_with_survey_passwords.credentials.add(machine_credential)
     job_template_with_survey_passwords.credentials.add(vault_credential)
     job_template_with_survey_passwords.admin_role.members.add(alice)
+    project.admin_role.members.add(alice)
+    inventory.admin_role.members.add(alice)
     assert get(
         reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
         alice, expect=200
@@ -26,6 +28,10 @@ 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(
+        reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
+        {'name': 'new jt name'}, alice, expect=403
+    )
     jt_copy_pk = post(
         reverse('api:job_template_copy', kwargs={'pk': job_template_with_survey_passwords.pk}),
         {'name': 'new jt name'}, admin, expect=201

awx/main/tests/unit/api/serializers/test_job_template_serializers.py

@@ -97,7 +97,6 @@ class TestJobTemplateSerializerGetSummaryFields():
         are put into the serializer user_capabilities"""
 
         jt_obj = job_template_factory('testJT', project='proj1', persisted=False).job_template
-        jt_obj.id = 5
         jt_obj.admin_role = Role(id=9, role_field='admin_role')
         jt_obj.execute_role = Role(id=8, role_field='execute_role')
         jt_obj.read_role = Role(id=7, role_field='execute_role')
@@ -115,7 +114,7 @@ class TestJobTemplateSerializerGetSummaryFields():
 
         with mocker.patch("awx.api.serializers.role_summary_fields_generator", return_value='Can eat pie'):
             with mocker.patch("awx.main.access.JobTemplateAccess.can_change", return_value='foobar'):
-                with mocker.patch("awx.main.access.JobTemplateAccess.can_add", return_value='foo'):
+                with mocker.patch("awx.main.access.JobTemplateAccess.can_copy", return_value='foo'):
                     with mock.patch.object(jt_obj.__class__, 'get_deprecated_credential', return_value=None):
                         response = serializer.get_summary_fields(jt_obj)