diff --git a/analytics/tests/test_counts.py b/analytics/tests/test_counts.py index 0726664405..f910e97495 100644 --- a/analytics/tests/test_counts.py +++ b/analytics/tests/test_counts.py @@ -1382,7 +1382,7 @@ class TestLoggingCountStats(AnalyticsTestCase): stream, _ = self.create_stream_with_recipient() invite_expires_in_minutes = 2 * 24 * 60 - with mock.patch("zerver.actions.invites.apply_invite_realm_heuristics"): + with mock.patch("zerver.actions.invites.too_many_recent_realm_invites", return_value=False): do_invite_users( user, ["user1@domain.tld", "user2@domain.tld"], @@ -1393,7 +1393,7 @@ class TestLoggingCountStats(AnalyticsTestCase): # We currently send emails when re-inviting users that haven't # turned into accounts, so count them towards the total - with mock.patch("zerver.actions.invites.apply_invite_realm_heuristics"): + with mock.patch("zerver.actions.invites.too_many_recent_realm_invites", return_value=False): do_invite_users( user, ["user1@domain.tld", "user2@domain.tld"], @@ -1404,7 +1404,7 @@ class TestLoggingCountStats(AnalyticsTestCase): # Test mix of good and malformed invite emails with self.assertRaises(InvitationError), mock.patch( - "zerver.actions.invites.apply_invite_realm_heuristics" + "zerver.actions.invites.too_many_recent_realm_invites", return_value=False ): do_invite_users( user, @@ -1416,7 +1416,7 @@ class TestLoggingCountStats(AnalyticsTestCase): # Test inviting existing users with self.assertRaises(InvitationError), mock.patch( - "zerver.actions.invites.apply_invite_realm_heuristics" + "zerver.actions.invites.too_many_recent_realm_invites", return_value=False ): do_invite_users( user, @@ -1433,7 +1433,7 @@ class TestLoggingCountStats(AnalyticsTestCase): assertInviteCountEquals(5) # Resending invite should cost you - with mock.patch("zerver.actions.invites.apply_invite_realm_heuristics"): + with mock.patch("zerver.actions.invites.too_many_recent_realm_invites", return_value=False): do_resend_user_invite_email(assert_is_not_none(PreregistrationUser.objects.first())) assertInviteCountEquals(6) diff --git a/zerver/actions/invites.py b/zerver/actions/invites.py index d68c04b9bb..14d0a42720 100644 --- a/zerver/actions/invites.py +++ b/zerver/actions/invites.py @@ -82,11 +82,16 @@ def estimate_recent_invites(realms: Collection[Realm], *, days: int) -> int: return recent_invites -def apply_invite_realm_heuristics(realm: Realm, recent_invites: int, num_invitees: int) -> None: +def too_many_recent_realm_invites(realm: Realm, num_invitees: int) -> bool: + # Basic check that we're blow the realm-set limit + recent_invites = estimate_recent_invites([realm], days=1) + if num_invitees + recent_invites > realm.max_invites: + return True + if realm.plan_type != Realm.PLAN_TYPE_LIMITED: - return + return False if realm.max_invites != settings.INVITES_DEFAULT_REALM_DAILY_MAX: - return + return False # If they're a non-paid plan with default invitation limits, # we further limit how many invitations can be sent in a day @@ -97,50 +102,50 @@ def apply_invite_realm_heuristics(realm: Realm, recent_invites: int, num_invitee # unlikely to hit these limits. If a real realm hits them, # the resulting message suggests that they contact support if # they have a real use case. - suspicion_score = 0 + warning_flags = [] if zxcvbn(realm.string_id)["score"] == 4: # Very high entropy realm names are suspicious - suspicion_score += 1 + warning_flags.append("random-realm-name") if not realm.description: - suspicion_score += 1 + warning_flags.append("no-realm-description") if realm.icon_source == Realm.ICON_FROM_GRAVATAR: - suspicion_score += 1 + warning_flags.append("no-realm-icon") if realm.date_created >= timezone_now() - datetime.timedelta(hours=1): - suspicion_score += 1 + warning_flags.append("realm-created-in-last-hour") current_user_count = len(UserProfile.objects.filter(realm=realm, is_bot=False, is_active=True)) if current_user_count == 1: - suspicion_score += 1 + warning_flags.append("only-one-user") estimated_sent = RealmCount.objects.filter( realm=realm, property="messages_sent:message_type:day" ).aggregate(messages=Sum("value")) if not estimated_sent["messages"]: - suspicion_score += 1 + warning_flags.append("no-messages-sent") - if suspicion_score == 6: + if len(warning_flags) == 6: permitted_ratio = 2 - elif suspicion_score >= 3: + elif len(warning_flags) >= 3: permitted_ratio = 3 else: permitted_ratio = 5 - # For now, simply log the data; this will change to a raise of - # InvitationError once we've done some auditing. - logging.warning( - "%s (suspicion %d/6) inviting %d more, have %d recent, %d max, but only %d current users. Ratio %.1f, %d allowed", + ratio = (num_invitees + recent_invites) / current_user_count + logging.log( + logging.WARNING if ratio > permitted_ratio else logging.INFO, + "%s (!: %s) inviting %d more, have %d recent, but only %d current users. Ratio %.1f, %d allowed", realm.string_id, - suspicion_score, + ",".join(warning_flags), num_invitees, recent_invites, - realm.max_invites, current_user_count, - (num_invitees + recent_invites) / current_user_count, + ratio, permitted_ratio, ) + return ratio > permitted_ratio def check_invite_limit(realm: Realm, num_invitees: int) -> None: @@ -151,8 +156,7 @@ def check_invite_limit(realm: Realm, num_invitees: int) -> None: if not settings.OPEN_REALM_CREATION: return - recent_invites = estimate_recent_invites([realm], days=1) - if num_invitees + recent_invites > realm.max_invites: + if too_many_recent_realm_invites(realm, num_invitees): raise InvitationError( msg, [], @@ -160,8 +164,6 @@ def check_invite_limit(realm: Realm, num_invitees: int) -> None: daily_limit_reached=True, ) - apply_invite_realm_heuristics(realm, recent_invites, num_invitees) - default_max = settings.INVITES_DEFAULT_REALM_DAILY_MAX newrealm_age = datetime.timedelta(days=settings.INVITES_NEW_REALM_DAYS) if realm.date_created <= timezone_now() - newrealm_age: diff --git a/zerver/tests/test_invite.py b/zerver/tests/test_invite.py index 0c849eb2a0..71d34aa4b6 100644 --- a/zerver/tests/test_invite.py +++ b/zerver/tests/test_invite.py @@ -11,6 +11,7 @@ from django.conf import settings from django.core import mail from django.core.mail.message import EmailMultiAlternatives from django.http import HttpRequest +from django.test import override_settings from django.urls import reverse from django.utils.timezone import now as timezone_now @@ -22,14 +23,16 @@ from confirmation.models import ( get_object_from_key, ) from corporate.lib.stripe import get_latest_seat_count +from zerver.actions.create_realm import do_change_realm_subdomain, do_create_realm from zerver.actions.create_user import do_create_user, process_new_human_user from zerver.actions.invites import ( do_create_multiuse_invite_link, do_get_invites_controlled_by_user, do_invite_users, do_revoke_multi_use_invite, + too_many_recent_realm_invites, ) -from zerver.actions.realm_settings import do_set_realm_property +from zerver.actions.realm_settings import do_change_realm_plan_type, do_set_realm_property from zerver.actions.user_settings import do_change_full_name from zerver.actions.users import change_user_is_active from zerver.context_processors import common_context @@ -267,6 +270,164 @@ class InviteUserTest(InviteUserBase): self.assert_json_error_contains(result, "reached the limit") self.check_sent_emails([]) + @override_settings(OPEN_REALM_CREATION=True) + def test_limited_plan_heuristics(self) -> None: + # There additional limits only apply if OPEN_REALM_CREATION is + # True and the plan is "limited," which is primarily only + # relevant on Zulip Cloud. + + realm = do_create_realm("sdfoijt23489fuskdfjhksdf", "Totally Normal") + realm.plan_type = Realm.PLAN_TYPE_LIMITED + realm.invite_required = False + realm.save() + + # Create a first user + admin_user = do_create_user( + "someone@example.com", + "password", + realm, + "full name", + role=UserProfile.ROLE_REALM_OWNER, + realm_creation=True, + acting_user=None, + ) + + # Inviting would work at all + with self.assertLogs(level="INFO") as m: + self.assertFalse(too_many_recent_realm_invites(realm, 1)) + self.assertEqual( + m.output, + [ + ( + "INFO:root:sdfoijt23489fuskdfjhksdf " + "(!: random-realm-name,no-realm-description,no-realm-icon,realm-created-in-last-hour,only-one-user,no-messages-sent) " + "inviting 1 more, have 0 recent, but only 1 current users. " + "Ratio 1.0, 2 allowed" + ) + ], + ) + + # This realm is currently very suspicious, so can only invite + # 2 users at once (2x current 1 user) + with self.assertLogs(level="INFO") as m: + self.assertFalse(too_many_recent_realm_invites(realm, 2)) + self.assertTrue(too_many_recent_realm_invites(realm, 3)) + self.assertEqual( + m.output, + [ + ( + "INFO:root:sdfoijt23489fuskdfjhksdf " + "(!: random-realm-name,no-realm-description,no-realm-icon,realm-created-in-last-hour,only-one-user,no-messages-sent) " + "inviting 2 more, have 0 recent, but only 1 current users. " + "Ratio 2.0, 2 allowed" + ), + ( + "WARNING:root:sdfoijt23489fuskdfjhksdf " + "(!: random-realm-name,no-realm-description,no-realm-icon,realm-created-in-last-hour,only-one-user,no-messages-sent) " + "inviting 3 more, have 0 recent, but only 1 current users. " + "Ratio 3.0, 2 allowed" + ), + ], + ) + + # Having another user makes it slightly less suspicious, and + # also able to invite more in ratio with the current count of + # users (3x current 2 users) + self.register("other@example.com", "test", subdomain=realm.string_id) + with self.assertLogs(level="INFO") as m: + self.assertFalse(too_many_recent_realm_invites(realm, 6)) + self.assertTrue(too_many_recent_realm_invites(realm, 7)) + self.assertEqual( + m.output, + [ + ( + "INFO:root:sdfoijt23489fuskdfjhksdf " + "(!: random-realm-name,no-realm-description,no-realm-icon,realm-created-in-last-hour,no-messages-sent) " + "inviting 6 more, have 0 recent, but only 2 current users. " + "Ratio 3.0, 3 allowed" + ), + ( + "WARNING:root:sdfoijt23489fuskdfjhksdf " + "(!: random-realm-name,no-realm-description,no-realm-icon,realm-created-in-last-hour,no-messages-sent) " + "inviting 7 more, have 0 recent, but only 2 current users. " + "Ratio 3.5, 3 allowed" + ), + ], + ) + + # Remove some more warning flags + do_change_realm_subdomain(realm, "reasonable", acting_user=None) + realm.description = "A real place" + realm.date_created = timezone_now() - datetime.timedelta(hours=2) + realm.save() + + # This is now more allowable (5x current 2 users) + with self.assertLogs(level="INFO") as m: + self.assertFalse(too_many_recent_realm_invites(realm, 10)) + self.assertTrue(too_many_recent_realm_invites(realm, 11)) + self.assertEqual( + m.output, + [ + ( + "INFO:root:reasonable " + "(!: no-realm-icon,no-messages-sent) " + "inviting 10 more, have 0 recent, but only 2 current users. " + "Ratio 5.0, 5 allowed" + ), + ( + "WARNING:root:reasonable " + "(!: no-realm-icon,no-messages-sent) " + "inviting 11 more, have 0 recent, but only 2 current users. " + "Ratio 5.5, 5 allowed" + ), + ], + ) + + # If we have a different max_invites on the realm that kicks in, though + realm.max_invites = 8 + realm.save() + self.assertFalse(too_many_recent_realm_invites(realm, 8)) + self.assertTrue(too_many_recent_realm_invites(realm, 9)) + + # And if we have a non-default max invite then that applies + # but not the heuristics (which would limit us to 10, here) + realm.max_invites = 12 + realm.save() + self.assertFalse(too_many_recent_realm_invites(realm, 12)) + self.assertTrue(too_many_recent_realm_invites(realm, 13)) + + # Not being a limited plan also opens us up from the + # heuristics. First, set us back to the default invite limit + realm.max_invites = settings.INVITES_DEFAULT_REALM_DAILY_MAX + realm.save() + with self.assertLogs(level="INFO") as m: + self.assertFalse(too_many_recent_realm_invites(realm, 10)) + self.assertTrue(too_many_recent_realm_invites(realm, 11)) + self.assertEqual( + m.output, + [ + ( + "INFO:root:reasonable " + "(!: no-realm-icon,no-messages-sent) " + "inviting 10 more, have 0 recent, but only 2 current users. " + "Ratio 5.0, 5 allowed" + ), + ( + "WARNING:root:reasonable " + "(!: no-realm-icon,no-messages-sent) " + "inviting 11 more, have 0 recent, but only 2 current users. " + "Ratio 5.5, 5 allowed" + ), + ], + ) + # Become a Standard plan + do_change_realm_plan_type(realm, Realm.PLAN_TYPE_STANDARD, acting_user=admin_user) + self.assertFalse(too_many_recent_realm_invites(realm, 3000)) + self.assertTrue(too_many_recent_realm_invites(realm, 3001)) + do_change_realm_plan_type(realm, Realm.PLAN_TYPE_STANDARD_FREE, acting_user=admin_user) + self.assertFalse(too_many_recent_realm_invites(realm, 3000)) + self.assertTrue(too_many_recent_realm_invites(realm, 3001)) + def test_invite_user_to_realm_on_manual_license_plan(self) -> None: user = self.example_user("hamlet") self.login_user(user)