Awx: Creating a Schedule with invalid `credentials` key raises 500 Error

[ryanpetrello]

SUMMARY

1. Create a normal user, peon, with "admin" access to a Job Template.
2. As that user, attempt to create a schedule for that Job Template, specifying credentials:

body = {
    'inventory': 1,
    'credentials': [1],
    'name': 'Example',
    'rrule': 'DTSTART:20151117T050000Z RRULE:FREQ=DAILY;INTERVAL=1;COUNT=1'
}
requests.post(
    'https://peon:[email protected]:8043/api/v2/job_templates/7/schedules/',
    json=body
)

AWX raises an uncaught exception:

2019-06-21 20:31:01,747 ERROR    django.request Internal Server Error: /api/v2/job_templates/7/schedules/
Traceback (most recent call last):
  File "/venv/awx/lib64/python3.6/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/venv/awx/lib64/python3.6/site-packages/django/core/handlers/base.py", line 187, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/venv/awx/lib64/python3.6/site-packages/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib64/python3.6/contextlib.py", line 52, in inner
    return func(*args, **kwds)
  File "/venv/awx/lib64/python3.6/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/venv/awx/lib64/python3.6/site-packages/django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "./awx/api/generics.py", line 293, in dispatch
    return super(APIView, self).dispatch(request, *args, **kwargs)
  File "/venv/awx/lib64/python3.6/site-packages/rest_framework/views.py", line 495, in dispatch
    response = self.handle_exception(exc)
  File "/venv/awx/lib64/python3.6/site-packages/rest_framework/views.py", line 455, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/venv/awx/lib64/python3.6/site-packages/rest_framework/views.py", line 492, in dispatch
    response = handler(request, *args, **kwargs)
  File "/venv/awx/lib64/python3.6/site-packages/rest_framework/generics.py", line 244, in post
    return self.create(request, *args, **kwargs)
  File "./awx/api/generics.py", line 570, in create
    if not request.user.can_access(self.model, 'add', serializer.initial_data):
  File "./awx/main/access.py", line 122, in check_user_access
    result = access_method(*args, **kwargs)
  File "./awx/main/access.py", line 162, in wrapper
    return func(self, *args, **kwargs)
  File "./awx/main/access.py", line 2445, in can_add
    if not JobLaunchConfigAccess(self.user).can_add(data):
  File "./awx/main/access.py", line 162, in wrapper
    return func(self, *args, **kwargs)
  File "./awx/main/access.py", line 1787, in can_add
    cred_pks = [cred.pk for cred in data['credentials']]
  File "./awx/main/access.py", line 1787, in <listcomp>
    cred_pks = [cred.pk for cred in data['credentials']]
AttributeError: 'int' object has no attribute 'pk'

[ryanpetrello]

The example here is an incorrect usage of the API (you're supposed to attach credentials using /api/v2/job_templates/X/schedules/Y/credentials/) but we still shouldn't be throwing an HTTP 500.

[ryanpetrello]

@AlanCoding what do you think is the cleanest way to resolve this?

Here's my Friday afternoon stab at it:

diff --git a/awx/api/serializers.py b/awx/api/serializers.py
index 2eb252a5c..f9913ab00 100644
--- a/awx/api/serializers.py
+++ b/awx/api/serializers.py
@@ -3415,6 +3415,12 @@ class LaunchConfigurationBaseSerializer(BaseSerializer):
         summary_fields.pop('credential', None)
         return summary_fields

+    def to_internal_value(self, data):
+        # promptable credentials are associated at `/credentials/` endpoints,
+        # not in the actual launch config
+        data.pop('credentials', None)
+        return super(LaunchConfigurationBaseSerializer, self).to_internal_value(data)
+

[AlanCoding]

For whatever reason, I can't reproduce this, although the traceback makes perfect sense to me. When I attempt it, it just creates the schedule and doesn't associate any credentials. Anyway, if I could reproduce it, I would test out this fix:

diff --git a/awx/api/generics.py b/awx/api/generics.py
index 7c17799d11..4058f7f358 100644
--- a/awx/api/generics.py
+++ b/awx/api/generics.py
@@ -567,7 +567,7 @@ class SubListCreateAPIView(SubListAPIView, ListCreateAPIView):
                             status=status.HTTP_400_BAD_REQUEST)

         # Verify we have permission to add the object as given.
-        if not request.user.can_access(self.model, 'add', serializer.initial_data):
+        if not request.user.can_access(self.model, 'add', serializer.validated_data):
             raise PermissionDenied()

         # save the object through the serializer, reload and returned the saved

The key should really only be used for internal purposes, and it's concerning that data the user supplied gets into that logic at all (although in practice it should get by using a reference object instead). This diff should avoid that, but it's been the current way for a long time.

[fosterseth]

I was able to reproduce the 500 error. I can also confirm that @AlanCoding's suggestion does prevent a 500 error and the request goes through cleanly with a 201 response.

If we like this solution I'd be happy to open a PR with the change, or I can look for another solution to address Alan's concern.

[fosterseth]

fixed and merged into awx devel #5084

[kdelee]

Tested this out by adding credentials key to a POST to a job templates's schedule, it was correctly ignored as was expected behavior (because this endpoint does not use that key and should be sanitized from payload according to our current practice). I did not get a 500 server error, and instead the request was processed correctly. Closing, thanks!