Harden user edit form authorization

dojo/user/ui/forms.py

@@ -57,6 +57,27 @@ def __init__(self, *args, **kwargs):
             self.fields["is_staff"].disabled = True
             self.fields["is_superuser"].disabled = True
 
+    def clean(self):
+        cleaned_data = super().clean()
+        # Only a superuser may change the username or email of an account other
+        # than their own: rewriting a victim's email and triggering a password
+        # reset is an account-takeover vector. Mirrors UserSerializer.validate()
+        # on the API path.
+        current_user = get_current_user()
+        if (
+            current_user is not None
+            and not current_user.is_superuser
+            and self.instance.pk is not None
+            and self.instance.pk != current_user.pk
+        ):
+            for identity_field in ("username", "email"):
+                if identity_field in cleaned_data and cleaned_data[identity_field] != getattr(self.instance, identity_field):
+                    self.add_error(
+                        identity_field,
+                        _("Only superusers are allowed to change the username or email of another user."),
+                    )
+        return cleaned_data
+
 
 class DeleteUserForm(forms.ModelForm):
     id = forms.IntegerField(required=True,

unittests/test_user_ui_identity_authz.py

@@ -0,0 +1,73 @@
+from crum import set_current_user
+from django.contrib.auth.models import Permission
+from django.urls import reverse
+
+from dojo.models import Dojo_User
+from dojo.user.ui.forms import EditDojoUserForm
+from unittests.dojo_test_case import DojoTestCase
+
+
+class UserUIIdentityFieldAuthzTest(DojoTestCase):
+
+    """
+    UI twin of test_apiv2_user_identity_authz. A non-superuser holding the
+    user-management configuration permission must not be able to change the
+    identity fields (username/email) of another account through the classic
+    edit_user view / EditDojoUserForm; changing a victim's email enables
+    account takeover via the password-reset flow.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.delegate = Dojo_User.objects.create(username="ui_identity_delegate", is_active=True)
+        cls.delegate.user_permissions.add(
+            Permission.objects.get(codename="view_user", content_type__app_label="auth"),
+            Permission.objects.get(codename="change_user", content_type__app_label="auth"),
+        )
+        cls.superuser = Dojo_User.objects.create(username="ui_identity_super", is_active=True, is_superuser=True, is_staff=True)
+        cls.target = Dojo_User.objects.create(username="ui_identity_target", email="target@example.com", is_active=True)
+
+    def tearDown(self):
+        set_current_user(None)
+        super().tearDown()
+
+    def _edit_data(self, **overrides):
+        data = {"username": self.target.username, "email": self.target.email, "is_active": "on"}
+        data.update(overrides)
+        return data
+
+    # form-level guard
+
+    def test_form_blocks_delegate_changing_another_email(self):
+        set_current_user(self.delegate)
+        form = EditDojoUserForm(self._edit_data(email="attacker@evil.example"), instance=self.target)
+        self.assertFalse(form.is_valid())
+        self.assertIn("email", form.errors)
+
+    def test_form_blocks_delegate_changing_another_username(self):
+        set_current_user(self.delegate)
+        form = EditDojoUserForm(self._edit_data(username="hijacked"), instance=self.target)
+        self.assertFalse(form.is_valid())
+        self.assertIn("username", form.errors)
+
+    def test_form_allows_superuser_changing_another_email(self):
+        set_current_user(self.superuser)
+        form = EditDojoUserForm(self._edit_data(email="newby-admin@example.com"), instance=self.target)
+        self.assertTrue(form.is_valid(), form.errors)
+
+    def test_form_allows_delegate_changing_own_email(self):
+        set_current_user(self.delegate)
+        data = {"username": self.delegate.username, "email": "mynew@example.com", "is_active": "on"}
+        form = EditDojoUserForm(data, instance=self.delegate)
+        self.assertTrue(form.is_valid(), form.errors)
+
+    # end-to-end: the reported PoC no longer changes the victim's email
+
+    def test_delegate_post_cannot_change_target_email(self):
+        self.client.force_login(self.delegate)
+        self.client.post(
+            reverse("edit_user", args=(self.target.id,)),
+            self._edit_data(email="attacker@evil.example"),
+        )
+        self.target.refresh_from_db()
+        self.assertEqual(self.target.email, "target@example.com")