From e94d69a524074262a36dbafbdc3faee630a600f5 Mon Sep 17 00:00:00 2001 From: catborise Date: Sat, 3 Oct 2026 15:51:09 +0300 Subject: [PATCH] feat(accounts): Django's LoginRequiredMiddleware replaces the package django-login-required-middleware is dropped; Django 5.1+ ships the same middleware. The login page is already marked login_not_required by Django; email_otp gets the decorator, which replaces LOGIN_REQUIRED_IGNORE_VIEW_NAMES. The datasource still requires login. Existing settings.py files still name the old middleware. A system check (accounts.E002) stops migrate and the server until MIDDLEWARE uses Django's middleware, instead of an ImportError at startup; the README has the steps. A new matrix test walks every URL anonymously: everything outside a short allowlist (login pages, the OpenAPI docs) sends the visitor to the login page, and API views deny with 401/403. It passed with the old middleware first and passes unchanged now. --- README.md | 7 ++++- accounts/checks.py | 17 +++++++++++++ accounts/test_checks.py | 23 +++++++++++++++++ accounts/views.py | 3 ++- conf/requirements.txt | 1 - webvirtcloud/settings.py.template | 3 +-- webvirtcloud/test_authorization_matrix.py | 31 +++++++++++++++++++++++ 7 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 accounts/test_checks.py diff --git a/README.md b/README.md index 2ad82c6d..280679d3 100644 --- a/README.md +++ b/README.md @@ -396,7 +396,12 @@ environment=CSRF_TRUSTED_ORIGINS="https://webvirtcloud.example.com" Back up `db.sqlite3` before updating (in Docker it lives in the `data` volume). -Upgrading from Django 4.2: delete `USE_L10N` from `webvirtcloud/settings.py` (Django 5 ignores it). Logging out now needs a POST; links or scripts that call `/accounts/logout/` with GET get 405. +Upgrading from Django 4.2, edit `webvirtcloud/settings.py`: + +- in `MIDDLEWARE`, replace `"login_required.middleware.LoginRequiredMiddleware"` with `"django.contrib.auth.middleware.LoginRequiredMiddleware"` (the old package is no longer a dependency; `migrate` refuses to run until this is done); +- delete `LOGIN_REQUIRED_IGNORE_VIEW_NAMES` and `USE_L10N`. + +Logging out now needs a POST; links or scripts that call `/accounts/logout/` with GET get 405. Before running migrations, add `accounts.middleware.ForcePasswordChangeMiddleware` to `MIDDLEWARE` in your existing `webvirtcloud/settings.py`, after diff --git a/accounts/checks.py b/accounts/checks.py index ca8ad8fe..d001eaa9 100644 --- a/accounts/checks.py +++ b/accounts/checks.py @@ -19,3 +19,20 @@ def password_change_middleware_check(app_configs, **kwargs): ) ] return [] + + +@register(Tags.security) +def login_required_middleware_check(app_configs, **kwargs): + middleware = list(settings.MIDDLEWARE) + required = "django.contrib.auth.middleware.LoginRequiredMiddleware" + old = "login_required.middleware.LoginRequiredMiddleware" + if old in middleware or required not in middleware: + return [ + Error( + "MIDDLEWARE must use Django's LoginRequiredMiddleware.", + hint=f"In webvirtcloud/settings.py replace '{old}' with '{required}' " + "and delete LOGIN_REQUIRED_IGNORE_VIEW_NAMES; see README.md.", + id="accounts.E002", + ) + ] + return [] diff --git a/accounts/test_checks.py b/accounts/test_checks.py new file mode 100644 index 00000000..a49cd916 --- /dev/null +++ b/accounts/test_checks.py @@ -0,0 +1,23 @@ +from django.conf import settings +from django.test import SimpleTestCase, override_settings + +from accounts.checks import login_required_middleware_check + +OLD = "login_required.middleware.LoginRequiredMiddleware" +NEW = "django.contrib.auth.middleware.LoginRequiredMiddleware" + + +class LoginRequiredMiddlewareCheckTestCase(SimpleTestCase): + def ids(self, middleware): + with override_settings(MIDDLEWARE=middleware): + return [e.id for e in login_required_middleware_check(None)] + + def test_current_settings_pass(self): + self.assertEqual(self.ids(settings.MIDDLEWARE), []) + + def test_settings_from_before_django_5_2_are_rejected(self): + old = [OLD if m == NEW else m for m in settings.MIDDLEWARE] + self.assertEqual(self.ids(old), ["accounts.E002"]) + + def test_missing_login_middleware_is_rejected(self): + self.assertEqual(self.ids([m for m in settings.MIDDLEWARE if m != NEW]), ["accounts.E002"]) diff --git a/accounts/views.py b/accounts/views.py index 152a726b..8cb386bd 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -5,7 +5,7 @@ from admin.decorators import superuser_only from django.conf import settings from django.contrib import messages from django.contrib.auth import get_user_model, update_session_auth_hash, login as auth_login -from django.contrib.auth.decorators import login_required +from django.contrib.auth.decorators import login_not_required, login_required from django.contrib.auth.forms import PasswordChangeForm from django.core.exceptions import PermissionDenied from django.db import transaction @@ -201,6 +201,7 @@ def user_instance_delete(request, pk): ) +@login_not_required def email_otp(request): form = EmailOTPForm(request.POST or None) if form.is_valid(): diff --git a/conf/requirements.txt b/conf/requirements.txt index 30d0260f..c59811f5 100644 --- a/conf/requirements.txt +++ b/conf/requirements.txt @@ -1,7 +1,6 @@ Django==5.2.17 django_bootstrap5==26.2 django-bootstrap-icons==0.9.0 -django-login-required-middleware==0.9.0 django-otp==1.7.3 django-qr-code==4.2.0 django-auth-ldap==5.3.0 diff --git a/webvirtcloud/settings.py.template b/webvirtcloud/settings.py.template index 9a35b25a..75941bd4 100644 --- a/webvirtcloud/settings.py.template +++ b/webvirtcloud/settings.py.template @@ -78,7 +78,7 @@ MIDDLEWARE = [ "django.middleware.csrf.CsrfViewMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", "django_otp.middleware.OTPMiddleware", - "login_required.middleware.LoginRequiredMiddleware", + "django.contrib.auth.middleware.LoginRequiredMiddleware", "accounts.middleware.ForcePasswordChangeMiddleware", "django.contrib.auth.middleware.RemoteUserMiddleware", "django.contrib.messages.middleware.MessageMiddleware", @@ -293,7 +293,6 @@ SHOW_PROFILE_EDIT_PASSWORD = True # OTP Config OTP_ENABLED = False -LOGIN_REQUIRED_IGNORE_VIEW_NAMES = ["accounts:login", "accounts:email_otp"] # REST Framework & OpenAPI (drf-spectacular) REST_FRAMEWORK = { diff --git a/webvirtcloud/test_authorization_matrix.py b/webvirtcloud/test_authorization_matrix.py index 26a05acd..566b10ac 100644 --- a/webvirtcloud/test_authorization_matrix.py +++ b/webvirtcloud/test_authorization_matrix.py @@ -55,6 +55,18 @@ ALLOWED_FOR_ANY_USER = { "schema-swagger-ui": "API docs (SERVE_PERMISSIONS is S-21, Wave 3)", } +# URL name -> why an anonymous visitor may reach it. Everything else must +# redirect to the login page. +ALLOWED_FOR_ANONYMOUS = { + "accounts:login": "login page", + "accounts:email_otp": "part of the login flow (S-12 tracks its own issues)", + "rest_framework:login": "DRF browsable API login page", + "schema": "OpenAPI schema (SERVE_PERMISSIONS is S-21, Wave 3)", + "schema-json": "OpenAPI schema (SERVE_PERMISSIONS is S-21, Wave 3)", + "schema-redoc": "API docs (SERVE_PERMISSIONS is S-21, Wave 3)", + "schema-swagger-ui": "API docs (SERVE_PERMISSIONS is S-21, Wave 3)", +} + # Endpoints that expose a VM's console (or its VNC password) and therefore # must follow the console rule: superuser or VM owner, not global view_instances. CONSOLE_ENDPOINTS = ["instances:getvvfile", "vdi_url"] @@ -210,6 +222,25 @@ class AuthorizationMatrixTestCase(TestCase): "Reachable by a user without any VM/compute relation:\n" + "\n".join(reachable), ) + def test_anonymous_visitor_is_sent_to_login_everywhere_else(self): + from django.conf import settings + + reachable = [] + for name, params in iter_patterns(get_resolver().url_patterns): + if name in ALLOWED_FOR_ANONYMOUS: + continue + url = self._url(name, params) + for method in (self.client.get, self.client.post): + self.client.logout() + response = method(url) + # DRF views opt out of login middleware and deny by their own + # permission classes. + api_denied = url.startswith("/api/") and response.status_code in (401, 403) + to_login = response.status_code == 302 and response["Location"].startswith(settings.LOGIN_URL) + if not (api_denied or to_login): + reachable.append(f"{method.__name__.upper()} {name} ({url}) -> {response.status_code}") + self.assertEqual(reachable, [], "Reachable without logging in:\n" + "\n".join(reachable)) + def test_allowlist_only_names_existing_urls(self): names = {name for name, _ in iter_patterns(get_resolver().url_patterns)} self.assertEqual(sorted(set(ALLOWED_FOR_ANY_USER) - names), [])