ensure_csrf_cookie decorator issues a "CSRF token missing or incorrect" warning.

[wrr@…]

I'm using ensure_csrf_cookie decorator to set CSRF protection token with a POST request. The decorator works correctly but it prints incorrect and confusing warning:

WARNING django.request Forbidden (CSRF token missing or incorrect.): /auth/api/csrftoken/

The warning for sure comes from the decorator, because the application does not use CsrfViewMiddleware. I briefly examined django/views/decorators/csrf.py and django/middleware/csrf.py and it seems that indeed such warning is printed when post method is decorated.

Relevant part of the code that produces warning:

from django.core.context_processors import csrf
from django.utils.decorators import method_decorator
from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.generic import View

class CsrfToken(View):
    """Establishes Cross Site Request Forgery protection token."""

    @method_decorator(ensure_csrf_cookie)
    def post(self, request):
        """Returns CSRF protection token in a cookie and a response body."""
        csrf_token = csrf(request).values()[0]
        return http.HttpResponseOKJson({'csrfToken': str(csrf_token)})

[Aymeric Augustin]

This string exists only once in the entire codebase, in django/middleware/csrf.py:

REASON_BAD_TOKEN = "CSRF token missing or incorrect."

REASON_BAD_TOKEN is used only once, in CsrfViewMiddleware.process_view.

I'm not sure how you determined that "the application does not use CsrfViewMiddleware"; as far as I can tell, it does.

[anonymous]

What I meant was that the warning for sure comes from ensure_csrf_cookie because the application does not use CsrfViewMiddleware directly. ensure_csrf_cookie internally uses CsrfViewMiddleware and the warning is indeed printed by this class, but this is ensure_csrf_cookie implementation detail.

[Russell Keith-Magee]

I can validate that the report is accurate -- a POST request to a view protected by ensure_csrf_cookie raises a 403.

I suppose the bigger question is whether this is expected or not. I'm having difficulty with understanding why you would want to ensure that there is CSRF cookie, but not actually use it for a POST. The ensure_csrf_cookie decorator was introduced as a fix for #15354. The intention appears to be to ensure that the cookie has been set on a GET request, so that subsequent POST requests will have the cookie in place.

Marking as DDN so we can sort out what the right behavior is here.

[wrr@…]

Actually, if you don't enable CsrfViewMiddleware, the decorator does not raise 403, it prints 'WARNING django.request Forbidden (CSRF token missing or incorrect.)' but always accepts the request (as it should). This is because the decorator overwrites _reject():

class _EnsureCsrfCookie(CsrfViewMiddleware):
    def _reject(self, request, reason):
        return None

I have a single view that establishes CSRF token with a POST request and does nothing more. And several views that require and validate the token (for severals reasons I do not use CsrfViewMiddleware to do the validation). I'm using POST to establish the token as an extra precaution to make sure the token does not leak to a different origin site.

Anyway, I'm not convinced this requires design discussion. It seems like an obvious bug in the implementation. The printed warning is incorrect, request is not forbidden, 403 is not returned. The decorator, according to the documentation, should only set a cookie, not perform any validation. It seems like the warning is a result of a shortcut in the decorator implementation that reuses CsrfViewMiddleware but does not disable validation related warnings, that are not relevant for the decorator.

[Luke Plant]

I agree with wrr - this is a clear (low priority) bug in the implementation. It also looks like the fix is trivial - move all the logger calls into the _reject() method. AFAICS from a brief review, this is straightforward and will reduce duplication and line count as well.

[Olivier Sels]

Patch added. Does this need tests?

[Aymeric Augustin]

As long as you haven't provided a convincing explanation of why it wouldn't need tests, it does :)

[Aymeric Augustin]

The patch changes the behavior of _EnsureCsrfToken; it no longer logs warnings. Is this change intended?

[Aymeric Augustin]

Oh -- that's actually the intended change. I got confused when you said it only shuffled things around on IRC.

I would like a test proving that ensure_csrf_cookie no longer logs warning messages. If for some reason such a test is impossible to write, let's talk again.

[Olivier Sels]

I added the test and opened a pull request at ​https://github.com/django/django/pull/1110.
It fails in current master and passes with the patch.

[Aymeric Augustin]

Very good, I'm going to merge this (after tweaking the test slightly).

[Aymeric Augustin <aymeric.augustin@…>]

In 63a9555d57069cee32de388821dbe580da1f97c0:

Fixed #19436 -- Don't log warnings in ensure_csrf_cookie.