Skip to content
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.

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

added missing {% csrf_token %} to some forms #3854

Closed
wants to merge 1 commit into from

Conversation

atombrella
Copy link
Contributor

Another pull request (#3703) pointed this out.

@atombrella
Copy link
Contributor Author

No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.

The above error is generated for some of the environments, but most of the builds are passing. Can somebody trigger a rebuild?

@xordoquy
Copy link
Collaborator

@atombrella I restarted the travis build.

@xordoquy
Copy link
Collaborator

Strange for the base template as it's used successfully for the browsable API.

@atombrella
Copy link
Contributor Author

To be honest, I inserted the tokens thinking it was weird that the forms didn't have them, and haven't worked with a setup of DRF.

@tomchristie
Copy link
Member

The CSRF tokens should be being sent in the AJAX request headers. Happy to reconsider this if someone verifies that there's an actual issue here.

@matteosimone
Copy link

@tomchristie

I am running into this problem as well:

If you enable CSRF_COOKIE_HTTPONLY, javascript does not have access to the CSRF token, and the csrfmiddlewaretoken is also missing from the form, so it fails verification.

I created a simple django app to demonstrate: https://github.com/matteosimone/drf-testing. The only modifications made to this app are adding CSRF_COOKIE_HTTP_ONLY and enforcing authentication for rest framework.

Clone and go into the testing directory.

python manage.py migrate
python manage.py createsuperuser
python manage.py runserver

Go into http://localhost:8000/users/, log in, click on "Raw Data Form", and then click "Submit". It will say CSRF Verification failed.

@egbertvanderhout
Copy link

egbertvanderhout commented Oct 27, 2016

@tomchristie

Somehow this is an issue.

According to https://docs.djangoproject.com/en/dev/ref/settings/#std:setting-CSRF_COOKIE_HTTPONLY you shouldn't get the CSRF token from a cookie but from a hidden form input: "If you enable this and need to send the value of the CSRF token with Ajax requests, your JavaScript will need to pull the value from a hidden CSRF token form input on the page instead of from the cookie.".

Combined with the following advise from manage.py check --deploy - (security.W017) You have 'django.middleware.csrf.CsrfViewMiddleware' in your MIDDLEWARE, but you have not set CSRF_COOKIE_HTTPONLY to True. Using an HttpOnly CSRF cookie makes it more difficult for cross-site scripting attacks to steal the CSRF token. - we have an issue.

What works - but is not tested to be secure, I'm no expert on this - is:

base.html

  <script>
    window.drf = {
      csrfHeaderName: "{{ csrf_header_name|default:'X-CSRFToken' }}",
      csrfCookieName: "{{ csrf_cookie_name|default:'csrftoken' }}",
      csrf: "{{ csrf_token }}" // <-- added this line
    };
  </script>

csrf.js

var csrftoken = getCookie(window.drf.csrfCookieName) || window.drf.csrf;

This solves the problem. I haven't made a PR because I'm not sure this is a valid, secure solution.

See also:

@egbertvanderhout
Copy link

egbertvanderhout commented Oct 27, 2016

Workaround while this isn't fixed:

Create a local template ([your-project-template-root]/rest_framework/api.html) and add this code:

{% extends "rest_framework/base.html" %}

{% block script %}
    {{ block.super }}
    <script>
        var csrftoken = "{{ csrf_token }}";
    </script>
{% endblock %}

@tomchristie
Copy link
Member

The docs for CSRF_COOKIE_HTTPONLY state that.

If this is set to True, client-side JavaScript will not to be able to access the CSRF cookie.

Don't set CSRF_COOKIE_HTTPONLY = True if you want to be able to use the browsable API. We won't be able to make that work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants