Add a validator to the /admin/ form for changing email addresses
authorMagnus Hagander <magnus@hagander.net>
Fri, 29 Jan 2021 10:17:14 +0000 (11:17 +0100)
committerMagnus Hagander <magnus@hagander.net>
Fri, 29 Jan 2021 10:17:14 +0000 (11:17 +0100)
As Jonathan recently demonstrated, it was possible to change the primary
email address on one account to be the secondary email address of
another one when using /admin/ (it was prevented when using the user
facing forms), which then caused conflicts in the change track and
replication steps later.

Add a validator to the admin form to avoid this mistake in the future.

This doesn't entirely remove all possibilities, so we should consider
adding a database trigger to enforce it as well, but it closes the most
obvious hole and makes the other ones much harder to hit (but you can
still cause the same damage through psql or so, of course).

pgweb/account/admin.py

index 0e8210b9bb0a17be99c548cb8e4c762de0c735ff..93d73fed0945bba7b46cd98af8052944eaa2fca4 100644 (file)
@@ -1,4 +1,5 @@
 from django.contrib import admin
+from django.core.validators import ValidationError
 from django.contrib.auth.admin import UserAdmin
 from django.contrib.auth.forms import UserChangeForm
 from django.contrib.auth.models import User
@@ -96,6 +97,15 @@ class PGUserChangeForm(UserChangeForm):
         else:
             return "Unknown"
 
+    def clean_email(self):
+        e = self.cleaned_data['email'].lower()
+        if User.objects.filter(email=e).exclude(pk=self.instance.pk):
+            raise ValidationError("There already exists a different user with this address")
+        if SecondaryEmail.objects.filter(email=e):
+            raise ValidationError("This address is already a secondary address attached to a user")
+
+        return e
+
 
 class PGUserAdmin(UserAdmin):
     """overrides default Django user admin"""