-
Notifications
You must be signed in to change notification settings - Fork 25
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
Anonymous CSRF cookie fix, test fixes, and ANON_ALWAYS cleanup #11
base: master
Are you sure you want to change the base?
Anonymous CSRF cookie fix, test fixes, and ANON_ALWAYS cleanup #11
Conversation
ensure that / is a simple view that doesn't have side effects that break the tests. session-csrf's tests should be independent of the Django project it's included in.
This fixes an issue with IE 7 and 8 not accepting the cookie if the client's date is set wrong. Session cookies (cookies that have no set expiry date and therefore expire at the end of the browser session) are already used by Django for storing the SESSION_COOKIE (sessionid by default).
This requires moving the common code into process_view so we can tell if the view has been decorated.
This caused test_anon_token_from_cookie to pass when it shouldn't
The view needs to be decorated with anonymous_csrf before calling process_view.
It turns out runtests.sh was using my global settings file, not the local version, so ANON_ALWAYS was True which cause the test to pass. I have fixed this issue by unsetting ANON_ALWAYS in setUp and fixed the failing test. I'll look into why runtests.sh was using the wrong settings file later. I have confirmed that it's not a result of my changes (it still happens with an unmodified version of session_csrf). |
@fwenzel Can you or someone you know look at this? I'm failing as a maintainer. |
@Osmose is this something you can take a look at? Thanks! |
I'm a bit swamped right now. Let's play r? hot potato! @pmclanahan! |
Code looks clean. Tests pass. r+ as far as I'm concerned. DISCLAIMER: Have not used this in practice. |
These commits fix an issue with the anonymous CSRF cookie affecting old versions of IE, fix some tests (including fixes needed by Django 1.4), and clean up the ANON_ALWAYS code I wrote 6 months ago by merging its functionality with @anonymous_csrf.