Skip to content

Commit be1b748

Browse files
committed
fix(backup): Don't assign is_unclaimed in global import
Importing a user in `ImportScope.Global` should never result in them being marked `is_unclaimed`. Issue: getsentry/team-ospo@203
1 parent 0ee6c61 commit be1b748

File tree

3 files changed

+21
-14
lines changed

3 files changed

+21
-14
lines changed

src/sentry/models/user.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -405,23 +405,27 @@ def normalize_before_relocation_import(
405405
if old_pk is None:
406406
return None
407407

408-
# New users are always unclaimed.
409-
self.is_unclaimed = True
410-
411-
# Give the user a cryptographically secure random password. The purpose here is to have a
412-
# password that NO ONE knows - the only way to log into this account is to use the "claim
413-
# your account" flow to create a new password (or to click "lost password" and end up there
414-
# anyway), at which point we'll detect the user's `is_unclaimed`` status and prompt them to
415-
# change their `username` as well.
416-
self.set_password(
417-
"".join(secrets.choice(RANDOM_PASSWORD_ALPHABET) for _ in range(RANDOM_PASSWORD_LENGTH))
418-
)
419-
420408
if scope not in {ImportScope.Config, ImportScope.Global}:
421409
self.is_staff = False
422410
self.is_superuser = False
423411
self.is_managed = False
424412

413+
# No need to mark users newly "unclaimed" when doing a global backup/restore.
414+
if scope != ImportScope.Global or self.is_unclaimed:
415+
# New users are marked unclaimed.
416+
self.is_unclaimed = True
417+
418+
# Give the user a cryptographically secure random password. The purpose here is to have
419+
# a password that NO ONE knows - the only way to log into this account is to use the
420+
# "claim your account" flow to create a new password (or to click "lost password" and
421+
# end up there anyway), at which point we'll detect the user's `is_unclaimed` status and
422+
# prompt them to change their `username` as well.
423+
self.set_password(
424+
"".join(
425+
secrets.choice(RANDOM_PASSWORD_ALPHABET) for _ in range(RANDOM_PASSWORD_LENGTH)
426+
)
427+
)
428+
425429
return old_pk
426430

427431
def write_relocation_import(

tests/sentry/backup/test_imports.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,8 @@ def test_users_unsanitized_in_global_scope(self):
226226
import_in_global_scope(tmp_file, printer=NOOP_PRINTER)
227227

228228
assert User.objects.count() == 4
229-
assert User.objects.filter(is_unclaimed=True).count() == 4
229+
# We don't mark `Global`ly imported `User`s unclaimed.
230+
assert User.objects.filter(is_unclaimed=True).count() == 0
230231
assert User.objects.filter(is_managed=True).count() == 1
231232
assert User.objects.filter(is_staff=True).count() == 2
232233
assert User.objects.filter(is_superuser=True).count() == 2

tests/sentry/backup/test_snapshots.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ def test_auto_suffix_username_and_organization_slug(tmp_path):
8787
)
8888

8989

90-
# Apps are often owned by a fake "proxy_user" with an autogenerated name and no email. This empty email is represented as such in the database, with an empty string as the entry in the `Email` table. This test ensures that this relationship is preserved through an import/export cycle.
90+
# Apps are often owned by a fake "proxy_user" with an autogenerated name and no email. This empty
91+
# email is represented as such in the database, with an empty string as the entry in the `Email`
92+
# table. This test ensures that this relationship is preserved through an import/export cycle.
9193
@django_db_all(transaction=True)
9294
def test_app_user_with_empty_email(tmp_path):
9395
import_export_from_fixture_then_validate(

0 commit comments

Comments
 (0)