allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From brond...@apache.org
Subject allura git commit: [#7800] factor out a common method for ip address lookup from a request
Date Fri, 21 Nov 2014 20:40:22 GMT
Repository: allura
Updated Branches:
  refs/heads/db/7800 [created] 5dfecc535


[#7800] factor out a common method for ip address lookup from a request


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

Branch: refs/heads/db/7800
Commit: 5dfecc5353c3593b927d1f8d3dbf6ed2922acbd9
Parents: 88b9926
Author: Dave Brondsema <dbrondsema@slashdotmedia.com>
Authored: Fri Nov 21 20:40:00 2014 +0000
Committer: Dave Brondsema <dbrondsema@slashdotmedia.com>
Committed: Fri Nov 21 20:40:00 2014 +0000

----------------------------------------------------------------------
 Allura/allura/lib/decorators.py               |  8 ++---
 Allura/allura/lib/helpers.py                  | 15 ++++----
 Allura/allura/lib/spam/akismetfilter.py       |  4 +--
 Allura/allura/lib/spam/mollomfilter.py        |  4 +--
 Allura/allura/lib/utils.py                    | 10 ++++--
 Allura/allura/model/artifact.py               |  5 ++-
 Allura/allura/model/auth.py                   |  5 +--
 Allura/allura/tests/functional/test_auth.py   |  3 +-
 Allura/allura/tests/test_utils.py             | 41 +++++++++++++++++-----
 Allura/allura/tests/unit/spam/__init__.py     |  0
 Allura/allura/tests/unit/spam/test_akismet.py | 22 ++++--------
 Allura/allura/tests/unit/spam/test_mollom.py  | 17 ++-------
 Allura/development.ini                        |  3 ++
 13 files changed, 73 insertions(+), 64 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/lib/decorators.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/decorators.py b/Allura/allura/lib/decorators.py
index 28ad5bf..d472758 100644
--- a/Allura/allura/lib/decorators.py
+++ b/Allura/allura/lib/decorators.py
@@ -25,11 +25,11 @@ from urllib import unquote
 from decorator import decorator
 from tg.decorators import before_validate
 from tg import request, redirect
-
 from webob import exc
-
 from pylons import tmpl_context as c
+
 from allura.lib import helpers as h
+from allura.lib import utils
 
 
 def task(*args, **kw):
@@ -161,9 +161,7 @@ class log_action(object):  # pragma no cover
         '''
         extra = self._extra_proto.copy()
         # Save the client IP address
-        client_ip = request.headers.get('X_FORWARDED_FOR', request.remote_addr)
-        client_ip = client_ip.split(',')[0].strip()
-        extra.update(client_ip=client_ip)
+        extra.update(client_ip=utils.ip_address(request))
         # Save the user info
         user = getattr(request, 'user', None)
         if user:

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/lib/helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index 981534c..3b6a1d5 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -59,10 +59,13 @@ from webhelpers import date, feedgenerator, html, number, misc, text
 from webob.exc import HTTPUnauthorized
 
 from allura.lib import exceptions as exc
-# Reimport to make available to templates
 from allura.lib import AsciiDammit
+from allura.lib import utils
+
+# import to make available to templates, don't delete:
 from .security import has_access
 
+
 log = logging.getLogger(__name__)
 
 # validates project, subproject, and user names
@@ -657,13 +660,7 @@ class log_action(object):
                 result['username'] = '*system'
             try:
                 result['url'] = request.url
-                ip_address = request.headers.get(
-                    'X_FORWARDED_FOR', request.remote_addr)
-                if ip_address is not None:
-                    ip_address = ip_address.split(',')[0].strip()
-                    result['ip_address'] = ip_address
-                else:
-                    result['ip_address'] = '0.0.0.0'
+                result['ip_address'] = utils.ip_address(request)
             except TypeError:
                 pass
             return result
@@ -1206,7 +1203,7 @@ def auditlog_user(message, *args, **kwargs):
     :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)
+    ip_address = utils.ip_address(request)
     message = 'IP Address: {}\n'.format(ip_address) + message
     if kwargs.get('user') and kwargs['user'] != c.user:
         message = 'Done by user: {}\n'.format(c.user.username) + message

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/lib/spam/akismetfilter.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/spam/akismetfilter.py b/Allura/allura/lib/spam/akismetfilter.py
index 775aecc..4913970 100644
--- a/Allura/allura/lib/spam/akismetfilter.py
+++ b/Allura/allura/lib/spam/akismetfilter.py
@@ -21,6 +21,7 @@ from pylons import request
 from pylons import tmpl_context as c
 
 from allura.lib import helpers as h
+from allura.lib import utils
 from allura.lib.spam import SpamFilter
 
 try:
@@ -66,8 +67,7 @@ class AkismetSpamFilter(SpamFilter):
             kw['comment_author'] = user.display_name or user.username
             kw['comment_author_email'] = user.email_addresses[
                 0] if user.email_addresses else ''
-        user_ip = request.headers.get('X_FORWARDED_FOR', request.remote_addr)
-        kw['user_ip'] = user_ip.split(',')[0].strip()
+        kw['user_ip'] = utils.ip_address(request)
         kw['user_agent'] = request.headers.get('USER_AGENT')
         kw['referrer'] = request.headers.get('REFERER')
         # kw will be urlencoded, need to utf8-encode

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/lib/spam/mollomfilter.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/spam/mollomfilter.py b/Allura/allura/lib/spam/mollomfilter.py
index d76ff59..3651a94 100644
--- a/Allura/allura/lib/spam/mollomfilter.py
+++ b/Allura/allura/lib/spam/mollomfilter.py
@@ -21,6 +21,7 @@ from pylons import request
 from pylons import tmpl_context as c
 
 from allura.lib import helpers as h
+from allura.lib import utils
 from allura.lib.spam import SpamFilter
 
 try:
@@ -75,8 +76,7 @@ class MollomSpamFilter(SpamFilter):
             kw['authorName'] = user.display_name or user.username
             kw['authorMail'] = user.email_addresses[
                 0] if user.email_addresses else ''
-        user_ip = request.headers.get('X_FORWARDED_FOR', request.remote_addr)
-        kw['authorIP'] = user_ip.split(',')[0].strip()
+        kw['authorIP'] = utils.ip_address(request)
         # kw will be urlencoded, need to utf8-encode
         for k, v in kw.items():
             kw[k] = h.really_unicode(v).encode('utf8')

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/lib/utils.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py
index ce1836f..35bc039 100644
--- a/Allura/allura/lib/utils.py
+++ b/Allura/allura/lib/utils.py
@@ -336,9 +336,7 @@ class AntiSpam(object):
         if timestamp is None:
             timestamp = self.timestamp
         try:
-            client_ip = self.request.headers.get(
-                'X_FORWARDED_FOR', self.request.remote_addr)
-            client_ip = client_ip.split(',')[0].strip()
+            client_ip = ip_address(self.request)
         except (TypeError, AttributeError), err:
             client_ip = '127.0.0.1'
         plain = '%d:%s:%s' % (
@@ -552,3 +550,9 @@ class ForgeHTMLSanitizer(html5lib.sanitizer.HTMLSanitizer):
                 self.allowed_elements.append('iframe')
         return super(ForgeHTMLSanitizer, self).sanitize_token(token)
 
+
+def ip_address(request):
+    ip = request.remote_addr
+    if tg.config.get('ip_address_header'):
+        ip = request.headers.get(tg.config['ip_address_header']) or ip
+    return ip
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/model/artifact.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/artifact.py b/Allura/allura/model/artifact.py
index 4632301..daba6c3 100644
--- a/Allura/allura/model/artifact.py
+++ b/Allura/allura/model/artifact.py
@@ -31,6 +31,7 @@ from webhelpers import feedgenerator as FG
 
 from allura.lib import helpers as h
 from allura.lib import security
+from allura.lib import utils
 
 from allura.lib.search import SearchIndexable
 from .session import main_orm_session
@@ -472,9 +473,7 @@ class VersionedArtifact(Artifact):
     def commit(self, update_stats=True):
         '''Save off a snapshot of the artifact and increment the version #'''
         try:
-            ip_address = request.headers.get(
-                'X_FORWARDED_FOR', request.remote_addr)
-            ip_address = ip_address.split(',')[0].strip()
+            ip_address = utils.ip_address(request)
         except:
             ip_address = '0.0.0.0'
         data = dict(

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/model/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 7928796..6217f42 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -42,6 +42,7 @@ import types
 import allura.tasks.mail_tasks
 from allura.lib import helpers as h
 from allura.lib import plugin
+from allura.lib import utils
 from allura.lib.decorators import memoize
 from allura.lib.search import SearchIndexable
 from .session import main_orm_session, main_doc_session
@@ -347,7 +348,7 @@ class User(MappedClass, ActivityNode, ActivityObject, SearchIndexable):
         return dict(provider.index_user(self), **fields)
 
     def track_login(self, req):
-        user_ip = req.headers.get('X_FORWARDED_FOR', req.remote_addr)
+        user_ip = utils.ip_address(req)
         user_agent = req.headers.get('User-Agent')
         self.last_access['login_date'] = datetime.utcnow()
         self.last_access['login_ip'] = user_ip
@@ -355,7 +356,7 @@ class User(MappedClass, ActivityNode, ActivityObject, SearchIndexable):
         session(self).flush(self)
 
     def track_active(self, req):
-        user_ip = req.headers.get('X_FORWARDED_FOR', req.remote_addr)
+        user_ip = utils.ip_address(req)
         user_agent = req.headers.get('User-Agent')
         now = datetime.utcnow()
         last_date = self.last_access['session_date']

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/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 2206f37..9d0f1b1 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -81,7 +81,8 @@ class TestAuth(TestController):
         assert_equal(user.last_access['login_ua'], None)
 
         self.app.post('/auth/do_login',
-                      headers={'X_FORWARDED_FOR': 'addr', 'User-Agent': 'browser'},
+                      headers={'User-Agent': 'browser'},
+                      extra_environ={'REMOTE_ADDR': 'addr'},
                       params=dict(
                           username='test-user',
                           password='foo'

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/tests/test_utils.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_utils.py b/Allura/allura/tests/test_utils.py
index 5e4135b..209d0df 100644
--- a/Allura/allura/tests/test_utils.py
+++ b/Allura/allura/tests/test_utils.py
@@ -21,17 +21,18 @@ import time
 import unittest
 from os import path
 
-import pylons
 from webob import Request
 from mock import Mock, patch
 from nose.tools import assert_equal
 from pygments import highlight
 from pygments.lexers import get_lexer_for_filename
+from tg import config
 
 from alluratest.controller import setup_unit_test
 
 from allura import model as M
 from allura.lib import utils
+from allura.lib import helpers as h
 
 
 @patch.dict('allura.lib.utils.tg.config', clear=True, foo='bar', baz='true')
@@ -96,7 +97,6 @@ class TestAntispam(unittest.TestCase):
 
     def setUp(self):
         setup_unit_test()
-        pylons.request.remote_addr = '127.0.0.1'
         self.a = utils.AntiSpam()
 
     def test_generate_fields(self):
@@ -105,12 +105,6 @@ class TestAntispam(unittest.TestCase):
         assert 'name="spinner"' in fields, fields
         assert ('class="%s"' % self.a.honey_class) in fields, fields
 
-    def test_valid_submit(self):
-        form = dict(a='1', b='2')
-        r = Request.blank('/', POST=self._encrypt_form(**form))
-        validated = utils.AntiSpam.validate_request(r)
-        assert dict(a='1', b='2') == validated, validated
-
     def test_invalid_old(self):
         form = dict(a='1', b='2')
         r = Request.blank('/', POST=self._encrypt_form(**form))
@@ -119,6 +113,13 @@ class TestAntispam(unittest.TestCase):
             utils.AntiSpam.validate_request,
             r, now=time.time() + 24 * 60 * 60 + 1)
 
+    def test_valid_submit(self):
+        form = dict(a='1', b='2')
+        r = Request.blank('/', POST=self._encrypt_form(**form),
+                          environ={'remote_addr': '127.0.0.1'})
+        validated = utils.AntiSpam.validate_request(r)
+        assert dict(a='1', b='2') == validated, validated
+
     def test_invalid_future(self):
         form = dict(a='1', b='2')
         r = Request.blank('/', POST=self._encrypt_form(**form))
@@ -253,3 +254,27 @@ class TestHTMLSanitizer(unittest.TestCase):
         p = utils.ForgeHTMLSanitizer('<div><iframe src="https://www.youtube.com/embed/kOLpSPEA72U?feature=oembed"></iframe></div>')
         assert_equal(
             self.simple_tag_list(p), ['div', 'iframe', 'div'])
+
+
+def test_ip_address():
+    req = Mock()
+    req.remote_addr = '1.2.3.4'
+    req.headers = {}
+    assert_equal(utils.ip_address(req),
+                 '1.2.3.4')
+
+def test_ip_address_header():
+    req = Mock()
+    req.remote_addr = '1.2.3.4'
+    req.headers = {'X_FORWARDED_FOR': '5.6.7.8'}
+    with h.push_config(config, **{'ip_address_header': 'X_FORWARDED_FOR'}):
+        assert_equal(utils.ip_address(req),
+                     '5.6.7.8')
+
+def test_ip_address_header_not_set():
+    req = Mock()
+    req.remote_addr = '1.2.3.4'
+    req.headers = {}
+    with h.push_config(config, **{'ip_address_header': 'X_FORWARDED_FOR'}):
+        assert_equal(utils.ip_address(req),
+                     '1.2.3.4')

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/tests/unit/spam/__init__.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/spam/__init__.py b/Allura/allura/tests/unit/spam/__init__.py
new file mode 100644
index 0000000..e69de29

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/tests/unit/spam/test_akismet.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/spam/test_akismet.py b/Allura/allura/tests/unit/spam/test_akismet.py
index 1b7215a..79802c3 100644
--- a/Allura/allura/tests/unit/spam/test_akismet.py
+++ b/Allura/allura/tests/unit/spam/test_akismet.py
@@ -41,8 +41,6 @@ class TestAkismet(unittest.TestCase):
         self.fake_user = mock.Mock(display_name=u'Søme User',
                                    email_addresses=['user@domain'])
         self.fake_headers = dict(
-            REMOTE_ADDR='fallback ip',
-            X_FORWARDED_FOR='some ip',
             USER_AGENT='some browser',
             REFERER='some url')
         self.content = u'spåm text'
@@ -57,6 +55,7 @@ class TestAkismet(unittest.TestCase):
     @mock.patch('allura.lib.spam.akismetfilter.request')
     def test_check(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.akismet.service.comment_check.side_effect({'side_effect': ''})
         self.akismet.check(self.content)
@@ -68,6 +67,7 @@ class TestAkismet(unittest.TestCase):
     @mock.patch('allura.lib.spam.akismetfilter.request')
     def test_check_with_explicit_content_type(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.akismet.check(self.content, content_type='some content type')
         self.expected_data['comment_type'] = 'some content type'
@@ -79,6 +79,7 @@ class TestAkismet(unittest.TestCase):
     @mock.patch('allura.lib.spam.akismetfilter.request')
     def test_check_with_artifact(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.akismet.check(self.content, artifact=self.fake_artifact)
         expected_data = self.expected_data
@@ -91,6 +92,7 @@ class TestAkismet(unittest.TestCase):
     @mock.patch('allura.lib.spam.akismetfilter.request')
     def test_check_with_user(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.akismet.check(self.content, user=self.fake_user)
         expected_data = self.expected_data
@@ -104,6 +106,7 @@ class TestAkismet(unittest.TestCase):
     @mock.patch('allura.lib.spam.akismetfilter.request')
     def test_check_with_implicit_user(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = self.fake_user
         self.akismet.check(self.content)
         expected_data = self.expected_data
@@ -115,21 +118,9 @@ class TestAkismet(unittest.TestCase):
 
     @mock.patch('allura.lib.spam.akismetfilter.c')
     @mock.patch('allura.lib.spam.akismetfilter.request')
-    def test_check_with_fallback_ip(self, request, c):
-        self.expected_data['user_ip'] = 'fallback ip'
-        self.fake_headers.pop('X_FORWARDED_FOR')
-        request.headers = self.fake_headers
-        request.remote_addr = self.fake_headers['REMOTE_ADDR']
-        c.user = None
-        self.akismet.check(self.content)
-        self.akismet.service.comment_check.assert_called_once_with(
-            self.content,
-            data=self.expected_data, build_data=False)
-
-    @mock.patch('allura.lib.spam.akismetfilter.c')
-    @mock.patch('allura.lib.spam.akismetfilter.request')
     def test_submit_spam(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.akismet.submit_spam(self.content)
         self.akismet.service.submit_spam.assert_called_once_with(
@@ -139,6 +130,7 @@ class TestAkismet(unittest.TestCase):
     @mock.patch('allura.lib.spam.akismetfilter.request')
     def test_submit_ham(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.akismet.submit_ham(self.content)
         self.akismet.service.submit_ham.assert_called_once_with(

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/allura/tests/unit/spam/test_mollom.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/spam/test_mollom.py b/Allura/allura/tests/unit/spam/test_mollom.py
index 931f8a3..cac2ab8 100644
--- a/Allura/allura/tests/unit/spam/test_mollom.py
+++ b/Allura/allura/tests/unit/spam/test_mollom.py
@@ -44,8 +44,6 @@ class TestMollom(unittest.TestCase):
         self.fake_user = mock.Mock(display_name=u'Søme User',
                                    email_addresses=['user@domain'])
         self.fake_headers = dict(
-            REMOTE_ADDR='fallback ip',
-            X_FORWARDED_FOR='some ip',
             USER_AGENT='some browser',
             REFERER='some url')
         self.content = u'spåm text'
@@ -59,6 +57,7 @@ class TestMollom(unittest.TestCase):
     @mock.patch('allura.lib.spam.mollomfilter.request')
     def test_check(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.mollom.check(self.content, artifact=self.artifact)
         self.mollom.service.checkContent.assert_called_once_with(
@@ -68,6 +67,7 @@ class TestMollom(unittest.TestCase):
     @mock.patch('allura.lib.spam.mollomfilter.request')
     def test_check_with_user(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = None
         self.mollom.check(self.content, user=self.fake_user,
                           artifact=self.artifact)
@@ -81,6 +81,7 @@ class TestMollom(unittest.TestCase):
     @mock.patch('allura.lib.spam.mollomfilter.request')
     def test_check_with_implicit_user(self, request, c):
         request.headers = self.fake_headers
+        request.remote_addr = 'some ip'
         c.user = self.fake_user
         self.mollom.check(self.content, artifact=self.artifact)
         expected_data = self.expected_data
@@ -89,18 +90,6 @@ class TestMollom(unittest.TestCase):
         self.mollom.service.checkContent.assert_called_once_with(
             **self.expected_data)
 
-    @mock.patch('allura.lib.spam.mollomfilter.c')
-    @mock.patch('allura.lib.spam.mollomfilter.request')
-    def test_check_with_fallback_ip(self, request, c):
-        self.expected_data['authorIP'] = 'fallback ip'
-        self.fake_headers.pop('X_FORWARDED_FOR')
-        request.headers = self.fake_headers
-        request.remote_addr = self.fake_headers['REMOTE_ADDR']
-        c.user = None
-        self.mollom.check(self.content, artifact=self.artifact)
-        self.mollom.service.checkContent.assert_called_once_with(
-            **self.expected_data)
-
     def test_submit_spam(self):
         self.mollom.submit_spam('test', artifact=self.artifact)
         assert self.mollom.service.sendFeedback.call_args[0] == (

http://git-wip-us.apache.org/repos/asf/allura/blob/5dfecc53/Allura/development.ini
----------------------------------------------------------------------
diff --git a/Allura/development.ini b/Allura/development.ini
index 0de4e80..e5be8b2 100644
--- a/Allura/development.ini
+++ b/Allura/development.ini
@@ -159,6 +159,9 @@ files_expires_header_secs = 1209600 ; 2 weeks
 
 ew.extra_headers = [ ('Access-Control-Allow-Origin', '*') ]
 
+; If your environment (e.g. behind a server-side proxy) needs to look at an http header to
get the actual remote addr
+;ip_address_header = X-Forwarded-For
+
 # SCM settings for local development
 scm.host.ro.git = /srv/git$path
 scm.host.rw.git = /srv/git$path


Mime
View raw message