Skip to content
This repository has been archived by the owner on Jan 29, 2019. It is now read-only.

CSRF token fetching does not work with Django 1.8 #282

Closed
abompard opened this issue Apr 4, 2015 · 4 comments
Closed

CSRF token fetching does not work with Django 1.8 #282

abompard opened this issue Apr 4, 2015 · 4 comments

Comments

@abompard
Copy link
Contributor

abompard commented Apr 4, 2015

The newly released Django 1.8 changed the way context processors are run in the RequestContext object. They were previously run on instanciation, they are now run when the RequestContext is bound to a template. As a result, the CsrfToken view fails to get the token and login does not work.
I see 3 ways of solving this problem, I have implemented all of them and all tests pass:

  • Use an extremely simple template to get the token as we used to: hyperkitty@090e7c7. There's only one issue I can see with this: Django 1.8 allows plugging other template engines, and the template syntax may not match if a completely different engine is chosen (Jinja would work too).
  • Get the token from the cookie: as recommended by the django documentation since version 1.4 (at least), AJAX calls should get the CSRF token from the cookies. Here's a commit that uses the cookie if there's a token there, and falls back to the backend: hyperkitty@626b32b
  • But since the documentation recommends using the cookie and certifies that the token will be there if CSRF protection is enabled, we can simply drop the CsrfToken view. This commit does exactly that: hyperkitty@fde6f86. It's a little more invasive but all tests pass, and it also means less code to maintain ;-)

I have tested all three options on my local python2.7-django1.8 install, and they all work. I would personally go with the third, but it's up to you in the end. I'll create a pull request for the one you choose.

@Osmose
Copy link
Contributor

Osmose commented Apr 6, 2015

First, thanks for the report / fixes!

The reason why we didn't do the second or third option before is that a lot of the sites using django-browserid (mainly the Mozilla ones since they're all on *.mozilla.org) also use django-session-csrf. Since django-session-csrf stores the token in the session, we can't rely on the token being in the cookie.

This first option seems doable, although the issue with multiple template engines is an annoying one. I'm not super-excited about it, but could we alter the current view to bind a dummy Template instance before fetching the value? That way we could just generate an empty template instance from whatever the default template engine is and avoid worrying about which engine they're using.

In a semi-related note, does 1.8's swappable engines mean our current templates might be parsed by the wrong engine? I did some searching but couldn't find any useful info on how libraries specify what template engine they want to use.

@Osmose
Copy link
Contributor

Osmose commented Apr 6, 2015

After some discussion with @carljm it seems out best bet for the latter question will be to document how to add an extra template engine definition specifically for django-browserid if you're using non-DTL templates on your site.

I filed #283 for that specific issue. In the case of this issue I think a dummy template from the default template engine would be better.

@abompard
Copy link
Contributor Author

abompard commented Apr 6, 2015

Gotcha, that's what the comment at the top of the method meant :-)
OK, I've committed a change to bind to a dummy template instead of rendering it, and submitted the pull request #284. Unfortunately I have to use a try-except block for Django 1.8+, I didn't see a way that would be API-compatible with Django 1.4 to 1.8.
I tested it on my local install with Django 1.6 to 1.8, but all tests pass in tox and they include Django 1.4 too.

@abompard abompard mentioned this issue Apr 6, 2015
@Osmose
Copy link
Contributor

Osmose commented Apr 30, 2015

This was fixed by #284.

@Osmose Osmose closed this as completed Apr 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants