From 5e31a6b1c011e9219ea7ab7c92f28cd5d1d88ef8 Mon Sep 17 00:00:00 2001 From: Ujjawal Modi Date: Sat, 5 Aug 2023 16:11:47 +0530 Subject: [PATCH] invites: Make it possible for non-admins to revoke multiuse invites. This commit makes changes to allow non-admins to revoke multiuse invitations created by them. --- api_docs/changelog.md | 9 +++++++ .../confirm_dialog/confirm_revoke_invite.hbs | 4 +++ zerver/actions/invites.py | 18 ++++++++----- zerver/tests/test_invite.py | 27 +++++++++++++++++-- zerver/views/invite.py | 7 ++--- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 12f7e4254a..019911374a 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -33,6 +33,15 @@ format used by the Zulip server that they are interacting with. to create reusable invitation links. Previously, this endpoint was restricted to admin users only. +* `GET /invites`: Endpoint response for non-admin users now includes both + email invitations and reusable invitation links that they have created. + Previously, non-admin users could only create email invitations, and + therefore the response did not include reusable invitation links for these users. + +* `DELETE /invites/multiuse/{invite_id}`: Non-admin users can now revoke + reusable invitation links they have created. Previously, only admin users could + create and revoke reusable invitation links. + **Feature level 208** * [`POST /users/me/subscriptions`](/api/subscribe), diff --git a/web/templates/confirm_dialog/confirm_revoke_invite.hbs b/web/templates/confirm_dialog/confirm_revoke_invite.hbs index d21c82826f..254c65be79 100644 --- a/web/templates/confirm_dialog/confirm_revoke_invite.hbs +++ b/web/templates/confirm_dialog/confirm_revoke_invite.hbs @@ -1,5 +1,9 @@ {{#if is_multiuse}} +{{#if referred_by}}

{{#tr}}Are you sure you want to revoke this invitation link created by {referred_by}?{{/tr}}

{{else}} +

{{#tr}}Are you sure you want to revoke this invitation link?{{/tr}}

+{{/if}} +{{else}}

{{#tr}}Are you sure you want to revoke the invitation to {email}?{{/tr}}

{{/if}} diff --git a/zerver/actions/invites.py b/zerver/actions/invites.py index 2e6e21e1a9..fbaf23a89c 100644 --- a/zerver/actions/invites.py +++ b/zerver/actions/invites.py @@ -362,13 +362,19 @@ def do_get_invites_controlled_by_user(user_profile: UserProfile) -> List[Dict[st ) ) - if not user_profile.is_realm_admin: - # We do not return multiuse invites to non-admin users. - return invites + if user_profile.is_realm_admin: + multiuse_confirmation_objs = Confirmation.objects.filter( + realm=user_profile.realm, type=Confirmation.MULTIUSE_INVITE + ).filter(Q(expiry_date__gte=timezone_now()) | Q(expiry_date=None)) + else: + multiuse_invite_ids = MultiuseInvite.objects.filter(referred_by=user_profile).values_list( + "id", flat=True + ) + multiuse_confirmation_objs = Confirmation.objects.filter( + type=Confirmation.MULTIUSE_INVITE, + object_id__in=multiuse_invite_ids, + ).filter(Q(expiry_date__gte=timezone_now()) | Q(expiry_date=None)) - multiuse_confirmation_objs = Confirmation.objects.filter( - 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 diff --git a/zerver/tests/test_invite.py b/zerver/tests/test_invite.py index e5f41de230..e415a2b00b 100644 --- a/zerver/tests/test_invite.py +++ b/zerver/tests/test_invite.py @@ -1747,8 +1747,11 @@ class InvitationsTestCase(InviteUserBase): do_create_multiuse_invite_link( user_profile, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes ) - self.assert_length(do_get_invites_controlled_by_user(user_profile), 5) - self.assert_length(do_get_invites_controlled_by_user(hamlet), 1) + do_create_multiuse_invite_link( + hamlet, PreregistrationUser.INVITE_AS["MEMBER"], invite_expires_in_minutes + ) + self.assert_length(do_get_invites_controlled_by_user(user_profile), 6) + self.assert_length(do_get_invites_controlled_by_user(hamlet), 2) self.assert_length(do_get_invites_controlled_by_user(othello), 1) def test_successful_get_open_invitations(self) -> None: @@ -1999,6 +2002,26 @@ class InvitationsTestCase(InviteUserBase): confirmation_settings.STATUS_REVOKED, ) + # Test non-admins can only delete invitations created by them. + multiuse_invite = MultiuseInvite.objects.create( + referred_by=self.example_user("hamlet"), realm=zulip_realm + ) + create_confirmation_link( + multiuse_invite, Confirmation.MULTIUSE_INVITE, validity_in_minutes=validity_in_minutes + ) + + self.login("cordelia") + error_result = self.client_delete("/json/invites/multiuse/" + str(multiuse_invite.id)) + self.assert_json_error(error_result, "Must be an organization administrator") + + self.login("hamlet") + result = self.client_delete("/json/invites/multiuse/" + str(multiuse_invite.id)) + self.assertEqual(result.status_code, 200) + self.assertEqual( + MultiuseInvite.objects.get(id=multiuse_invite.id).status, + confirmation_settings.STATUS_REVOKED, + ) + # Test deleting multiuse invite from another realm mit_realm = get_realm("zephyr") multiuse_invite_in_mit = MultiuseInvite.objects.create( diff --git a/zerver/views/invite.py b/zerver/views/invite.py index ae4a7d30de..1c449e1708 100644 --- a/zerver/views/invite.py +++ b/zerver/views/invite.py @@ -14,7 +14,7 @@ from zerver.actions.invites import ( do_revoke_multi_use_invite, do_revoke_user_invite, ) -from zerver.decorator import require_member_or_admin, require_realm_admin +from zerver.decorator import require_member_or_admin from zerver.lib.exceptions import JsonableError, OrganizationOwnerRequiredError from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success @@ -140,7 +140,7 @@ def revoke_user_invite( return json_success(request) -@require_realm_admin +@require_member_or_admin @has_request_variables def revoke_multiuse_invite( request: HttpRequest, user_profile: UserProfile, invite_id: int @@ -153,7 +153,8 @@ def revoke_multiuse_invite( if invite.realm != user_profile.realm: raise JsonableError(_("No such invitation")) - check_role_based_permissions(invite.invited_as, user_profile, require_admin=True) + if invite.referred_by_id != user_profile.id: + check_role_based_permissions(invite.invited_as, user_profile, require_admin=True) if invite.status == confirmation_settings.STATUS_REVOKED: raise JsonableError(_("Invitation has already been revoked"))