[#7800] factor out a common method for ip address lookup from a request
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 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 @@
'''
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:
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 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 @@
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 @@
: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
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 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 @@
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
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 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 @@
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')
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 @@
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 @@
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
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 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 @@
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(
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 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 @@
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 @@
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']
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 @@
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'
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 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 @@
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 @@
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 @@
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 @@
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')
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
--- /dev/null
+++ b/Allura/allura/tests/unit/spam/__init__.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 @@
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 @@
@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 @@
@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 @@
@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 @@
@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 @@
@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 @@
@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 @@
@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(
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 @@
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 @@
@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 @@
@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 @@
@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 @@
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] == (
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 @@
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
