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 <arunikayadav42@gmail.com>
This commit is contained in:
clarammdantas 2020-04-30 20:52:45 -03:00 committed by Tim Abbott
parent 923c5e3063
commit edcf4f0ea2
5 changed files with 73 additions and 12 deletions

View File

@ -3,3 +3,4 @@
__revision__ = '$Id: settings.py 12 2008-11-23 19:38:52Z jarek.zgoda $' __revision__ = '$Id: settings.py 12 2008-11-23 19:38:52Z jarek.zgoda $'
STATUS_ACTIVE = 1 STATUS_ACTIVE = 1
STATUS_REVOKED = 2

View File

@ -0,0 +1,14 @@
{% extends "zerver/portico_signup.html" %}
{% block portico_content %}
<div class="app portico-page">
<div class="app-main portico-page-container center-block flex full-page account-creation new-style">
<div class="inline-block">
<div class="app-main white-box">
<h1>{{ _("The registration link has expired or is not valid.") }}</h1>
<a href="{{ login_url }}">{{ _("Log in") }}</a>.
</div>
</div>
</div>
</div>
{% endblock %}

View File

@ -401,12 +401,15 @@ def process_new_human_user(user_profile: UserProfile,
# inactive so we can keep track of the PreregistrationUser we # inactive so we can keep track of the PreregistrationUser we
# actually used for analytics # actually used for analytics
if prereg_user is not None: if prereg_user is not None:
PreregistrationUser.objects.filter(email__iexact=user_profile.delivery_email).exclude( PreregistrationUser.objects.filter(
id=prereg_user.id).update(status=0) 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: if prereg_user.referred_by is not None:
notify_invites_changed(user_profile) notify_invites_changed(user_profile)
else: 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) notify_new_user(user_profile)
# Clear any scheduled invitation emails to prevent them # 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]]: def do_get_user_invites(user_profile: UserProfile) -> List[Dict[str, Any]]:
days_to_activate = settings.INVITATION_LINK_VALIDITY_DAYS 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) 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) invited_at__gte=lowest_datetime)
if user_profile.is_realm_admin: if user_profile.is_realm_admin:

View File

@ -8,6 +8,7 @@ from django.http import HttpResponse
from django.test import TestCase, override_settings from django.test import TestCase, override_settings
from django.utils.timezone import now as timezone_now from django.utils.timezone import now as timezone_now
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.urls import reverse
from unittest.mock import patch, MagicMock from unittest.mock import patch, MagicMock
from zerver.lib.test_helpers import ( from zerver.lib.test_helpers import (
@ -41,7 +42,8 @@ from zerver.lib.actions import (
do_create_default_stream_group, do_create_default_stream_group,
do_add_default_stream, do_add_default_stream,
do_create_realm, 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, \ from zerver.lib.send_email import send_future_email, FromAddress, \
deliver_email deliver_email
from zerver.lib.initial_password import initial_password 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 " self.assert_in_success_response(["Whoops. The confirmation link has expired "
"or been deactivated."], result) "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: def test_validate_email_not_already_in_realm(self) -> None:
email = self.nonreg_email('alice') email = self.nonreg_email('alice')
password = 'password' password = 'password'
@ -2763,11 +2799,9 @@ class UserSignUpTest(InviteUserBase):
'terms': True, 'terms': True,
'full_name': "New Guy", 'full_name': "New Guy",
'from_confirmation': '1'}) 'from_confirmation': '1'})
# We should get redirected back to the login page. # Error page should be displayed
expected_url = ('/accounts/login/' + '?email=' + self.assert_in_success_response(["The registration link has expired or is not valid."], result)
urllib.parse.quote_plus(email)) self.assertEqual(result.status_code, 200)
self.assertEqual(result.status_code, 302)
self.assertEqual(result["Location"], expected_url)
def test_signup_with_multiple_default_stream_groups(self) -> None: def test_signup_with_multiple_default_stream_groups(self) -> None:
# Check if user is subscribed to the streams of default # Check if user is subscribed to the streams of default

View File

@ -43,18 +43,24 @@ from confirmation.models import Confirmation, RealmCreationKey, ConfirmationKeyE
validate_key, create_confirmation_link, get_object_from_key, \ validate_key, create_confirmation_link, get_object_from_key, \
render_confirmation_key_error render_confirmation_key_error
from confirmation import settings as confirmation_settings
import logging import logging
import smtplib import smtplib
import urllib import urllib
def check_prereg_key_and_redirect(request: HttpRequest, confirmation_key: str) -> HttpResponse: 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() confirmation = Confirmation.objects.filter(confirmation_key=confirmation_key).first()
if confirmation is None or confirmation.type not in [ if confirmation is None or confirmation.type not in [
Confirmation.USER_REGISTRATION, Confirmation.INVITATION, Confirmation.REALM_CREATION]: Confirmation.USER_REGISTRATION, Confirmation.INVITATION, Confirmation.REALM_CREATION]:
return render_confirmation_key_error( return render_confirmation_key_error(
request, ConfirmationKeyException(ConfirmationKeyException.DOES_NOT_EXIST)) 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: try:
get_object_from_key(confirmation_key, confirmation.type, activate_object=False) get_object_from_key(confirmation_key, confirmation.type, activate_object=False)
except ConfirmationKeyException as exception: except ConfirmationKeyException as exception:
@ -73,6 +79,8 @@ def accounts_register(request: HttpRequest) -> HttpResponse:
key = request.POST['key'] key = request.POST['key']
confirmation = Confirmation.objects.get(confirmation_key=key) confirmation = Confirmation.objects.get(confirmation_key=key)
prereg_user = confirmation.content_object 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 email = prereg_user.email
realm_creation = prereg_user.realm_creation realm_creation = prereg_user.realm_creation
password_required = prereg_user.password_required password_required = prereg_user.password_required