From 27c4e46b307a82f0b7f45ada42df2323b252ef59 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Sun, 19 May 2024 01:30:36 +0200 Subject: [PATCH] do_deactivate_realm: Add deactivation_reason kwarg. It's going to be helpful in the future to record the reason for realm deactivation. - For information tracking - For making a distinction between cases where we can allow realm owners to reactivate their realm via a self-serve flow (e.g. "owner_request") vs where we can't (ToS abuse). --- corporate/tests/test_stripe.py | 8 ++++++-- corporate/tests/test_support_views.py | 6 +++++- corporate/views/support.py | 6 +++++- tools/test-api | 2 +- zerver/actions/create_realm.py | 4 +++- zerver/actions/realm_settings.py | 19 +++++++++++++++++- .../management/commands/deactivate_realm.py | 13 ++++++++++-- zerver/management/commands/export.py | 4 +++- zerver/tests/test_audit_log.py | 6 +++++- zerver/tests/test_auth_backends.py | 20 ++++++++++++++----- zerver/tests/test_decorators.py | 8 ++++++-- zerver/tests/test_email_change.py | 4 +++- zerver/tests/test_email_mirror.py | 4 +++- zerver/tests/test_events.py | 2 +- zerver/tests/test_push_notifications.py | 2 +- zerver/tests/test_realm.py | 20 +++++++++---------- zerver/tests/test_signup.py | 8 ++++++-- zerver/views/realm.py | 2 +- 18 files changed, 103 insertions(+), 35 deletions(-) diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index 22a11eb8f5..96d17ff404 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -4318,7 +4318,9 @@ class StripeTest(StripeTestCase): self.assertEqual(last_ledger_entry.licenses, 20) self.assertEqual(last_ledger_entry.licenses_at_next_renewal, 20) - do_deactivate_realm(get_realm("zulip"), acting_user=None) + do_deactivate_realm( + get_realm("zulip"), acting_user=None, deactivation_reason="owner_request" + ) plan.refresh_from_db() self.assertTrue(get_realm("zulip").deactivated) @@ -4352,7 +4354,9 @@ class StripeTest(StripeTestCase): self.seat_count, True, CustomerPlan.BILLING_SCHEDULE_ANNUAL, True, False ) - do_deactivate_realm(get_realm("zulip"), acting_user=None) + do_deactivate_realm( + get_realm("zulip"), acting_user=None, deactivation_reason="owner_request" + ) self.assertTrue(get_realm("zulip").deactivated) do_reactivate_realm(get_realm("zulip")) diff --git a/corporate/tests/test_support_views.py b/corporate/tests/test_support_views.py index 7e452815a4..ea32a2def9 100644 --- a/corporate/tests/test_support_views.py +++ b/corporate/tests/test_support_views.py @@ -1422,7 +1422,11 @@ class TestSupportEndpoint(ZulipTestCase): result = self.client_post( "/activity/support", {"realm_id": f"{lear_realm.id}", "status": "deactivated"} ) - m.assert_called_once_with(lear_realm, acting_user=self.example_user("iago")) + m.assert_called_once_with( + lear_realm, + acting_user=self.example_user("iago"), + deactivation_reason="owner_request", + ) self.assert_in_success_response(["lear deactivated"], result) with mock.patch("corporate.views.support.do_send_realm_reactivation_email") as m: diff --git a/corporate/views/support.py b/corporate/views/support.py index 978eba6c46..be09f474aa 100644 --- a/corporate/views/support.py +++ b/corporate/views/support.py @@ -447,7 +447,11 @@ def support( f"Realm reactivation email sent to admins of {realm.string_id}." ) elif status == "deactivated": - do_deactivate_realm(realm, acting_user=acting_user) + # TODO: Add support for deactivation reason in the support UI that'll be passed + # here. + do_deactivate_realm( + realm, acting_user=acting_user, deactivation_reason="owner_request" + ) context["success_message"] = f"{realm.string_id} deactivated." elif scrub_realm: do_scrub_realm(realm, acting_user=acting_user) diff --git a/tools/test-api b/tools/test-api index 0eea5f7446..3dffb6a86a 100755 --- a/tools/test-api +++ b/tools/test-api @@ -116,7 +116,7 @@ with test_server_running( do_reactivate_user(guest_user, acting_user=None) # Test realm deactivated error - do_deactivate_realm(guest_user.realm, acting_user=None) + do_deactivate_realm(guest_user.realm, acting_user=None, deactivation_reason="owner_request") client = Client( email=email, diff --git a/zerver/actions/create_realm.py b/zerver/actions/create_realm.py index 32ca3c8465..306a4cf767 100644 --- a/zerver/actions/create_realm.py +++ b/zerver/actions/create_realm.py @@ -88,7 +88,9 @@ def do_change_realm_subdomain( # the realm has been moved to a new subdomain. if add_deactivated_redirect: placeholder_realm = do_create_realm(old_subdomain, realm.name) - do_deactivate_realm(placeholder_realm, acting_user=None) + do_deactivate_realm( + placeholder_realm, acting_user=None, deactivation_reason="subdomain_change" + ) do_add_deactivated_redirect(placeholder_realm, realm.url) diff --git a/zerver/actions/realm_settings.py b/zerver/actions/realm_settings.py index cadf78dceb..b9b74a5d6b 100644 --- a/zerver/actions/realm_settings.py +++ b/zerver/actions/realm_settings.py @@ -455,7 +455,23 @@ def do_set_realm_user_default_setting( send_event(realm, event, active_user_ids(realm.id)) -def do_deactivate_realm(realm: Realm, *, acting_user: Optional[UserProfile]) -> None: +RealmDeactivationReasonType = Literal[ + "owner_request", + "tos_violation", + "inactivity", + "self_hosting_migration", + # When we change the subdomain of a realm, we leave + # behind a deactivated gravestone realm. + "subdomain_change", +] + + +def do_deactivate_realm( + realm: Realm, + *, + acting_user: Optional[UserProfile], + deactivation_reason: RealmDeactivationReasonType, +) -> None: """ Deactivate this realm. Do NOT deactivate the users -- we need to be able to tell the difference between users that were intentionally deactivated, @@ -481,6 +497,7 @@ def do_deactivate_realm(realm: Realm, *, acting_user: Optional[UserProfile]) -> acting_user=acting_user, extra_data={ RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(realm), + "deactivation_reason": deactivation_reason, }, ) diff --git a/zerver/management/commands/deactivate_realm.py b/zerver/management/commands/deactivate_realm.py index 91b7dbb167..6f4ba0f739 100644 --- a/zerver/management/commands/deactivate_realm.py +++ b/zerver/management/commands/deactivate_realm.py @@ -1,5 +1,5 @@ from argparse import ArgumentParser -from typing import Any +from typing import Any, cast from typing_extensions import override @@ -15,11 +15,18 @@ class Command(ZulipBaseCommand): parser.add_argument( "--redirect_url", metavar="", help="URL to which the realm has moved" ) + parser.add_argument( + "--deactivation_reason", + metavar="", + help="Reason for deactivation", + required=True, + ) self.add_realm_args(parser, required=True) @override def handle(self, *args: Any, **options: str) -> None: realm = self.get_realm(options) + deactivation_reason = options["deactivation_reason"] assert realm is not None # Should be ensured by parser @@ -32,5 +39,7 @@ class Command(ZulipBaseCommand): return print("Deactivating", options["realm_id"]) - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm( + realm, acting_user=None, deactivation_reason=cast(Any, deactivation_reason) + ) print("Done!") diff --git a/zerver/management/commands/export.py b/zerver/management/commands/export.py index 20dd19e7ed..7bb3a8011b 100644 --- a/zerver/management/commands/export.py +++ b/zerver/management/commands/export.py @@ -202,7 +202,9 @@ class Command(ZulipBaseCommand): if options["deactivate_realm"]: print(f"\033[94mDeactivating realm\033[0m: {realm.string_id}") - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm( + realm, acting_user=None, deactivation_reason="self_hosting_migration" + ) def percent_callback(bytes_transferred: Any) -> None: print(end=".", flush=True) diff --git a/zerver/tests/test_audit_log.py b/zerver/tests/test_audit_log.py index 8ad8377110..c622f755c6 100644 --- a/zerver/tests/test_audit_log.py +++ b/zerver/tests/test_audit_log.py @@ -421,11 +421,15 @@ class TestRealmAuditLog(ZulipTestCase): def test_realm_activation(self) -> None: realm = get_realm("zulip") user = self.example_user("desdemona") - do_deactivate_realm(realm, acting_user=user) + do_deactivate_realm(realm, acting_user=user, deactivation_reason="owner_request") log_entry = RealmAuditLog.objects.get( realm=realm, event_type=RealmAuditLog.REALM_DEACTIVATED, acting_user=user ) extra_data = log_entry.extra_data + + deactivation_reason = extra_data["deactivation_reason"] + self.assertEqual(deactivation_reason, "owner_request") + self.check_role_count_schema(extra_data[RealmAuditLog.ROLE_COUNT]) do_reactivate_realm(realm) diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 3e83d72033..050d72edcd 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -214,7 +214,9 @@ class AuthBackendTest(ZulipTestCase): self.assertEqual(user_profile, result) # Verify auth fails with a deactivated realm - do_deactivate_realm(user_profile.realm, acting_user=None) + do_deactivate_realm( + user_profile.realm, acting_user=None, deactivation_reason="owner_request" + ) result = backend.authenticate(**good_kwargs) self.assertIsNone(result) @@ -4989,7 +4991,9 @@ class FetchAPIKeyTest(ZulipTestCase): self.assert_json_error_contains(result, "Account is deactivated", 401) def test_deactivated_realm(self) -> None: - do_deactivate_realm(self.user_profile.realm, acting_user=None) + do_deactivate_realm( + self.user_profile.realm, acting_user=None, deactivation_reason="owner_request" + ) result = self.client_post( "/api/v1/fetch_api_key", dict(username=self.email, password=initial_password(self.email)), @@ -5051,7 +5055,9 @@ class DevFetchAPIKeyTest(ZulipTestCase): self.assert_json_error_contains(result, "Account is deactivated", 401) def test_deactivated_realm(self) -> None: - do_deactivate_realm(self.user_profile.realm, acting_user=None) + do_deactivate_realm( + self.user_profile.realm, acting_user=None, deactivation_reason="owner_request" + ) result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=self.email)) self.assert_json_error_contains(result, "This organization has been deactivated", 401) @@ -6343,7 +6349,9 @@ class TestLDAP(ZulipLDAPTestCase): with self.settings(AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map): backend = self.backend email = "nonexisting@zulip.com" - do_deactivate_realm(backend._realm, acting_user=None) + do_deactivate_realm( + backend._realm, acting_user=None, deactivation_reason="owner_request" + ) with self.assertRaisesRegex(Exception, "Realm has been deactivated"): backend.get_or_build_user(email, _LDAPUser()) @@ -7464,7 +7472,9 @@ class JWTFetchAPIKeyTest(ZulipTestCase): def test_inactive_realm_failure(self) -> None: payload = {"email": self.email} - do_deactivate_realm(self.user_profile.realm, acting_user=None) + do_deactivate_realm( + self.user_profile.realm, acting_user=None, deactivation_reason="owner_request" + ) with self.settings(JWT_AUTH_KEYS={"zulip": {"key": "key1", "algorithms": ["HS256"]}}): key = settings.JWT_AUTH_KEYS["zulip"]["key"] [algorithm] = settings.JWT_AUTH_KEYS["zulip"]["algorithms"] diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index 3eb5663cc0..ca7f7935d9 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -582,7 +582,9 @@ class DeactivatedRealmTest(ZulipTestCase): """ realm = get_realm("zulip") - do_deactivate_realm(get_realm("zulip"), acting_user=None) + do_deactivate_realm( + get_realm("zulip"), acting_user=None, deactivation_reason="owner_request" + ) result = self.client_post( "/json/messages", @@ -649,7 +651,9 @@ class DeactivatedRealmTest(ZulipTestCase): Using a webhook while in a deactivated realm fails """ - do_deactivate_realm(get_realm("zulip"), acting_user=None) + do_deactivate_realm( + get_realm("zulip"), acting_user=None, deactivation_reason="owner_request" + ) user_profile = self.example_user("hamlet") api_key = get_api_key(user_profile) url = f"/api/v1/external/jira?api_key={api_key}&stream=jira_custom" diff --git a/zerver/tests/test_email_change.py b/zerver/tests/test_email_change.py index 65009f30f7..5c91f9ad65 100644 --- a/zerver/tests/test_email_change.py +++ b/zerver/tests/test_email_change.py @@ -160,7 +160,9 @@ class EmailChangeTestCase(ZulipTestCase): self.login_user(user_profile) activation_url = self.generate_email_change_link(new_email) - do_deactivate_realm(user_profile.realm, acting_user=None) + do_deactivate_realm( + user_profile.realm, acting_user=None, deactivation_reason="owner_request" + ) response = self.client_get(activation_url) self.assertEqual(response.status_code, 302) diff --git a/zerver/tests/test_email_mirror.py b/zerver/tests/test_email_mirror.py index 855836587a..53486d1cb6 100644 --- a/zerver/tests/test_email_mirror.py +++ b/zerver/tests/test_email_mirror.py @@ -1282,7 +1282,9 @@ class TestMissedMessageEmailMessages(ZulipTestCase): mm_address = create_missed_message_address(user_profile, message) - do_deactivate_realm(user_profile.realm, acting_user=None) + do_deactivate_realm( + user_profile.realm, acting_user=None, deactivation_reason="owner_request" + ) incoming_valid_message = EmailMessage() incoming_valid_message.set_content("TestMissedMessageEmailMessages body") diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 60ce4bc525..3bef842c0d 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2932,7 +2932,7 @@ class NormalActionsTest(BaseAction): # correct because were one to somehow compute page_params (as # this test does), but that's not actually possible. with self.verify_action(state_change_expected=False) as events: - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") check_realm_deactivated("events[0]", events[0]) def test_do_mark_onboarding_step_as_read(self) -> None: diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 28ed65cb9b..4091d51060 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1668,7 +1668,7 @@ class AnalyticsBouncerTest(BouncerTestCase): do_set_realm_authentication_methods(zephyr_realm, new_auth_method_dict, acting_user=user) # Deactivation is synced. - do_deactivate_realm(zephyr_realm, acting_user=None) + do_deactivate_realm(zephyr_realm, acting_user=None, deactivation_reason="owner_request") send_server_data_to_push_bouncer() check_counts(5, 5, 1, 1, 7) diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 97aaac7e8f..2879ad3140 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -295,7 +295,7 @@ class RealmTest(ZulipTestCase): hamlet_id = self.example_user("hamlet").id get_user_profile_by_id(hamlet_id) realm = get_realm("zulip") - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") user = get_user_profile_by_id(hamlet_id) self.assertTrue(user.realm.deactivated) @@ -342,7 +342,7 @@ class RealmTest(ZulipTestCase): delay=timedelta(hours=1), ) self.assertEqual(ScheduledEmail.objects.count(), 1) - do_deactivate_realm(user.realm, acting_user=None) + do_deactivate_realm(user.realm, acting_user=None, deactivation_reason="owner_request") self.assertEqual(ScheduledEmail.objects.count(), 0) def test_do_change_realm_description_clears_cached_descriptions(self) -> None: @@ -366,10 +366,10 @@ class RealmTest(ZulipTestCase): realm = get_realm("zulip") self.assertFalse(realm.deactivated) - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") self.assertTrue(realm.deactivated) - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") self.assertTrue(realm.deactivated) def test_do_set_deactivated_redirect_on_deactivated_realm(self) -> None: @@ -377,7 +377,7 @@ class RealmTest(ZulipTestCase): realm = get_realm("zulip") redirect_url = "new_server.zulip.com" - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") self.assertTrue(realm.deactivated) do_add_deactivated_redirect(realm, redirect_url) self.assertEqual(realm.deactivated_redirect, redirect_url) @@ -389,7 +389,7 @@ class RealmTest(ZulipTestCase): def test_do_reactivate_realm(self) -> None: realm = get_realm("zulip") - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") self.assertTrue(realm.deactivated) do_reactivate_realm(realm) @@ -419,7 +419,7 @@ class RealmTest(ZulipTestCase): def test_realm_reactivation_link(self) -> None: realm = get_realm("zulip") - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") self.assertTrue(realm.deactivated) obj = RealmReactivationStatus.objects.create(realm=realm) @@ -432,13 +432,13 @@ class RealmTest(ZulipTestCase): self.assertFalse(realm.deactivated) # Make sure the link can't be reused. - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") response = self.client_get(confirmation_url) self.assertEqual(response.status_code, 404) def test_realm_reactivation_confirmation_object(self) -> None: realm = get_realm("zulip") - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") self.assertTrue(realm.deactivated) obj = RealmReactivationStatus.objects.create(realm=realm) create_confirmation_link(obj, Confirmation.REALM_REACTIVATION) @@ -449,7 +449,7 @@ class RealmTest(ZulipTestCase): def test_do_send_realm_reactivation_email(self) -> None: realm = get_realm("zulip") - do_deactivate_realm(realm, acting_user=None) + do_deactivate_realm(realm, acting_user=None, deactivation_reason="owner_request") self.assertEqual(realm.deactivated, True) iago = self.example_user("iago") do_send_realm_reactivation_email(realm, acting_user=iago) diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index c0316bbf29..0706da466b 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -556,7 +556,9 @@ class PasswordResetTest(ZulipTestCase): def test_password_reset_with_deactivated_realm(self) -> None: user_profile = self.example_user("hamlet") email = user_profile.delivery_email - do_deactivate_realm(user_profile.realm, acting_user=None) + do_deactivate_realm( + user_profile.realm, acting_user=None, deactivation_reason="owner_request" + ) # start the password reset process by supplying an email address with self.assertLogs(level="INFO") as m: @@ -4350,7 +4352,9 @@ class TestFindMyTeam(ZulipTestCase): self.assertIn("Unfortunately, no Zulip Cloud accounts", message.body) def test_find_team_deactivated_realm(self) -> None: - do_deactivate_realm(get_realm("zulip"), acting_user=None) + do_deactivate_realm( + get_realm("zulip"), acting_user=None, deactivation_reason="owner_request" + ) data = {"emails": self.example_email("hamlet")} result = self.client_post("/accounts/find/", data) self.assertEqual(result.status_code, 200) diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 6037c11f17..f33f02e071 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -477,7 +477,7 @@ def update_realm( @has_request_variables def deactivate_realm(request: HttpRequest, user: UserProfile) -> HttpResponse: realm = user.realm - do_deactivate_realm(realm, acting_user=user) + do_deactivate_realm(realm, acting_user=user, deactivation_reason="owner_request") return json_success(request)