From 63389b3bd361f7edd9b793db7ce9beb50621afd7 Mon Sep 17 00:00:00 2001 From: sahil839 Date: Thu, 18 Jun 2020 16:33:06 +0530 Subject: [PATCH] invite: Add option to invite user as an organization owner. We can now invite new users as realm owners. We restrict only owners to invite new users as owners both for single invite and multiuse invite link. Also, only owners can revoke or resend owner invitations. --- static/js/settings_invites.js | 2 + static/templates/admin_invites_list.hbs | 4 +- templates/zerver/api/changelog.md | 5 ++ templates/zerver/app/invite_user.html | 3 + templates/zerver/help/invite-new-users.md | 7 +- version.py | 2 +- zerver/lib/create_user.py | 2 +- zerver/models.py | 1 + zerver/tests/test_signup.py | 105 ++++++++++++++++++++-- zerver/views/invite.py | 23 +++-- 10 files changed, 134 insertions(+), 20 deletions(-) diff --git a/static/js/settings_invites.js b/static/js/settings_invites.js index 85f0fd6515..56af4ef9b5 100644 --- a/static/js/settings_invites.js +++ b/static/js/settings_invites.js @@ -19,6 +19,7 @@ exports.invited_as_values = new Map([ [1, i18n.t("Member")], [2, i18n.t("Organization administrator")], [3, i18n.t("Guest")], + [4, i18n.t("Organization owner")], ]); function add_invited_as_text(invites) { @@ -51,6 +52,7 @@ function populate_invites(invites_data) { modifier: function (item) { item.invited_absolute_time = timerender.absolute_time(item.invited * 1000); item.is_admin = page_params.is_admin; + item.disable_buttons = item.invited_as === 4 && !page_params.is_owner; return render_admin_invites_list({ invite: item }); }, filter: { diff --git a/static/templates/admin_invites_list.hbs b/static/templates/admin_invites_list.hbs index 7fd88dc0dc..2835573c09 100644 --- a/static/templates/admin_invites_list.hbs +++ b/static/templates/admin_invites_list.hbs @@ -23,11 +23,11 @@ {{invited_as_text}} - {{#unless is_multiuse}} - {{/unless}} diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 8b7cdd2a7c..84843f0637 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -10,6 +10,11 @@ below features are supported. ## Changes in Zulip 2.2 +**Feature level 20** + +* Added support for inviting users as organization owners to the + invitation endpoints. + **Feature level 19** * [`GET /events`](/api/get-events): `subscriptions` event with diff --git a/templates/zerver/app/invite_user.html b/templates/zerver/app/invite_user.html index 8506b4cab8..f072ae3bea 100644 --- a/templates/zerver/app/invite_user.html +++ b/templates/zerver/app/invite_user.html @@ -38,6 +38,9 @@ {% endif %} + {% if is_owner %} + + {% endif %} diff --git a/templates/zerver/help/invite-new-users.md b/templates/zerver/help/invite-new-users.md index 8d0501a2e7..d99d9a0748 100644 --- a/templates/zerver/help/invite-new-users.md +++ b/templates/zerver/help/invite-new-users.md @@ -12,7 +12,7 @@ the article below describes each in more detail. * Share a **reusable invitation link**. The last two, invite-based, techniques also allow you to control the -[role (admin, member, or guest)](/help/roles-and-permissions) that the +[role (owner, admin, member, or guest)](/help/roles-and-permissions) that the invited people will have. You can also manage access by @@ -133,8 +133,9 @@ restrict invites to admins only. ## Manage pending invitations -Organization administrators can revoke or resend any invitation or reusable -invitation link. +Organization owners can revoke or resend any invitation or reusable +invitation link. Organization administrators can can do the same +except for invitations for the organization owners role. {start_tabs} diff --git a/version.py b/version.py index c80a9a5988..cab5abf5d8 100644 --- a/version.py +++ b/version.py @@ -29,7 +29,7 @@ DESKTOP_WARNING_VERSION = "5.2.0" # # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md. -API_FEATURE_LEVEL = 18 +API_FEATURE_LEVEL = 20 # 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 diff --git a/zerver/lib/create_user.py b/zerver/lib/create_user.py index 28764886bf..f2e9cb3364 100644 --- a/zerver/lib/create_user.py +++ b/zerver/lib/create_user.py @@ -45,7 +45,7 @@ def get_display_email_address(user_profile: UserProfile, realm: Realm) -> str: return user_profile.delivery_email def get_role_for_new_user(invited_as: int, realm_creation: bool=False) -> int: - if realm_creation: + if realm_creation or invited_as == PreregistrationUser.INVITE_AS['REALM_OWNER']: return UserProfile.ROLE_REALM_OWNER elif invited_as == PreregistrationUser.INVITE_AS['REALM_ADMIN']: return UserProfile.ROLE_REALM_ADMINISTRATOR diff --git a/zerver/models.py b/zerver/models.py index 0020967f0f..ba1682f7b7 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1355,6 +1355,7 @@ class PreregistrationUser(models.Model): MEMBER = 1, REALM_ADMIN = 2, GUEST_USER = 3, + REALM_OWNER = 4, ) invited_as: int = models.PositiveSmallIntegerField(default=INVITE_AS['MEMBER']) diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 00b81b5329..c5f9c7fc32 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -942,11 +942,27 @@ class InviteUserTest(InviteUserBase): inviter.email, ) + def test_successful_invite_user_as_owner_from_owner_account(self) -> None: + self.login('desdemona') + invitee = self.nonreg_email('alice') + result = self.invite(invitee, ["Denmark"], + invite_as=PreregistrationUser.INVITE_AS['REALM_OWNER']) + self.assert_json_success(result) + self.assertTrue(find_key_by_email(invitee)) + + self.submit_reg_form_for_user(invitee, "password") + invitee_profile = self.nonreg_user('alice') + self.assertTrue(invitee_profile.is_realm_owner) + self.assertFalse(invitee_profile.is_guest) + + def test_invite_user_as_owner_from_admin_account(self) -> None: + self.login('iago') + invitee = self.nonreg_email('alice') + response = self.invite(invitee, ["Denmark"], + invite_as=PreregistrationUser.INVITE_AS['REALM_OWNER']) + self.assert_json_error(response, "Must be an organization owner") + def test_successful_invite_user_as_admin_from_admin_account(self) -> None: - """ - Test that a new user invited to a stream receives some initial - history but only from public streams. - """ self.login('iago') invitee = self.nonreg_email('alice') result = self.invite(invitee, ["Denmark"], @@ -957,13 +973,10 @@ class InviteUserTest(InviteUserBase): self.submit_reg_form_for_user(invitee, "password") invitee_profile = self.nonreg_user('alice') self.assertTrue(invitee_profile.is_realm_admin) + self.assertFalse(invitee_profile.is_realm_owner) self.assertFalse(invitee_profile.is_guest) def test_invite_user_as_admin_from_normal_account(self) -> None: - """ - Test that a new user invited to a stream receives some initial - history but only from public streams. - """ self.login('hamlet') invitee = self.nonreg_email('alice') response = self.invite(invitee, ["Denmark"], @@ -1721,6 +1734,26 @@ class InvitationsTestCase(InviteUserBase): lambda: ScheduledEmail.objects.get(address__iexact=invitee, type=ScheduledEmail.INVITATION_REMINDER)) + def test_delete_owner_invitation(self) -> None: + self.login('desdemona') + owner = self.example_user('desdemona') + + invitee = "DeleteMe@zulip.com" + self.assert_json_success(self.invite(invitee, ['Denmark'], + invite_as=PreregistrationUser.INVITE_AS['REALM_OWNER'])) + prereg_user = PreregistrationUser.objects.get(email=invitee) + result = self.api_delete(self.example_user('iago'), + '/api/v1/invites/' + str(prereg_user.id)) + self.assert_json_error(result, "Must be an organization owner") + + result = self.api_delete(owner, '/api/v1/invites/' + str(prereg_user.id)) + self.assert_json_success(result) + result = self.api_delete(owner, '/api/v1/invites/' + str(prereg_user.id)) + self.assert_json_error(result, "No such invitation") + self.assertRaises(ScheduledEmail.DoesNotExist, + lambda: ScheduledEmail.objects.get(address__iexact=invitee, + type=ScheduledEmail.INVITATION_REMINDER)) + def test_delete_multiuse_invite(self) -> None: """ A DELETE call to /json/invites/multiuse should delete the @@ -1738,6 +1771,18 @@ class InvitationsTestCase(InviteUserBase): error_result = self.client_delete('/json/invites/multiuse/' + str(multiuse_invite.id)) self.assert_json_error(error_result, "No such invitation") + # Test deleting owner mutiuse_invite. + multiuse_invite = MultiuseInvite.objects.create(referred_by=self.example_user("desdemona"), realm=zulip_realm, + invited_as=PreregistrationUser.INVITE_AS['REALM_OWNER']) + create_confirmation_link(multiuse_invite, Confirmation.MULTIUSE_INVITE) + error_result = self.client_delete('/json/invites/multiuse/' + str(multiuse_invite.id)) + self.assert_json_error(error_result, 'Must be an organization owner') + + self.login('desdemona') + result = self.client_delete('/json/invites/multiuse/' + str(multiuse_invite.id)) + self.assert_json_success(result) + self.assertIsNone(MultiuseInvite.objects.filter(id=multiuse_invite.id).first()) + # Test deleting multiuse invite from another realm mit_realm = get_realm("zephyr") multiuse_invite_in_mit = MultiuseInvite.objects.create(referred_by=self.mit_user("sipbtest"), realm=mit_realm) @@ -1833,6 +1878,36 @@ class InvitationsTestCase(InviteUserBase): error_result = self.client_post('/json/invites/' + str(prereg_user.id) + '/resend') self.assert_json_error(error_result, "Must be an organization administrator") + def test_resend_owner_invitation(self) -> None: + self.login("desdemona") + + invitee = "resend_owner@zulip.com" + self.assert_json_success(self.invite(invitee, ['Denmark'], + invite_as=PreregistrationUser.INVITE_AS['REALM_OWNER'])) + self.check_sent_emails([invitee], custom_from_name="Zulip") + scheduledemail_filter = ScheduledEmail.objects.filter( + address__iexact=invitee, type=ScheduledEmail.INVITATION_REMINDER) + self.assertEqual(scheduledemail_filter.count(), 1) + original_timestamp = scheduledemail_filter.values_list('scheduled_timestamp', flat=True) + + # Test only organization owners can resend owner invitation. + self.login('iago') + prereg_user = PreregistrationUser.objects.get(email=invitee) + error_result = self.client_post('/json/invites/' + str(prereg_user.id) + '/resend') + self.assert_json_error(error_result, "Must be an organization owner") + + self.login('desdemona') + result = self.client_post('/json/invites/' + str(prereg_user.id) + '/resend') + self.assert_json_success(result) + + self.assertEqual(ScheduledEmail.objects.filter( + address__iexact=invitee, type=ScheduledEmail.INVITATION_REMINDER).count(), 1) + + # Check that we have exactly one scheduled email, and that it is different + self.assertEqual(scheduledemail_filter.count(), 1) + self.assertNotEqual(original_timestamp, + scheduledemail_filter.values_list('scheduled_timestamp', flat=True)) + def test_accessing_invites_in_another_realm(self) -> None: inviter = UserProfile.objects.exclude(realm=get_realm('zulip')).first() prereg_user = PreregistrationUser.objects.create( @@ -2042,6 +2117,20 @@ class MultiuseInviteTest(ZulipTestCase): result = self.client_post('/json/invites/multiuse') self.assert_json_error(result, "Must be an organization administrator") + def test_multiuse_link_for_inviting_as_owner(self) -> None: + self.login('iago') + result = self.client_post('/json/invites/multiuse', + {"invite_as": ujson.dumps(PreregistrationUser.INVITE_AS['REALM_OWNER'])}) + self.assert_json_error(result, "Must be an organization owner") + + self.login('desdemona') + result = self.client_post('/json/invites/multiuse', + {"invite_as": ujson.dumps(PreregistrationUser.INVITE_AS['REALM_OWNER'])}) + self.assert_json_success(result) + + invite_link = result.json()["invite_link"] + self.check_user_able_to_register(self.nonreg_email("test"), invite_link) + def test_create_multiuse_link_invalid_stream_api_call(self) -> None: self.login('iago') result = self.client_post('/json/invites/multiuse', diff --git a/zerver/views/invite.py b/zerver/views/invite.py index 88ec3d2d27..d23ce24057 100644 --- a/zerver/views/invite.py +++ b/zerver/views/invite.py @@ -13,7 +13,7 @@ from zerver.lib.actions import ( do_revoke_multi_use_invite, do_revoke_user_invite, ) -from zerver.lib.exceptions import OrganizationAdministratorRequired +from zerver.lib.exceptions import OrganizationAdministratorRequired, OrganizationOwnerRequired from zerver.lib.request import REQ, JsonableError, has_request_variables from zerver.lib.response import json_error, json_success from zerver.lib.streams import access_stream_by_id @@ -21,6 +21,10 @@ from zerver.lib.validator import check_int, check_list from zerver.models import MultiuseInvite, PreregistrationUser, Stream, UserProfile +def check_if_owner_required(invited_as: int, user_profile: UserProfile) -> None: + if invited_as == PreregistrationUser.INVITE_AS['REALM_OWNER'] and not user_profile.is_realm_owner: + raise OrganizationOwnerRequired() + @require_member_or_admin @has_request_variables def invite_users_backend(request: HttpRequest, user_profile: UserProfile, @@ -34,6 +38,7 @@ def invite_users_backend(request: HttpRequest, user_profile: UserProfile, raise OrganizationAdministratorRequired() if invite_as not in PreregistrationUser.INVITE_AS.values(): return json_error(_("Must be invited as an valid type of user")) + check_if_owner_required(invite_as, user_profile) if invite_as == PreregistrationUser.INVITE_AS['REALM_ADMIN'] and not user_profile.is_realm_admin: return json_error(_("Must be an organization administrator")) if not invitee_emails_raw: @@ -82,8 +87,10 @@ def revoke_user_invite(request: HttpRequest, user_profile: UserProfile, if prereg_user.referred_by.realm != user_profile.realm: raise JsonableError(_("No such invitation")) - if prereg_user.referred_by_id != user_profile.id and not user_profile.is_realm_admin: - raise JsonableError(_("Must be an organization administrator")) + if prereg_user.referred_by_id != user_profile.id: + check_if_owner_required(prereg_user.invited_as, user_profile) + if not user_profile.is_realm_admin: + raise JsonableError(_("Must be an organization administrator")) do_revoke_user_invite(prereg_user) return json_success() @@ -101,6 +108,8 @@ def revoke_multiuse_invite(request: HttpRequest, user_profile: UserProfile, if invite.realm != user_profile.realm: raise JsonableError(_("No such invitation")) + check_if_owner_required(invite.invited_as, user_profile) + do_revoke_multi_use_invite(invite) return json_success() @@ -118,8 +127,10 @@ def resend_user_invite_email(request: HttpRequest, user_profile: UserProfile, if prereg_user.referred_by is None or prereg_user.referred_by.realm != user_profile.realm: raise JsonableError(_("No such invitation")) - if prereg_user.referred_by_id != user_profile.id and not user_profile.is_realm_admin: - raise JsonableError(_("Must be an organization administrator")) + if prereg_user.referred_by_id != user_profile.id: + check_if_owner_required(prereg_user.invited_as, user_profile) + if not user_profile.is_realm_admin: + raise JsonableError(_("Must be an organization administrator")) timestamp = do_resend_user_invite_email(prereg_user) return json_success({'timestamp': timestamp}) @@ -130,6 +141,8 @@ def generate_multiuse_invite_backend( request: HttpRequest, user_profile: UserProfile, invite_as: int=REQ(validator=check_int, default=PreregistrationUser.INVITE_AS['MEMBER']), stream_ids: Sequence[int]=REQ(validator=check_list(check_int), default=[])) -> HttpResponse: + check_if_owner_required(invite_as, user_profile) + streams = [] for stream_id in stream_ids: try: