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 <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

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
 

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
 

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'),

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):

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
 

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'),

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