8000 Creating a Schedule with invalid `credentials` key raises 500 Error · Issue #4147 · ansible/awx · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Creating a Schedule with invalid credentials key raises 500 Error #4147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ryanpetrello opened this issue Jun 21, 2019 · 6 comments
Closed

Creating a Schedule with invalid credentials key raises 500 Error #4147

ryanpetrello opened this issue Jun 21, 2019 · 6 comments
Assignees
@fosterseth @kdelee
Labels
component:api type:bug

Comments

@ryanpetrello
Copy link
Contributor
ryanpetrello commented Jun 21, 2019 •
edited
Loading
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:peon@awx.example.org: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 ryanpetrello self-assigned this Jun 21, 2019
@ryanpetrello
Copy link
Contributor Author

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
Copy link
Contributor Author
ryanpetrello commented Jun 21, 2019 •
edited
Loading

@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)
+

@ryanpetrello ryanpetrello changed the title Creating a Schedule with credentials you don't have access to raises 500 Error Creating a Schedule invalid credentials key raises 500 Error Jun 22, 2019
@ryanpetrello ryanpetrello changed the title Creating a Schedule invalid credentials key raises 500 Error Creating a Schedule with invalid credentials key raises 500 Error Jun 24, 2019
@AlanCoding
Copy link
Member

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.

@ryanpetrello ryanpetrello removed their assignment Sep 12, 2019
@fosterseth fosterseth self-assigned this Oct 23, 2019
@fosterseth
Copy link
Member
fosterseth commented Oct 23, 2019 •
edited
Loading

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 added a commit to fosterseth/awx that referenced this issue Oct 23, 2019
@fosterseth
Fix 500 error when creating a job schedule
58e84a4 
- 500 error occurs when a non-admin user attempts to add an invalid
  credential during schedule creation
- This change checks that the user can add the object to
  serializer.validated_data, instead of serializer.initial_data
- The invalid credential field is purged in .validated_data, so the
  request passes through cleanly
- Fix for awx issue ansible#4147
@fosterseth fosterseth mentioned this issue Oct 23, 2019
Merged
@fosterseth
Copy link
Member

fixed and merged into awx devel #5084

ryanpetrello pushed a commit to ryanpetrello/awx that referenced this issue Oct 29, 2019
@fosterseth @ryanpetrello
Fix 500 error when creating a job schedule
ff72be6 
- 500 error occurs when a non-admin user attempts to add an invalid
  credential during schedule creation
- This change checks that the user can add the object to
  serializer.validated_data, instead of serializer.initial_data
- The invalid credential field is purged in .validated_data, so the
  request passes through cleanly
- Fix for awx issue ansible#4147
@kdelee kdelee self-assigned this Dec 23, 2019
@kdelee
Copy link
Member
kdelee commented Dec 23, 2019

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!

@kdelee kdelee closed this as completed Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fosterseth fosterseth

@kdelee kdelee

Labels
component:api type:bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 par 30EF ticipants
@ryanpetrello @AlanCoding @fosterseth @kdelee
0