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 2014/11/21 21:40:22 UTC
allura git commit: [#7800] factor out a common method for ip address
lookup from a request
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 <db...@slashdotmedia.com>
Authored: Fri Nov 21 20:40:00 2014 +0000
Committer: Dave Brondsema <db...@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