You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by he...@apache.org on 2015/06/15 14:27:21 UTC
[02/46] allura git commit: [#7893] stop CSRF login requests;
add tests for CSRF
[#7893] stop CSRF login requests; add tests for CSRF
Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/10e1f51e
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/10e1f51e
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/10e1f51e
Branch: refs/heads/hs/7873
Commit: 10e1f51e1cd3c1ca31ea65a4ab0e1074a9ff92b7
Parents: 3ed25d5
Author: Dave Brondsema <db...@slashdotmedia.com>
Authored: Mon Jun 8 20:31:39 2015 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Mon Jun 8 22:05:59 2015 +0000
----------------------------------------------------------------------
Allura/allura/lib/custom_middleware.py | 15 +++++++++-
Allura/allura/lib/widgets/forms.py | 2 +-
Allura/allura/templates/jinja_master/lib.html | 2 +-
Allura/allura/tests/functional/test_auth.py | 34 ++++++++++++++++++++++
4 files changed, 50 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/allura/blob/10e1f51e/Allura/allura/lib/custom_middleware.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/custom_middleware.py b/Allura/allura/lib/custom_middleware.py
index 8212cd2..13c3db5 100644
--- a/Allura/allura/lib/custom_middleware.py
+++ b/Allura/allura/lib/custom_middleware.py
@@ -129,6 +129,8 @@ class CSRFMiddleware(object):
def __call__(self, environ, start_response):
req = Request(environ)
+
+ # enforce POSTs
cookie = req.cookies.get(self._cookie_name, None)
if cookie is None:
cookie = h.cryptographic_nonce()
@@ -136,7 +138,17 @@ class CSRFMiddleware(object):
param = req.str_POST.pop(self._param_name, None)
if cookie != param:
log.warning('CSRF attempt detected, %r != %r', cookie, param)
- environ.pop('HTTP_COOKIE', None)
+ environ.pop('HTTP_COOKIE', None) # effectively kill the existing session
+ if req.path.startswith('/auth/'):
+ # for operations where you're not logged in yet (e.g. login form, pwd recovery, etc), then killing
+ # the session doesn't help, so we block the request entirely
+ resp = exc.HTTPForbidden()
+ return resp(environ, start_response)
+
+ # Set cookie for use in later forms:
+
+ # in addition to setting a cookie, set this so its available on first response before cookie gets created in browser
+ environ[self._cookie_name] = cookie
def session_start_response(status, headers, exc_info=None):
if dict(headers).get('Content-Type', '').startswith('text/html'):
@@ -144,6 +156,7 @@ class CSRFMiddleware(object):
('Set-cookie',
str('%s=%s; Path=/' % (self._cookie_name, cookie))))
return start_response(status, headers, exc_info)
+
return self._app(environ, session_start_response)
http://git-wip-us.apache.org/repos/asf/allura/blob/10e1f51e/Allura/allura/lib/widgets/forms.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/forms.py b/Allura/allura/lib/widgets/forms.py
index 0cf8c18..714a32b 100644
--- a/Allura/allura/lib/widgets/forms.py
+++ b/Allura/allura/lib/widgets/forms.py
@@ -1090,7 +1090,7 @@ class CsrfForm(ew.SimpleForm):
def context_for(self, field):
ctx = super(CsrfForm, self).context_for(field)
if field.name == '_session_id':
- ctx['value'] = tg.request.cookies.get('_session_id')
+ ctx['value'] = tg.request.cookies.get('_session_id') or tg.request.environ['_session_id']
return ctx
http://git-wip-us.apache.org/repos/asf/allura/blob/10e1f51e/Allura/allura/templates/jinja_master/lib.html
----------------------------------------------------------------------
diff --git a/Allura/allura/templates/jinja_master/lib.html b/Allura/allura/templates/jinja_master/lib.html
index 78ab9c0..75acb9e 100644
--- a/Allura/allura/templates/jinja_master/lib.html
+++ b/Allura/allura/templates/jinja_master/lib.html
@@ -19,7 +19,7 @@
{% macro csrf() -%}
{% if request -%}
- {{request.cookies['_session_id']}}
+ {{ request.cookies['_session_id'] or request.environ['_session_id'] }}
{%- endif %}
{%- endmacro %}
http://git-wip-us.apache.org/repos/asf/allura/blob/10e1f51e/Allura/allura/tests/functional/test_auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py
index 0f9a8ee..20ceda3 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -1792,3 +1792,37 @@ class TestPasswordExpire(TestController):
f['return_to'] = return_to
r = f.submit(extra_environ={'username': 'test-user'}, status=302)
assert_equal(r.location, 'http://localhost/p/test/tickets/?milestone=1.0&page=2')
+
+
+class TestCSRFProtection(TestController):
+
+ def test_blocks_invalid(self):
+ # so test-admin isn't automatically logged in for all requests
+ self.app.extra_environ = {'disable_auth_magic': 'True'}
+
+ # regular login
+ r = self.app.get('/auth/')
+ r.form['username'] = 'test-admin'
+ r.form['password'] = 'foo'
+ r.form.submit()
+
+ # regular form submit
+ r = self.app.get('/admin/overview')
+ r = r.form.submit()
+ assert_equal(r.location, 'http://localhost/admin/overview')
+
+ # invalid form submit
+ r = self.app.get('/admin/overview')
+ r.form['_session_id'] = 'bogus'
+ r = r.form.submit()
+ assert_equal(r.location, 'http://localhost/auth/')
+
+ def test_blocks_invalid_on_login(self):
+ r = self.app.get('/auth/')
+ r.form['_session_id'] = 'bogus'
+ r.form.submit(status=403)
+
+ def test_token_present_on_first_request(self):
+ r = self.app.get('/auth/')
+ assert_true(r.form['_session_id'].value)
+