Anonymous CSRF cookie fix, test fixes, and ANON_ALWAYS cleanup #11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.