settings_users: Support guest user in admin-user-table.

This supports guest user in the user-info-form-modal as well as in the
role section of the admin-user-table.

With some fixes by Tim Abbott and Shubham Dhama.

static/js/settings_users.js

@@ -64,9 +64,12 @@ exports.update_user_data = function (user_id, new_data) {
         }
     }
 
-    if (new_data.is_admin !== undefined) {
+    if (new_data.is_admin !== undefined || new_data.is_guest !== undefined) {
-        if (new_data.is_admin) {
+        var person_obj = people.get_person_from_user_id(user_id);
+        if (person_obj.is_admin) {
             user_row.find(".user_role").text(i18n.t("Administrator"));
+        } else if (person_obj.is_guest) {
+            user_row.find(".user_role").text(i18n.t("Guest"));
         } else {
             user_row.find(".user_role").text(i18n.t("Member"));
         }
@@ -309,6 +312,7 @@ exports.on_load_success = function (realm_people_data) {
             user_id: person.user_id,
             full_name: people.get_full_name(person.user_id),
             is_admin: person.is_admin,
+            is_guest: person.is_guest,
             is_bot: person.is_bot,
         });
         var user_info_form_modal = $(html);
@@ -363,6 +367,7 @@ exports.on_load_success = function (realm_people_data) {
                 data = {
                     full_name: JSON.stringify(full_name.val()),
                     is_admin: JSON.stringify(user_role_select_value === 'admin'),
+                    is_guest: JSON.stringify(user_role_select_value === 'guest'),
                 };
             }
 

static/js/user_events.js

@@ -52,6 +52,11 @@ exports.update_person = function update(person) {
         }
     }
 
+    if (_.has(person, 'is_guest')) {
+        person_obj.is_guest = person.is_guest;
+        settings_users.update_user_data(person.user_id, person);
+    }
+
     if (_.has(person, 'avatar_url')) {
         var url = person.avatar_url;
         person_obj.avatar_url = url;

static/templates/admin_user_list.handlebars

@@ -10,6 +10,8 @@
         <span class="user_role">
             {{#if is_admin}}
             {{t "Administrator" }}
+            {{else if is_guest}}
+            {{t "Guest" }}
             {{else}}
             {{t "Member" }}
             {{/if}}

static/templates/user-info-form-modal.handlebars

@@ -21,6 +21,7 @@
                     <select name="user-role-select" id="user-role-select">
                         <option value="regular">{{t 'Regular member' }}</option>
                         <option value="admin" {{#if is_admin}}selected{{/if}}>{{t 'Organization administrator' }}</option>
+                        <option value="guest" {{#if is_guest}}selected{{/if}}>{{t 'Guest'}}</option>
                     </select>
                 </div>
                 {{/if}}

zerver/lib/actions.py

@@ -482,6 +482,7 @@ def notify_created_user(user_profile: UserProfile) -> None:
                              avatar_url=avatar_url(user_profile),
                              timezone=user_profile.timezone,
                              date_joined=user_profile.date_joined.isoformat(),
+                             is_guest=user_profile.is_guest,
                              is_bot=user_profile.is_bot))  # type: Dict[str, Any]
     if not user_profile.is_bot:
         event["person"]["profile_data"] = {}
@@ -3175,6 +3176,16 @@ def do_change_is_admin(user_profile: UserProfile, value: bool,
                                  is_admin=value))
         send_event(event, active_user_ids(user_profile.realm_id))
 
+def do_change_is_guest(user_profile: UserProfile, value: bool) -> None:
+    user_profile.is_guest = value
+    user_profile.save(update_fields=["is_guest"])
+    event = dict(type="realm_user", op="update",
+                 person=dict(email=user_profile.email,
+                             user_id=user_profile.id,
+                             is_guest=value))
+    send_event(event, active_user_ids(user_profile.realm_id))
+
+
 def do_change_stream_invite_only(stream: Stream, invite_only: bool,
                                  history_public_to_subscribers: Optional[bool]=None) -> None:
     history_public_to_subscribers = get_default_value_for_history_public_to_subscribers(

zerver/lib/cache.py

@@ -324,7 +324,7 @@ realm_user_dict_fields = [
     'id', 'full_name', 'short_name', 'email',
     'avatar_source', 'avatar_version', 'is_active',
     'is_realm_admin', 'is_bot', 'realm_id', 'timezone',
-    'date_joined'
+    'date_joined', 'is_guest'
 ]  # type: List[str]
 
 def realm_user_dicts_cache_key(realm_id: int) -> str:

zerver/lib/events.py

@@ -73,6 +73,7 @@ def get_raw_user_data(realm_id: int, client_gravatar: bool) -> Dict[int, Dict[st
         )
 
         is_admin = row['is_realm_admin']
+        is_guest = row['is_guest']
         is_bot = row['is_bot']
         # This format should align with get_cross_realm_dicts() and notify_created_user
         result = dict(
@@ -80,6 +81,7 @@ def get_raw_user_data(realm_id: int, client_gravatar: bool) -> Dict[int, Dict[st
             user_id=row['id'],
             avatar_url=avatar_url,
             is_admin=is_admin,
+            is_guest=is_guest,
             is_bot=is_bot,
             full_name=row['full_name'],
             timezone=row['timezone'],

zerver/tests/test_events.py

@@ -1154,6 +1154,7 @@ class EventsRegisterTest(ZulipTestCase):
                 ('full_name', check_string),
                 ('is_admin', check_bool),
                 ('is_bot', check_bool),
+                ('is_guest', check_bool),
                 ('profile_data', check_dict_only([])),
                 ('timezone', check_string),
                 ('date_joined', check_string),

zerver/tests/test_users.py

@@ -34,6 +34,7 @@ from zerver.lib.actions import (
     do_deactivate_user,
     do_reactivate_user,
     do_change_is_admin,
+    do_change_is_guest,
     do_create_user,
 )
 from zerver.lib.create_user import copy_user_settings
@@ -230,6 +231,118 @@ class PermissionTest(ZulipTestCase):
         with self.assertRaises(JsonableError):
             access_user_by_id(self.example_user("cordelia"), self.example_user("aaron").id)
 
+    def test_change_regular_member_to_guest(self) -> None:
+        iago = self.example_user("iago")
+        self.login(iago.email)
+
+        hamlet = self.example_user("hamlet")
+        self.assertFalse(hamlet.is_guest)
+
+        # Test failure of making user both admin and guest
+        req = dict(is_guest=ujson.dumps(True), is_admin=ujson.dumps(True))
+        result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
+        self.assert_json_error(result, 'Guests cannot be organization administrators')
+        self.assertFalse(hamlet.is_guest)
+        self.assertFalse(hamlet.is_realm_admin)
+        hamlet = self.example_user("hamlet")
+
+        req = dict(is_guest=ujson.dumps(True))
+        events = []  # type: List[Mapping[str, Any]]
+        with tornado_redirected_to_list(events):
+            result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
+        self.assert_json_success(result)
+
+        hamlet = self.example_user("hamlet")
+        self.assertTrue(hamlet.is_guest)
+        person = events[0]['event']['person']
+        self.assertEqual(person['email'], hamlet.email)
+        self.assertTrue(person['is_guest'])
+
+    def test_change_guest_to_regular_member(self) -> None:
+        iago = self.example_user("iago")
+        self.login(iago.email)
+
+        polonius = self.example_user("polonius")
+        self.assertTrue(polonius.is_guest)
+        req = dict(is_guest=ujson.dumps(False))
+        events = []  # type: List[Mapping[str, Any]]
+        with tornado_redirected_to_list(events):
+            result = self.client_patch('/json/users/{}'.format(polonius.id), req)
+        self.assert_json_success(result)
+
+        polonius = self.example_user("polonius")
+        self.assertFalse(polonius.is_guest)
+        person = events[0]['event']['person']
+        self.assertEqual(person['email'], polonius.email)
+        self.assertFalse(person['is_guest'])
+
+    def test_change_admin_to_guest(self) -> None:
+        iago = self.example_user("iago")
+        self.login(iago.email)
+        hamlet = self.example_user("hamlet")
+        do_change_is_admin(hamlet, True)
+        self.assertFalse(hamlet.is_guest)
+        self.assertTrue(hamlet.is_realm_admin)
+
+        # Test failure of making a admin to guest without revoking admin status
+        req = dict(is_guest=ujson.dumps(True))
+        result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
+        self.assert_json_error(result, 'Guests cannot be organization administrators')
+
+        # Test changing a user from admin to guest and revoking admin status
+        hamlet = self.example_user("hamlet")
+        self.assertFalse(hamlet.is_guest)
+        req = dict(is_admin=ujson.dumps(False), is_guest=ujson.dumps(True))
+        events = []  # type: List[Mapping[str, Any]]
+        with tornado_redirected_to_list(events):
+            result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
+        self.assert_json_success(result)
+
+        hamlet = self.example_user("hamlet")
+        self.assertTrue(hamlet.is_guest)
+        self.assertFalse(hamlet.is_realm_admin)
+
+        person = events[0]['event']['person']
+        self.assertEqual(person['email'], hamlet.email)
+        self.assertFalse(person['is_admin'])
+
+        person = events[1]['event']['person']
+        self.assertEqual(person['email'], hamlet.email)
+        self.assertTrue(person['is_guest'])
+
+    def test_change_guest_to_admin(self) -> None:
+        iago = self.example_user("iago")
+        self.login(iago.email)
+        polonius = self.example_user("polonius")
+        self.assertTrue(polonius.is_guest)
+        self.assertFalse(polonius.is_realm_admin)
+
+        # Test failure of making a guest to admin without revoking guest status
+        req = dict(is_admin=ujson.dumps(True))
+        result = self.client_patch('/json/users/{}'.format(polonius.id), req)
+        self.assert_json_error(result, 'Guests cannot be organization administrators')
+
+        # Test changing a user from guest to admin and revoking guest status
+        polonius = self.example_user("polonius")
+        self.assertFalse(polonius.is_realm_admin)
+        req = dict(is_admin=ujson.dumps(True), is_guest=ujson.dumps(False))
+        events = []  # type: List[Mapping[str, Any]]
+        with tornado_redirected_to_list(events):
+            result = self.client_patch('/json/users/{}'.format(polonius.id), req)
+        self.assert_json_success(result)
+
+        polonius = self.example_user("polonius")
+        self.assertFalse(polonius.is_guest)
+        self.assertTrue(polonius.is_realm_admin)
+
+        person = events[0]['event']['person']
+        self.assertEqual(person['email'], polonius.email)
+        self.assertTrue(person['is_admin'])
+
+        person = events[1]['event']['person']
+        self.assertEqual(person['email'], polonius.email)
+        self.assertFalse(person['is_guest'])
+
 class AdminCreateUserTest(ZulipTestCase):
     def test_create_user_backend(self) -> None:
 

zerver/views/users.py

@@ -18,7 +18,7 @@ from zerver.lib.actions import do_change_avatar_fields, do_change_bot_owner, \
     do_change_default_events_register_stream, do_change_default_sending_stream, \
     do_create_user, do_deactivate_user, do_reactivate_user, do_regenerate_api_key, \
     check_change_full_name, notify_created_bot, do_update_outgoing_webhook_service, \
-    do_update_bot_config_data, check_change_bot_full_name
+    do_update_bot_config_data, check_change_bot_full_name, do_change_is_guest
 from zerver.lib.avatar import avatar_url, get_gravatar_url, get_avatar_field
 from zerver.lib.bot_config import set_bot_config
 from zerver.lib.exceptions import JsonableError
@@ -78,14 +78,27 @@ def reactivate_user_backend(request: HttpRequest, user_profile: UserProfile,
 @has_request_variables
 def update_user_backend(request: HttpRequest, user_profile: UserProfile, user_id: int,
                         full_name: Optional[str]=REQ(default="", validator=check_string),
-                        is_admin: Optional[bool]=REQ(default=None, validator=check_bool)) -> HttpResponse:
+                        is_admin: Optional[bool]=REQ(default=None, validator=check_bool),
+                        is_guest: Optional[bool]=REQ(default=None, validator=check_bool)) -> HttpResponse:
     target = access_user_by_id(user_profile, user_id, allow_deactivated=True, allow_bots=True)
 
+    # This condition is a bit complicated, because the user could
+    # already be a guest/admin, or the request could be to make the
+    # user a guest/admin.  In any case, the point is that we outright
+    # reject requests that would result in a user who is both an admin
+    # and a guest.
+    if (((is_guest is None and target.is_guest) or is_guest) and
+            ((is_admin is None and target.is_realm_admin) or is_admin)):
+        return json_error(_("Guests cannot be organization administrators"))
+
     if is_admin is not None and target.is_realm_admin != is_admin:
         if not is_admin and check_last_admin(user_profile):
             return json_error(_('Cannot remove the only organization administrator'))
         do_change_is_admin(target, is_admin)
 
+    if is_guest is not None and target.is_guest != is_guest:
+        do_change_is_guest(target, is_guest)
+
     if (full_name is not None and target.full_name != full_name and
             full_name.strip() != ""):
         # We don't respect `name_changes_disabled` here because the request
@@ -381,6 +394,7 @@ def get_members_backend(request: HttpRequest, user_profile: UserProfile,
         'is_bot',
         'is_realm_admin',
         'is_active',
+        'is_guest',
         'bot_type',
         'avatar_source',
         'avatar_version',
@@ -399,6 +413,7 @@ def get_members_backend(request: HttpRequest, user_profile: UserProfile,
             is_active=row['is_active'],
             is_admin=row['is_realm_admin'],
             bot_type=row['bot_type'],
+            is_guest=row['is_guest'],
         )
 
         result['avatar_url'] = get_avatar_field(