[#1672] WIP for CI

src/open_inwoner/accounts/backends.py

@@ -26,14 +26,23 @@ 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)
+                user = User.objects.get(
+                    email__iexact=username,
+                    login_type=LoginTypeChoices.default,
+                    is_active=True,
+                )
                 if check_password(password, user.password):
                     return user
-            except get_user_model().DoesNotExist:
+            except User.MultipleObjectsReturned:
+                # Found multiple users with this email (shouldn't happen if we added checks)
+                # TODO regular backends have timing-attack hardening here
+                return None
+            except User.DoesNotExist:
                 # No user was found, return None - triggers default login failed
+                # TODO regular backends have timing-attack hardening here
                 return None
 
         # 2FA with sms verification
@@ -57,18 +66,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 +99,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

@@ -211,6 +211,11 @@ def __init__(self, user, *args, **kwargs):
 
 
 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,
@@ -222,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):
@@ -322,6 +325,9 @@ def find_user(
     ):
         """Try to find a user by email alone, or email + first_name + last_name"""
 
+        raise NotImplemented("this feels weird")
+        # TODO fix contact create form
+
         existing_users = User.objects.filter(email__iexact=email)
 
         # no user found: return, then send invitation (no ValidationError raised)

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/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",
+            ),
+        ),
+    ]

src/open_inwoner/accounts/models.py

@@ -7,6 +7,7 @@
 from django.contrib.contenttypes.fields import GenericRelation
 from django.core.exceptions import ValidationError
 from django.db import models
+from django.db.models import CheckConstraint, Q, UniqueConstraint
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.crypto import get_random_string
@@ -114,6 +115,7 @@ class User(AbstractBaseUser, PermissionsMixin):
     date_joined = models.DateTimeField(
         verbose_name=_("Date joined"), default=timezone.now
     )
+    # TODO shouldn't rsin & bsn be unique?
     rsin = models.CharField(verbose_name=_("Rsin"), max_length=9, null=True, blank=True)
     bsn = NLBSNField(verbose_name=_("Bsn"), null=True, blank=True)
     login_type = models.CharField(
@@ -219,44 +221,74 @@ class Meta:
         verbose_name = _("User")
         verbose_name_plural = _("Users")
 
+        # TODO enforce email/is_active/login_type unique constraints on database?
+        constraints = [
+            UniqueConstraint(
+                fields=["email"],
+                condition=~Q(login_type=LoginTypeChoices.digid),
+                name="unique_email_when_not_digid",
+            ),
+            # CheckConstraint(
+            #     # maybe this is not correct?
+            #     check=(Q(bsn="") | Q(bsn__isnull=True))
+            #     & ~Q(login_type=LoginTypeChoices.digid),
+            #     name="check_digid_bsn_required_when_digid",
+            # ),
+            # UniqueConstraint(
+            #     fields=["bsn"],
+            #     condition=Q(login_type=LoginTypeChoices.digid),
+            #     name="unique_bsn_when_digid",
+            # ),
+            # ideally we'd have a lot more here
+        ]
+
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self._old_bsn = self.bsn
 
     def __str__(self):
-        return (
-            f"{self.first_name} {self.last_name} ({self.get_contact_email()})".strip()
-        )
+        name = self.get_full_name()
+        email = self.get_contact_email()
+        if name and email:
+            return f"{name} ({email})"
+        else:
+            return name or email or str(self.uuid)[:8]
 
     def clean(self, *args, **kwargs):
         """Reject non-unique emails, except for users with login_type DigiD"""
 
-        existing_users = self.__class__.objects.filter(email__iexact=self.email)
+        existing_users = User.objects.filter(email__iexact=self.email)
+        if self.pk:
+            existing_users = existing_users.exclude(pk=self.pk)
 
         # no duplicates
         if not existing_users:
             return
 
-        # the current user is editing their profile
-        if self in existing_users:
-            return
-
         # account has been deactivated
-        if any(user.bsn == self.bsn and not user.is_active for user in existing_users):
-            raise ValidationError(
-                {"email": ValidationError(_("This account has been deactivated"))}
-            )
+        for user in existing_users:
+            if (
+                user.login_type == LoginTypeChoices.digid
+                and user.bsn == self.bsn
+                and not user.is_active
+            ):
+                raise ValidationError(
+                    {"email": ValidationError(_("This account has been deactivated"))}
+                )
 
         # all accounts with duplicate emails have login_type digid
-        if self.login_type == LoginTypeChoices.digid and all(
-            (user.login_type == LoginTypeChoices.digid for user in existing_users)
-        ):
-            return
-
-        # some account does not have login_type digid
-        raise ValidationError(
-            {"email": ValidationError(_("This email is already taken."))}
-        )
+        if self.login_type == LoginTypeChoices.digid:
+            for user in existing_users:
+                if user.login_type != LoginTypeChoices.digid:
+                    # some account does not have login_type digid
+                    raise ValidationError(
+                        {"email": ValidationError(_("This email is already taken."))}
+                    )
+        else:
+            # non-digid must be unique
+            raise ValidationError(
+                {"email": ValidationError(_("This email is already taken."))}
+            )
 
     @property
     def seed(self):