diff --git a/static/js/settings_users.js b/static/js/settings_users.js index 9170308ba0..21c563fc6c 100644 --- a/static/js/settings_users.js +++ b/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) { + if (new_data.is_admin !== undefined || new_data.is_guest !== undefined) { + 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'), }; } diff --git a/static/js/user_events.js b/static/js/user_events.js index 445fb805db..10efd020ce 100644 --- a/static/js/user_events.js +++ b/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; diff --git a/static/templates/admin_user_list.handlebars b/static/templates/admin_user_list.handlebars index a2ba8c95f6..eb4d58e4d8 100644 --- a/static/templates/admin_user_list.handlebars +++ b/static/templates/admin_user_list.handlebars @@ -10,6 +10,8 @@ {{#if is_admin}} {{t "Administrator" }} + {{else if is_guest}} + {{t "Guest" }} {{else}} {{t "Member" }} {{/if}} diff --git a/static/templates/user-info-form-modal.handlebars b/static/templates/user-info-form-modal.handlebars index 14d8b9172b..9dc84847de 100644 --- a/static/templates/user-info-form-modal.handlebars +++ b/static/templates/user-info-form-modal.handlebars @@ -21,6 +21,7 @@ {{/if}} diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 0839d6cd02..ecf1e25365 100644 --- a/zerver/lib/actions.py +++ b/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( diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py index d085d94f35..5acb9e847e 100644 --- a/zerver/lib/cache.py +++ b/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: diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 53450d9dc9..451a265876 100644 --- a/zerver/lib/events.py +++ b/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'], diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 015d4998ae..b96d85dce8 100644 --- a/zerver/tests/test_events.py +++ b/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), diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 9b2a62a5a2..c9a445df88 100644 --- a/zerver/tests/test_users.py +++ b/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: diff --git a/zerver/views/users.py b/zerver/views/users.py index fc552891fe..155ed4dcad 100644 --- a/zerver/views/users.py +++ b/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(