From 676edb3802a5ad38a9c95cd797c1cf677e59f21d Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Mon, 2 Aug 2021 20:45:55 +0200 Subject: [PATCH] confirmation: Migration to add expiry_date step 1. The commit: 1. Adds the new field as nullable. 2. Adds code that'll create new Confirmation with the field set correctly. 3. For verifying validity of Confirmation object this still uses the old logic in get_object_from_key() to keep things functioning until we backfill the old objects in the next step. Thus this commit is deployable. Next we'll have a commit to run a backfill migration. --- .../0008_confirmation_expiry_date.py | 17 +++++++ confirmation/models.py | 11 +++++ zerver/tests/test_auth_backends.py | 6 ++- zerver/tests/test_signup.py | 45 ++++++++----------- 4 files changed, 51 insertions(+), 28 deletions(-) create mode 100644 confirmation/migrations/0008_confirmation_expiry_date.py diff --git a/confirmation/migrations/0008_confirmation_expiry_date.py b/confirmation/migrations/0008_confirmation_expiry_date.py new file mode 100644 index 0000000000..9e22076f8b --- /dev/null +++ b/confirmation/migrations/0008_confirmation_expiry_date.py @@ -0,0 +1,17 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("confirmation", "0007_add_indexes"), + ] + + operations = [ + migrations.AddField( + model_name="confirmation", + name="expiry_date", + field=models.DateTimeField(db_index=True, null=True), + preserve_default=False, + ), + ] diff --git a/confirmation/models.py b/confirmation/models.py index ce3ea79123..276eab32f4 100644 --- a/confirmation/models.py +++ b/confirmation/models.py @@ -85,6 +85,7 @@ def get_object_from_key( def create_confirmation_link( obj: Union[Realm, HasRealmObject, OptionalHasRealmObject], confirmation_type: int, + validity_in_days: Optional[int] = None, url_args: Mapping[str, str] = {}, ) -> str: key = generate_key() @@ -94,11 +95,20 @@ def create_confirmation_link( elif hasattr(obj, "realm"): realm = obj.realm + expiry_date = None + if validity_in_days: + expiry_date = timezone_now() + datetime.timedelta(days=validity_in_days) + else: + expiry_date = timezone_now() + datetime.timedelta( + days=_properties[confirmation_type].validity_in_days + ) + Confirmation.objects.create( content_object=obj, date_sent=timezone_now(), confirmation_key=key, realm=realm, + expiry_date=expiry_date, type=confirmation_type, ) return confirmation_url(key, realm, confirmation_type, url_args) @@ -124,6 +134,7 @@ class Confirmation(models.Model): content_object = GenericForeignKey("content_type", "object_id") date_sent: datetime.datetime = models.DateTimeField(db_index=True) confirmation_key: str = models.CharField(max_length=40, db_index=True) + expiry_date: datetime.datetime = models.DateTimeField(db_index=True, null=True) realm: Optional[Realm] = models.ForeignKey(Realm, null=True, on_delete=CASCADE) # The following list is the set of valid types diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index aa243d7cb2..e006060d0a 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -1510,7 +1510,8 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase): referrer = self.example_user("hamlet") multiuse_obj = MultiuseInvite.objects.create(realm=realm, referred_by=referrer) multiuse_obj.streams.set(streams) - create_confirmation_link(multiuse_obj, Confirmation.MULTIUSE_INVITE) + validity_in_days = 2 + create_confirmation_link(multiuse_obj, Confirmation.MULTIUSE_INVITE, validity_in_days) multiuse_confirmation = Confirmation.objects.all().last() assert multiuse_confirmation is not None multiuse_object_key = multiuse_confirmation.confirmation_key @@ -4036,7 +4037,8 @@ class GoogleAuthBackendTest(SocialAuthBase): referrer = self.example_user("hamlet") multiuse_obj = MultiuseInvite.objects.create(realm=realm, referred_by=referrer) multiuse_obj.streams.set(streams) - create_confirmation_link(multiuse_obj, Confirmation.MULTIUSE_INVITE) + validity_in_days = 2 + create_confirmation_link(multiuse_obj, Confirmation.MULTIUSE_INVITE, validity_in_days) multiuse_confirmation = Confirmation.objects.all().last() assert multiuse_confirmation is not None multiuse_object_key = multiuse_confirmation.confirmation_key diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 59847c67e5..fd3868d89c 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -20,9 +20,7 @@ from confirmation import settings as confirmation_settings from confirmation.models import ( Confirmation, ConfirmationKeyException, - confirmation_url, create_confirmation_link, - generate_key, get_object_from_key, one_click_unsubscribe_link, ) @@ -1921,7 +1919,8 @@ so we didn't send them an invitation. We did send invitations to everyone else!" data = {"email": invitee_email, "referrer_email": current_user.email} invitee = PreregistrationUser.objects.get(email=data["email"]) referrer = self.example_user(referrer_name) - link = create_confirmation_link(invitee, Confirmation.INVITATION) + validity_in_days = 2 + link = create_confirmation_link(invitee, Confirmation.INVITATION, validity_in_days) context = common_context(referrer) context.update( activate_url=link, @@ -2019,13 +2018,9 @@ so we didn't send them an invitation. We did send invitations to everyone else!" prereg_user = PreregistrationUser.objects.create( email=email, referred_by=inviter, realm=realm ) - url = create_confirmation_link(prereg_user, Confirmation.USER_REGISTRATION) - registration_key = url.split("/")[-1] - - conf = Confirmation.objects.filter(confirmation_key=registration_key).first() - assert conf is not None - conf.date_sent -= datetime.timedelta(weeks=3) - conf.save() + date_sent = timezone_now() - datetime.timedelta(weeks=3) + with patch("confirmation.models.timezone_now", return_value=date_sent): + url = create_confirmation_link(prereg_user, Confirmation.USER_REGISTRATION) target_url = "/" + url.split("/", 3)[3] result = self.client_get(target_url) @@ -2350,7 +2345,8 @@ class InvitationsTestCase(InviteUserBase): multiuse_invite = MultiuseInvite.objects.create( referred_by=self.example_user("hamlet"), realm=zulip_realm ) - create_confirmation_link(multiuse_invite, Confirmation.MULTIUSE_INVITE) + validity_in_days = 2 + create_confirmation_link(multiuse_invite, Confirmation.MULTIUSE_INVITE, validity_in_days) result = self.client_delete("/json/invites/multiuse/" + str(multiuse_invite.id)) self.assertEqual(result.status_code, 200) self.assertIsNone(MultiuseInvite.objects.filter(id=multiuse_invite.id).first()) @@ -2364,7 +2360,8 @@ class InvitationsTestCase(InviteUserBase): realm=zulip_realm, invited_as=PreregistrationUser.INVITE_AS["REALM_OWNER"], ) - create_confirmation_link(multiuse_invite, Confirmation.MULTIUSE_INVITE) + validity_in_days = 2 + create_confirmation_link(multiuse_invite, Confirmation.MULTIUSE_INVITE, validity_in_days) error_result = self.client_delete("/json/invites/multiuse/" + str(multiuse_invite.id)) self.assert_json_error(error_result, "Must be an organization owner") @@ -2378,7 +2375,10 @@ class InvitationsTestCase(InviteUserBase): multiuse_invite_in_mit = MultiuseInvite.objects.create( referred_by=self.mit_user("sipbtest"), realm=mit_realm ) - create_confirmation_link(multiuse_invite_in_mit, Confirmation.MULTIUSE_INVITE) + validity_in_days = 2 + create_confirmation_link( + multiuse_invite_in_mit, Confirmation.MULTIUSE_INVITE, validity_in_days + ) error_result = self.client_delete( "/json/invites/multiuse/" + str(multiuse_invite_in_mit.id) ) @@ -2617,15 +2617,9 @@ class MultiuseInviteTest(ZulipTestCase): if date_sent is None: date_sent = timezone_now() - key = generate_key() - Confirmation.objects.create( - content_object=invite, - date_sent=date_sent, - confirmation_key=key, - type=Confirmation.MULTIUSE_INVITE, - ) - - return confirmation_url(key, self.realm, Confirmation.MULTIUSE_INVITE) + validity_in_days = 2 + with patch("confirmation.models.timezone_now", return_value=date_sent): + return create_confirmation_link(invite, Confirmation.MULTIUSE_INVITE, validity_in_days) def check_user_able_to_register(self, email: str, invite_link: str) -> None: password = "password" @@ -2652,9 +2646,8 @@ class MultiuseInviteTest(ZulipTestCase): email2 = self.nonreg_email("test1") email3 = self.nonreg_email("alice") - date_sent = timezone_now() - datetime.timedelta( - days=settings.INVITATION_LINK_VALIDITY_DAYS - 1 - ) + validity_in_days = 2 + date_sent = timezone_now() - datetime.timedelta(days=validity_in_days - 1) invite_link = self.generate_multiuse_invite_link(date_sent=date_sent) self.check_user_able_to_register(email1, invite_link) @@ -2663,7 +2656,7 @@ class MultiuseInviteTest(ZulipTestCase): def test_expired_multiuse_link(self) -> None: email = self.nonreg_email("newuser") - date_sent = timezone_now() - datetime.timedelta(days=settings.INVITATION_LINK_VALIDITY_DAYS) + date_sent = timezone_now() - datetime.timedelta(days=2) invite_link = self.generate_multiuse_invite_link(date_sent=date_sent) result = self.client_post(invite_link, {"email": email})