mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 11:16:47 +00:00
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.
This commit is contained in:
parent
920f318601
commit
e94d69a524
7 changed files with 80 additions and 5 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 []
|
||||
|
|
|
|||
23
accounts/test_checks.py
Normal file
23
accounts/test_checks.py
Normal file
|
|
@ -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"])
|
||||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 = {
|
||||
|
|
|
|||
|
|
@ -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), [])
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue