fix(backup): UserEmail handling (#55433)

The previous code assumed UserEmail collisions are possible. It is now
refactored to treat them all as unique, and always reset them from their
imported state for all `ImportScope`s except `Global`.

Issue: getsentry/team-ospo#181

fixtures/backup/user-with-maximum-privileges.json

@@ -5,9 +5,9 @@
         "fields": {
             "password": "pbkdf2_sha256$150000$iEvdIknqYjTr$+QsGn0tfIJ1FZLxQI37mVU1gL2KbL/wqjMtG/dFhsMA=",
             "last_login": null,
-            "username": "testing@example.com",
+            "username": "maximum@example.com",
             "name": "",
-            "email": "testing@example.com",
+            "email": "maximum@example.com",
             "is_staff": true,
             "is_active": true,
             "is_superuser": true,
@@ -39,7 +39,7 @@
         "pk": 1,
         "fields": {
             "user": 1,
-            "email": "testing@example.com",
+            "email": "maximum@example.com",
             "validation_hash": "mCnWesSVvYQcq7qXQ36AZHwosAd6cghE",
             "date_hash_added": "2023-06-22T22:59:55.521Z",
             "is_verified": true

src/sentry/models/user.py

@@ -381,7 +381,9 @@ def _normalize_before_relocation_import(
             self.is_staff = False
             self.is_superuser = False
 
-        # TODO(getsentry/team-ospo#181): Handle usernames that already exist.
+        # TODO(getsentry/team-ospo#181): Handle usernames that already exist. This will involve
+        # marking the user "unclaimed", wiping their password, and adding a random suffix to their
+        # username.
 
         return old_pk
 

src/sentry/models/useremail.py

@@ -6,7 +6,6 @@
 
 from django.conf import settings
 from django.db import models
-from django.forms import model_to_dict
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 
@@ -83,17 +82,33 @@ def get_primary_email(cls, user: User) -> UserEmail:
         """@deprecated"""
         return cls.objects.get_primary_email(user)
 
-    # TODO(getsentry/team-ospo#181): what's up with email/useremail here? Seems like both get added
-    # with `sentry.user` simultaneously? Will need to make more robust user handling logic, and to
-    # test what happens when a UserEmail already exists.
+    def _normalize_before_relocation_import(
+        self, pk_map: PrimaryKeyMap, scope: ImportScope
+    ) -> Optional[int]:
+        old_pk = super()._normalize_before_relocation_import(pk_map, scope)
+        if old_pk is None:
+            return None
+
+        # Only preserve validation hashes in global scope - in all others, have the user verify
+        # their email again.
+        if scope != ImportScope.Global:
+            self.is_verified = False
+            self.validation_hash = get_secure_token()
+            self.date_hash_added = timezone.now()
+
+        return old_pk
+
     def write_relocation_import(
         self, pk_map: PrimaryKeyMap, scope: ImportScope
     ) -> Optional[Tuple[int, int]]:
-        old_pk = super()._normalize_before_relocation_import(pk_map, scope)
+        old_pk = self._normalize_before_relocation_import(pk_map, scope)
         if old_pk is None:
             return None
 
-        (useremail, _) = self.__class__.objects.get_or_create(
-            user=self.user, email=self.email, defaults=model_to_dict(self)
-        )
+        useremail = self.__class__.objects.get(user=self.user, email=self.email)
+        for f in self._meta.fields:
+            if f.name not in ["id", "pk"]:
+                setattr(useremail, f.name, getattr(self, f.name))
+        useremail.save()
+
         return (old_pk, useremail.pk)

tests/sentry/backup/test_imports.py

@@ -2,6 +2,7 @@
 
 import tempfile
 from copy import deepcopy
+from datetime import datetime
 from functools import cached_property
 from pathlib import Path
 
@@ -104,6 +105,18 @@ def test_user_sanitized_in_user_scope(self):
         assert User.objects.count() == 4
         assert User.objects.filter(is_staff=False, is_superuser=False).count() == 4
 
+        # Every user except `max_user` shares an email.
+        assert Email.objects.count() == 2
+
+        # All `UserEmail`s must have their verification status reset in this scope.
+        assert UserEmail.objects.count() == 4
+        assert UserEmail.objects.filter(is_verified=True).count() == 0
+        assert UserEmail.objects.filter(date_hash_added__lt=datetime(2023, 7, 1, 0, 0)).count() == 0
+        assert (
+            UserEmail.objects.filter(validation_hash="mCnWesSVvYQcq7qXQ36AZHwosAd6cghE").count()
+            == 0
+        )
+
         assert User.objects.filter(is_staff=True).count() == 0
         assert User.objects.filter(is_superuser=True).count() == 0
         assert Authenticator.objects.count() == 0
@@ -121,6 +134,18 @@ def test_user_sanitized_in_organization_scope(self):
         assert User.objects.count() == 4
         assert User.objects.filter(is_staff=False, is_superuser=False).count() == 4
 
+        # Every user except `max_user` shares an email.
+        assert Email.objects.count() == 2
+
+        # All `UserEmail`s must have their verification status reset in this scope.
+        assert UserEmail.objects.count() == 4
+        assert UserEmail.objects.filter(is_verified=True).count() == 0
+        assert UserEmail.objects.filter(date_hash_added__lt=datetime(2023, 7, 1, 0, 0)).count() == 0
+        assert (
+            UserEmail.objects.filter(validation_hash="mCnWesSVvYQcq7qXQ36AZHwosAd6cghE").count()
+            == 0
+        )
+
         assert User.objects.filter(is_staff=True).count() == 0
         assert User.objects.filter(is_superuser=True).count() == 0
         assert Authenticator.objects.count() == 0
@@ -140,6 +165,19 @@ def test_users_sanitized_in_global_scope(self):
         assert User.objects.filter(is_superuser=True).count() == 2
         assert User.objects.filter(is_staff=False, is_superuser=False).count() == 2
         assert Authenticator.objects.count() == 4
+        assert UserEmail.objects.count() == 4
+
+        # Every user except `max_user` shares an email.
+        assert Email.objects.count() == 2
+
+        # All `UserEmail`s must keep their imported verification status reset in this scope.
+        assert UserEmail.objects.count() == 4
+        assert UserEmail.objects.filter(is_verified=True).count() == 4
+        assert UserEmail.objects.filter(date_hash_added__lt=datetime(2023, 7, 1, 0, 0)).count() == 4
+        assert (
+            UserEmail.objects.filter(validation_hash="mCnWesSVvYQcq7qXQ36AZHwosAd6cghE").count()
+            == 4
+        )
 
         # 1 from `max_user`, 1 from `permission_user`.
         assert UserPermission.objects.count() == 2