invite: Add backend support for "Never expires" option.

The database value for expiry_date is None for the invite
that will never expire and the clients send -1 as value
in the API similar to the message retention setting.

Also, when passing invite_expire_in_days as an argument
in various functions, invite_expire_in_days is passed as
-1 for "Never expires" option since invite_expire_in_days
is an optional argument in some functions and thus we cannot
pass "None" value.
This commit is contained in:
Sahil Batra 2021-11-30 18:04:37 +05:30 committed by Tim Abbott
parent c19d6fb3ef
commit 392b17da5f
12 changed files with 241 additions and 35 deletions

View File

@ -92,7 +92,9 @@ def get_confirmations(
link_status = ""
now = timezone_now()
if now < expiry_date:
if expiry_date is None:
expires_in = "Never"
elif now < expiry_date:
expires_in = timesince(now, expiry_date)
else:
expires_in = "Expired"

View File

@ -0,0 +1,18 @@
# Generated by Django 3.2.9 on 2021-11-30 17:44
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("confirmation", "0010_alter_confirmation_expiry_date"),
]
operations = [
migrations.AlterField(
model_name="confirmation",
name="expiry_date",
field=models.DateTimeField(db_index=True, null=True),
),
]

View File

@ -18,6 +18,7 @@ from django.urls import reverse
from django.utils.timezone import now as timezone_now
from typing_extensions import Protocol
from zerver.lib.types import UnspecifiedValue
from zerver.models import EmailChangeStatus, MultiuseInvite, PreregistrationUser, Realm, UserProfile
@ -70,7 +71,7 @@ def get_object_from_key(
except Confirmation.DoesNotExist:
raise ConfirmationKeyException(ConfirmationKeyException.DOES_NOT_EXIST)
if timezone_now() > confirmation.expiry_date:
if confirmation.expiry_date is not None and timezone_now() > confirmation.expiry_date:
raise ConfirmationKeyException(ConfirmationKeyException.EXPIRED)
obj = confirmation.content_object
@ -85,7 +86,7 @@ def create_confirmation_link(
obj: Union[Realm, HasRealmObject, OptionalHasRealmObject],
confirmation_type: int,
*,
validity_in_days: Optional[int] = None,
validity_in_days: Union[Optional[int], UnspecifiedValue] = UnspecifiedValue(),
url_args: Mapping[str, str] = {},
) -> str:
# validity_in_days is an override for the default values which are
@ -100,7 +101,11 @@ def create_confirmation_link(
current_time = timezone_now()
expiry_date = None
if validity_in_days:
if not isinstance(validity_in_days, UnspecifiedValue):
if validity_in_days is None:
expiry_date = None
else:
assert validity_in_days is not None
expiry_date = current_time + datetime.timedelta(days=validity_in_days)
else:
expiry_date = current_time + datetime.timedelta(
@ -138,7 +143,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)
expiry_date: Optional[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

View File

@ -21,6 +21,12 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 5.0
**Feature level 117**
* `POST /invites`, `POST /invites/multiuse`: Added support for passing
`null` as the `invite_expires_in_days` parameter to request an
invitation that never expires.
**Feature level 116**
* [`GET /server_settings`](/api/get-server-settings): Added

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md, as well as
# "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 116
API_FEATURE_LEVEL = 117
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

View File

@ -174,7 +174,7 @@ from zerver.lib.topic import (
update_messages_for_topic_edit,
)
from zerver.lib.topic_mutes import add_topic_mute, get_topic_mutes, remove_topic_mute
from zerver.lib.types import ProfileDataElementValue, ProfileFieldData
from zerver.lib.types import ProfileDataElementValue, ProfileFieldData, UnspecifiedValue
from zerver.lib.upload import (
claim_attachment,
delete_avatar_image,
@ -7526,7 +7526,7 @@ def do_send_confirmation_email(
invitee: PreregistrationUser,
referrer: UserProfile,
email_language: str,
invite_expires_in_days: Optional[int] = None,
invite_expires_in_days: Union[Optional[int], UnspecifiedValue] = UnspecifiedValue(),
) -> str:
"""
Send the confirmation/welcome e-mail to an invited user.
@ -7625,7 +7625,7 @@ def do_invite_users(
invitee_emails: Collection[str],
streams: Collection[Stream],
*,
invite_expires_in_days: int,
invite_expires_in_days: Optional[int],
invite_as: int = PreregistrationUser.INVITE_AS["MEMBER"],
) -> None:
num_invites = len(invitee_emails)
@ -7737,6 +7737,13 @@ def do_invite_users(
notify_invites_changed(user_profile.realm)
def get_invitation_expiry_date(confirmation_obj: Confirmation) -> Optional[int]:
expiry_date = confirmation_obj.expiry_date
if expiry_date is None:
return expiry_date
return datetime_to_timestamp(expiry_date)
def do_get_invites_controlled_by_user(user_profile: UserProfile) -> List[Dict[str, Any]]:
"""
Returns a list of dicts representing invitations that can be controlled by user_profile.
@ -7755,13 +7762,12 @@ def do_get_invites_controlled_by_user(user_profile: UserProfile) -> List[Dict[st
invites = []
for invitee in prereg_users:
expiry_date = invitee.confirmation.get().expiry_date
invites.append(
dict(
email=invitee.email,
invited_by_user_id=invitee.referred_by.id,
invited=datetime_to_timestamp(invitee.invited_at),
expiry_date=datetime_to_timestamp(expiry_date),
expiry_date=get_invitation_expiry_date(invitee.confirmation.get()),
id=invitee.id,
invited_as=invitee.invited_as,
is_multiuse=False,
@ -7773,8 +7779,8 @@ def do_get_invites_controlled_by_user(user_profile: UserProfile) -> List[Dict[st
return invites
multiuse_confirmation_objs = Confirmation.objects.filter(
realm=user_profile.realm, type=Confirmation.MULTIUSE_INVITE, expiry_date__gte=timezone_now()
)
realm=user_profile.realm, type=Confirmation.MULTIUSE_INVITE
).filter(Q(expiry_date__gte=timezone_now()) | Q(expiry_date=None))
for confirmation_obj in multiuse_confirmation_objs:
invite = confirmation_obj.content_object
assert invite is not None
@ -7782,7 +7788,7 @@ def do_get_invites_controlled_by_user(user_profile: UserProfile) -> List[Dict[st
dict(
invited_by_user_id=invite.referred_by.id,
invited=datetime_to_timestamp(confirmation_obj.date_sent),
expiry_date=datetime_to_timestamp(confirmation_obj.expiry_date),
expiry_date=get_invitation_expiry_date(confirmation_obj),
id=invite.id,
link_url=confirmation_url(
confirmation_obj.confirmation_key,
@ -7812,9 +7818,8 @@ def get_valid_invite_confirmations_generated_by_user(
confirmations += list(
Confirmation.objects.filter(
type=Confirmation.MULTIUSE_INVITE,
expiry_date__gte=timezone_now(),
object_id__in=multiuse_invite_ids,
)
).filter(Q(expiry_date__gte=timezone_now()) | Q(expiry_date=None))
)
return confirmations
@ -7834,7 +7839,7 @@ def revoke_invites_generated_by_user(user_profile: UserProfile) -> None:
def do_create_multiuse_invite_link(
referred_by: UserProfile,
invited_as: int,
invite_expires_in_days: int,
invite_expires_in_days: Optional[int],
streams: Sequence[Stream] = [],
) -> str:
realm = referred_by.realm
@ -7883,9 +7888,14 @@ def do_resend_user_invite_email(prereg_user: PreregistrationUser) -> int:
prereg_user.invited_at = timezone_now()
prereg_user.save()
invite_expires_in_days = (
prereg_user.confirmation.get().expiry_date - prereg_user.invited_at
).days
expiry_date = prereg_user.confirmation.get().expiry_date
if expiry_date is None:
invite_expires_in_days = None
else:
# The resent invitation is reset to expire as long after the
# reminder is sent as it lasted originally.
invite_expires_in_days = (expiry_date - prereg_user.invited_at).days
prereg_user.confirmation.clear()
do_increment_logging_stat(

View File

@ -72,3 +72,20 @@ class SAMLIdPConfigDict(TypedDict, total=False):
extra_attrs: List[str]
x509cert: str
x509cert_path: str
class UnspecifiedValue:
"""In most API endpoints, we use a default value of `None"` to encode
parameters that the client did not pass, which is nicely Pythonic.
However, that design does not work for those few endpoints where
we want to allow clients to pass an explicit `null` (which
JSON-decodes to `None`).
We use this type as an explicit sentinel value for such endpoints.
TODO: Can this be merged with the _NotSpecified class, which is
currently an internal implementation detail of the REQ class?
"""
pass

View File

@ -86,6 +86,7 @@ from zerver.lib.types import (
ProfileDataElementBase,
ProfileDataElementValue,
RealmUserValidator,
UnspecifiedValue,
UserFieldElement,
Validator,
)
@ -2184,17 +2185,30 @@ class PreregistrationUser(models.Model):
def filter_to_valid_prereg_users(
query: QuerySet,
invite_expires_in_days: Optional[int] = None,
invite_expires_in_days: Union[Optional[int], UnspecifiedValue] = UnspecifiedValue(),
) -> QuerySet:
"""
If invite_expires_in_days is specified, we return only those PreregistrationUser
objects that were created at most that many days in the past.
"""
active_value = confirmation_settings.STATUS_ACTIVE
revoked_value = confirmation_settings.STATUS_REVOKED
query = query.exclude(status__in=[active_value, revoked_value])
if invite_expires_in_days:
if invite_expires_in_days is None:
# Since invite_expires_in_days is None, we're invitation will never
# expire, we do not need to check anything else and can simply return
# after excluding objects with active and revoked status.
return query
assert invite_expires_in_days is not None
if not isinstance(invite_expires_in_days, UnspecifiedValue):
lowest_datetime = timezone_now() - datetime.timedelta(days=invite_expires_in_days)
return query.filter(invited_at__gte=lowest_datetime)
else:
return query.filter(confirmation__expiry_date__gte=timezone_now())
return query.filter(
Q(confirmation__expiry_date=None) | Q(confirmation__expiry_date__gte=timezone_now())
)
class MultiuseInvite(models.Model):

View File

@ -1045,7 +1045,7 @@ class InviteUserBase(ZulipTestCase):
self,
invitee_emails: str,
stream_names: Sequence[str],
invite_expires_in_days: int = settings.INVITATION_LINK_VALIDITY_DAYS,
invite_expires_in_days: Optional[int] = settings.INVITATION_LINK_VALIDITY_DAYS,
body: str = "",
invite_as: int = PreregistrationUser.INVITE_AS["MEMBER"],
) -> HttpResponse:
@ -1060,11 +1060,16 @@ class InviteUserBase(ZulipTestCase):
stream_ids = []
for stream_name in stream_names:
stream_ids.append(self.get_stream_id(stream_name))
invite_expires_in: Union[str, Optional[int]] = invite_expires_in_days
if invite_expires_in is None:
invite_expires_in = orjson.dumps(None).decode()
return self.client_post(
"/json/invites",
{
"invitee_emails": invitee_emails,
"invite_expires_in_days": invite_expires_in_days,
"invite_expires_in_days": invite_expires_in,
"stream_ids": orjson.dumps(stream_ids).decode(),
"invite_as": invite_as,
},
@ -2085,6 +2090,26 @@ so we didn't send them an invitation. We did send invitations to everyone else!"
"Whoops. The confirmation link has expired or been deactivated.", result
)
def test_never_expire_confirmation_obejct(self) -> None:
email = self.nonreg_email("alice")
realm = get_realm("zulip")
inviter = self.example_user("iago")
prereg_user = PreregistrationUser.objects.create(
email=email, referred_by=inviter, realm=realm
)
activation_url = create_confirmation_link(
prereg_user, Confirmation.INVITATION, validity_in_days=None
)
confirmation = Confirmation.objects.last()
assert confirmation is not None
self.assertEqual(confirmation.expiry_date, None)
activation_key = activation_url.split("/")[-1]
response = self.client_post(
"/accounts/register/",
{"key": activation_key, "from_confirmation": 1, "full_nme": "alice"},
)
self.assertEqual(response.status_code, 200)
def test_send_more_than_one_invite_to_same_user(self) -> None:
self.user_profile = self.example_user("iago")
streams = []
@ -2349,6 +2374,53 @@ class InvitationsTestCase(InviteUserBase):
self.assertTrue(invites[1]["is_multiuse"])
self.assertEqual(invites[1]["invited_by_user_id"], hamlet.id)
def test_get_never_expiring_invitations(self) -> None:
self.login("iago")
user_profile = self.example_user("iago")
streams = []
for stream_name in ["Denmark", "Scotland"]:
streams.append(get_stream(stream_name, user_profile.realm))
with patch(
"confirmation.models.timezone_now",
return_value=timezone_now() - datetime.timedelta(days=1000),
):
# Testing the invitation with expiry date set to "None" exists
# after a large amount of days.
do_invite_users(
user_profile,
["TestOne@zulip.com"],
streams,
invite_expires_in_days=None,
)
do_invite_users(
user_profile,
["TestTwo@zulip.com"],
streams,
invite_expires_in_days=100,
)
do_create_multiuse_invite_link(
user_profile, PreregistrationUser.INVITE_AS["MEMBER"], None
)
do_create_multiuse_invite_link(
user_profile, PreregistrationUser.INVITE_AS["MEMBER"], 100
)
result = self.client_get("/json/invites")
self.assertEqual(result.status_code, 200)
invites = orjson.loads(result.content)["invites"]
# We only get invitations that will never expire because we have mocked time such
# that the other invitations are created in the deep past.
self.assert_length(invites, 2)
self.assertFalse(invites[0]["is_multiuse"])
self.assertEqual(invites[0]["email"], "TestOne@zulip.com")
self.assertEqual(invites[0]["expiry_date"], None)
self.assertTrue(invites[1]["is_multiuse"])
self.assertEqual(invites[1]["invited_by_user_id"], user_profile.id)
self.assertEqual(invites[1]["expiry_date"], None)
def test_successful_delete_invitation(self) -> None:
"""
A DELETE call to /json/invites/<ID> should delete the invite and
@ -2635,6 +2707,23 @@ class InvitationsTestCase(InviteUserBase):
original_timestamp, scheduledemail_filter.values_list("scheduled_timestamp", flat=True)
)
def test_resend_never_expiring_invitation(self) -> None:
self.login("iago")
invitee = "resend@zulip.com"
self.assert_json_success(self.invite(invitee, ["Denmark"], None))
prereg_user = PreregistrationUser.objects.get(email=invitee)
# Verify and then clear from the outbox the original invite email
self.check_sent_emails([invitee])
from django.core.mail import outbox
outbox.pop()
result = self.client_post("/json/invites/" + str(prereg_user.id) + "/resend")
self.assert_json_success(result)
self.check_sent_emails([invitee])
def test_accessing_invites_in_another_realm(self) -> None:
inviter = UserProfile.objects.exclude(realm=get_realm("zulip")).first()
assert inviter is not None

View File

@ -1461,6 +1461,21 @@ class ActivateTest(ZulipTestCase):
invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"],
)
do_invite_users(
iago,
["new5@zulip.com"],
[],
invite_expires_in_days=None,
invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"],
)
do_invite_users(
desdemona,
["new6@zulip.com"],
[],
invite_expires_in_days=None,
invite_as=PreregistrationUser.INVITE_AS["REALM_ADMIN"],
)
iago_multiuse_key = do_create_multiuse_invite_link(
iago, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_days
).split("/")[-2]
@ -1468,17 +1483,24 @@ class ActivateTest(ZulipTestCase):
desdemona, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_days
).split("/")[-2]
iago_never_expire_multiuse_key = do_create_multiuse_invite_link(
iago, PreregistrationUser.INVITE_AS["MEMBER"], None
).split("/")[-2]
desdemona_never_expire_multiuse_key = do_create_multiuse_invite_link(
desdemona, PreregistrationUser.INVITE_AS["MEMBER"], None
).split("/")[-2]
self.assertEqual(
filter_to_valid_prereg_users(
PreregistrationUser.objects.filter(referred_by=iago)
).count(),
2,
3,
)
self.assertEqual(
filter_to_valid_prereg_users(
PreregistrationUser.objects.filter(referred_by=desdemona)
).count(),
2,
3,
)
self.assertTrue(
Confirmation.objects.get(confirmation_key=iago_multiuse_key).expiry_date
@ -1488,6 +1510,14 @@ class ActivateTest(ZulipTestCase):
Confirmation.objects.get(confirmation_key=desdemona_multiuse_key).expiry_date
> timezone_now()
)
self.assertIsNone(
Confirmation.objects.get(confirmation_key=iago_never_expire_multiuse_key).expiry_date
)
self.assertIsNone(
Confirmation.objects.get(
confirmation_key=desdemona_never_expire_multiuse_key
).expiry_date
)
do_deactivate_user(iago, acting_user=None)
@ -1503,7 +1533,7 @@ class ActivateTest(ZulipTestCase):
filter_to_valid_prereg_users(
PreregistrationUser.objects.filter(referred_by=desdemona)
).count(),
2,
3,
)
self.assertTrue(
Confirmation.objects.get(confirmation_key=iago_multiuse_key).expiry_date
@ -1513,6 +1543,15 @@ class ActivateTest(ZulipTestCase):
Confirmation.objects.get(confirmation_key=desdemona_multiuse_key).expiry_date
> timezone_now()
)
self.assertTrue(
Confirmation.objects.get(confirmation_key=iago_never_expire_multiuse_key).expiry_date
<= timezone_now()
)
self.assertIsNone(
Confirmation.objects.get(
confirmation_key=desdemona_never_expire_multiuse_key
).expiry_date
)
def test_clear_scheduled_jobs(self) -> None:
user = self.example_user("hamlet")

View File

@ -1,5 +1,5 @@
import re
from typing import List, Sequence, Set
from typing import List, Optional, Sequence, Set
from django.conf import settings
from django.http import HttpRequest, HttpResponse
@ -18,7 +18,7 @@ from zerver.lib.exceptions import JsonableError, OrganizationOwnerRequired
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_success
from zerver.lib.streams import access_stream_by_id
from zerver.lib.validator import check_int, check_list
from zerver.lib.validator import check_int, check_list, check_none_or
from zerver.models import MultiuseInvite, PreregistrationUser, Stream, UserProfile
@ -36,8 +36,8 @@ def invite_users_backend(
request: HttpRequest,
user_profile: UserProfile,
invitee_emails_raw: str = REQ("invitee_emails"),
invite_expires_in_days: int = REQ(
json_validator=check_int, default=settings.INVITATION_LINK_VALIDITY_DAYS
invite_expires_in_days: Optional[int] = REQ(
json_validator=check_none_or(check_int), default=settings.INVITATION_LINK_VALIDITY_DAYS
),
invite_as: int = REQ(json_validator=check_int, default=PreregistrationUser.INVITE_AS["MEMBER"]),
stream_ids: List[int] = REQ(json_validator=check_list(check_int)),
@ -174,8 +174,8 @@ def resend_user_invite_email(
def generate_multiuse_invite_backend(
request: HttpRequest,
user_profile: UserProfile,
invite_expires_in_days: int = REQ(
json_validator=check_int, default=settings.INVITATION_LINK_VALIDITY_DAYS
invite_expires_in_days: Optional[int] = REQ(
json_validator=check_none_or(check_int), default=settings.INVITATION_LINK_VALIDITY_DAYS
),
invite_as: int = REQ(json_validator=check_int, default=PreregistrationUser.INVITE_AS["MEMBER"]),
stream_ids: Sequence[int] = REQ(json_validator=check_list(check_int), default=[]),

View File

@ -461,6 +461,12 @@ class ConfirmationEmailWorker(QueueProcessingWorker):
activate_url = do_send_confirmation_email(
invitee, referrer, email_language, invite_expires_in_days
)
if invite_expires_in_days is None:
# We do not queue reminder email for never expiring
# invitations. This is probably a low importance bug; it
# would likely be more natural to send a reminder after 7
# days.
return
# queue invitation reminder
if invite_expires_in_days >= 4: