diff --git a/src/sentry/tasks/relocation.py b/src/sentry/tasks/relocation.py index 6709cf2a611625..8a6002475a75fe 100644 --- a/src/sentry/tasks/relocation.py +++ b/src/sentry/tasks/relocation.py @@ -1097,6 +1097,7 @@ def postprocessing(uuid: str) -> None: user_id=relocation.owner_id, role="owner", ) + # Last, but certainly not least: trigger signals, so that interested subscribers in eg: # getsentry can do whatever postprocessing they need to. If even a single one fails, we fail # the entire task. @@ -1104,8 +1105,8 @@ def postprocessing(uuid: str) -> None: if isinstance(result, Exception): raise result - # This signal nust come after the relocated signal, to ensure that the subscription and customer models - # have been appropriately set up before attempting to redeem a promo code. + # This signal must come after the relocated signal, to ensure that the subscription and + # customer models have been appropriately set up before attempting to redeem a promo code. relocation_redeem_promo_code.send_robust( sender=postprocessing, user_id=relocation.owner_id, @@ -1166,6 +1167,12 @@ def notifying_users(uuid: str) -> None: for chunk in chunks: imported_user_ids = imported_user_ids.union(set(chunk.inserted_map.values())) + imported_org_slugs: set[int] = set() + for chunk in RegionImportChunk.objects.filter( + import_uuid=str(uuid), model="sentry.organization" + ): + imported_org_slugs = imported_org_slugs.union(set(chunk.inserted_identifiers.values())) + # Do a sanity check on pk-mapping before we go and reset the passwords of random users - are # all of these usernames plausibly ones that were included in the import, based on username # prefix matching? @@ -1184,7 +1191,7 @@ def notifying_users(uuid: str) -> None: # Okay, everything seems fine - go ahead and send those emails. for user in imported_users: hash = lost_password_hash_service.get_or_create(user_id=user.id).hash - LostPasswordHash.send_relocate_account_email(user, hash, relocation.want_org_slugs) + LostPasswordHash.send_relocate_account_email(user, hash, list(imported_org_slugs)) relocation.latest_unclaimed_emails_sent_at = datetime.now(UTC) relocation.save() @@ -1223,12 +1230,18 @@ def notifying_owner(uuid: str) -> None: attempts_left, ERR_NOTIFYING_INTERNAL, ): + imported_org_slugs: set[int] = set() + for chunk in RegionImportChunk.objects.filter( + import_uuid=str(uuid), model="sentry.organization" + ): + imported_org_slugs = imported_org_slugs.union(set(chunk.inserted_identifiers.values())) + send_relocation_update_email( relocation, Relocation.EmailKind.SUCCEEDED, { "uuid": str(relocation.uuid), - "orgs": relocation.want_org_slugs, + "orgs": list(imported_org_slugs), }, ) diff --git a/src/sentry/templates/sentry/account/relocate/claimed.html b/src/sentry/templates/sentry/account/relocate/claimed.html new file mode 100644 index 00000000000000..5f074a9e517157 --- /dev/null +++ b/src/sentry/templates/sentry/account/relocate/claimed.html @@ -0,0 +1,12 @@ +{% extends "sentry/bases/auth.html" %} + +{% load crispy_forms_tags %} +{% load i18n %} + +{% block title %}{% trans "Claim Account" %} | {{ block.super }}{% endblock %} + +{% block auth_main %} +

{% trans "Claim Account" %}

+ +

This account has already been claimed - you should be able to log in as normal. You can always reset your password if you have forgotten it.

+{% endblock %} diff --git a/src/sentry/templates/sentry/account/relocate/error.html b/src/sentry/templates/sentry/account/relocate/error.html new file mode 100644 index 00000000000000..445b707b968374 --- /dev/null +++ b/src/sentry/templates/sentry/account/relocate/error.html @@ -0,0 +1,14 @@ +{% extends "sentry/bases/auth.html" %} + +{% load crispy_forms_tags %} +{% load i18n %} + +{% block title %}{% trans "Claim Account" %} | {{ block.super }}{% endblock %} + +{% block auth_main %} +

{% trans "Claim Account" %}

+ +

{% blocktrans %}We were unable to locate this account.{% endblocktrans %}

+ +

Please contact support for further assistance if necessary.

+{% endblock %} diff --git a/src/sentry/templates/sentry/account/relocate/failure.html b/src/sentry/templates/sentry/account/relocate/failure.html index e2dba15c9c4f2b..eb331dc22f8b17 100644 --- a/src/sentry/templates/sentry/account/relocate/failure.html +++ b/src/sentry/templates/sentry/account/relocate/failure.html @@ -7,7 +7,11 @@ {% block auth_main %}

{% trans "Claim Account" %}

-

{% blocktrans %}This password link has expired. Request a new password recovery code to set - your account password and claim your account.{% endblocktrans %}

-

Request Reset Code

+

{% blocktrans %}This link has expired. Request a new claim account email to set your account + password and claim your account.{% endblocktrans %}

+
+
+ +
+
{% endblock %} diff --git a/src/sentry/templates/sentry/account/relocate/sent.html b/src/sentry/templates/sentry/account/relocate/sent.html new file mode 100644 index 00000000000000..da5c71b7ae2b0b --- /dev/null +++ b/src/sentry/templates/sentry/account/relocate/sent.html @@ -0,0 +1,12 @@ +{% extends "sentry/bases/auth.html" %} + +{% load crispy_forms_tags %} +{% load i18n %} + +{% block title %}{% trans "Claim Account" %} | {{ block.super }}{% endblock %} + +{% block auth_main %} +

{% trans "Claim Account" %}

+ +

{% blocktrans %}We have sent an email to the address registered with this account containing further instructions to set your password and claim this account.{% endblocktrans %}

+{% endblock %} diff --git a/src/sentry/web/frontend/accounts.py b/src/sentry/web/frontend/accounts.py index 7504c756e59819..5067387849cfff 100644 --- a/src/sentry/web/frontend/accounts.py +++ b/src/sentry/web/frontend/accounts.py @@ -110,6 +110,70 @@ def recover(request): return render_to_response(get_template("recover", "index"), context, request) +@set_referrer_policy("strict-origin-when-cross-origin") +@control_silo_function +def relocate_reclaim(request, user_id): + """ + Ask to receive a new "claim this user" email. + """ + from sentry import ratelimits as ratelimiter + + extra = { + "ip_address": request.META["REMOTE_ADDR"], + "user_agent": request.META.get("HTTP_USER_AGENT"), + "user_id": user_id, + } + if request.method != "POST": + logger.warning("reclaim.error", extra=extra) + return render_to_response(get_template("relocate", "error"), {}, request) + + if ratelimiter.backend.is_limited( + "accounts:reclaim:{}".format(extra["ip_address"]), + limit=5, + window=60, # 5 per minute should be enough for anyone + ): + logger.warning("reclaim.rate-limited", extra=extra) + + return HttpResponse( + "You have made too many password recovery attempts. Please try again later.", + content_type="text/plain", + status=429, + ) + + # Verify that the user is unclaimed. If they are already claimed, tell the requester that this + # is the case, since of course claiming this account would be impossible. + user = User.objects.filter(id=user_id).first() + if user is None: + logger.warning("reclaim.user_not_found", extra=extra) + return render_to_response(get_template("relocate", "error"), {}, request) + if not user.is_unclaimed: + logger.warning("reclaim.already_claimed", extra=extra) + return render_to_response(get_template("relocate", "claimed"), {}, request) + + # Get all orgs for user. We'll need this info to properly compose the new relocation email. + org_ids = OrganizationMemberMapping.objects.filter(user_id=user_id).values_list( + "organization_id", flat=True + ) + org_slugs = list( + OrganizationMapping.objects.filter(organization_id__in=org_ids).values_list( + "slug", flat=True + ) + ) + if len(org_slugs) == 0: + logger.warning("reclaim.error", extra=extra) + return render_to_response(get_template("relocate", "error"), {}, request) + + # Make a new `LostPasswordHash`, and send the "this user has been relocated ..." email again. + password_hash = lost_password_hash_service.get_or_create(user_id=user_id) + LostPasswordHash.send_relocate_account_email(user, password_hash.hash, org_slugs) + extra["passwordhash_id"] = password_hash.id + extra["org_slugs"] = org_slugs + + # Let the user know that we've sent them a new email. + logger.info("recover.sent", extra=extra) + return render_to_response(get_template("relocate", "sent"), {}, request) + + @set_referrer_policy("strict-origin-when-cross-origin") @control_silo_function def recover_confirm(request, user_id, hash, mode="recover"): @@ -120,7 +184,7 @@ def recover_confirm(request, user_id, hash, mode="recover"): raise LostPasswordHash.DoesNotExist user = password_hash.user except LostPasswordHash.DoesNotExist: - return render_to_response(get_template(mode, "failure"), {}, request) + return render_to_response(get_template(mode, "failure"), {"user_id": user_id}, request) # TODO(getsentry/team-ospo#190): Clean up ternary logic and only show relocation form if user is unclaimed form_cls = RelocationForm if mode == "relocate" else ChangePasswordRecoverForm diff --git a/src/sentry/web/urls.py b/src/sentry/web/urls.py index 8e0d92f649849e..ce4b1596f35295 100644 --- a/src/sentry/web/urls.py +++ b/src/sentry/web/urls.py @@ -298,6 +298,11 @@ accounts.recover_confirm, name="sentry-account-recover-confirm", ), + re_path( + r"^relocation/reclaim/(?P[\d]+)/$", + accounts.relocate_reclaim, + name="sentry-account-relocate-reclaim", + ), re_path( r"^password/confirm/(?P[\d]+)/(?P[0-9a-zA-Z]+)/$", accounts.set_password_confirm, diff --git a/tests/sentry/tasks/test_relocation.py b/tests/sentry/tasks/test_relocation.py index 6d92864b3e7714..9ec4bdb2636041 100644 --- a/tests/sentry/tasks/test_relocation.py +++ b/tests/sentry/tasks/test_relocation.py @@ -98,6 +98,17 @@ class FakeCloudBuildClient: class RelocationTaskTestCase(TestCase): def setUp(self): super().setUp() + + # Create a collision with the org slug we'll be requesting. + self.requested_org_slug = "testing" + self.existing_org_owner = self.create_user( + email="existing_org_owner@example.com", + is_superuser=False, + is_staff=False, + is_active=True, + ) + self.existing_org = self.create_organization(name=self.requested_org_slug) + self.owner = self.create_user( email="owner@example.com", is_superuser=False, is_staff=False, is_active=True ) @@ -1859,11 +1870,20 @@ def setUp(self): printer=Printer(), ) - self.imported_users = ControlImportChunkReplica.objects.get( - import_uuid=self.uuid, model="sentry.user" + self.imported_orgs = sorted( + RegionImportChunk.objects.get( + import_uuid=self.uuid, model="sentry.organization" + ).inserted_identifiers.values() + ) + assert len(self.imported_orgs) == 1 + assert ( + len( + ControlImportChunkReplica.objects.get( + import_uuid=self.uuid, model="sentry.user" + ).inserted_map + ) + == 2 ) - - assert len(self.imported_users.inserted_map) == 2 def test_success( self, @@ -1881,8 +1901,8 @@ def test_success( mock_relocation_email.call_args_list[0][0][0].username, mock_relocation_email.call_args_list[1][0][0].username, ] - assert mock_relocation_email.call_args_list[0][0][2] == ["testing"] - assert mock_relocation_email.call_args_list[1][0][2] == ["testing"] + assert sorted(mock_relocation_email.call_args_list[0][0][2]) == self.imported_orgs + assert sorted(mock_relocation_email.call_args_list[1][0][2]) == self.imported_orgs assert "admin@example.com" in email_targets assert "member@example.com" in email_targets @@ -1969,6 +1989,18 @@ def setUp(self): self.relocation.latest_task = OrderedTask.NOTIFYING_USERS.name self.relocation.save() + RegionImportChunk.objects.create( + import_uuid=self.relocation.uuid, + model="sentry.organization", + min_ordinal=0, + max_ordinal=0, + min_source_pk=1, + max_source_pk=1, + inserted_map={1: 1234}, + inserted_identifiers={1: "testing-ab"}, + ) + self.imported_orgs = ["testing-ab"] + def test_success_admin_assisted_relocation( self, completed_mock: Mock, @@ -1980,6 +2012,7 @@ def test_success_admin_assisted_relocation( assert fake_message_builder.call_count == 1 assert fake_message_builder.call_args.kwargs["type"] == "relocation.succeeded" + assert fake_message_builder.call_args.kwargs["context"]["orgs"] == self.imported_orgs fake_message_builder.return_value.send_async.assert_called_once_with( to=[self.owner.email, self.superuser.email] ) @@ -2002,6 +2035,7 @@ def test_success_self_serve_relocation( assert fake_message_builder.call_count == 1 assert fake_message_builder.call_args.kwargs["type"] == "relocation.succeeded" + assert fake_message_builder.call_args.kwargs["context"]["orgs"] == self.imported_orgs fake_message_builder.return_value.send_async.assert_called_once_with(to=[self.owner.email]) assert completed_mock.call_count == 1 diff --git a/tests/sentry/web/frontend/test_accounts.py b/tests/sentry/web/frontend/test_accounts.py index 8da315419d49e1..4bf6a4a7aadcdf 100644 --- a/tests/sentry/web/frontend/test_accounts.py +++ b/tests/sentry/web/frontend/test_accounts.py @@ -1,13 +1,17 @@ +from datetime import timedelta from functools import cached_property from unittest.mock import call, patch +from django.core import mail from django.test import override_settings from django.urls import reverse +from django.utils import timezone from sentry.models.lostpasswordhash import LostPasswordHash from sentry.models.useremail import UserEmail from sentry.services.hybrid_cloud.organization import organization_service from sentry.testutils.cases import TestCase +from sentry.testutils.helpers.task_runner import BurstTaskRunner from sentry.testutils.silo import control_silo_test from sentry.web.frontend.accounts import recover_confirm @@ -26,6 +30,9 @@ def relocation_recover_path(self, user_id, hash_): "sentry-account-relocate-confirm", kwargs={"user_id": user_id, "hash": hash_} ) + def relocation_reclaim_path(self, user_id): + return reverse("sentry-account-relocate-reclaim", kwargs={"user_id": user_id}) + def test_get_renders_form(self): resp = self.client.get(self.path) assert resp.status_code == 200 @@ -118,9 +125,27 @@ def test_relocate_recovery_no_inputs(self): header_name = "Referrer-Policy" assert resp.has_header(header_name) - assert resp.templates[0].name == ("sentry/account/relocate/confirm.html") assert resp.status_code == 200 assert resp[header_name] == "strict-origin-when-cross-origin" + self.assertTemplateUsed("sentry/account/relocate/confirm.html") + + def test_relocate_expired_lost_password_hash(self): + user = self.create_user() + lost_password = LostPasswordHash.objects.create( + user=user, date_added=timezone.now() - timedelta(hours=2) + ) + assert not lost_password.is_valid() + + resp = self.client.get( + self.relocation_recover_path(lost_password.user_id, lost_password.hash), + ) + + header_name = "Referrer-Policy" + + assert resp.has_header(header_name) + assert resp.status_code == 200 + assert resp[header_name] == "strict-origin-when-cross-origin" + self.assertTemplateUsed("sentry/account/relocate/failure.html") @patch("sentry.signals.terms_accepted.send_robust") def test_relocate_recovery_post_multiple_orgs(self, terms_accepted_signal_mock): @@ -148,7 +173,7 @@ def test_relocate_recovery_post_multiple_orgs(self, terms_accepted_signal_mock): user.refresh_from_db() assert resp.has_header(header_name) - assert resp.templates[0].name == ("sentry/emails/password-changed.txt") + self.assertTemplateUsed("sentry/emails/password-changed.txt") assert not user.is_unclaimed assert user.username == new_username assert user.password != old_password @@ -200,7 +225,7 @@ def test_relocate_recovery_post(self, terms_accepted_signal_mock): user.refresh_from_db() assert resp.has_header(header_name) - assert resp.templates[0].name == ("sentry/emails/password-changed.txt") + self.assertTemplateUsed("sentry/emails/password-changed.txt") assert not user.is_unclaimed assert user.username == new_username assert user.password != old_password @@ -267,13 +292,90 @@ def test_relocate_recovery_invalid_password(self): user.refresh_from_db() assert resp.has_header(header_name) - assert resp.templates[0].name == ("sentry/account/relocate/failure.html") + self.assertTemplateUsed("sentry/account/relocate/failure.html") assert user.is_unclaimed assert user.username != new_username assert user.password == old_password assert resp.status_code == 200 assert resp[header_name] == "strict-origin-when-cross-origin" + def test_relocate_reclaim_success(self): + user = self.create_user(email="member@example.com", is_unclaimed=True) + lost_password = LostPasswordHash.objects.create(user=user) + org = self.create_organization(name="test-org") + self.create_member(user=user, organization=org, role="member", teams=[]) + + with BurstTaskRunner() as burst: + resp = self.client.post( + self.relocation_reclaim_path(lost_password.user_id), + ) + + # Sends the async email. + burst() + + assert len(mail.outbox) == 1 + assert mail.outbox[0].to == ["member@example.com"] + assert "Set Username and Password" in mail.outbox[0].subject + assert "test-org" in mail.outbox[0].body + assert "Claim Account" in mail.outbox[0].body + assert f"/relocation/confirm/{user.id}/{lost_password.hash}/" in mail.outbox[0].body + + header_name = "Referrer-Policy" + + assert resp.has_header(header_name) + assert resp.status_code == 200 + assert resp[header_name] == "strict-origin-when-cross-origin" + self.assertTemplateUsed("sentry/account/relocate/sent.html") + + def test_relocate_reclaim_user_not_found(self): + user = self.create_user(email="member@example.com", is_unclaimed=True) + lost_password = LostPasswordHash.objects.create(user=user) + org = self.create_organization(name="test-org") + self.create_member(user=user, organization=org, role="member", teams=[]) + + resp = self.client.post( + self.relocation_reclaim_path(lost_password.user_id + 12345), + ) + + header_name = "Referrer-Policy" + + assert resp.has_header(header_name) + assert resp.status_code == 200 + assert resp[header_name] == "strict-origin-when-cross-origin" + self.assertTemplateUsed("sentry/account/relocate/error.html") + + def test_relocate_reclaim_already_claimed(self): + user = self.create_user(email="member@example.com", is_unclaimed=False) + lost_password = LostPasswordHash.objects.create(user=user) + org = self.create_organization(name="test-org") + self.create_member(user=user, organization=org, role="member", teams=[]) + + resp = self.client.post( + self.relocation_reclaim_path(lost_password.user_id), + ) + + header_name = "Referrer-Policy" + + assert resp.has_header(header_name) + assert resp.status_code == 200 + assert resp[header_name] == "strict-origin-when-cross-origin" + self.assertTemplateUsed("sentry/account/relocate/claimed.html") + + def test_relocate_reclaim_user_not_in_any_orgs(self): + user = self.create_user(email="member@example.com", is_unclaimed=True) + lost_password = LostPasswordHash.objects.create(user=user) + + resp = self.client.post( + self.relocation_reclaim_path(lost_password.user_id), + ) + + header_name = "Referrer-Policy" + + assert resp.has_header(header_name) + assert resp.status_code == 200 + assert resp[header_name] == "strict-origin-when-cross-origin" + self.assertTemplateUsed("sentry/account/relocate/error.html") + def test_confirm_email(self): self.login_as(self.user)