From 770974c8ce86ae45afc847c3bcf440a9ea788bed Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 15 Aug 2023 11:54:02 -0700 Subject: [PATCH] sso: Switch to django-axes >= 5.0 - Add explicit dependency on django-ipware >=3. django-axes >= 6 adds only and optional dependency on django-ipware. Adding explicit dependency make the behavior safer. - Depend on django-axes >= 5 where the authentication backend and other features are available. The new code won't work with older versions. The new approach uses and authentication backend to deny access to the login form on lockout and a middleware to redirect user to locked out form when limit of attempts have been reached. - Drop old code used for compatibility with django-axes 3.x. - Suppress verbose and debug messages as django-axes is too chatty. - Re-implment the CAPTCHA form entirely. In the old style, we have a login form with CAPTCHA field. That would not work with the new django-axes authentication middle. On submission of the form, auth.authenticate() will be called. This call invokes various authentication backends include django-axes authentication backend. This backend's behavior is to reject all authentication attempts when the IP is listed in locked table. The new approach is to provide a simple CAPTCHA form with just the CAPTCHA field. If the form is successfully validated (correct CAPTCHA is provided), then the lock on the IP address is reset. The user is then free to perform 3 more attempts to login. - Update firstboot form to send the request parameter when using auth.authenticate() method. This needed by Django axes' authentication method which will be triggered. Tests: - Run tests on Debian Bookworm and Debian testing. - Axes verbose messages and debug messages are not printed on the console when running FreedomBox in debug mode. - Only three invalid attempts are allowed at the login page. After the final incorrect attempt, user is redirected to CAPTCHA page. Visiting the login page using the URL works but entering the correct credentials still takes the user to CAPTCHA page. - CAPTCHA form appears as expected. Clicking the CAPTCHA images downloads the audio file corresponding to the image. Incorrect CAPTCHA shows an error. Correct CAPTCHA takes the user to login form where they are able to login with correct credentials. Entering incorrect credentials 3 times will take the user again to CAPTCHA page. - Creating user account during firstboot works. - Blocked IP address the IP of the client such as 10.42.0.1 and not the local IP address 127.0.0.1 according the django-axes log messages. While one client IP address is blocked, another IP is able to login to the same user account that was attempted by the blocked client. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- debian/control | 10 ++++- plinth/__main__.py | 6 +-- plinth/axes_app_config.py | 14 ------- plinth/log.py | 3 ++ plinth/modules/sso/forms.py | 9 +++-- plinth/modules/sso/templates/captcha.html | 28 +++++++++++++ plinth/modules/sso/urls.py | 9 ++--- plinth/modules/sso/views.py | 48 ++++++++--------------- plinth/modules/users/forms.py | 4 +- plinth/modules/users/urls.py | 12 ++---- plinth/settings.py | 19 ++++++++- plinth/utils.py | 19 +-------- plinth/web_framework.py | 11 ------ 13 files changed, 92 insertions(+), 100 deletions(-) delete mode 100644 plinth/axes_app_config.py create mode 100644 plinth/modules/sso/templates/captcha.html diff --git a/debian/control b/debian/control index 340bb9a89..af482d8d1 100644 --- a/debian/control +++ b/debian/control @@ -26,8 +26,11 @@ Build-Depends: python3-configobj, python3-dbus, python3-django (>= 1.11), - python3-django-axes (>= 3.0.3), + python3-django-axes (>= 5.0.0), python3-django-captcha, +# Explictly depend on ipware as it is optional dependecy for future versions +# of django-axes. + python3-django-ipware (>= 3), python3-django-stronghold (>= 0.3.0), python3-flake8, python3-gi, @@ -108,8 +111,11 @@ Depends: python3-configobj, python3-dbus, python3-django (>= 1.11), - python3-django-axes (>= 3.0.3), + python3-django-axes (>= 5.0.0), python3-django-captcha, +# Explictly depend on ipware as it is optional dependecy for future versions +# of django-axes. + python3-django-ipware (>= 3), python3-django-stronghold, python3-gi, python3-markupsafe, diff --git a/plinth/__main__.py b/plinth/__main__.py index 12f898879..3fa050e1c 100644 --- a/plinth/__main__.py +++ b/plinth/__main__.py @@ -7,13 +7,9 @@ import sys from . import __version__ from . import app as app_module -from . import (cfg, frontpage, glib, log, menu, module_loader, setup, utils, +from . import (cfg, frontpage, glib, log, menu, module_loader, setup, web_framework, web_server) -if utils.is_axes_old(): - import axes - axes.default_app_config = 'plinth.axes_app_config.AppConfig' - precedence_commandline_arguments = ["server_dir", "develop"] logger = logging.getLogger(__name__) diff --git a/plinth/axes_app_config.py b/plinth/axes_app_config.py deleted file mode 100644 index e13a76b02..000000000 --- a/plinth/axes_app_config.py +++ /dev/null @@ -1,14 +0,0 @@ -# SPDX-License-Identifier: AGPL-3.0-or-later -""" -Overridden AppConfig from django-axes to avoid monkey-patched LoginView -""" - -from django import apps - - -class AppConfig(apps.AppConfig): - name = 'axes' - - def ready(self): - # Signals must be loaded for axes to get the login_failed signals - from axes import signals # noqa pylint: disable=unused-import isort:skip diff --git a/plinth/log.py b/plinth/log.py index 653e42eba..5b5b7974f 100644 --- a/plinth/log.py +++ b/plinth/log.py @@ -125,6 +125,9 @@ def get_configuration(): 'loggers': { 'django.db.backends': { 'level': 'INFO' # Set to 'DEBUG' to log database queries + }, + 'axes': { + 'level': 'INFO' # Too verbose during DEBUG } } } diff --git a/plinth/modules/sso/forms.py b/plinth/modules/sso/forms.py index 1de76a7b4..605df651e 100644 --- a/plinth/modules/sso/forms.py +++ b/plinth/modules/sso/forms.py @@ -4,8 +4,10 @@ Forms for the Single Sign On app of FreedomBox. """ from captcha.fields import CaptchaField +from django import forms from django.contrib.auth.forms import \ AuthenticationForm as DjangoAuthenticationForm +from django.utils.translation import gettext_lazy as _ class AuthenticationForm(DjangoAuthenticationForm): @@ -20,6 +22,7 @@ class AuthenticationForm(DjangoAuthenticationForm): }) -class CaptchaAuthenticationForm(AuthenticationForm): - """Authentication form with an additional Captcha field.""" - captcha = CaptchaField() +class CaptchaForm(forms.Form): + """Form with a CAPTCHA field to use after 3 invalid login attempts.""" + captcha = CaptchaField( + label=_('Enter the letters in the image to proceed to the login page')) diff --git a/plinth/modules/sso/templates/captcha.html b/plinth/modules/sso/templates/captcha.html new file mode 100644 index 000000000..c9521cd61 --- /dev/null +++ b/plinth/modules/sso/templates/captcha.html @@ -0,0 +1,28 @@ +{% extends "base.html" %} +{% comment %} +# SPDX-License-Identifier: AGPL-3.0-or-later +{% endcomment %} + +{% load bootstrap %} +{% load i18n %} + +{% block content %} + +
+ +
+ {% csrf_token %} +
+ {{ form|bootstrap }} + +
+ +
+
+
+ + +
+ +{% endblock %} diff --git a/plinth/modules/sso/urls.py b/plinth/modules/sso/urls.py index 457314d73..8e4441649 100644 --- a/plinth/modules/sso/urls.py +++ b/plinth/modules/sso/urls.py @@ -3,21 +3,20 @@ URLs for the Single Sign On module. """ -from axes.decorators import axes_dispatch from django.urls import re_path from stronghold.decorators import public from plinth.utils import non_admin_view -from .views import CaptchaLoginView, SSOLoginView, refresh +from .views import CaptchaView, SSOLoginView, refresh urlpatterns = [ - re_path(r'^accounts/sso/login/$', - public(axes_dispatch(SSOLoginView.as_view())), name='sso-login'), + re_path(r'^accounts/sso/login/$', public(SSOLoginView.as_view()), + name='sso-login'), re_path(r'^accounts/sso/refresh/$', non_admin_view(refresh), name='sso-refresh'), # Locked URL from django-axes - re_path(r'accounts/sso/login/locked/$', public(CaptchaLoginView.as_view()), + re_path(r'accounts/sso/login/locked/$', public(CaptchaView.as_view()), name='locked_out'), ] diff --git a/plinth/modules/sso/views.py b/plinth/modules/sso/views.py index e9f84d4b0..f94d0411f 100644 --- a/plinth/modules/sso/views.py +++ b/plinth/modules/sso/views.py @@ -6,7 +6,6 @@ import os import urllib import axes.utils -from axes.decorators import axes_form_invalid from django import shortcuts from django.contrib import messages from django.contrib.auth import REDIRECT_FIELD_NAME @@ -15,11 +14,12 @@ from django.contrib.auth.views import LoginView from django.http import HttpResponseRedirect from django.utils.translation import gettext as _ from django.views.decorators.http import require_POST +from django.views.generic.edit import FormView -from plinth import translation, utils, web_framework +from plinth import translation from . import privileged -from .forms import AuthenticationForm, CaptchaAuthenticationForm +from .forms import AuthenticationForm, CaptchaForm PRIVATE_KEY_FILE_NAME = 'privkey.pem' SSO_COOKIE_NAME = 'auth_pubtkt' @@ -58,39 +58,23 @@ class SSOLoginView(LoginView): return response - # XXX: Use axes middleware and authentication backend instead of - # axes_form_invalid when axes >= 5.0.0 becomes available in Debian stable. - @axes_form_invalid - def form_invalid(self, *args, **kwargs): - """Trigger django-axes logic to deal with too many attempts.""" - return super().form_invalid(*args, **kwargs) +class CaptchaView(FormView): + """A simple form view with a CAPTCHA image. -class CaptchaLoginView(LoginView): - """A login view with mandatory CAPTCHA image.""" + When a user performs too many login attempts, they will no longer be able + to login with the typical login view. They will be redirected to this view. + On successfully solving the CAPTCHA in this form, their ability to use the + login form will be reset. + """ - redirect_authenticated_user = True - template_name = 'login.html' - form_class = CaptchaAuthenticationForm + template_name = 'captcha.html' + form_class = CaptchaForm - def dispatch(self, request, *args, **kwargs): - """Handle a request and return a HTTP response.""" - response = super().dispatch(request, *args, **kwargs) - if not request.POST: - return response - - if not request.user.is_authenticated: - return response - - # Successful authentication - if utils.is_axes_old(): - 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) + def form_valid(self, form): + """Reset login attempts and redirect to login page.""" + axes.utils.reset_request(self.request) + return shortcuts.redirect('users:login') @require_POST diff --git a/plinth/modules/users/forms.py b/plinth/modules/users/forms.py index e221e9f1a..41df46aa1 100644 --- a/plinth/modules/users/forms.py +++ b/plinth/modules/users/forms.py @@ -398,7 +398,9 @@ class FirstBootForm(ValidNewUsernameCheckMixin, auth.forms.UserCreationForm): def login_user(self, username, password): """Try to login the user with the credentials provided""" try: - user = auth.authenticate(username=username, password=password) + # Django axes requires the request attribute + user = auth.authenticate(request=self.request, username=username, + password=password) auth.login(self.request, user) except Exception: pass diff --git a/plinth/modules/users/urls.py b/plinth/modules/users/urls.py index 7bce71b59..2e721b5d5 100644 --- a/plinth/modules/users/urls.py +++ b/plinth/modules/users/urls.py @@ -3,11 +3,10 @@ URLs for the Users module """ -from axes.decorators import axes_dispatch from django.urls import re_path from stronghold.decorators import public -from plinth.modules.sso.views import CaptchaLoginView, SSOLoginView, logout +from plinth.modules.sso.views import CaptchaView, SSOLoginView, logout from plinth.utils import non_admin_view from . import views @@ -24,14 +23,11 @@ urlpatterns = [ name='change_password'), # Authnz is handled by SSO - - # XXX: Use axes authentication backend and middleware instead of - # axes_dispatch after axes 5.x becomes available in Debian stable. - re_path(r'^accounts/login/$', - public(axes_dispatch(SSOLoginView.as_view())), name='login'), + re_path(r'^accounts/login/$', public(SSOLoginView.as_view()), + name='login'), re_path(r'^accounts/logout/$', public(logout), name='logout'), re_path(r'^users/firstboot/$', public(views.FirstBootView.as_view()), name='firstboot'), - re_path(r'accounts/login/locked/$', public(CaptchaLoginView.as_view()), + re_path(r'accounts/login/locked/$', public(CaptchaView.as_view()), name='locked_out'), ] diff --git a/plinth/settings.py b/plinth/settings.py index 278ebe247..882fe688a 100644 --- a/plinth/settings.py +++ b/plinth/settings.py @@ -48,9 +48,21 @@ AUTH_PASSWORD_VALIDATORS = [ }, ] +AUTHENTICATION_BACKENDS = [ + # AxesStandaloneBackend should be the first backend. If the user is locked + # out due to too many attempts, the backend denies further attempts untill + # unlocked by a CAPTCHA form. + 'axes.backends.AxesStandaloneBackend', + + # Django ModelBackend is the default authentication backend. + 'django.contrib.auth.backends.ModelBackend', +] + AXES_LOCKOUT_URL = 'locked/' -AXES_RESET_ON_SUCCESS = True # Only used with axes >= 4.4.3 +AXES_RESET_ON_SUCCESS = True + +AXES_VERBOSE = False CACHES = { 'default': { @@ -128,6 +140,11 @@ MIDDLEWARE = ( 'plinth.middleware.FirstSetupMiddleware', 'plinth.modules.first_boot.middleware.FirstBootMiddleware', 'plinth.middleware.SetupMiddleware', + + # AxesMiddleware should be the last middleware. It only formats user + # lockout messages and renders Axes lockout responses on failed user + # authentication attempts from login views. + 'axes.middleware.AxesMiddleware', ) PASSWORD_HASHERS = [ diff --git a/plinth/utils.py b/plinth/utils.py index 6f0f6043b..d7804766e 100644 --- a/plinth/utils.py +++ b/plinth/utils.py @@ -14,7 +14,7 @@ import markupsafe import ruamel.yaml from django.utils.functional import lazy -from plinth.version import Version +from plinth.version import Version # noqa def import_from_gi(library, version): @@ -169,23 +169,6 @@ def is_non_empty_file(file_path): return os.path.isfile(file_path) and os.path.getsize(file_path) > 0 -def is_axes_old(): - """Return true if using django-axes version strictly less than 5.0.0. - - XXX: Remove this method and allow code that uses it after django-axes >= - 5.0.0 becomes available in Debian stable. - - """ - import axes - try: - version = axes.get_version() - except AttributeError: - # axes.get_version() was removed in 5.0.13 - return False - - return Version(version) < Version('5.0') - - def is_authenticated_user(username, password): """Return true if the user authentication succeeds.""" import pam # Minimize dependencies for running tests diff --git a/plinth/web_framework.py b/plinth/web_framework.py index 9fc8b6433..691b0228a 100644 --- a/plinth/web_framework.py +++ b/plinth/web_framework.py @@ -129,14 +129,3 @@ 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