From dc9ab52edc46b410d483ac8f7981925bc74c19c9 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 23 Jan 2019 16:53:56 -0800 Subject: [PATCH] axes: Minor fixes to configuration for IP blocking - Use the X-Forwarded-For header only if specified in the configuration. This makes FreedomBox safe to use when not behind a reverse proxy server (although we are unlikely to do this). - When fetching the IP address to reset after successful login, use the X-Forwarded-For header only if specified in the configuration. - Minor flake8 refactorings. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- data/etc/plinth/plinth.config | 2 ++ plinth.config | 2 ++ plinth/cfg.py | 2 ++ plinth/modules/sso/views.py | 36 +++++++++------------- plinth/tests/data/etc/plinth/plinth.config | 2 ++ plinth/tests/test_cfg.py | 6 +++- plinth/web_framework.py | 17 +++++++++- 7 files changed, 44 insertions(+), 23 deletions(-) diff --git a/data/etc/plinth/plinth.config b/data/etc/plinth/plinth.config index a3a373462..2b874c152 100644 --- a/data/etc/plinth/plinth.config +++ b/data/etc/plinth/plinth.config @@ -17,6 +17,7 @@ port = 8000 # Enable the following only if Plinth is behind a proxy server. The # proxy server should properly clean and the following HTTP headers: +# X-Forwarded-For # X-Forwarded-Host # X-Forwarded-Proto # If you enable these unnecessarily, this will lead to serious security @@ -27,6 +28,7 @@ port = 8000 # configuration allows only connections from localhost # # Leave the values blank to disable +use_x_forwarded_for = True use_x_forwarded_host = True secure_proxy_ssl_header = HTTP_X_FORWARDED_PROTO diff --git a/plinth.config b/plinth.config index 3b7c89ba3..340e92f53 100644 --- a/plinth.config +++ b/plinth.config @@ -17,6 +17,7 @@ port = 8000 # Enable the following only if Plinth is behind a proxy server. The # proxy server should properly clean and the following HTTP headers: +# X-Forwarded-For # X-Forwarded-Host # X-Forwarded-Proto # If you enable these unnecessarily, this will lead to serious security @@ -27,6 +28,7 @@ port = 8000 # configuration allows only connections from localhost # # Leave the values blank to disable +use_x_forwarded_for = True use_x_forwarded_host = True secure_proxy_ssl_header = HTTP_X_FORWARDED_PROTO diff --git a/plinth/cfg.py b/plinth/cfg.py index 86155db6d..95630da6a 100644 --- a/plinth/cfg.py +++ b/plinth/cfg.py @@ -32,6 +32,7 @@ actions_dir = None doc_dir = None host = None port = None +use_x_forwarded_for = False use_x_forwarded_host = False secure_proxy_ssl_header = None develop = False @@ -98,6 +99,7 @@ def read(config_path=None, root_directory=None): ('Network', 'host', 'string'), ('Network', 'port', 'int'), ('Network', 'secure_proxy_ssl_header', 'string'), + ('Network', 'use_x_forwarded_for', 'bool'), ('Network', 'use_x_forwarded_host', 'bool'), ('Misc', 'box_name', 'string'), ('Misc', 'danube_edition', 'bool'), diff --git a/plinth/modules/sso/views.py b/plinth/modules/sso/views.py index eea5cd02d..2ecbe135d 100644 --- a/plinth/modules/sso/views.py +++ b/plinth/modules/sso/views.py @@ -22,13 +22,13 @@ import logging import os import urllib +import axes.utils +from axes.decorators import axes_form_invalid from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.views import LoginView, LogoutView from django.http import HttpResponseRedirect -from axes.decorators import axes_form_invalid -from axes.utils import reset -from plinth import actions +from plinth import actions, web_framework from .forms import AuthenticationForm @@ -66,8 +66,8 @@ class SSOLoginView(LoginView): response = super(SSOLoginView, self).dispatch(request, *args, **kwargs) if request.user.is_authenticated: return set_ticket_cookie(request.user, response) - else: - return response + + return response @axes_form_invalid def form_invalid(self, *args, **kwargs): @@ -82,24 +82,18 @@ class CaptchaLoginView(LoginView): def dispatch(self, request, *args, **kwargs): response = super(CaptchaLoginView, self).dispatch( request, *args, **kwargs) - if request.POST: - if request.user.is_authenticated: - ip = get_ip_address_from_request(request) - reset(ip=ip) - return set_ticket_cookie(request.user, response) - else: - return response - return response + if not request.POST: + return response + if not request.user.is_authenticated: + return response -def get_ip_address_from_request(request): - x_forwarded_for = request.META.get('HTTP_X_FORWARDED_FOR') - if x_forwarded_for: - ip = x_forwarded_for.split(',')[0] - else: - ip = request.META.get('REMOTE_ADDR') - logger.warning("IP address is " + ip) - return ip + # Successful authentication + ip_address = web_framework.get_ip_address_from_request(request) + axes.utils.reset(ip=ip_address) + logger.info('Login attempts reset for IP after successful login: %s', + ip_address) + return set_ticket_cookie(request.user, response) class SSOLogoutView(LogoutView): diff --git a/plinth/tests/data/etc/plinth/plinth.config b/plinth/tests/data/etc/plinth/plinth.config index 3b7c89ba3..340e92f53 100644 --- a/plinth/tests/data/etc/plinth/plinth.config +++ b/plinth/tests/data/etc/plinth/plinth.config @@ -17,6 +17,7 @@ port = 8000 # Enable the following only if Plinth is behind a proxy server. The # proxy server should properly clean and the following HTTP headers: +# X-Forwarded-For # X-Forwarded-Host # X-Forwarded-Proto # If you enable these unnecessarily, this will lead to serious security @@ -27,6 +28,7 @@ port = 8000 # configuration allows only connections from localhost # # Leave the values blank to disable +use_x_forwarded_for = True use_x_forwarded_host = True secure_proxy_ssl_header = HTTP_X_FORWARDED_PROTO diff --git a/plinth/tests/test_cfg.py b/plinth/tests/test_cfg.py index 41df6a90c..2b0fe990e 100644 --- a/plinth/tests/test_cfg.py +++ b/plinth/tests/test_cfg.py @@ -126,12 +126,16 @@ class TestCfg(unittest.TestCase): self.assertEqual(parser.get('Path', 'actions_dir'), cfg.actions_dir) self.assertEqual(parser.get('Path', 'doc_dir'), cfg.doc_dir) - self.assertEqual(5, len(parser.items('Network'))) + self.assertEqual(6, len(parser.items('Network'))) self.assertEqual(parser.get('Network', 'host'), cfg.host) self.assertEqual(int(parser.get('Network', 'port')), cfg.port) self.assertEqual( parser.get('Network', 'secure_proxy_ssl_header'), cfg.secure_proxy_ssl_header) + self.assertIsInstance(cfg.use_x_forwarded_for, bool) + self.assertEqual( + parser.get('Network', 'use_x_forwarded_for'), + str(cfg.use_x_forwarded_for)) self.assertIsInstance(cfg.use_x_forwarded_host, bool) self.assertEqual( parser.get('Network', 'use_x_forwarded_host'), diff --git a/plinth/web_framework.py b/plinth/web_framework.py index 2ec7a9386..770e432ac 100644 --- a/plinth/web_framework.py +++ b/plinth/web_framework.py @@ -72,6 +72,10 @@ def init(): if cfg.secure_proxy_ssl_header: secure_proxy_ssl_header = (cfg.secure_proxy_ssl_header, 'https') + ipware_meta_precedence_order = ('REMOTE_ADDR', ) + if cfg.use_x_forwarded_for: + ipware_meta_precedence_order = ('HTTP_X_FORWARDED_FOR', ) + pwd = 'django.contrib.auth.password_validation' django.conf.settings.configure( @@ -113,7 +117,7 @@ def init(): DEBUG=cfg.develop, FORCE_SCRIPT_NAME=cfg.server_dir, INSTALLED_APPS=applications, - IPWARE_META_PRECEDENCE_ORDER=('HTTP_X_FORWARDED_FOR', ), + IPWARE_META_PRECEDENCE_ORDER=ipware_meta_precedence_order, LANGUAGES=get_languages(), LOGGING=log.get_configuration(), LOGIN_URL='users:login', @@ -191,3 +195,14 @@ def get_wsgi_application(): def get_static_url(): """Return Django static URL.""" return django.conf.settings.STATIC_URL + + +def get_ip_address_from_request(request): + """Return the IP address of the original client.""" + if cfg.use_x_forwarded_for: + x_forwarded_for = request.META.get('HTTP_X_FORWARDED_FOR') + ip_address = x_forwarded_for.split(',')[0] + else: + ip_address = request.META.get('REMOTE_ADDR') + + return ip_address