diff --git a/frontend_tests/node_tests/settings_user_groups.js b/frontend_tests/node_tests/settings_user_groups.js index 9b1051d437..c2a43c8639 100644 --- a/frontend_tests/node_tests/settings_user_groups.js +++ b/frontend_tests/node_tests/settings_user_groups.js @@ -26,6 +26,8 @@ set_global('ui_report', {}); set_global('people', { my_current_user_id: noop, }); +set_global('page_params', {}); + function reset_test_setup(pill_container_stub) { function input_pill_stub(opts) { assert.equal(opts.container, pill_container_stub); @@ -39,9 +41,15 @@ function reset_test_setup(pill_container_stub) { } run_test('can_edit', () => { + page_params.is_guest = false; page_params.is_admin = true; assert(settings_user_groups.can_edit(1)); + page_params.is_admin = false; + page_params.is_guest = true; + assert(!settings_user_groups.can_edit(1)); + + page_params.is_guest = false; page_params.is_admin = false; user_groups.is_member_of = (group_id, user_id) => { assert.equal(group_id, 1); diff --git a/static/js/settings_user_groups.js b/static/js/settings_user_groups.js index 6023ab3c96..ae238c2227 100644 --- a/static/js/settings_user_groups.js +++ b/static/js/settings_user_groups.js @@ -25,8 +25,13 @@ exports.can_edit = function (group_id) { return true; } + if (page_params.is_guest) { + return false; + } + return user_groups.is_member_of(group_id, people.my_current_user_id()); }; + exports.populate_user_groups = function () { var user_groups_section = $('#user-groups').expectOne(); diff --git a/static/templates/user-groups-admin.handlebars b/static/templates/user-groups-admin.handlebars index 46edb4f076..dbe0b6635b 100644 --- a/static/templates/user-groups-admin.handlebars +++ b/static/templates/user-groups-admin.handlebars @@ -2,6 +2,7 @@ {{#unless is_admin}}
Only group members and organization administrators can modify a group.
{{/unless}} + {{#unless is_guest}}
@@ -21,6 +22,7 @@
+ {{/unless}}

{{#tr this}}Add members of your organization to mentionable user groups.{{/tr}}

diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index ef134d09ee..f0c5191897 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -110,6 +110,19 @@ class UserGroupAPITestCase(ZulipTestCase): self.assert_json_error(result, "User group 'support' already exists.") self.assert_length(UserGroup.objects.all(), 2) + def test_user_group_create_by_guest_user(self) -> None: + guest_user = self.example_user('polonius') + + # Guest users can't create user group + self.login(guest_user.email) + params = { + 'name': 'support', + 'members': ujson.dumps([guest_user.id]), + 'description': 'Support team', + } + result = self.client_post('/json/user_groups/create', info=params) + self.assert_json_error(result, "Not allowed for guest users") + def test_user_group_update(self) -> None: hamlet = self.example_user('hamlet') self.login(self.example_email("hamlet")) @@ -162,6 +175,28 @@ class UserGroupAPITestCase(ZulipTestCase): self.assert_json_success(result) self.assertEqual(result.json()['description'], 'Description successfully updated.') + def test_user_group_update_by_guest_user(self) -> None: + hamlet = self.example_user('hamlet') + guest_user = self.example_user('polonius') + self.login(hamlet.email) + params = { + 'name': 'support', + 'members': ujson.dumps([hamlet.id, guest_user.id]), + 'description': 'Support team', + } + result = self.client_post('/json/user_groups/create', info=params) + self.assert_json_success(result) + user_group = UserGroup.objects.get(name='support') + + # Guest user can't edit any detail of an user group + self.login(guest_user.email) + params = { + 'name': 'help', + 'description': 'Troubleshooting team', + } + result = self.client_patch('/json/user_groups/{}'.format(user_group.id), info=params) + self.assert_json_error(result, "Not allowed for guest users") + def test_user_group_delete(self) -> None: hamlet = self.example_user('hamlet') self.login(self.example_email("hamlet")) @@ -211,6 +246,24 @@ class UserGroupAPITestCase(ZulipTestCase): self.assertEqual(UserGroup.objects.count(), 1) self.assertEqual(UserGroupMembership.objects.count(), 2) + def test_user_group_delete_by_guest_user(self) -> None: + hamlet = self.example_user('hamlet') + guest_user = self.example_user('polonius') + self.login(hamlet.email) + params = { + 'name': 'support', + 'members': ujson.dumps([hamlet.id, guest_user.id]), + 'description': 'Support team', + } + result = self.client_post('/json/user_groups/create', info=params) + self.assert_json_success(result) + user_group = UserGroup.objects.get(name='support') + + # Guest users can't delete any user group(not even those of which they are a member) + self.login(guest_user.email) + result = self.client_delete('/json/user_groups/{}'.format(user_group.id)) + self.assert_json_error(result, "Not allowed for guest users") + def test_update_members_of_user_group(self) -> None: hamlet = self.example_user('hamlet') self.login(self.example_email("hamlet")) diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index 73e6e103dd..88a9a8f8bc 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -3,6 +3,7 @@ from django.utils.translation import ugettext as _ from typing import List +from zerver.decorator import require_non_guest_human_user from zerver.context_processors import get_realm_from_request from zerver.lib.actions import check_add_user_group, do_update_user_group_name, \ do_update_user_group_description, bulk_add_members_to_user_group, \ @@ -17,6 +18,7 @@ from zerver.lib.user_groups import access_user_group_by_id, get_memberships_of_u from zerver.models import UserProfile, UserGroup, UserGroupMembership from zerver.views.streams import compose_views, FuncKwargPair +@require_non_guest_human_user @has_request_variables def add_user_group(request: HttpRequest, user_profile: UserProfile, name: str=REQ(), @@ -26,6 +28,7 @@ def add_user_group(request: HttpRequest, user_profile: UserProfile, check_add_user_group(user_profile.realm, name, user_profiles, description) return json_success() +@require_non_guest_human_user @has_request_variables def edit_user_group(request: HttpRequest, user_profile: UserProfile, user_group_id: int=REQ(validator=check_int), @@ -47,6 +50,7 @@ def edit_user_group(request: HttpRequest, user_profile: UserProfile, return json_success(result) +@require_non_guest_human_user @has_request_variables def delete_user_group(request: HttpRequest, user_profile: UserProfile, user_group_id: int=REQ(validator=check_int)) -> HttpResponse: @@ -54,6 +58,7 @@ def delete_user_group(request: HttpRequest, user_profile: UserProfile, check_delete_user_group(user_group_id, user_profile) return json_success() +@require_non_guest_human_user @has_request_variables def update_user_group_backend(request: HttpRequest, user_profile: UserProfile, user_group_id: int=REQ(validator=check_int),