Merge pull request #736 from maykinmedia/feature/1672-email-unique

[#1672] Enabled non-unique emails for digid users

src/open_inwoner/accounts/backends.py

@@ -26,14 +26,28 @@ def authenticate(
         self, request, username=None, password=None, user=None, token=None, **kwargs
     ):
         config = SiteConfiguration.get_solo()
-
+        User = get_user_model()
         if username and password and not config.login_2fa_sms:
             try:
-                user = get_user_model().objects.get(email__iexact=username)
-                if check_password(password, user.password):
+                user = User.objects.get(
+                    email__iexact=username,
+                    login_type=LoginTypeChoices.default,
+                )
+                if check_password(
+                    password, user.password
+                ) and self.user_can_authenticate(user):
                     return user
-            except get_user_model().DoesNotExist:
+            except User.MultipleObjectsReturned:
+                # Found multiple users with this email (shouldn't happen if we added checks)
+                # Run the default password hasher once to reduce the timing
+                # difference between an existing and a nonexistent user (#20760).
+                User().set_password(password)
+                return None
+            except User.DoesNotExist:
                 # No user was found, return None - triggers default login failed
+                # Run the default password hasher once to reduce the timing
+                # difference between an existing and a nonexistent user (#20760).
+                User().set_password(password)
                 return None
 
         # 2FA with sms verification
@@ -57,18 +71,29 @@ def authenticate(self, request=None, *args, **kwargs):
 
 class CustomOIDCBackend(OIDCAuthenticationBackend):
     def create_user(self, claims):
-        """Return object for a newly created user account."""
+        """
+        Return object for a newly created user account.
+
+        before we got here we already checked for existing users based on the overriden queryset from the .filter_users_by_claims()
+        """
         unique_id = self.retrieve_identifier_claim(claims)
 
-        email = generate_email_from_string(unique_id)
         if "email" in claims:
             email = claims["email"]
+        else:
+            email = generate_email_from_string(unique_id)
 
-        existing_user = self.UserModel.objects.filter(email__iexact=email).first()
+        existing_user = self.UserModel.objects.filter(
+            email__iexact=email,
+            login_type=LoginTypeChoices.default,
+            is_active=True,
+        ).first()
         if existing_user:
             logger.debug("Updating OIDC user: %s with email %s", unique_id, email)
             existing_user.oidc_id = unique_id
             existing_user.login_type = LoginTypeChoices.oidc
+            # TODO verify we want unusable_password
+            existing_user.set_unusable_password()
             existing_user.save()
             return existing_user
         else:
@@ -79,9 +104,10 @@ def create_user(self, claims):
                 "email": email,
                 "login_type": LoginTypeChoices.oidc,
             }
-
             user = self.UserModel.objects.create_user(**kwargs)
             self.update_user(user, claims)
+            # TODO verify we want unusable_password
+            user.set_unusable_password()
 
             return user
 

src/open_inwoner/accounts/forms.py

@@ -1,9 +1,12 @@
+from typing import Optional
+
 from django import forms
 from django.conf import settings
 from django.contrib.auth import authenticate
 from django.contrib.auth.forms import PasswordResetForm
 from django.core.exceptions import ValidationError
 from django.core.mail import EmailMultiAlternatives
+from django.db.models import Q
 from django.template import loader
 from django.utils.translation import ugettext_lazy as _
 
@@ -120,18 +123,6 @@ def __init__(self, *args, **kwargs):
         else:
             self.fields["phonenumber"].required = True
 
-    def clean_email(self):
-        email = self.cleaned_data["email"]
-
-        existing_user = User.objects.filter(email__iexact=email).first()
-        if not existing_user:
-            return email
-
-        if existing_user.is_active:
-            raise ValidationError(_("The user with this email already exists"))
-
-        raise ValidationError(_("This user has been deactivated"))
-
 
 class BaseUserForm(forms.ModelForm):
     class Meta:
@@ -153,6 +144,10 @@ def __init__(self, user, *args, **kwargs):
 
 
 class UserForm(BaseUserForm):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.user = kwargs.pop("user")
+
     class Meta:
         model = User
         fields = (
@@ -214,17 +209,13 @@ def __init__(self, user, *args, **kwargs):
             if not user.infix:
                 del self.fields["infix"]
 
-    def clean_email(self):
-        email = self.cleaned_data["email"]
-
-        is_existing_user = User.objects.filter(email__iexact=email).exists()
-        if is_existing_user:
-            raise ValidationError(_("The user with this email already exists"))
-
-        return email
-
 
 class CustomPasswordResetForm(PasswordResetForm):
+    def get_users(self, email):
+        users = super().get_users(email)
+        # filter regular email login users
+        return [u for u in users if u.login_type == LoginTypeChoices.default]
+
     def send_mail(
         self,
         subject_template_name,
@@ -236,28 +227,26 @@ def send_mail(
     ):
         """
         Send a django.core.mail.EmailMultiAlternatives to `to_email`.
+
+        Note: the context has the user specific information / tokens etc
         """
-        email = self.cleaned_data.get("email")
-        user = User.objects.get(email=email)
-
-        if user.login_type == LoginTypeChoices.default:
-            subject = loader.render_to_string(subject_template_name, context)
-            # Email subject *must not* contain newlines
-            subject = "".join(subject.splitlines())
-            body = loader.render_to_string(email_template_name, context)
-
-            email_message = EmailMultiAlternatives(
-                subject,
-                body,
-                from_email,
-                [to_email],
-                headers={"X-Mail-Queue-Priority": "now"},
-            )
-            if html_email_template_name is not None:
-                html_email = loader.render_to_string(html_email_template_name, context)
-                email_message.attach_alternative(html_email, "text/html")
+        subject = loader.render_to_string(subject_template_name, context)
+        # Email subject *must not* contain newlines
+        subject = "".join(subject.splitlines())
+        body = loader.render_to_string(email_template_name, context)
+
+        email_message = EmailMultiAlternatives(
+            subject,
+            body,
+            from_email,
+            [to_email],
+            headers={"X-Mail-Queue-Priority": "now"},
+        )
+        if html_email_template_name is not None:
+            html_email = loader.render_to_string(html_email_template_name, context)
+            email_message.attach_alternative(html_email, "text/html")
 
-            email_message.send()
+        email_message.send()
 
 
 class CategoriesForm(forms.ModelForm):
@@ -316,24 +305,60 @@ def __init__(self, user, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
     def clean(self):
+        """
+        Note cleaning and lookup is a bit convoluted as we have to deal with non-unique emails:
+         - adding multiple contacts at same time
+         - users adding their own email, to add their other account as contact
+
+        But we still want to provide some error feedback
+        """
         cleaned_data = super().clean()
         email = cleaned_data.get("email")
-
-        if email:
-            if email == self.user.email:
-                raise ValidationError(
-                    _("Please enter a valid email address of a contact.")
-                )
-
-            if self.user.is_email_of_contact(email):
+        if not email:
+            return
+
+        # use sets for simplicity, and use .only("id")
+        existing_users = set(User.objects.filter(email__iexact=email))
+        user_contacts = set(self.user.user_contacts.all().only("id"))
+        contacts_for_approval = set(self.user.contacts_for_approval.all().only("id"))
+
+        # check if this was our own email and if we just found ourselves
+        if self.user in existing_users:
+            existing_users.remove(self.user)
+            if not existing_users:
+                raise ValidationError(_("You cannot add yourself as a contact."))
+
+        if not existing_users:
+            # no users found, pass and let the view send an Invite to the email
+            return
+
+        # best effort, we're going to return successful if we find at least one good contact
+        #   or only report the worst error (to not confuse the end-user)
+        not_active = False
+        has_contact = False
+        added_contacts = set()
+
+        # check if these users are valid and not already added
+        for contact_user in existing_users:
+            if not contact_user.is_active:
+                not_active = True
+            elif contact_user in user_contacts or contact_user in contacts_for_approval:
+                has_contact = True
+            else:
+                added_contacts.add(contact_user)
+
+        # remember the contacts and let the view add records, logs and the emails
+        if added_contacts:
+            cleaned_data["added_contacts"] = added_contacts
+        else:
+            # produce some feedback, check most interesting first
+            if has_contact:
                 raise ValidationError(
                     _(
                         "Het ingevoerde e-mailadres komt al voor in uw contactpersonen. Pas de gegevens aan en probeer het opnieuw."
                     )
                 )
-
-            existing_user = User.objects.filter(email__iexact=email)
-            if existing_user and existing_user.get().is_not_active():
+            elif not_active:
                 raise ValidationError(
                     _("The user cannot be added, their account has been deleted.")
                 )

src/open_inwoner/accounts/managers.py

@@ -33,6 +33,8 @@ def get_by_rsin(self, rsin):
         return self.get_queryset().get(rsin=rsin)
 
     def eherkenning_create(self, rsin, **kwargs):
+        raise NotImplementedError("old code, please verify before use")
+        # TODO what is this @rsin.com email hostname? @example.org is bad enough but this actually exists
         return super().create(
             email="user-{}@rsin.com".format(rsin),
             login_type=LoginTypeChoices.eherkenning,

src/open_inwoner/accounts/migrations/0064_alter_user_email.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.20 on 2023-08-18 12:41
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("accounts", "0063_alter_user_phonenumber"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="user",
+            name="email",
+            field=models.EmailField(max_length=254, verbose_name="Email address"),
+        ),
+    ]

src/open_inwoner/accounts/migrations/0065_user_unique_email_when_not_digid.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.2.20 on 2023-09-07 09:00
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("accounts", "0064_alter_user_email"),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name="user",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(("login_type", "digid"), _negated=True),
+                fields=("email",),
+                name="unique_email_when_not_digid",
+            ),
+        ),
+    ]