allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hei...@apache.org
Subject [02/46] allura git commit: [#7893] stop CSRF login requests; add tests for CSRF
Date Mon, 15 Jun 2015 12:27:21 GMT
[#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 <dbrondsema@slashdotmedia.com>
Authored: Mon Jun 8 20:31:39 2015 +0000
Committer: Dave Brondsema <dbrondsema@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)
+


Mime
View raw message