From edcf4f0ea266b582279c304772aad784ea2f686c Mon Sep 17 00:00:00 2001 From: clarammdantas Date: Thu, 30 Apr 2020 20:52:45 -0300 Subject: [PATCH] invitations: Revoke remaining invitations after user registers. If a user receives more than one invite to join a realm, after that user registers, all the remaining invitations should be revoked, preventing them to be listed in active invitations on admin panel. To do this, we added a new prereg_user status, STATUS_REVOKED. We also added a confirmation_link_expired_error page in case the user tries click on a revoked invitaion. This page has a link to login page. Fixes: #12629 Co-authored-by: Arunika --- confirmation/settings.py | 1 + .../confirmation_link_expired_error.html | 14 ++++++ zerver/lib/actions.py | 14 ++++-- zerver/tests/test_signup.py | 46 ++++++++++++++++--- zerver/views/registration.py | 10 +++- 5 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 templates/zerver/confirmation_link_expired_error.html diff --git a/confirmation/settings.py b/confirmation/settings.py index f141d4055b..63278f9ada 100644 --- a/confirmation/settings.py +++ b/confirmation/settings.py @@ -3,3 +3,4 @@ __revision__ = '$Id: settings.py 12 2008-11-23 19:38:52Z jarek.zgoda $' STATUS_ACTIVE = 1 +STATUS_REVOKED = 2 diff --git a/templates/zerver/confirmation_link_expired_error.html b/templates/zerver/confirmation_link_expired_error.html new file mode 100644 index 0000000000..a423755f1f --- /dev/null +++ b/templates/zerver/confirmation_link_expired_error.html @@ -0,0 +1,14 @@ +{% extends "zerver/portico_signup.html" %} + +{% block portico_content %} +
+ +
+{% endblock %} diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 2e86a74ede..1fe52d4ce7 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -401,12 +401,15 @@ def process_new_human_user(user_profile: UserProfile, # inactive so we can keep track of the PreregistrationUser we # actually used for analytics if prereg_user is not None: - PreregistrationUser.objects.filter(email__iexact=user_profile.delivery_email).exclude( - id=prereg_user.id).update(status=0) + PreregistrationUser.objects.filter( + email__iexact=user_profile.delivery_email).exclude(id=prereg_user.id)\ + .update(status=confirmation_settings.STATUS_REVOKED) + if prereg_user.referred_by is not None: notify_invites_changed(user_profile) else: - PreregistrationUser.objects.filter(email__iexact=user_profile.delivery_email).update(status=0) + PreregistrationUser.objects.filter(email__iexact=user_profile.delivery_email)\ + .update(status=confirmation_settings.STATUS_REVOKED) notify_new_user(user_profile) # Clear any scheduled invitation emails to prevent them @@ -5072,9 +5075,10 @@ def do_invite_users(user_profile: UserProfile, def do_get_user_invites(user_profile: UserProfile) -> List[Dict[str, Any]]: days_to_activate = settings.INVITATION_LINK_VALIDITY_DAYS - active_value = getattr(confirmation_settings, 'STATUS_ACTIVE', 1) + active_value = confirmation_settings.STATUS_ACTIVE + revoked_value = confirmation_settings.STATUS_REVOKED lowest_datetime = timezone_now() - datetime.timedelta(days=days_to_activate) - base_query = PreregistrationUser.objects.exclude(status=active_value).filter( + base_query = PreregistrationUser.objects.exclude(status__in=[active_value, revoked_value]).filter( invited_at__gte=lowest_datetime) if user_profile.is_realm_admin: diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 1dc3f2ddb0..dd3d7885ff 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -8,6 +8,7 @@ from django.http import HttpResponse from django.test import TestCase, override_settings from django.utils.timezone import now as timezone_now from django.core.exceptions import ValidationError +from django.urls import reverse from unittest.mock import patch, MagicMock from zerver.lib.test_helpers import ( @@ -41,7 +42,8 @@ from zerver.lib.actions import ( do_create_default_stream_group, do_add_default_stream, do_create_realm, - get_default_streams_for_realm) + get_default_streams_for_realm, + do_invite_users, do_create_user) from zerver.lib.send_email import send_future_email, FromAddress, \ deliver_email from zerver.lib.initial_password import initial_password @@ -1506,6 +1508,40 @@ so we didn't send them an invitation. We did send invitations to everyone else!" self.assert_in_success_response(["Whoops. The confirmation link has expired " "or been deactivated."], result) + def test_send_more_than_one_invite_to_same_user(self) -> None: + self.user_profile = self.example_user('iago') + streams = [] + for stream_name in ["Denmark", "Scotland"]: + streams.append(get_stream(stream_name, self.user_profile.realm)) + + do_invite_users(self.user_profile, ["foo@zulip.com"], streams, False) + prereg_user = PreregistrationUser.objects.get(email="foo@zulip.com") + do_invite_users(self.user_profile, ["foo@zulip.com"], streams, False) + do_invite_users(self.user_profile, ["foo@zulip.com"], streams, False) + + invites = PreregistrationUser.objects.filter(email__iexact="foo@zulip.com") + self.assertEqual(len(invites), 3) + + do_create_user( + 'foo@zulip.com', + 'password', + self.user_profile.realm, + 'full name', 'short name', + prereg_user=prereg_user, + ) + + accepted_invite = PreregistrationUser.objects.filter( + email__iexact="foo@zulip.com", status=confirmation_settings.STATUS_ACTIVE) + revoked_invites = PreregistrationUser.objects.filter( + email__iexact="foo@zulip.com", status=confirmation_settings.STATUS_REVOKED) + # If a user was invited more than once, when it accepts one invite and register + # the others must be canceled. + self.assertEqual(len(accepted_invite), 1) + self.assertEqual(accepted_invite[0].id, prereg_user.id) + + expected_revoked_invites = set(invites.exclude(id=prereg_user.id)) + self.assertEqual(set(revoked_invites), expected_revoked_invites) + def test_validate_email_not_already_in_realm(self) -> None: email = self.nonreg_email('alice') password = 'password' @@ -2763,11 +2799,9 @@ class UserSignUpTest(InviteUserBase): 'terms': True, 'full_name': "New Guy", 'from_confirmation': '1'}) - # We should get redirected back to the login page. - expected_url = ('/accounts/login/' + '?email=' + - urllib.parse.quote_plus(email)) - self.assertEqual(result.status_code, 302) - self.assertEqual(result["Location"], expected_url) + # Error page should be displayed + self.assert_in_success_response(["The registration link has expired or is not valid."], result) + self.assertEqual(result.status_code, 200) def test_signup_with_multiple_default_stream_groups(self) -> None: # Check if user is subscribed to the streams of default diff --git a/zerver/views/registration.py b/zerver/views/registration.py index 90bcf70815..38df23cf90 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -43,18 +43,24 @@ from confirmation.models import Confirmation, RealmCreationKey, ConfirmationKeyE validate_key, create_confirmation_link, get_object_from_key, \ render_confirmation_key_error +from confirmation import settings as confirmation_settings + import logging import smtplib import urllib def check_prereg_key_and_redirect(request: HttpRequest, confirmation_key: str) -> HttpResponse: - # If the key isn't valid, show the error message on the original URL confirmation = Confirmation.objects.filter(confirmation_key=confirmation_key).first() if confirmation is None or confirmation.type not in [ Confirmation.USER_REGISTRATION, Confirmation.INVITATION, Confirmation.REALM_CREATION]: return render_confirmation_key_error( request, ConfirmationKeyException(ConfirmationKeyException.DOES_NOT_EXIST)) + + prereg_user = confirmation.content_object + if prereg_user.status == confirmation_settings.STATUS_REVOKED: + return render(request, "zerver/confirmation_link_expired_error.html") + try: get_object_from_key(confirmation_key, confirmation.type, activate_object=False) except ConfirmationKeyException as exception: @@ -73,6 +79,8 @@ def accounts_register(request: HttpRequest) -> HttpResponse: key = request.POST['key'] confirmation = Confirmation.objects.get(confirmation_key=key) prereg_user = confirmation.content_object + if prereg_user.status == confirmation_settings.STATUS_REVOKED: + return render(request, "zerver/confirmation_link_expired_error.html") email = prereg_user.email realm_creation = prereg_user.realm_creation password_required = prereg_user.password_required