fix(admin): a forced password change cannot trap a user or be skipped with a token

- The administrator's password form took "password-based
  authentication: Disabled" with "must change it at the next login":
  the change form asks for the old password, which the user then never
  had, so they could not get past it. That combination is refused, and
  an administrator cannot disable their own password.
- A user who must change the password could still use the API with
  their tokens. A token is now refused until they change it, as the
  pages are.

accounts/auth.py

@@ -3,8 +3,12 @@
 from datetime import timedelta
 
 from django.utils import timezone
+from django.utils.translation import gettext_lazy as _
 from drf_spectacular.contrib.knox_auth_token import KnoxTokenScheme
 from knox.auth import TokenAuthentication as KnoxTokenAuthentication
+from rest_framework import exceptions
+
+from accounts.models import UserAttributes
 
 # a token's last use is written at most once a minute, not on every request
 LAST_USED_EVERY = timedelta(minutes=1)
@@ -13,6 +17,9 @@ LAST_USED_EVERY = timedelta(minutes=1)
 class TokenAuthentication(KnoxTokenAuthentication):
     def validate_user(self, auth_token):
         user, token = super().validate_user(auth_token)
+        # as the pages do (ForcePasswordChangeMiddleware), until the user changes the password
+        if UserAttributes.objects.filter(user=user, must_change_password=True).exists():
+            raise exceptions.AuthenticationFailed(_("Change your password before using the API."))
         now = timezone.now()
         if token.last_used is None or now - token.last_used >= LAST_USED_EVERY:
             type(token).objects.filter(pk=token.pk).update(last_used=now)

accounts/tests/test_api_tokens.py

@@ -5,7 +5,7 @@ import re
 from datetime import timedelta
 from unittest.mock import patch
 
-from accounts.models import ApiToken
+from accounts.models import ApiToken, UserAttributes
 from django.contrib.auth import get_user_model
 from django.db import connection
 from django.test import TestCase
@@ -116,6 +116,13 @@ class ApiTokenTests(TestCase):
         self.client.force_login(self.user)
         self.assertIn("Last used", self.client.get(reverse("accounts:profile")).content.decode())
 
+    def test_a_token_stops_while_its_user_must_change_the_password(self):
+        token = self.make()
+        UserAttributes.objects.update_or_create(user=self.user, defaults={"must_change_password": True})
+        self.assertEqual(self.api(token).status_code, 401)
+        UserAttributes.objects.filter(user=self.user).update(must_change_password=False)
+        self.assertEqual(self.api(token).status_code, 200)
+
     def test_a_user_has_at_most_ten_tokens(self):
         for number in range(10):
             self.make(name=f"t{number}")

admin/forms.py

@@ -133,3 +133,10 @@ class AdminPasswordForm(AdminPasswordChangeForm):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.fields["must_change_password"] = self.fields.pop("must_change_password")  # last, after Django's own
+
+    def clean(self):
+        cleaned = super().clean()
+        # the change form asks for the old password: a user without one could never pass it
+        if cleaned.get("must_change_password") and not cleaned.get("set_usable_password", True):
+            raise ValidationError(_("A user without a password cannot be made to change it"))
+        return cleaned

admin/tests/test_user_pages.py

@@ -79,3 +79,19 @@ class UserPagesTests(TestCase):
         self.client.force_login(self.admin)
         self.client.post(url, {"password1": "New-pw-4567y", "password2": "New-pw-4567y"})
         self.assertFalse(UserAttributes.objects.get(user=self.alice).must_change_password)
+
+    def test_a_user_without_a_password_cannot_be_made_to_change_it(self):
+        # the change form asks for the old password: one they never had
+        url = reverse("admin:user_update_password", args=[self.alice.pk])
+        response = self.client.post(url, {"usable_password": "false", "must_change_password": "on"})
+        self.assertContains(response, "A user without a password cannot be made to change it")
+        self.alice.refresh_from_db()
+        self.assertTrue(self.alice.has_usable_password())
+
+    def test_an_administrator_keeps_their_own_password(self):
+        url = reverse("admin:user_update_password", args=[self.admin.pk])
+        response = self.client.post(url, {"usable_password": "false"})
+        self.assertContains(response, "You cannot block, delete or demote your own account")
+        self.admin.refresh_from_db()
+        self.assertTrue(self.admin.has_usable_password())
+

admin/views.py

@@ -187,7 +187,11 @@ def user_update_password(request, pk):
     user = get_object_or_404(User, pk=pk)
     if request.method == "POST":
         form = forms.AdminPasswordForm(user, request.POST)
-        if form.is_valid():
+        valid = form.is_valid()
+        if valid and user == request.user and not form.cleaned_data["set_usable_password"]:
+            form.add_error(None, OWN_ACCOUNT)
+            valid = False
+        if valid:
             with audit(request, None, f"User {user.username}", "set password"), transaction.atomic():
                 user = form.save()
                 UserAttributes.objects.update_or_create(