From 0b1f8c05e3738e9162eab0f7254af517c738a870 Mon Sep 17 00:00:00 2001 From: Somesh Ranjan Date: Sat, 7 May 2022 12:33:02 +0530 Subject: [PATCH] org_settings: Add backend support to change bot role. This commit attempts to add the backend support by extending the /json/bots/{bot_id}/ url support to accept the role field as a parameter. This was previously already possible via `/json/users/{user_id}`, so this change just simplifies client implementation. --- templates/zerver/api/changelog.md | 6 ++++++ version.py | 2 +- zerver/tests/test_bots.py | 27 ++++++++++++++++++++++++++- zerver/views/users.py | 16 ++++++++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 1b0857ac9b..a134297360 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 6.0 +**Feature level 130** + +* `PATCH /bots/{bot_user_id}`: Added support for changing a bot's role + via this endpoint. Previously, this could only be done via [`PATCH + /users/{user_id}`](/api/update-user). + **Feature level 129** * [`POST /register`](/api/register-queue), diff --git a/version.py b/version.py index 546a879c5d..634919a4d7 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 129 +API_FEATURE_LEVEL = 130 # 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/tests/test_bots.py b/zerver/tests/test_bots.py index f553ab76f0..054fbc9ef7 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -10,7 +10,7 @@ from zulip_bots.custom_exceptions import ConfigValidationError from zerver.actions.realm_settings import do_set_realm_property from zerver.actions.streams import do_change_stream_permission -from zerver.actions.users import do_change_can_create_users, do_deactivate_user +from zerver.actions.users import do_change_can_create_users, do_change_user_role, do_deactivate_user from zerver.lib.bot_config import ConfigError, get_bot_config from zerver.lib.bot_lib import get_bot_handler from zerver.lib.integrations import EMBEDDED_BOTS, WebhookIntegration @@ -1169,6 +1169,31 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): bot = self.get_bot() self.assertEqual(None, bot["default_sending_stream"]) + def test_patch_bot_role(self) -> None: + self.login("desdemona") + + email = "default-bot@zulip.com" + user_profile = self.get_bot_user(email) + + do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=user_profile) + + req = dict(role=UserProfile.ROLE_GUEST) + + result = self.client_patch(f"/json/bots/{self.get_bot_user(email).id}", req) + self.assert_json_success(result) + + user_profile = self.get_bot_user(email) + self.assertEqual(user_profile.role, UserProfile.ROLE_GUEST) + + # Test for not allowing a non-owner user to make assign a bot an owner role + desdemona = self.example_user("desdemona") + do_change_user_role(desdemona, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None) + + req = dict(role=UserProfile.ROLE_REALM_OWNER) + + result = self.client_patch(f"/json/bots/{self.get_bot_user(email).id}", req) + self.assert_json_error(result, "Must be an organization owner") + def test_patch_bot_to_stream_private_allowed(self) -> None: self.login("hamlet") user_profile = self.example_user("hamlet") diff --git a/zerver/views/users.py b/zerver/views/users.py index d98a6bff9e..5612f37c80 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -193,6 +193,8 @@ def update_user_backend( # grant/remove the role in question. access_user_by_id has # already verified we're an administrator; here we enforce # that only owners can toggle the is_realm_owner flag. + # + # Logic replicated in patch_bot_backend. if UserProfile.ROLE_REALM_OWNER in [role, target.role] and not user_profile.is_realm_owner: raise OrganizationOwnerRequired() @@ -302,6 +304,12 @@ def patch_bot_backend( user_profile: UserProfile, bot_id: int, full_name: Optional[str] = REQ(default=None), + role: Optional[int] = REQ( + default=None, + json_validator=check_int_in( + UserProfile.ROLE_TYPES, + ), + ), bot_owner_id: Optional[int] = REQ(json_validator=check_int, default=None), config_data: Optional[Dict[str, str]] = REQ( default=None, json_validator=check_dict(value_validator=check_string) @@ -316,6 +324,14 @@ def patch_bot_backend( if full_name is not None: check_change_bot_full_name(bot, full_name, user_profile) + + if role is not None and bot.role != role: + # Logic duplicated from update_user_backend. + if UserProfile.ROLE_REALM_OWNER in [role, bot.role] and not user_profile.is_realm_owner: + raise OrganizationOwnerRequired() + + do_change_user_role(bot, role, acting_user=user_profile) + if bot_owner_id is not None: try: owner = get_user_profile_by_id_in_realm(bot_owner_id, user_profile.realm)