Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #15808 -- Added optional HttpOnly flag to the CSRF Cookie.

Thanks Samuel Lavitt for the report and Sascha Peilicke for the patch.
  • Loading branch information...
commit 720888a14699a80a6cd07d32514b9dcd5b1005fb 1 parent 6afc85a
@aaugustin aaugustin authored
View
1  django/conf/global_settings.py
@@ -529,6 +529,7 @@
CSRF_COOKIE_DOMAIN = None
CSRF_COOKIE_PATH = '/'
CSRF_COOKIE_SECURE = False
+CSRF_COOKIE_HTTPONLY = False
############
# MESSAGES #
View
3  django/middleware/csrf.py
@@ -210,7 +210,8 @@ def process_response(self, request, response):
max_age = 60 * 60 * 24 * 7 * 52,
domain=settings.CSRF_COOKIE_DOMAIN,
path=settings.CSRF_COOKIE_PATH,
- secure=settings.CSRF_COOKIE_SECURE
+ secure=settings.CSRF_COOKIE_SECURE,
+ httponly=settings.CSRF_COOKIE_HTTPONLY
)
# Content varies with the CSRF cookie, so set the Vary header.
patch_vary_headers(response, ('Cookie',))
View
1  docs/ref/contrib/csrf.txt
@@ -491,6 +491,7 @@ Settings
A number of settings can be used to control Django's CSRF behavior:
* :setting:`CSRF_COOKIE_DOMAIN`
+* :setting:`CSRF_COOKIE_HTTPONLY`
* :setting:`CSRF_COOKIE_NAME`
* :setting:`CSRF_COOKIE_PATH`
* :setting:`CSRF_COOKIE_SECURE`
View
13 docs/ref/settings.txt
@@ -281,6 +281,19 @@ Please note that the presence of this setting does not imply that Django's CSRF
protection is safe from cross-subdomain attacks by default - please see the
:ref:`CSRF limitations <csrf-limitations>` section.
+.. setting:: CSRF_COOKIE_HTTPONLY
+
+CSRF_COOKIE_HTTPONLY
+--------------------
+
+.. versionadded:: 1.6
+
+Default: ``False``
+
+Whether to use HttpOnly flag on the CSRF cookie. If this is set to ``True``,
+client-side JavaScript will not to be able to access the CSRF cookie. See
+:setting:`SESSION_COOKIE_HTTPONLY` for details on HttpOnly.
+
.. setting:: CSRF_COOKIE_NAME
CSRF_COOKIE_NAME
View
3  docs/releases/1.6.txt
@@ -36,6 +36,9 @@ Minor features
* Authentication backends can raise ``PermissionDenied`` to immediately fail
the authentication chain.
+* The HttpOnly flag can be set on the CSRF cookie with
+ :setting:`CSRF_COOKIE_HTTPONLY`.
+
* The ``assertQuerysetEqual()`` now checks for undefined order and raises
``ValueError`` if undefined order is spotted. The order is seen as
undefined if the given ``QuerySet`` isn't ordered and there are more than
View
4 tests/regressiontests/csrf_tests/tests.py
@@ -101,7 +101,8 @@ def test_process_response_get_token_used(self):
with self.settings(CSRF_COOKIE_NAME='myname',
CSRF_COOKIE_DOMAIN='.example.com',
CSRF_COOKIE_PATH='/test/',
- CSRF_COOKIE_SECURE=True):
+ CSRF_COOKIE_SECURE=True,
+ CSRF_COOKIE_HTTPONLY=True):
# token_view calls get_token() indirectly
CsrfViewMiddleware().process_view(req, token_view, (), {})
resp = token_view(req)
@@ -110,6 +111,7 @@ def test_process_response_get_token_used(self):
self.assertNotEqual(csrf_cookie, False)
self.assertEqual(csrf_cookie['domain'], '.example.com')
self.assertEqual(csrf_cookie['secure'], True)
+ self.assertEqual(csrf_cookie['httponly'], True)
self.assertEqual(csrf_cookie['path'], '/test/')
self.assertTrue('Cookie' in resp2.get('Vary',''))

4 comments on commit 720888a

@saschpe

Maybe I'm missing something but why did you not simply merge the pull request instead of squashing and commiting it under your name? That's just not how git works but maybe that helps to keep the AUTHORS file small....

@mjtamlyn
Collaborator

It's nothing to do with AUTHORS - as you wrote this patch you should be able to add yourself to AUTHORS if you wish. I believe it's an effort to keep the history cleaner - it helps to see the complete accepted solution to a ticket in one go.

@saschpe

Agreed on the AUTHORS part but the rest doesn't make sense. Acceping the pull is "one go". Either way, you still have to close the ticket in the other (i.e. not github) tool. If a single commit would have been requested, I could have squashed it myself.

@aaugustin
Owner

There are several differences between the PR and what I committed. One (the wrong versionadded directive) had been pointed out in comments. Others (the missing entry in the changelog, settings not in alphabetical order in the docs, wording in the settings, etc.) hadn't. At some point, it's faster to polish the patch myself than to walk you through every change — all the more since I usually change my mind 10 times in the process. That makes the difference between "the ticket gets fixed" and "the patch languishes until it's outdated".

I'm missing your point about "how git works": git is just a toolkit and each project can use it in whatever way is most appropriate. I applied the diff, fine-tuned it, and credited you in the commit message. We've always done things this way for Django. We're using PRs as "patches on steroids" or "diff viewers with inline commenting".

To tell the truth — I don't care about this feature at all. There's only one reason why I did this commit: I felt bad that your previous PR had gone stale and I thought you'd like seeing the fix in Django. I'm really sorry it offended you.

That said, in the future, I'm not willing to play the "merge-as-is or close" game. It isn't an efficient use of my time. If you prefer that your PRs not be merged rather than be merged with edits, say it, because that isn't our expectation.

I hope you understand!

Please sign in to comment.
Something went wrong with that request. Please try again.