mirror of https://github.com/zulip/zulip.git
invites: Switch new LIMITED-plan heuristic to enforcing.
This commit is contained in:
parent
50a2a54393
commit
330141f55d
|
@ -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)
|
||||
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue