Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(backup): Don't assign is_unclaimed in global import #57738

Merged
merged 3 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions src/sentry/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,23 +403,27 @@ def normalize_before_relocation_import(
if old_pk is None:
return None

# New users are always unclaimed.
self.is_unclaimed = True

# Give the user a cryptographically secure random password. The purpose here is to have a
# password that NO ONE knows - the only way to log into this account is to use the "claim
# your account" flow to create a new password (or to click "lost password" and end up there
# anyway), at which point we'll detect the user's `is_unclaimed`` status and prompt them to
# change their `username` as well.
self.set_password(
"".join(secrets.choice(RANDOM_PASSWORD_ALPHABET) for _ in range(RANDOM_PASSWORD_LENGTH))
)

if scope not in {ImportScope.Config, ImportScope.Global}:
self.is_staff = False
self.is_superuser = False
self.is_managed = False

# No need to mark users newly "unclaimed" when doing a global backup/restore.
if scope != ImportScope.Global or self.is_unclaimed:
# New users are marked unclaimed.
self.is_unclaimed = True

# Give the user a cryptographically secure random password. The purpose here is to have
# a password that NO ONE knows - the only way to log into this account is to use the
# "claim your account" flow to create a new password (or to click "lost password" and
# end up there anyway), at which point we'll detect the user's `is_unclaimed` status and
# prompt them to change their `username` as well.
self.set_password(
"".join(
secrets.choice(RANDOM_PASSWORD_ALPHABET) for _ in range(RANDOM_PASSWORD_LENGTH)
)
)

return old_pk

def write_relocation_import(
Expand Down
3 changes: 2 additions & 1 deletion tests/sentry/backup/test_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ def test_users_unsanitized_in_global_scope(self):
import_in_global_scope(tmp_file, printer=NOOP_PRINTER)

assert User.objects.count() == 4
assert User.objects.filter(is_unclaimed=True).count() == 4
# We don't mark `Global`ly imported `User`s unclaimed.
assert User.objects.filter(is_unclaimed=True).count() == 0
assert User.objects.filter(is_managed=True).count() == 1
assert User.objects.filter(is_staff=True).count() == 2
assert User.objects.filter(is_superuser=True).count() == 2
Expand Down
4 changes: 3 additions & 1 deletion tests/sentry/backup/test_snapshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ def test_auto_suffix_username_and_organization_slug(tmp_path):
)


# 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.
# 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.
@django_db_all(transaction=True)
def test_app_user_with_empty_email(tmp_path):
import_export_from_fixture_then_validate(
Expand Down