diff --git a/requirements/dev.in b/requirements/dev.in index 2044878a66..981944438d 100644 --- a/requirements/dev.in +++ b/requirements/dev.in @@ -72,3 +72,6 @@ natsort # For spell check linter codespell + +# For mocking time +time-machine diff --git a/requirements/dev.txt b/requirements/dev.txt index 63723e028a..2e58996671 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1739,6 +1739,7 @@ python-dateutil==2.8.2 \ # arrow # botocore # moto + # time-machine python-debian==0.1.49 \ --hash=sha256:880f3bc52e31599f2a9b432bd7691844286825087fccdcf2f6ffd5cd79a26f9f \ --hash=sha256:8cf677a30dbcb4be7a99536c17e11308a827a4d22028dc59a67f6c6dd3f0f58c @@ -2252,6 +2253,61 @@ tblib==1.7.0 \ testslide==2.7.0 \ --hash=sha256:23ece0b9f5b704b33b978e9c8797936034516adc3c4743ebdaed664aa700c3cb # 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 \ --hash=sha256:2b80a96d41e7c3914b8cda8bc7f705a4d9c49275616e886103dd839dfc847847 \ --hash=sha256:8cff3a8f066c2ec677c06dbc7b45619804a6938478d9d73c284b29d14ecb0627 diff --git a/zerver/tests/test_invite.py b/zerver/tests/test_invite.py index 0ab2db5d93..0c849eb2a0 100644 --- a/zerver/tests/test_invite.py +++ b/zerver/tests/test_invite.py @@ -6,6 +6,7 @@ from unittest.mock import patch from urllib.parse import urlencode import orjson +import time_machine from django.conf import settings from django.core import mail from django.core.mail.message import EmailMultiAlternatives @@ -28,9 +29,7 @@ from zerver.actions.invites import ( do_invite_users, do_revoke_multi_use_invite, ) -from zerver.actions.realm_settings import ( - do_set_realm_property, -) +from zerver.actions.realm_settings import 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 @@ -40,11 +39,7 @@ from zerver.lib.send_email import ( send_future_email, ) from zerver.lib.test_classes import ZulipTestCase -from zerver.lib.test_helpers import ( - cache_tries_captured, - find_key_by_email, - queries_captured, -) +from zerver.lib.test_helpers import find_key_by_email from zerver.models import ( Message, MultiuseInvite, @@ -66,7 +61,7 @@ if TYPE_CHECKING: 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)) email_recipients = [email.recipients()[0] for email in mail.outbox] 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.assertEqual(mail.outbox[0].extra_headers["List-Id"], "Zulip Dev ") + if clear: + mail.outbox = [] def invite( self, @@ -89,6 +85,7 @@ class InviteUserBase(ZulipTestCase): invite_expires_in_minutes: Optional[int] = INVITATION_LINK_VALIDITY_MINUTES, body: str = "", invite_as: int = PreregistrationUser.INVITE_AS["MEMBER"], + realm: Optional[Realm] = None, ) -> "TestHttpResponse": """ Invites the specified users to Zulip with the specified streams. @@ -100,7 +97,7 @@ class InviteUserBase(ZulipTestCase): """ stream_ids = [] 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 if invite_expires_in is None: @@ -114,6 +111,7 @@ class InviteUserBase(ZulipTestCase): "stream_ids": orjson.dumps(stream_ids).decode(), "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: user_profile = self.example_user("hamlet") 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) - 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.save() - - 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() + result = try_invite(30, default_realm_max=50, new_realm_max=20, realm_max=40) self.assert_json_error_contains(result, "reached the limit") + self.check_sent_emails([]) - # Next show that aggregate limits expire once the realm is old - # enough. + # If some other realm consumes some invites, it affects our realm. Invite 20 users in lear: + 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.save() - - with queries_captured() as queries: - with cache_tries_captured() as cache_tries: - result = try_invite() - + result = try_invite(10, default_realm_max=50, new_realm_max=20, realm_max=40) 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. - # - # TODO: There is some test OTHER than this one - # that is leaking some kind of state change - # 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() + # We've sent 30 invites. None of the limits matter if open + # realm creation is disabled. + result = try_invite( + 10, default_realm_max=30, new_realm_max=20, realm_max=10, open_realm_creation=False + ) 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): - result = self.invite(invitees, [stream_name]) + # We've sent 5 invites. Ensure we can trip the fresh "today" limit for the realm + 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: user = self.example_user("hamlet") @@ -713,24 +739,6 @@ earl-test@zulip.com""", realm.max_invites = settings.INVITES_DEFAULT_REALM_DAILY_MAX 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: """ Tests inviting with various missing or invalid parameters.