You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2017/07/17 17:19:39 UTC

allura git commit: [#8158] Adds antispam measures to Login page

Repository: allura
Updated Branches:
  refs/heads/master 3470ed196 -> 0c76ca6b6


[#8158] Adds antispam measures to Login page


Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/0c76ca6b
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/0c76ca6b
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/0c76ca6b

Branch: refs/heads/master
Commit: 0c76ca6b6a0fc75039615dec4a82d8fc396b08da
Parents: 3470ed1
Author: Kenton Taylor <kt...@slashdotmedia.com>
Authored: Thu Jul 13 19:08:03 2017 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Mon Jul 17 17:05:26 2017 +0000

----------------------------------------------------------------------
 Allura/allura/controllers/auth.py               |  2 +
 Allura/allura/lib/utils.py                      | 11 ++-
 Allura/allura/lib/widgets/auth_widgets.py       | 10 ++-
 Allura/allura/templates/login_fragment.html     |  3 +
 .../templates/phone_verification_fragment.html  |  3 +
 Allura/allura/tests/functional/test_auth.py     | 90 +++++++++++++-------
 Allura/allura/tests/functional/test_rest.py     |  5 +-
 AlluraTest/alluratest/validation.py             | 21 ++++-
 .../forgeuserstats/tests/test_stats.py          |  4 +-
 9 files changed, 106 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/Allura/allura/controllers/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index 47246ae..81ef645 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -51,6 +51,7 @@ from allura.lib.widgets import (
 from allura.lib.widgets import forms, form_fields as ffw
 from allura.lib import mail_util
 from allura.lib.multifactor import TotpService, RecoveryCodeService
+from allura.lib import utils
 from allura.controllers import BaseController
 from allura.tasks.mail_tasks import send_system_mail_to_user
 
@@ -320,6 +321,7 @@ class AuthController(BaseController):
     @expose()
     @require_post()
     @validate(F.login_form, error_handler=index)
+    @utils.AntiSpam.validate('Spambot protection engaged')
     def do_login(self, return_to=None, **kw):
         location = '/'
 

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/Allura/allura/lib/utils.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py
index e68e287..0b740d8 100644
--- a/Allura/allura/lib/utils.py
+++ b/Allura/allura/lib/utils.py
@@ -247,12 +247,12 @@ class AntiSpam(object):
         Please don't fill out this field.</label><br>
     <input id="$fld_id" name="$fld_name" type="text"><br></p>''')
 
-    def __init__(self, request=None, num_honey=2):
+    def __init__(self, request=None, num_honey=2, timestamp=None, spinner=None):
         self.num_honey = num_honey
         if request is None or request.method == 'GET':
             self.request = pylons.request
-            self.timestamp = int(time.time())
-            self.spinner = self.make_spinner()
+            self.timestamp = timestamp if timestamp else int(time.time())
+            self.spinner = spinner if spinner else self.make_spinner()
             self.timestamp_text = str(self.timestamp)
             self.spinner_text = self._wrap(self.spinner)
         else:
@@ -382,6 +382,10 @@ class AntiSpam(object):
             try:
                 new_params = cls.validate_request(params=params)
                 params.update(new_params)
+
+                if tg.request.POST:
+                    # request.params is immutable, but will reflect changes to request.POST
+                    tg.request.POST.update(new_params)
             except (ValueError, TypeError, binascii.Error):
                 testing = pylons.request.environ.get('paste.testing', False)
                 if testing:
@@ -389,6 +393,7 @@ class AntiSpam(object):
                     raise
                 else:
                     # regular antispam failure handling
+                    tg.flash(error_msg, 'error')
                     raise Invalid(error_msg, params, None)
         return before_validate(antispam_hook)
 

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/Allura/allura/lib/widgets/auth_widgets.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/widgets/auth_widgets.py b/Allura/allura/lib/widgets/auth_widgets.py
index f2ba4e7..7bac270 100644
--- a/Allura/allura/lib/widgets/auth_widgets.py
+++ b/Allura/allura/lib/widgets/auth_widgets.py
@@ -24,6 +24,7 @@ from formencode import Invalid
 from webob import exc
 
 from .forms import ForgeForm
+from pylons import tmpl_context as c, app_globals as g
 
 from allura.lib import plugin
 from allura import model as M
@@ -36,10 +37,10 @@ class LoginForm(ForgeForm):
     @property
     def fields(self):
         fields = [
-            ew.TextField(name='username', label='Username', attrs={
+            ew.TextField(name=g.antispam.enc('username'), label='Username', attrs={
                 'autofocus': 'autofocus',
             }),
-            ew.PasswordField(name='password', label='Password'),
+            ew.PasswordField(name=g.antispam.enc('password'), label='Password'),
             ew.Checkbox(
                 name='rememberme',
                 label='Remember Me',
@@ -53,6 +54,11 @@ class LoginForm(ForgeForm):
                     name='link',
                     text='<a href="/auth/forgotten_password" style="margin-left:162px" target="_top">'
                          'Forgot password?</a>'))
+
+        for fld in g.antispam.extra_fields():
+            fields.append(
+                ew_core.Widget(template=ew.Snippet(fld)))
+
         return fields
 
     @validator

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/Allura/allura/templates/login_fragment.html
----------------------------------------------------------------------
diff --git a/Allura/allura/templates/login_fragment.html b/Allura/allura/templates/login_fragment.html
index 31fb26d..e49bb91 100644
--- a/Allura/allura/templates/login_fragment.html
+++ b/Allura/allura/templates/login_fragment.html
@@ -48,6 +48,9 @@
                 padding-top: 1em;
                 width: 1000px;
             }
+            .{{ g.antispam.honey_class }} {
+                display: none
+            }
         </style>
     </head>
     <body>

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/Allura/allura/templates/phone_verification_fragment.html
----------------------------------------------------------------------
diff --git a/Allura/allura/templates/phone_verification_fragment.html b/Allura/allura/templates/phone_verification_fragment.html
index 458f8ba..2651958 100644
--- a/Allura/allura/templates/phone_verification_fragment.html
+++ b/Allura/allura/templates/phone_verification_fragment.html
@@ -52,6 +52,9 @@
               margin-bottom: .5em;
             }
             .step-message { display: none; }
+            .{{ g.antispam.honey_class }} {
+                display: none
+            }
         </style>
     </head>
     <body>

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/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 1e0a086..64b4da5 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -40,8 +40,9 @@ from nose.tools import (
     assert_not_in,
     assert_true,
     assert_false,
+    assert_raises
 )
-from pylons import tmpl_context as c
+from pylons import tmpl_context as c, app_globals as g
 from webob import exc
 
 from allura.tests import TestController
@@ -75,16 +76,22 @@ class TestAuth(TestController):
         with audits('Successful login', user=True):
             r = self.app.post('/auth/do_login', params=dict(
                 username='test-user', password='foo',
-                _session_id=self.app.cookies['_session_id']))
+                _session_id=self.app.cookies['_session_id']),
+                antispam=True)
             assert_equal(r.headers['Location'], 'http://localhost/')
 
+        with assert_raises(ValueError) as ex:
+            r = self.app.post('/auth/do_login', antispam=True, params=dict(
+                username='test-user', password='foo', honey1='robot',
+                _session_id=self.app.cookies['_session_id']))
+
         with audits('Failed login', user=True):
-            r = self.app.post('/auth/do_login', params=dict(
+            r = self.app.post('/auth/do_login', antispam=True, params=dict(
                 username='test-user', password='food',
                 _session_id=self.app.cookies['_session_id']))
             assert 'Invalid login' in str(r), r.showbrowser()
 
-        r = self.app.post('/auth/do_login', params=dict(
+        r = self.app.post('/auth/do_login', antispam=True, params=dict(
             username='test-usera', password='foo',
             _session_id=self.app.cookies['_session_id']))
         assert 'Invalid login' in str(r), r.showbrowser()
@@ -93,10 +100,13 @@ class TestAuth(TestController):
         self.app.extra_environ = {'disable_auth_magic': 'True'}
         nav_pattern = ('nav', {'class': 'nav-main'})
         r = self.app.get('/auth/')
-        f = r.forms[0]
-        f['username'] = 'test-user'
-        f['password'] = 'foo'
-        r = f.submit().follow()
+
+        r = self.app.post('/auth/do_login', params=dict(
+            username='test-user', password='foo',
+            _session_id=self.app.cookies['_session_id']),
+            extra_environ={'REMOTE_ADDR': '127.0.0.1'},
+            antispam=True).follow()
+
         logged_in_session = r.session['_id']
         links = r.html.find(*nav_pattern).findAll('a')
         assert_equal(links[-1].string, "Log Out")
@@ -116,15 +126,17 @@ class TestAuth(TestController):
         self.app.get('/')  # establish session
         self.app.post('/auth/do_login',
                       headers={'User-Agent': 'browser'},
-                      extra_environ={'REMOTE_ADDR': 'addr'},
+                      extra_environ={'REMOTE_ADDR': '127.0.0.1'},
                       params=dict(
                           username='test-user',
                           password='foo',
                           _session_id=self.app.cookies['_session_id'],
-                      ))
+                      ),
+                      antispam=True,
+                      )
         user = M.User.by_username('test-user')
         assert_not_equal(user.last_access['login_date'], None)
-        assert_equal(user.last_access['login_ip'], 'addr')
+        assert_equal(user.last_access['login_ip'], '127.0.0.1')
         assert_equal(user.last_access['login_ua'], 'browser')
 
     def test_rememberme(self):
@@ -136,7 +148,7 @@ class TestAuth(TestController):
         r = self.app.post('/auth/do_login', params=dict(
             username='test-user', password='foo',
             _session_id=self.app.cookies['_session_id'],
-        ))
+        ), antispam=True)
         assert_equal(r.session['username'], username)
         assert_equal(r.session['login_expires'], True)
 
@@ -148,7 +160,7 @@ class TestAuth(TestController):
         r = self.app.post('/auth/do_login', params=dict(
             username='test-user', password='foo', rememberme='on',
             _session_id=self.app.cookies['_session_id'],
-        ))
+        ), antispam=True)
         assert_equal(r.session['username'], username)
         assert_not_equal(r.session['login_expires'], True)
 
@@ -752,7 +764,7 @@ class TestAuth(TestController):
         r = self.app.post(
             '/auth/do_login',
             params=dict(username='aaa', password='12345678',
-                        _session_id=self.app.cookies['_session_id']),
+                        _session_id=self.app.cookies['_session_id']), antispam=True,
             status=302)
 
     def test_create_account_require_email(self):
@@ -886,25 +898,27 @@ class TestAuth(TestController):
         r = self.app.post('/auth/do_login', params=dict(
             username='test-user', password='foo',
             return_to='/foo',
-            _session_id=self.app.cookies['_session_id']))
+            _session_id=self.app.cookies['_session_id']),
+            antispam=True
+        )
         assert_equal(r.location, 'http://localhost/foo')
 
         r = self.app.get('/auth/logout')
-        r = self.app.post('/auth/do_login', params=dict(
+        r = self.app.post('/auth/do_login', antispam=True, params=dict(
             username='test-user', password='foo',
             return_to='http://localhost:8080/foo',
             _session_id=self.app.cookies['_session_id']))
         assert_equal(r.location, 'http://localhost:8080/foo')
 
         r = self.app.get('/auth/logout')
-        r = self.app.post('/auth/do_login', params=dict(
+        r = self.app.post('/auth/do_login', antispam=True, params=dict(
             username='test-user', password='foo',
             return_to='http://example.com/foo',
             _session_id=self.app.cookies['_session_id']))
         assert_equal(r.location, 'http://localhost/')
 
         r = self.app.get('/auth/logout')
-        r = self.app.post('/auth/do_login', params=dict(
+        r = self.app.post('/auth/do_login', antispam=True, params=dict(
             username='test-user', password='foo',
             return_to='//example.com/foo',
             _session_id=self.app.cookies['_session_id']))
@@ -1812,10 +1826,13 @@ class TestDisableAccount(TestController):
 
 class TestPasswordExpire(TestController):
     def login(self, username='test-user', pwd='foo', query_string=''):
-        r = self.app.get('/auth/' + query_string, extra_environ={'username': '*anonymous'})
+        extra = extra_environ={'username': '*anonymous', 'REMOTE_ADDR':'127.0.0.1'}
+        r = self.app.get('/auth/' + query_string, extra_environ=extra)
+
         f = r.forms[0]
-        f['username'] = username
-        f['password'] = pwd
+        encoded = self.app.antispam_field_names(f)
+        f[encoded['username']] = username
+        f[encoded['password']] = pwd
         return f.submit(extra_environ={'username': '*anonymous'})
 
     def assert_redirects(self, where='/'):
@@ -2007,13 +2024,15 @@ class TestPasswordExpire(TestController):
 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'}
+        self.app.extra_environ = {'disable_auth_magic': 'True', 'REMOTE_ADDR': '127.0.0.1'}
 
         # regular login
         r = self.app.get('/auth/')
-        r.form['username'] = 'test-admin'
-        r.form['password'] = 'foo'
-        r.form.submit()
+
+        r = self.app.post('/auth/do_login', params=dict(
+            username='test-admin', password='foo',
+            _session_id=self.app.cookies['_session_id']),
+            antispam=True)
 
         # regular form submit
         r = self.app.get('/admin/overview')
@@ -2233,8 +2252,9 @@ class TestTwoFactor(TestController):
 
         # regular login
         r = self.app.get('/auth/?return_to=/p/foo')
-        r.form['username'] = 'test-admin'
-        r.form['password'] = 'foo'
+        encoded = self.app.antispam_field_names(r.form)
+        r.form[encoded['username']] = 'test-admin'
+        r.form[encoded['password']] = 'foo'
         with audits('Multifactor login - password ok, code not entered yet', user=True):
             r = r.form.submit()
 
@@ -2269,8 +2289,10 @@ class TestTwoFactor(TestController):
 
         # regular login
         r = self.app.get('/auth/?return_to=/p/foo')
-        r.form['username'] = 'test-admin'
-        r.form['password'] = 'foo'
+        encoded = self.app.antispam_field_names(r.form)
+
+        r.form[encoded['username']] = 'test-admin'
+        r.form[encoded['password']] = 'foo'
         r = r.form.submit()
         r = r.follow()
 
@@ -2298,8 +2320,9 @@ class TestTwoFactor(TestController):
 
         # regular login
         r = self.app.get('/auth/')
-        r.form['username'] = 'test-admin'
-        r.form['password'] = 'foo'
+        encoded = self.app.antispam_field_names(r.form)
+        r.form[encoded['username']] = 'test-admin'
+        r.form[encoded['password']] = 'foo'
         r = r.form.submit()
         r = r.follow()
 
@@ -2326,8 +2349,9 @@ class TestTwoFactor(TestController):
 
         # regular login
         r = self.app.get('/auth/?return_to=/p/foo')
-        r.form['username'] = 'test-admin'
-        r.form['password'] = 'foo'
+        encoded = self.app.antispam_field_names(r.form)
+        r.form[encoded['username']] = 'test-admin'
+        r.form[encoded['password']] = 'foo'
         r = r.form.submit()
 
         # check results

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/Allura/allura/tests/functional/test_rest.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_rest.py b/Allura/allura/tests/functional/test_rest.py
index 91cf40f..6a54972 100644
--- a/Allura/allura/tests/functional/test_rest.py
+++ b/Allura/allura/tests/functional/test_rest.py
@@ -375,8 +375,9 @@ class TestRestHome(TestRestApiBase):
 
         # regular login to get a session cookie set up
         r = self.app.get('/auth/')
-        r.form['username'] = 'test-admin'
-        r.form['password'] = 'foo'
+        encoded = self.app.antispam_field_names(r.form)
+        r.form[encoded['username']] = 'test-admin'
+        r.form[encoded['password']] = 'foo'
         r.form.submit()
 
         # simulate CORS ajax request withCredentials (cookie headers)

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/AlluraTest/alluratest/validation.py
----------------------------------------------------------------------
diff --git a/AlluraTest/alluratest/validation.py b/AlluraTest/alluratest/validation.py
index fba20af..23ac398 100644
--- a/AlluraTest/alluratest/validation.py
+++ b/AlluraTest/alluratest/validation.py
@@ -214,8 +214,8 @@ def validate_page(html_or_response):
 class AntiSpamTestApp(TestApp):
 
     def post(self, *args, **kwargs):
+        antispam = utils.AntiSpam()
         if kwargs.pop('antispam', False):
-            antispam = utils.AntiSpam()
             params = {
                 'timestamp': antispam.timestamp_text,
                 'spinner': antispam.spinner_text,
@@ -224,9 +224,28 @@ class AntiSpamTestApp(TestApp):
             }
             for k, v in kwargs['params'].iteritems():
                 params[antispam.enc(k)] = v
+            params['_session_id'] = kwargs['params'].get('_session_id')  # exclude csrf token from encryption
             kwargs['params'] = params
         return super(AntiSpamTestApp, self).post(*args, **kwargs)
 
+    def antispam_field_names(self, form):
+        """
+        :param form: a WebTest form (i.e. from a self.app.get response)
+        :return: a dict of field names -> antispam encoded field names
+        """
+        timestamp = form['timestamp'].value
+        spinner = form['spinner'].value
+        antispam = utils.AntiSpam(timestamp=int(timestamp), spinner=utils.AntiSpam._unwrap(spinner))
+        names = form.fields.keys()
+        name_mapping = {}
+        for name in names:
+            try:
+                decoded = antispam.dec(name)
+            except Exception:
+                decoded = name
+            name_mapping[decoded] = name
+        return name_mapping
+
 
 class PostParamCheckingTestApp(AntiSpamTestApp):
 

http://git-wip-us.apache.org/repos/asf/allura/blob/0c76ca6b/ForgeUserStats/forgeuserstats/tests/test_stats.py
----------------------------------------------------------------------
diff --git a/ForgeUserStats/forgeuserstats/tests/test_stats.py b/ForgeUserStats/forgeuserstats/tests/test_stats.py
index ff76087..173f9b9 100644
--- a/ForgeUserStats/forgeuserstats/tests/test_stats.py
+++ b/ForgeUserStats/forgeuserstats/tests/test_stats.py
@@ -40,9 +40,9 @@ class TestStats(TestController):
         user = User.by_username('test-user')
         init_logins = user.stats.tot_logins_count
         self.app.get('/')  # establish session
-        self.app.post('/auth/do_login', params=dict(
+        self.app.post('/auth/do_login', antispam=True, params=dict(
             username=user.username, password='foo',
-            _session_id=self.app.cookies['_session_id']
+            _session_id=self.app.cookies['_session_id'],
         ))
 
         assert user.stats.tot_logins_count == 1 + init_logins