You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by je...@apache.org on 2014/09/18 09:29:43 UTC

[08/43] git commit: [#7674] Include IP address in user audit logs

[#7674] Include IP address in user audit logs

Extracted a new method so that we don't further clutter the model with
references to the current web request


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

Branch: refs/heads/je/42cc_7656
Commit: 505174a00b0de8303147dc24a41dfe9026b42ab3
Parents: 0855719
Author: Dave Brondsema <db...@slashdotmedia.com>
Authored: Fri Sep 12 19:50:41 2014 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Fri Sep 12 19:50:41 2014 +0000

----------------------------------------------------------------------
 Allura/allura/controllers/auth.py               | 18 ++++++------
 Allura/allura/lib/helpers.py                    | 14 +++++++++
 Allura/allura/lib/plugin.py                     |  4 +--
 Allura/allura/tests/decorators.py               | 30 +++++++++++++++++---
 Allura/allura/tests/functional/test_auth.py     | 10 +++----
 .../allura/tests/functional/test_site_admin.py  |  4 +--
 6 files changed, 58 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/allura/blob/505174a0/Allura/allura/controllers/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index d2c97a7..61f068e 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -166,7 +166,7 @@ class AuthController(BaseController):
         user = self._validate_hash(hash)
         user.set_password(pw)
         user.set_tool_data('AuthPasswordReset', hash='', hash_expiry='')
-        M.AuditLog.log_user('Password changed (through recovery process)', user=user)
+        h.auditlog_user('Password changed (through recovery process)', user=user)
         flash('Password changed')
         redirect('/auth/')
 
@@ -205,7 +205,7 @@ class AuthController(BaseController):
                 message_id=h.gen_message_id(),
                 text=text)
 
-        M.AuditLog.log_user('Password recovery link sent to: %s', email, user=user_record)
+        h.auditlog_user('Password recovery link sent to: %s', email, user=user_record)
         flash('A password reset email has been sent, if the given email address is on record in our system.')
         redirect('/')
 
@@ -256,7 +256,7 @@ class AuthController(BaseController):
             })
 
             flash('Email address confirmed')
-            M.AuditLog.log_user('Email address verified: %s', addr._id)
+            h.auditlog_user('Email address verified: %s', addr._id)
         else:
             flash('Unknown verification link', 'error')
         redirect('/auth/preferences/')
@@ -387,7 +387,7 @@ class AuthController(BaseController):
         flash('Password changed')
         del session['pwd-expired']
         session.save()
-        M.AuditLog.log_user('Password reset (via expiration process)')
+        h.auditlog_user('Password reset (via expiration process)')
         if return_to and return_to != request.url:
             redirect(return_to)
         else:
@@ -435,7 +435,7 @@ class PreferencesController(BaseController):
             old = c.user.get_pref('display_name')
             c.user.set_pref('display_name', preferences['display_name'])
             if old != preferences['display_name']:
-                M.AuditLog.log_user('Display Name changed %s => %s', old, preferences['display_name'])
+                h.auditlog_user('Display Name changed %s => %s', old, preferences['display_name'])
             for i, (old_a, data) in enumerate(zip(c.user.email_addresses, addr or [])):
                 obj = c.user.address_object(old_a)
                 if data.get('delete') or not obj:
@@ -451,7 +451,7 @@ class PreferencesController(BaseController):
                             # clear it now, a new one will get set below
                             c.user.set_pref('email_address', None)
                             primary_addr = None
-                    M.AuditLog.log_user('Email address deleted: %s', c.user.email_addresses[i])
+                    h.auditlog_user('Email address deleted: %s', c.user.email_addresses[i])
                     del c.user.email_addresses[i]
                     if obj:
                         obj.delete()
@@ -467,7 +467,7 @@ class PreferencesController(BaseController):
                     em = M.EmailAddress.create(new_addr['addr'])
                     em.claimed_by_user_id = c.user._id
                     em.send_verification_link()
-                    M.AuditLog.log_user('New email address: %s', new_addr['addr'])
+                    h.auditlog_user('New email address: %s', new_addr['addr'])
                     flash('A verification email has been sent.  Please check your email and click to confirm.')
                 else:
                     flash('Email address %s is invalid' % new_addr['addr'], 'error')
@@ -478,7 +478,7 @@ class PreferencesController(BaseController):
                     if not kw.get('password') or not provider.validate_password(c.user, kw.get('password')):
                         flash('You must provide your current password to change primary address', 'error')
                         redirect('.')
-                    M.AuditLog.log_user(
+                    h.auditlog_user(
                         'Primary email changed: %s => %s',
                         c.user.get_pref('email_address'),
                         primary_addr)
@@ -501,7 +501,7 @@ class PreferencesController(BaseController):
             flash('Incorrect password', 'error')
             redirect('.')
         flash('Password changed')
-        M.AuditLog.log_user('Password changed')
+        h.auditlog_user('Password changed')
         redirect('.')
 
     @expose()

http://git-wip-us.apache.org/repos/asf/allura/blob/505174a0/Allura/allura/lib/helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index 5d978fa..dbed410 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -1190,3 +1190,17 @@ def unidiff(old, new):
         fromfile='old',
         tofile='new',
         lineterm=''))
+
+
+def auditlog_user(message, *args, **kwargs):
+    """
+    Create an audit log entry for a user, including the IP address
+
+    :param str message:
+    :param user: a :class:`allura.model.auth.User`
+    """
+    from allura import model as M
+    ip_address = request.headers.get('X-Remote-Addr', request.remote_addr)
+    message = 'IP Address: {}\n'.format(ip_address) + message
+    M.AuditLog.log_user(message, *args, **kwargs)
+

http://git-wip-us.apache.org/repos/asf/allura/blob/505174a0/Allura/allura/lib/plugin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 7f34559..14a2328 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -142,7 +142,7 @@ class AuthenticationProvider(object):
             if self.is_password_expired(user):
                 self.session['pwd-expired'] = True
                 from allura.model import AuditLog
-                AuditLog.log_user('Password expired', user=user)
+                h.auditlog_user('Password expired', user=user)
             if 'rememberme' in self.request.params:
                 remember_for = int(config.get('auth.remember_for', 365))
                 self.session['login_expires'] = datetime.utcnow() + timedelta(remember_for)
@@ -306,7 +306,7 @@ class LocalAuthenticationProvider(AuthenticationProvider):
         user.disabled = True
         session(user).flush(user)
         from allura.model import AuditLog
-        AuditLog.log_user('Account disabled', user=user)
+        h.auditlog_user('Account disabled', user=user)
 
     def validate_password(self, user, password):
         return self._validate_password(user, password)

http://git-wip-us.apache.org/repos/asf/allura/blob/505174a0/Allura/allura/tests/decorators.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/decorators.py b/Allura/allura/tests/decorators.py
index 3796de7..df0f0b9 100644
--- a/Allura/allura/tests/decorators.py
+++ b/Allura/allura/tests/decorators.py
@@ -154,18 +154,40 @@ class patch_middleware_config(object):
 
 
 @contextlib.contextmanager
-def audits(*messages):
+def audits(*messages, **kwargs):
+    """
+    Asserts all the messages exist in audit log
+
+    :param messages: regex strings
+    :param bool user: if this is a user log
+
+    """
     M.AuditLog.query.remove()
     yield
+    if kwargs.get('user'):
+        preamble = 'IP Address: .*\n'
+    else:
+        preamble = ''
     for message in messages:
         assert M.AuditLog.query.find(dict(
-            message=re.compile(message))).count(), 'Could not find "%s"' % message
+            message=re.compile(preamble + message))).count(), 'Could not find "%s"' % message
 
 
 @contextlib.contextmanager
-def out_audits(*messages):
+def out_audits(*messages, **kwargs):
+    """
+    Asserts none the messages exist in audit log.  "without audits"
+
+    :param messages: list of regex strings
+    :param bool user: if this is a user log
+
+    """
     M.AuditLog.query.remove()
     yield
+    if kwargs.get('user'):
+        preamble = 'IP Address: .*\n'
+    else:
+        preamble = ''
     for message in messages:
         assert not M.AuditLog.query.find(dict(
-            message=re.compile(message))).count(), 'Found unexpected: "%s"' % message
+            message=re.compile(preamble + message))).count(), 'Found unexpected: "%s"' % message

http://git-wip-us.apache.org/repos/asf/allura/blob/505174a0/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 77367ec..9ab122f 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -181,7 +181,7 @@ class TestAuth(TestController):
                      'test-admin@users.localhost')
 
         # add test@example
-        with td.audits('New email address: test@example.com'):
+        with td.audits('New email address: test@example.com', user=True):
             r = self.app.post('/auth/preferences/update', params={
                 'preferences.display_name': 'Test Admin',
                 'new_addr.addr': 'test@example.com',
@@ -196,7 +196,7 @@ class TestAuth(TestController):
         assert_equal(user.get_pref('email_address'), 'test-admin@users.localhost')
 
         # remove test-admin@users.localhost
-        with td.audits('Email address deleted: test-admin@users.localhost'):
+        with td.audits('Email address deleted: test-admin@users.localhost', user=True):
             r = self.app.post('/auth/preferences/update', params={
                 'preferences.display_name': 'Test Admin',
                 'addr-1.ord': '1',
@@ -213,7 +213,7 @@ class TestAuth(TestController):
         user = M.User.query.get(username='test-admin')
         assert_equal(user.get_pref('email_address'), None)
 
-        with td.audits('Display Name changed Test Admin => Admin'):
+        with td.audits('Display Name changed Test Admin => Admin', user=True):
             r = self.app.post('/auth/preferences/update', params={
                 'preferences.display_name': 'Admin',
                 'new_addr.addr': ''},
@@ -872,7 +872,7 @@ class TestPasswordReset(TestController):
         email.confirmed = True
         ThreadLocalORMSession.flush_all()
         old_pw_hash = user.password
-        with td.audits('Password recovery link sent to: test-admin@users.localhost'):
+        with td.audits('Password recovery link sent to: test-admin@users.localhost', user=True):
             r = self.app.post('/auth/password_recovery_hash', {'email': email.email})
         hash = user.get_tool_data('AuthPasswordReset', 'hash')
         hash_expiry = user.get_tool_data('AuthPasswordReset', 'hash_expiry')
@@ -885,7 +885,7 @@ class TestPasswordReset(TestController):
         assert_in('New Password (again):', r)
         form = r.forms[0]
         form['pw'] = form['pw2'] = new_password = '154321'
-        with td.audits('Password changed \(through recovery process\)'):
+        with td.audits('Password changed \(through recovery process\)', user=True):
             # escape parentheses, so they would not be treated as regex group
             r = form.submit()
         user = M.User.query.get(username='test-admin')

http://git-wip-us.apache.org/repos/asf/allura/blob/505174a0/Allura/allura/tests/functional/test_site_admin.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_site_admin.py b/Allura/allura/tests/functional/test_site_admin.py
index ab1ffc4..b7ae16a 100644
--- a/Allura/allura/tests/functional/test_site_admin.py
+++ b/Allura/allura/tests/functional/test_site_admin.py
@@ -170,8 +170,8 @@ class TestSiteAdmin(TestController):
     def test_users(self, request):
         request.url = 'http://host.domain/path/'
         c.user = M.User.by_username('test-user-1')
-        M.AuditLog.log_user('test activity user 1')
-        M.AuditLog.log_user('test activity user 2', user=M.User.by_username('test-user-2'))
+        h.auditlog_user('test activity user 1')
+        h.auditlog_user('test activity user 2', user=M.User.by_username('test-user-2'))
         r = self.app.get('/nf/admin/users')
         assert_not_in('test activity', r)
         r = self.app.get('/nf/admin/users?username=admin1')