Issue #3906
closed
browsable API inserts a csrf token field into all form submissions
Description
Pulp's REST API validates that only acceptable fields are submitted with each request. The list of fields does not include the csrf token. As a result of this validation, the browsable API forms produce responses that look like this:
HTTP 400 Bad Request
Allow: GET, POST, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept
{
"csrfmiddlewaretoken": [
"Unexpected field"
]
}
We need to investigate how to configure DRF to stop including this field with each form. Otherwise a fix from comment 3 would be appropriate.
Notes¶
To easily reproduce, navigate your browser to e.g http://localhost:8000/pulp/api/v3/repositories/ and create a repository by filling-in the create repository form.
This happens when session authentication is used such as with a browser; non-session authentication won't trigger the CSRF protection.
Unfortunately it seems the session-authentication is somehow mandatory or at least I wasn't able to just switch it off:
pulp: django.request:ERROR: Internal Server Error: /pulp/api/v3/repositories/
Traceback (most recent call last):
File "/home/vagrant/.virtualenvs/pulp/lib64/python3.6/site-packages/django/core/handlers/exception.py", line 34, in inner
response = get_response(request)
File "/home/vagrant/.virtualenvs/pulp/lib64/python3.6/site-packages/django/utils/deprecation.py", line 90, in __call__
response = self.process_request(request)
File "/home/vagrant/.virtualenvs/pulp/lib64/python3.6/site-packages/django/contrib/auth/middleware.py", line 23, in process_request
) % ("_CLASSES" if settings.MIDDLEWARE is None else "")
AssertionError: The Django authentication middleware requires session middleware to be installed. Edit your MIDDLEWARE setting to insert 'django.contrib.sessions.middleware.SessionMiddleware' before 'django.contrib.auth.middleware.AuthenticationMiddleware'.
[13/Aug/2018 13:15:14] "GET /pulp/api/v3/repositories/ HTTP/1.1" 500 69693
Discussion¶
The session authentication is configured for particular Django application in app/settings.py:
MIDDLEWARE = [
...
django.middleware.csrf.CsrfViewMiddleware,
django.contrib.auth.middleware.AuthenticationMiddleware,
]
...
REST_FRAMEWORK = {
...
'DEFAULT_AUTHENTICATION_CLASSES': [
rest_framework.authentication.SessionAuthentication,
rest_framework.authentication.BasicAuthentication,
]
}
DRF uses a custom authentication class SessionAuthentication that in turn utilizes the Django middleware CsrfViewMiddleware class thru the CSRFCheck class, so the CsrfViewMiddleware:process_view gets eventually triggered. The request_csrf_token is then extracted from the request.POST.get('csrfmiddlewaretoken') form field, which is unset neither in the Django nor in the DRF middleware classes and the csrfmiddlewaretoken form field gets eventually propagated into a Pulp create object view.
Good news is we should be fine to just ignore it; "greping" over Github, this particular csrfmiddlewaretoken string shows quite often in the code so often in fact that I can't confirm some of it being DRF/Django workarounds.
Related issues
Updated by daviddavis over 6 years ago
- Related to Issue #2970: REST API silently ignores object attributes that don't exist on the serializer added
Updated by dkliban@redhat.com over 6 years ago
- Subject changed from csrf middleware breaks browsable API to browsable API inserts a csrf token field into all form submissions
- Description updated (diff)
Updated by daviddavis over 6 years ago
One possible solution is to update our unknown field validator to ignore the field:
diff --git a/pulpcore/pulpcore/app/serializers/base.py b/pulpcore/pulpcore/app/serializers/base.py
index 2a1aef1..66410dc 100644
--- a/pulpcore/pulpcore/app/serializers/base.py
+++ b/pulpcore/pulpcore/app/serializers/base.py
@@ -81,7 +81,8 @@ def validate_unknown_fields(initial_data, defined_fields):
"""
This will raise a `ValidationError` if a serializer is passed fields that are unknown.
"""
- unknown_fields = set(initial_data) - set(defined_fields)
+ ignored_fields = {'csrfmiddlewaretoken'}
+ unknown_fields = set(initial_data) - set(defined_fields) - ignored_fields
if unknown_fields:
unknown_fields = {field: _('Unexpected field') for field in unknown_fields}
raise serializers.ValidationError(unknown_fields)
Updated by CodeHeeler over 6 years ago
- Triaged changed from No to Yes
- Sprint set to Sprint 41
Updated by milan over 6 years ago
- Status changed from NEW to ASSIGNED
- Assignee set to milan
Updated by milan over 6 years ago
When trying to address this in DRF (authentication classes) by stripping the csrfmiddlewaretoken from the request POST object I get this error:
___________________________________________________________ NoDropdownWithoutAuthTests.test_name_shown_when_logged_in ____________________________________________________________
tests/browsable_api/test_browsable_api.py:63: in test_name_shown_when_logged_in
response = self.client.get('/')
rest_framework/test.py:292: in get
response = super(APIClient, self).get(path, data=data, **extra)
rest_framework/test.py:209: in get
return self.generic('GET', path, **r)
rest_framework/test.py:238: in generic
method, path, data, content_type, secure, **extra)
env/lib/python2.7/site-packages/django/test/client.py:416: in generic
return self.request(**r)
rest_framework/test.py:289: in request
return super(APIClient, self).request(**kwargs)
rest_framework/test.py:241: in request
request = super(APIRequestFactory, self).request(**kwargs)
env/lib/python2.7/site-packages/django/test/client.py:483: in request
response = self.handler(environ)
env/lib/python2.7/site-packages/django/test/client.py:144: in __call__
response = self.get_response(request)
rest_framework/test.py:261: in get_response
return super(ForceAuthClientHandler, self).get_response(request)
env/lib/python2.7/site-packages/django/core/handlers/base.py:124: in get_response
response = self._middleware_chain(request)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:43: in inner
response = response_for_exception(request, exc)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:93: in response_for_exception
response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
env/lib/python2.7/site-packages/django/core/handlers/exception.py:41: in inner
response = get_response(request)
env/lib/python2.7/site-packages/django/utils/deprecation.py:140: in __call__
response = self.get_response(request)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:43: in inner
response = response_for_exception(request, exc)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:93: in response_for_exception
response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
env/lib/python2.7/site-packages/django/core/handlers/exception.py:41: in inner
response = get_response(request)
env/lib/python2.7/site-packages/django/utils/deprecation.py:140: in __call__
response = self.get_response(request)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:43: in inner
response = response_for_exception(request, exc)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:93: in response_for_exception
response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
env/lib/python2.7/site-packages/django/core/handlers/exception.py:41: in inner
response = get_response(request)
env/lib/python2.7/site-packages/django/utils/deprecation.py:140: in __call__
response = self.get_response(request)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:43: in inner
response = response_for_exception(request, exc)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:93: in response_for_exception
response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
env/lib/python2.7/site-packages/django/core/handlers/exception.py:41: in inner
response = get_response(request)
env/lib/python2.7/site-packages/django/utils/deprecation.py:140: in __call__
response = self.get_response(request)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:43: in inner
response = response_for_exception(request, exc)
env/lib/python2.7/site-packages/django/core/handlers/exception.py:93: in response_for_exception
response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
env/lib/python2.7/site-packages/django/core/handlers/exception.py:41: in inner
response = get_response(request)
env/lib/python2.7/site-packages/django/core/handlers/base.py:187: in _get_response
response = self.process_exception_by_middleware(e, request)
env/lib/python2.7/site-packages/django/core/handlers/base.py:185: in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
env/lib/python2.7/site-packages/django/views/decorators/csrf.py:58: in wrapped_view
return view_func(*args, **kwargs)
env/lib/python2.7/site-packages/django/views/generic/base.py:68: in view
return self.dispatch(request, *args, **kwargs)
rest_framework/views.py:495: in dispatch
response = self.handle_exception(exc)
rest_framework/views.py:455: in handle_exception
self.raise_uncaught_exception(exc)
rest_framework/views.py:483: in dispatch
self.initial(request, *args, **kwargs)
rest_framework/views.py:400: in initial
self.perform_authentication(request)
rest_framework/views.py:326: in perform_authentication
request.user
rest_framework/request.py:222: in user
self._authenticate()
/usr/lib64/python2.7/contextlib.py:35: in __exit__
self.gen.throw(type, value, traceback)
rest_framework/request.py:80: in wrap_attributeerrors
six.reraise(type(exc), exc, info[2])
rest_framework/request.py:76: in wrap_attributeerrors
yield
rest_framework/request.py:222: in user
self._authenticate()
rest_framework/request.py:375: in _authenticate
user_auth_tuple = authenticator.authenticate(self)
rest_framework/authentication.py:129: in authenticate
self.enforce_csrf(request)
rest_framework/authentication.py:148: in enforce_csrf
del(request.POST['csrfmiddlewaretoken'])
env/lib/python2.7/site-packages/django/http/request.py:440: in __delitem__
self._assert_mutable()
env/lib/python2.7/site-packages/django/http/request.py:431: in _assert_mutable
raise AttributeError("This QueryDict instance is immutable")
E WrappedAttributeError: This QueryDict instance is immutable
Which makes me think that the request data shouldn't be touched (in the outside of the middleware?)
So I guess the simplest thing to do is to treat it as @daviddavis suggests: ignore
Updated by daviddavis over 6 years ago
+1 to just ignoring the param in validate_unknown_fields.
Updated by milan over 6 years ago
- Status changed from ASSIGNED to POST
Updated by milan over 6 years ago
I've done some more investigation and I guess this is actually some DRF bug; see this call stack from my PDB session:
/home/vagrant/devel/django/django/core/handlers/base.py(124)_get_response()
-> response = wrapped_callback(request, *callback_args, **callback_kwargs)
/home/vagrant/devel/django/django/views/decorators/csrf.py(54)wrapped_view()
-> return view_func(*args, **kwargs)
> /home/vagrant/.virtualenvs/pulp/lib64/python3.6/site-packages/rest_framework/viewsets.py(82)view()
....
> /home/vagrant/.virtualenvs/pulp/lib64/python3.6/site-packages/rest_framework/viewsets.py(103)view()-><Response sta...harset=utf-8">
-> return self.dispatch(request, *args, **kwargs)
(Pdb) list
98 self.request = request
99 self.args = args
100 self.kwargs = kwargs
101
102 # And continue as usual
103 -> return self.dispatch(request, *args, **kwargs)
104
105 # take name and docstring from class
106 update_wrapper(view, cls, updated=())
107
108 # and possible attributes set by decorators
(Pdb) next
--Return--
> /home/vagrant/devel/django/django/views/decorators/csrf.py(54)wrapped_view()-><Response sta...harset=utf-8">
-> return view_func(*args, **kwargs)
(Pdb) list
49 def csrf_exempt(view_func):
50 """Mark a view function as being exempt from the CSRF view protection."""
51 # view_func.csrf_exempt = True would also work, but decorators are nicer
52 # if they don't have side effects, so return a new function.
53 def wrapped_view(*args, **kwargs):
54 -> return view_func(*args, **kwargs)
55 wrapped_view.csrf_exempt = True
56 return wraps(view_func)(wrapped_view)
So it seems the csrf_exempt is actually always applied to our views.
Added by milan over 6 years ago
Added by milan over 6 years ago
Revision 7ce37c36 | View on GitHub
Serializer ignore the csrfmiddlewaretoken field
The CSRF protection middleware kicks-in when a browser is used to access
Pulp API, after the user having successfully logged into Pulp, i.e
whenever a Session-based authentication is used by the DRF/Django
The middleware uses a custom (and invisible) form field:
csrfmiddlewaretoken that the page in the browser populates from a CSRF
cookie previously generated by Django whenever issuing a
POST/PATCH/DELETE... request to prevent the cross-site-request-forgery.
The whole mechanism is nicely described on wikipedia.
This csrfmiddlewaretoken field unfortunately isn't stripped from the
POSTed data by either DRF or Django, and causes our serializers to fail
validate form data because of it being an unknown field to our models.
This patch just makes our validation ignore the unknow
csrfmiddlewaretoken field.
Updated by milan over 6 years ago
- Status changed from POST to MODIFIED
Applied in changeset pulp|7ce37c36aae8a53a17870c09612cf0da424424f1.
Updated by bmbouter almost 5 years ago
- Status changed from MODIFIED to CLOSED - CURRENTRELEASE
.
Serializer ignore the csrfmiddlewaretoken field
The CSRF protection middleware kicks-in when a browser is used to access Pulp API, after the user having successfully logged into Pulp, i.e whenever a Session-based authentication is used by the DRF/Django The middleware uses a custom (and invisible) form field:
csrfmiddlewaretokenthat the page in the browser populates from a CSRF cookie previously generated by Django whenever issuing a POST/PATCH/DELETE... request to prevent the cross-site-request-forgery. The whole mechanism is nicely described on wikipedia.This
csrfmiddlewaretokenfield unfortunately isn't stripped from the POSTed data by either DRF or Django, and causes our serializers to fail validate form data because of it being an unknown field to our models.This patch just makes our validation ignore the unknow
csrfmiddlewaretokenfield.Fixes: #3906 https://pulp.plan.io/issues/3906