test_invite: Rework and expand invitation limit tests.

This adds tests for more corner cases, in exchange for dropping the
query count tests, which were of dubious utility.  It also adds the
time-machine library to mock the current time to test that the limits
do expire.
This commit is contained in:
Alex Vandiver 2023-03-15 17:13:28 +00:00 committed by Tim Abbott
parent 6971c6d62d
commit 50a2a54393
3 changed files with 168 additions and 101 deletions

View File

@ -72,3 +72,6 @@ natsort
# For spell check linter # For spell check linter
codespell codespell
# For mocking time
time-machine

View File

@ -1739,6 +1739,7 @@ python-dateutil==2.8.2 \
# arrow # arrow
# botocore # botocore
# moto # moto
# time-machine
python-debian==0.1.49 \ python-debian==0.1.49 \
--hash=sha256:880f3bc52e31599f2a9b432bd7691844286825087fccdcf2f6ffd5cd79a26f9f \ --hash=sha256:880f3bc52e31599f2a9b432bd7691844286825087fccdcf2f6ffd5cd79a26f9f \
--hash=sha256:8cf677a30dbcb4be7a99536c17e11308a827a4d22028dc59a67f6c6dd3f0f58c --hash=sha256:8cf677a30dbcb4be7a99536c17e11308a827a4d22028dc59a67f6c6dd3f0f58c
@ -2252,6 +2253,61 @@ tblib==1.7.0 \
testslide==2.7.0 \ testslide==2.7.0 \
--hash=sha256:23ece0b9f5b704b33b978e9c8797936034516adc3c4743ebdaed664aa700c3cb --hash=sha256:23ece0b9f5b704b33b978e9c8797936034516adc3c4743ebdaed664aa700c3cb
# via pyre-check # via pyre-check
time-machine==2.9.0 \
--hash=sha256:010a58a8de1120308befae19e6c9de2ef5ca5206635cea33cb264998725cc027 \
--hash=sha256:0b9c36240876622b7f2f9e11bf72f100857c0a1e1a59af2da3d5067efea62c37 \
--hash=sha256:1d0ab46ce8a60baf9d86525694bf698fed9efefd22b8cbe1ca3e74abbb3239e1 \
--hash=sha256:2f080f6f7ca8cfca43bc5639288aebd0a273b4b5bd0acff609c2318728b13a18 \
--hash=sha256:359c806e5b9a7a3c73dbb808d19dca297f5504a5eefdc5d031db8d918f43e364 \
--hash=sha256:36dde844d28549929fab171d683c28a8db1c206547bcf6b7aca77319847d2046 \
--hash=sha256:372a97da01db89533d2f4ce50bbd908e5c56df7b8cfd6a005b177d0b14dc2938 \
--hash=sha256:3ce445775fcf7cb4040cfdba4b7c4888e7fd98bbcccfe1dc3fa8a798ed1f1d24 \
--hash=sha256:3ff5148e2e73392db8418a1fe2f0b06f4a0e76772933502fb61e4c3000b5324e \
--hash=sha256:49df5eea2160068e5b2bf28c22fc4c5aea00862ad88ddc3b62fc0f0683e97538 \
--hash=sha256:4b55654aaeaba380fcd6c004b8ada2978fdd4ece1e61e6b9717c6d4cc7fbbcd9 \
--hash=sha256:4f3755d9342ca1f1019418db52072272dfd75eb818fa4726fa8aabe208b38c26 \
--hash=sha256:5657e0e6077cf15b37f0d8cf78e868113bbb3ecccc60064c40fe52d8166ca8b1 \
--hash=sha256:60222d43f6e93a926adc36ed37a54bc8e4d0d8d1c4d449096afcfe85086129c2 \
--hash=sha256:6211beee9f5dace08b1bbbb1fb09e34a69c52d87eea676729f14c8660481dff6 \
--hash=sha256:6463e302c96eb8c691c4340e281bd54327a213b924fa189aea81accf7e7f78df \
--hash=sha256:68ec8b83197db32c7a12da5f6b83c91271af3ed7f5dc122d2900a8de01dff9f0 \
--hash=sha256:69898aed9b2315a90f5855343d9aa34d05fa06032e2e3bb14f2528941ec89dc1 \
--hash=sha256:6b632d60aa0883dc7292ac3d32050604d26ec2bbd5c4d42fb0de3b4ef17343e2 \
--hash=sha256:728263611d7940fda34d21573bd2b3f1491bdb52dbf75c5fe6c226dfe4655201 \
--hash=sha256:748d701228e646c224f2adfa6a11b986cd4aa90f1b8c13ef4534a3919c796bc0 \
--hash=sha256:8367fd03f2d7349c7fc20f14de186974eaca2502c64b948212de663742c8fd11 \
--hash=sha256:8670cb5cfda99f483d60de6ce56ceb0ec5d359193e79e4688e1c3c9db3937383 \
--hash=sha256:8830510adbf0a231184da277db9de1d55ef93ed228a575d217aaee295505abf1 \
--hash=sha256:8976b7b1f7de13598b655d459f5640f90f3cd587283e1b914a22e45946c5485b \
--hash=sha256:8bcc86b5a07ea9745f26dfad958dde0a4f56748c2ae0c9a96200a334d1b55055 \
--hash=sha256:8e2a90b8300812d8d774f2d2fc216fec3c7d94132ac589e062489c395061f16c \
--hash=sha256:8e797e5a2a99d1b237183e52251abfc1ad85c376278b39d1aca76a451a97861a \
--hash=sha256:948ca690f9770ad4a93fa183061c11346505598f5f0b721965bc85ec83bb103d \
--hash=sha256:9ba5fc2655749066d68986de8368984dad4082db2fbeade78f40506dc5b65672 \
--hash=sha256:9ee553f7732fa51e019e3329a6984593184c4e0410af1e73d91ce38a5d4b34ab \
--hash=sha256:a2cf80e5deaaa68c6cefb25303a4c870490b4e7591ed8e2435a65728920bc097 \
--hash=sha256:ae4e3f02ab5dabb35adca606237c7e1a515c86d69c0b7092bbe0e1cfe5cffc61 \
--hash=sha256:b16a2129f9146faa080bfd1b53447761f7386ec5c72890c827a65f33ab200336 \
--hash=sha256:b32addbf56639a9a8261fb62f8ea83473447671c83ca2c017ab1eabf4841157f \
--hash=sha256:b8faff03231ee55d5a216ce3e9171c5205459f866f54d4b5ee8aa1d860e4ce11 \
--hash=sha256:bb15b2b79b00d3f6cf7d62096f5e782fa740ecedfe0540c09f1d1e4d3d7b81ba \
--hash=sha256:bdbe785e046d124f73cca603ee37d5fae0b15dc4c13702488ad19de56aae08ba \
--hash=sha256:bfa82614a98ecee70272bb6038d210b2ad7b2a6b8a678b400c34bdaf776802a7 \
--hash=sha256:c01dbc3671d0649023daf623e952f9f0b4d904d57ab546d6d35a4aeb14915e8d \
--hash=sha256:c5dbc8b87cdc7be070a499f2bd1cd405c7f647abeb3447dfd397639df040bc64 \
--hash=sha256:cb51432652ad663b4cbd631c73c90f9e94f463382b86c0b6b854173700512a70 \
--hash=sha256:cc6bf01211b5ea40f633d5502c5aa495b415ebaff66e041820997dae70a508e1 \
--hash=sha256:d329578abe47ce95baa015ef3825acebb1b73b5fa6f818fdf2d4685a00ca457f \
--hash=sha256:d4380bd6697cc7db3c9e6843f24779ac0550affa9d9a8e5f9e5d5cc139cb6583 \
--hash=sha256:d79d374e32488c76cdb06fbdd4464083aeaa715ddca3e864bac7c7760eb03729 \
--hash=sha256:eaf334477bc0a9283d5150a56be8670a07295ef676e5b5a7f086952929d1a56b \
--hash=sha256:f6e79643368828d4651146a486be5a662846ac223ab5e2c73ddd519acfcc243c \
--hash=sha256:f92d5d2eb119a6518755c4c9170112094c706d1c604460f50afc1308eeb97f0e \
--hash=sha256:f97ed8bc5b517844a71030f74e9561de92f4902c306e6ccc8331a5b0c8dd0e00 \
--hash=sha256:fcdef7687aed5c4331c9808f4a414a41987441c3e7a2ba554e4dccfa4218e788 \
--hash=sha256:fd72c0b2e7443fff6e4481991742b72c17f73735e5fdd176406ca48df187a5c9 \
--hash=sha256:fe013942ab7f3241fcbe66ee43222d47f499d1e0cb69e913791c52e638ddd7f0
# via -r requirements/dev.in
tinycss2==1.2.1 \ tinycss2==1.2.1 \
--hash=sha256:2b80a96d41e7c3914b8cda8bc7f705a4d9c49275616e886103dd839dfc847847 \ --hash=sha256:2b80a96d41e7c3914b8cda8bc7f705a4d9c49275616e886103dd839dfc847847 \
--hash=sha256:8cff3a8f066c2ec677c06dbc7b45619804a6938478d9d73c284b29d14ecb0627 --hash=sha256:8cff3a8f066c2ec677c06dbc7b45619804a6938478d9d73c284b29d14ecb0627

View File

@ -6,6 +6,7 @@ from unittest.mock import patch
from urllib.parse import urlencode from urllib.parse import urlencode
import orjson import orjson
import time_machine
from django.conf import settings from django.conf import settings
from django.core import mail from django.core import mail
from django.core.mail.message import EmailMultiAlternatives from django.core.mail.message import EmailMultiAlternatives
@ -28,9 +29,7 @@ from zerver.actions.invites import (
do_invite_users, do_invite_users,
do_revoke_multi_use_invite, do_revoke_multi_use_invite,
) )
from zerver.actions.realm_settings import ( from zerver.actions.realm_settings import do_set_realm_property
do_set_realm_property,
)
from zerver.actions.user_settings import do_change_full_name from zerver.actions.user_settings import do_change_full_name
from zerver.actions.users import change_user_is_active from zerver.actions.users import change_user_is_active
from zerver.context_processors import common_context from zerver.context_processors import common_context
@ -40,11 +39,7 @@ from zerver.lib.send_email import (
send_future_email, send_future_email,
) )
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import ( from zerver.lib.test_helpers import find_key_by_email
cache_tries_captured,
find_key_by_email,
queries_captured,
)
from zerver.models import ( from zerver.models import (
Message, Message,
MultiuseInvite, MultiuseInvite,
@ -66,7 +61,7 @@ if TYPE_CHECKING:
class InviteUserBase(ZulipTestCase): class InviteUserBase(ZulipTestCase):
def check_sent_emails(self, correct_recipients: List[str]) -> None: def check_sent_emails(self, correct_recipients: List[str], clear: bool = False) -> None:
self.assert_length(mail.outbox, len(correct_recipients)) self.assert_length(mail.outbox, len(correct_recipients))
email_recipients = [email.recipients()[0] for email in mail.outbox] email_recipients = [email.recipients()[0] for email in mail.outbox]
self.assertEqual(sorted(email_recipients), sorted(correct_recipients)) self.assertEqual(sorted(email_recipients), sorted(correct_recipients))
@ -80,7 +75,8 @@ class InviteUserBase(ZulipTestCase):
self.email_display_from(mail.outbox[0]), rf" <{self.TOKENIZED_NOREPLY_REGEX}>\Z" self.email_display_from(mail.outbox[0]), rf" <{self.TOKENIZED_NOREPLY_REGEX}>\Z"
) )
self.assertEqual(mail.outbox[0].extra_headers["List-Id"], "Zulip Dev <zulip.testserver>") if clear:
mail.outbox = []
def invite( def invite(
self, self,
@ -89,6 +85,7 @@ class InviteUserBase(ZulipTestCase):
invite_expires_in_minutes: Optional[int] = INVITATION_LINK_VALIDITY_MINUTES, invite_expires_in_minutes: Optional[int] = INVITATION_LINK_VALIDITY_MINUTES,
body: str = "", body: str = "",
invite_as: int = PreregistrationUser.INVITE_AS["MEMBER"], invite_as: int = PreregistrationUser.INVITE_AS["MEMBER"],
realm: Optional[Realm] = None,
) -> "TestHttpResponse": ) -> "TestHttpResponse":
""" """
Invites the specified users to Zulip with the specified streams. Invites the specified users to Zulip with the specified streams.
@ -100,7 +97,7 @@ class InviteUserBase(ZulipTestCase):
""" """
stream_ids = [] stream_ids = []
for stream_name in stream_names: for stream_name in stream_names:
stream_ids.append(self.get_stream_id(stream_name)) stream_ids.append(self.get_stream_id(stream_name, realm=realm))
invite_expires_in: Union[str, Optional[int]] = invite_expires_in_minutes invite_expires_in: Union[str, Optional[int]] = invite_expires_in_minutes
if invite_expires_in is None: if invite_expires_in is None:
@ -114,6 +111,7 @@ class InviteUserBase(ZulipTestCase):
"stream_ids": orjson.dumps(stream_ids).decode(), "stream_ids": orjson.dumps(stream_ids).decode(),
"invite_as": invite_as, "invite_as": invite_as,
}, },
subdomain=realm.string_id if realm else "zulip",
) )
@ -153,93 +151,121 @@ class InviteUserTest(InviteUserBase):
def test_invite_limits(self) -> None: def test_invite_limits(self) -> None:
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
realm = user_profile.realm realm = user_profile.realm
stream_name = "Denmark"
# These constants only need to be in descending order
# for this test to trigger an InvitationError based
# on max daily counts.
site_max = 50
realm_max = 40
num_invitees = 30
max_daily_count = 20
daily_counts = [(1, max_daily_count)]
invite_emails = [f"foo-{i:02}@zulip.com" for i in range(num_invitees)]
invitees = ",".join(invite_emails)
self.login_user(user_profile) self.login_user(user_profile)
realm.max_invites = realm_max def try_invite(
num_invitees: int,
*,
default_realm_max: int,
new_realm_max: int,
realm_max: int,
open_realm_creation: bool = True,
realm: Optional[Realm] = None,
stream_name: str = "Denmark",
) -> "TestHttpResponse":
if realm is None:
realm = get_realm("zulip")
invitees = ",".join(
[f"{realm.string_id}-{i:02}@zulip.com" for i in range(num_invitees)]
)
with self.settings(
OPEN_REALM_CREATION=open_realm_creation,
INVITES_DEFAULT_REALM_DAILY_MAX=default_realm_max,
INVITES_NEW_REALM_LIMIT_DAYS=[(1, new_realm_max)],
):
realm.max_invites = realm_max
realm.save()
return self.invite(invitees, [stream_name], realm=realm)
# Trip the "new realm" limits
realm.date_created = timezone_now() realm.date_created = timezone_now()
realm.save() realm.save()
result = try_invite(30, default_realm_max=50, new_realm_max=20, realm_max=40)
def try_invite() -> "TestHttpResponse":
with self.settings(
OPEN_REALM_CREATION=True,
INVITES_DEFAULT_REALM_DAILY_MAX=site_max,
INVITES_NEW_REALM_LIMIT_DAYS=daily_counts,
):
result = self.invite(invitees, [stream_name])
return result
result = try_invite()
self.assert_json_error_contains(result, "reached the limit") self.assert_json_error_contains(result, "reached the limit")
self.check_sent_emails([])
# Next show that aggregate limits expire once the realm is old # If some other realm consumes some invites, it affects our realm. Invite 20 users in lear:
# enough. lear_realm = get_realm("lear")
lear_realm.date_created = timezone_now()
lear_realm.save()
self.login_user(self.lear_user("king"))
result = try_invite(
20,
default_realm_max=50,
new_realm_max=20,
realm_max=40,
realm=lear_realm,
stream_name="general",
)
self.assert_json_success(result)
self.check_sent_emails([f"lear-{i:02}@zulip.com" for i in range(20)], clear=True)
# Which prevents inviting 1 in our realm:
self.login_user(user_profile)
result = try_invite(1, default_realm_max=50, new_realm_max=20, realm_max=40)
self.assert_json_error_contains(result, "reached the limit")
self.check_sent_emails([])
# If our realm max is over the default realm's, we're exempt from INVITES_NEW_REALM_LIMIT_DAYS
result = try_invite(10, default_realm_max=15, new_realm_max=5, realm_max=20)
self.assert_json_success(result)
self.check_sent_emails([f"zulip-{i:02}@zulip.com" for i in range(10)], clear=True)
# We've sent 10 invites. Trying to invite 15 people, even if
# 10 of them are the same, still trips the limit (10 previous
# + 15 in this submission > 20 realm max)
result = try_invite(15, default_realm_max=15, new_realm_max=5, realm_max=20)
self.assert_json_error_contains(result, "reached the limit")
self.check_sent_emails([])
# Inviting 10 more people (to the realm max of 20) works, and
# sends emails to the same 10 users again.
result = try_invite(10, default_realm_max=15, new_realm_max=5, realm_max=20)
self.assert_json_success(result)
self.check_sent_emails([f"zulip-{i:02}@zulip.com" for i in range(10)], clear=True)
# We've sent 20 invites. The 10 we just sent do count against
# us if we send to them again, since we did send mail
result = try_invite(10, default_realm_max=15, new_realm_max=5, realm_max=20)
self.assert_json_error_contains(result, "reached the limit")
self.check_sent_emails([])
# We've sent 20 invites. The realm is exempt from the new realm max
# (INVITES_NEW_REALM_LIMIT_DAYS) if it is old enough
realm.date_created = timezone_now() - datetime.timedelta(days=8) realm.date_created = timezone_now() - datetime.timedelta(days=8)
realm.save() realm.save()
result = try_invite(10, default_realm_max=50, new_realm_max=20, realm_max=40)
with queries_captured() as queries:
with cache_tries_captured() as cache_tries:
result = try_invite()
self.assert_json_success(result) self.assert_json_success(result)
self.check_sent_emails([f"zulip-{i:02}@zulip.com" for i in range(10)], clear=True)
# TODO: Fix large query count here. # We've sent 30 invites. None of the limits matter if open
# # realm creation is disabled.
# TODO: There is some test OTHER than this one result = try_invite(
# that is leaking some kind of state change 10, default_realm_max=30, new_realm_max=20, realm_max=10, open_realm_creation=False
# that throws off the query count here. It )
# is hard to investigate currently (due to
# the large number of queries), so I just
# use an approximate equality check.
actual_count = len(queries)
expected_count = 251
if abs(actual_count - expected_count) > 1:
raise AssertionError(
f"""
Unexpected number of queries:
expected query count: {expected_count}
actual: {actual_count}
"""
)
# Almost all of these cache hits are to re-fetch each one of the
# invitees. These happen inside our queue processor for sending
# confirmation emails, so they are somewhat difficult to avoid.
#
# TODO: Mock the call to queue_json_publish, so we can measure the
# queue impact separately from the user-perceived impact.
self.assert_length(cache_tries, 32)
# Next get line coverage on bumping a realm's max_invites.
realm.date_created = timezone_now()
realm.max_invites = site_max + 10
realm.save()
result = try_invite()
self.assert_json_success(result) self.assert_json_success(result)
self.check_sent_emails([f"zulip-{i:02}@zulip.com" for i in range(10)], clear=True)
# Finally get coverage on the case that OPEN_REALM_CREATION is False. # We've sent 40 invites "today". Fast-forward 48 hours
# and ensure that we can invite more people
with time_machine.travel(timezone_now() + datetime.timedelta(hours=48)):
result = try_invite(5, default_realm_max=30, new_realm_max=20, realm_max=10)
self.assert_json_success(result)
self.check_sent_emails([f"zulip-{i:02}@zulip.com" for i in range(5)], clear=True)
with self.settings(OPEN_REALM_CREATION=False): # We've sent 5 invites. Ensure we can trip the fresh "today" limit for the realm
result = self.invite(invitees, [stream_name]) result = try_invite(10, default_realm_max=30, new_realm_max=20, realm_max=10)
self.assert_json_error_contains(result, "reached the limit")
self.check_sent_emails([])
self.assert_json_success(result) # We've sent 5 invites. Reset the realm to be "recently"
# created, and ensure that we can trip the whole-server
# limit
realm.date_created = timezone_now() - datetime.timedelta(days=3)
realm.save()
result = try_invite(10, default_realm_max=50, new_realm_max=10, realm_max=40)
self.assert_json_error_contains(result, "reached the limit")
self.check_sent_emails([])
def test_invite_user_to_realm_on_manual_license_plan(self) -> None: def test_invite_user_to_realm_on_manual_license_plan(self) -> None:
user = self.example_user("hamlet") user = self.example_user("hamlet")
@ -713,24 +739,6 @@ earl-test@zulip.com""",
realm.max_invites = settings.INVITES_DEFAULT_REALM_DAILY_MAX realm.max_invites = settings.INVITES_DEFAULT_REALM_DAILY_MAX
realm.save() realm.save()
def test_invite_too_many_users(self) -> None:
# Only a light test of this pathway; e.g. doesn't test that
# the limit gets reset after 24 hours
self.login("iago")
invitee_emails = "1@zulip.com, 2@zulip.com"
self.invite(invitee_emails, ["Denmark"])
self.check_sent_emails(["1@zulip.com", "2@zulip.com"])
mail.outbox = []
invitee_emails = ", ".join(
f"more-{i}@zulip.com" for i in range(get_realm("zulip").max_invites - 1)
)
self.assert_json_error(
self.invite(invitee_emails, ["Denmark"]),
"To protect users, Zulip limits the number of invitations you can send in one day. Because you have reached the limit, no invitations were sent.",
)
self.check_sent_emails([])
def test_missing_or_invalid_params(self) -> None: def test_missing_or_invalid_params(self) -> None:
""" """
Tests inviting with various missing or invalid parameters. Tests inviting with various missing or invalid parameters.