From d48164ce1ef4c038b9b48e57213322b36d6b7654 Mon Sep 17 00:00:00 2001 From: Vector73 Date: Tue, 28 Jan 2025 11:21:58 +0000 Subject: [PATCH] settings: Add two new realm settings to restrict bot creation. Added `can_create_bots_group` setting which controls who can create any type of bots in the organization. Added `can_create_write_only_bots_group` setting which controls who can create incoming webhooks in the organization in additon to those who are in `can_create_bots_group`. --- api_docs/changelog.md | 12 + help/restrict-bot-creation.md | 16 +- version.py | 2 +- web/src/admin.ts | 7 +- web/src/group_permission_settings.ts | 2 + web/src/server_events_dispatch.js | 10 +- web/src/settings.ts | 8 +- web/src/settings_bots.ts | 67 ++-- web/src/settings_components.ts | 6 + web/src/settings_org.ts | 2 + web/src/state_data.ts | 3 +- web/templates/settings/bot_settings_tip.hbs | 14 +- .../organization_permissions_admin.hbs | 13 +- web/tests/dispatch.test.cjs | 25 +- web/tests/lib/events.cjs | 8 +- web/tests/settings_bots.test.cjs | 12 - web/tests/settings_org.test.cjs | 27 +- zerver/lib/event_types.py | 2 + zerver/lib/users.py | 32 +- ...56_realm_can_create_bots_group_and_more.py | 33 ++ ...alue_for_can_create_bots_group_and_more.py | 65 ++++ ...er_realm_can_create_bots_group_and_more.py | 31 ++ zerver/models/realms.py | 28 ++ zerver/models/users.py | 25 +- zerver/openapi/zulip.yaml | 46 +++ zerver/tests/test_bots.py | 309 +++++++++++++----- zerver/tests/test_home.py | 8 +- zerver/views/realm.py | 2 + zerver/views/users.py | 6 +- 29 files changed, 568 insertions(+), 253 deletions(-) create mode 100644 zerver/migrations/0656_realm_can_create_bots_group_and_more.py create mode 100644 zerver/migrations/0657_set_default_value_for_can_create_bots_group_and_more.py create mode 100644 zerver/migrations/0658_alter_realm_can_create_bots_group_and_more.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 0c6f733cbf..67371798d5 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,18 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 344** + +* `PATCH /realm`, [`GET /events`](/api/get-events), + [`POST /register`](/api/register-queue): + Added two new realm settings, `can_create_bots_group` which is a + [group-setting value](/api/group-setting-values) describing the set of users + with permission to create bot users in the organization, and + `can_create_write_only_bots_group` which is a + [group-setting value](/api/group-setting-values) describing the set of users + with permission to create bot users who can only send messages in the organization + in addition to the users who are in `can_create_bots_group`. + **Feature level 343** * [`GET /events`](/api/get-events): Added a new field `stream_ids` to replace diff --git a/help/restrict-bot-creation.md b/help/restrict-bot-creation.md index 09524d032e..5cb5c8ec6f 100644 --- a/help/restrict-bot-creation.md +++ b/help/restrict-bot-creation.md @@ -6,13 +6,25 @@ By default, anyone other than guests can [add a bot](/help/add-a-bot-or-integrat or integration to the Zulip organization. Organization administrators can change who is allowed to add bots. -## Configure who can add bots +## Configure who can create any type of bot {start_tabs} {settings_tab|organization-permissions} -2. Under **Other permissions**, configure **Who can add bots**. +2. Under **Other permissions**, configure **Who can create any bot**. + +{!save-changes.md!} + +{end_tabs} + +## Configure who can create bots that can only send messages + +{start_tabs} + +{settings_tab|organization-permissions} + +2. Under **Other permissions**, configure **Who can create bots that send messages into Zulip**. {!save-changes.md!} diff --git a/version.py b/version.py index 29a352d8cc..e74f90837f 100644 --- a/version.py +++ b/version.py @@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 343 # Last bumped for `stream_ids` field in stream deletion events. +API_FEATURE_LEVEL = 344 # Last bumped for `can_create_bots_group`. # 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/web/src/admin.ts b/web/src/admin.ts index a9ced4b36f..e4edad6dd9 100644 --- a/web/src/admin.ts +++ b/web/src/admin.ts @@ -148,7 +148,7 @@ export function build_page(): void { realm_email_changes_disabled: realm.realm_email_changes_disabled, realm_avatar_changes_disabled: realm.realm_avatar_changes_disabled, can_add_emojis: settings_data.user_can_add_custom_emoji(), - can_create_new_bots: settings_bots.can_create_new_bots(), + can_create_new_bots: settings_bots.can_create_incoming_webhooks(), realm_message_content_edit_limit_minutes: settings_components.get_realm_time_limits_in_minutes( "realm_message_content_edit_limit_seconds", @@ -194,7 +194,6 @@ export function build_page(): void { msg_edit_limit_dropdown_values: settings_config.msg_edit_limit_dropdown_values, msg_delete_limit_dropdown_values: settings_config.msg_delete_limit_dropdown_values, msg_move_limit_dropdown_values: settings_config.msg_move_limit_dropdown_values, - bot_creation_policy_values: settings_bots.bot_creation_policy_values, email_address_visibility_values: settings_config.email_address_visibility_values, waiting_period_threshold_dropdown_values: settings_config.waiting_period_threshold_dropdown_values, @@ -259,7 +258,7 @@ export function build_page(): void { $("#settings_content .organization-box").html(rendered_admin_tab); $("#settings_content .alert").removeClass("show"); - settings_bots.update_bot_settings_tip($("#admin-bot-settings-tip"), true); + settings_bots.update_bot_settings_tip($("#admin-bot-settings-tip")); settings_invites.update_invite_user_panel(); insert_tip_box(); @@ -268,8 +267,6 @@ export function build_page(): void { demo_organizations_ui.handle_demo_organization_conversion(); } - $("#id_realm_bot_creation_policy").val(realm.realm_bot_creation_policy); - $("#id_realm_digest_weekday").val(realm.realm_digest_weekday); const is_plan_plus = realm.realm_plan_type === 10; diff --git a/web/src/group_permission_settings.ts b/web/src/group_permission_settings.ts index a91cfcd347..5a969dcbf6 100644 --- a/web/src/group_permission_settings.ts +++ b/web/src/group_permission_settings.ts @@ -44,9 +44,11 @@ export const realm_group_setting_name_schema = z.enum([ "can_add_custom_emoji_group", "can_add_subscribers_group", "can_create_groups", + "can_create_bots_group", "can_create_public_channel_group", "can_create_private_channel_group", "can_create_web_public_channel_group", + "can_create_write_only_bots_group", "can_delete_any_message_group", "can_delete_own_message_group", "can_invite_users_group", diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index cd49d2c160..802c790bd7 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -211,12 +211,13 @@ export function dispatch_normal_event(event) { allow_edit_history: noop, allow_message_editing: noop, avatar_changes_disabled: settings_account.update_avatar_change_display, - bot_creation_policy: settings_bots.update_bot_permissions_ui, can_add_custom_emoji_group: noop, can_add_subscribers_group: noop, + can_create_bots_group: noop, can_create_groups: noop, can_create_private_channel_group: noop, can_create_public_channel_group: noop, + can_create_write_only_bots_group: noop, can_delete_any_message_group: noop, can_delete_own_message_group: noop, can_manage_all_groups: noop, @@ -319,6 +320,13 @@ export function dispatch_normal_event(event) { settings_emoji.update_custom_emoji_ui(); } + if ( + key === "can_create_bots_group" || + key === "can_create_write_only_bots_group" + ) { + settings_bots.update_bot_permissions_ui(); + } + if ( key === "can_create_public_channel_group" || key === "can_create_private_channel_group" || diff --git a/web/src/settings.ts b/web/src/settings.ts index 359e542515..8611f02a7b 100644 --- a/web/src/settings.ts +++ b/web/src/settings.ts @@ -52,7 +52,7 @@ export function update_lock_icon_in_sidebar(): void { $(".org-settings-list .locked").show(); - if (settings_bots.can_create_new_bots()) { + if (settings_bots.can_create_incoming_webhooks()) { $(".org-settings-list li[data-section='bot-list-admin'] .locked").hide(); } @@ -99,7 +99,7 @@ export function build_page(): void { zuliprc: "zuliprc", botserverrc: "botserverrc", timezones: timezones.timezones, - can_create_new_bots: settings_bots.can_create_new_bots(), + can_create_new_bots: settings_bots.can_create_incoming_webhooks(), settings_label, demote_inactive_streams_values: settings_config.demote_inactive_streams_values, web_mark_read_on_scroll_policy_values: @@ -146,8 +146,8 @@ export function build_page(): void { settings_config.automatically_follow_or_unmute_topics_policy_values, }); - settings_bots.update_bot_settings_tip($("#personal-bot-settings-tip"), false); $(".settings-box").html(rendered_settings_tab); + settings_bots.update_bot_settings_tip($("#personal-bot-settings-tip")); common.adjust_mac_kbd_tags("#user_enter_sends_label kbd"); } @@ -180,7 +180,7 @@ export function initialize(): void { is_guest: current_user.is_guest, show_uploaded_files_section: realm.max_file_upload_size_mib > 0, show_emoji_settings_lock: !settings_data.user_can_add_custom_emoji(), - can_create_new_bots: settings_bots.can_create_new_bots(), + can_create_new_bots: settings_bots.can_create_incoming_webhooks(), can_edit_user_panel: current_user.is_admin || settings_data.user_can_create_multiuse_invite() || diff --git a/web/src/settings_bots.ts b/web/src/settings_bots.ts index b365dec391..6863f80185 100644 --- a/web/src/settings_bots.ts +++ b/web/src/settings_bots.ts @@ -20,7 +20,7 @@ import * as list_widget from "./list_widget.ts"; import {page_params} from "./page_params.ts"; import * as people from "./people.ts"; import * as settings_data from "./settings_data.ts"; -import {current_user, realm} from "./state_data.ts"; +import {realm} from "./state_data.ts"; import type {HTMLSelectOneElement} from "./types.ts"; import * as ui_report from "./ui_report.ts"; import type {UploadWidget} from "./upload_widget.ts"; @@ -154,59 +154,43 @@ export function generate_botserverrc_content( ); } -export const bot_creation_policy_values = { - admins_only: { - code: 3, - description: $t({defaultMessage: "Admins"}), - }, - everyone: { - code: 1, - description: $t({defaultMessage: "Admins, moderators and members"}), - }, - restricted: { - code: 2, - description: $t({ - defaultMessage: "Admins, moderators and members, but only admins can add generic bots", - }), - }, -}; - export function can_create_new_bots(): boolean { - if (current_user.is_admin) { - return true; - } - - if (current_user.is_guest) { - return false; - } - - return realm.realm_bot_creation_policy !== bot_creation_policy_values.admins_only.code; + return settings_data.user_has_permission_for_group_setting( + realm.realm_can_create_bots_group, + "can_create_bots_group", + "realm", + ); } -export function update_bot_settings_tip($tip_container: JQuery, for_org_settings: boolean): void { - if ( - !current_user.is_admin && - realm.realm_bot_creation_policy === bot_creation_policy_values.everyone.code - ) { - $tip_container.hide(); - return; - } +export function can_create_incoming_webhooks(): boolean { + // User who have the permission to create any bot can also + // create incoming webhooks. + return ( + can_create_new_bots() || + settings_data.user_has_permission_for_group_setting( + realm.realm_can_create_write_only_bots_group, + "can_create_write_only_bots_group", + "realm", + ) + ); +} - if (current_user.is_admin && !for_org_settings) { +export function update_bot_settings_tip($tip_container: JQuery): void { + if (can_create_new_bots()) { $tip_container.hide(); return; } const rendered_tip = render_bot_settings_tip({ - realm_bot_creation_policy: realm.realm_bot_creation_policy, - permission_type: bot_creation_policy_values, + can_create_any_bots: can_create_new_bots(), + can_create_incoming_webhooks: can_create_incoming_webhooks(), }); $tip_container.show(); $tip_container.html(rendered_tip); } function update_add_bot_button(): void { - if (can_create_new_bots()) { + if (can_create_incoming_webhooks()) { $("#bot-settings .add-a-new-bot").show(); $("#admin-bot-list .add-new-bots").show(); $("#admin-bot-list .manage-your-bots").hide(); @@ -223,10 +207,9 @@ function update_add_bot_button(): void { } export function update_bot_permissions_ui(): void { - update_bot_settings_tip($("#admin-bot-settings-tip"), true); - update_bot_settings_tip($("#personal-bot-settings-tip"), false); + update_bot_settings_tip($("#admin-bot-settings-tip")); + update_bot_settings_tip($("#personal-bot-settings-tip")); update_add_bot_button(); - $("#id_realm_bot_creation_policy").val(realm.realm_bot_creation_policy); } export function add_a_new_bot(): void { diff --git a/web/src/settings_components.ts b/web/src/settings_components.ts index db7ab8f288..dc912a5543 100644 --- a/web/src/settings_components.ts +++ b/web/src/settings_components.ts @@ -803,9 +803,11 @@ export function check_realm_settings_property_changed(elem: HTMLElement): boolea break; case "realm_can_add_custom_emoji_group": case "realm_can_add_subscribers_group": + case "realm_can_create_bots_group": case "realm_can_create_groups": case "realm_can_create_public_channel_group": case "realm_can_create_private_channel_group": + case "realm_can_create_write_only_bots_group": case "realm_can_delete_any_message_group": case "realm_can_delete_own_message_group": case "realm_can_invite_users_group": @@ -1050,10 +1052,12 @@ export function populate_data_for_realm_settings_request( "can_access_all_users_group", "can_add_custom_emoji_group", "can_add_subscribers_group", + "can_create_bots_group", "can_create_groups", "can_create_private_channel_group", "can_create_public_channel_group", "can_create_web_public_channel_group", + "can_create_write_only_bots_group", "can_manage_all_groups", "can_delete_any_message_group", "can_delete_own_message_group", @@ -1513,9 +1517,11 @@ export const group_setting_widget_map = new Map{{t "This organization is configured so that only administrators can add bots." }} -{{else if (eq realm_bot_creation_policy permission_type.restricted.code) }} -
{{t "This organization is configured so that only administrators can add generic bots." }}
-{{else}} -
{{t "This organization is configured so that anyone can add bots." }}
-{{/if}} +{{#unless can_create_any_bots}} + {{#if can_create_incoming_webhooks}} +
{{t "You can create bots that can only send messages." }}
+ {{else}} +
{{t "You do not have permission to create bots." }}
+ {{/if}} +{{/unless}} diff --git a/web/templates/settings/organization_permissions_admin.hbs b/web/templates/settings/organization_permissions_admin.hbs index 2497d25bc5..fcc56530e1 100644 --- a/web/templates/settings/organization_permissions_admin.hbs +++ b/web/templates/settings/organization_permissions_admin.hbs @@ -335,12 +335,13 @@ {{> settings_save_discard_widget section_name="other-permissions" }}
-
- - -
+ {{> group_setting_value_pill_input + setting_name="realm_can_create_write_only_bots_group" + label=(t 'Who can create bots that send messages into Zulip')}} + + {{> group_setting_value_pill_input + setting_name="realm_can_create_bots_group" + label=(t 'Who can create any bot')}} {{> group_setting_value_pill_input setting_name="realm_can_add_custom_emoji_group" diff --git a/web/tests/dispatch.test.cjs b/web/tests/dispatch.test.cjs index 92924f06a4..8f470824f9 100644 --- a/web/tests/dispatch.test.cjs +++ b/web/tests/dispatch.test.cjs @@ -510,28 +510,7 @@ run_test("realm settings", ({override}) => { assert.equal(realm[parameter_name], true); } - function test_realm_integer(event, parameter_name) { - override(realm, parameter_name, 1); - event = {...event}; - event.value = 2; - dispatch(event); - assert.equal(realm[parameter_name], 2); - - event = {...event}; - event.value = 3; - dispatch(event); - assert.equal(realm[parameter_name], 3); - - event = {...event}; - event.value = 1; - dispatch(event); - assert.equal(realm[parameter_name], 1); - } - - let event = event_fixtures.realm__update__bot_creation_policy; - test_realm_integer(event, "realm_bot_creation_policy"); - - event = event_fixtures.realm__update__invite_required; + let event = event_fixtures.realm__update__invite_required; test_realm_boolean(event, "realm_invite_required"); event = event_fixtures.realm__update__want_advertise_in_communities_directory; @@ -597,6 +576,7 @@ run_test("realm settings", ({override}) => { override(realm, "realm_authentication_methods", {Google: {enabled: false, available: true}}); override(realm, "realm_can_add_custom_emoji_group", 1); override(realm, "realm_can_add_subscribers_group", 1); + override(realm, "realm_can_create_bots_group", 1); override(realm, "realm_can_create_public_channel_group", 1); override(realm, "realm_can_invite_users_group", 1); override(realm, "realm_can_move_messages_between_topics_group", 1); @@ -620,6 +600,7 @@ run_test("realm settings", ({override}) => { }); assert_same(realm.realm_can_add_custom_emoji_group, 3); assert_same(realm.realm_can_add_subscribers_group, 3); + assert_same(realm.realm_can_create_bots_group, 3); assert_same(realm.realm_can_create_public_channel_group, 3); assert_same(realm.realm_can_invite_users_group, 3); assert_same(realm.realm_can_move_messages_between_topics_group, 3); diff --git a/web/tests/lib/events.cjs b/web/tests/lib/events.cjs index ee83175bed..3ef1c41e69 100644 --- a/web/tests/lib/events.cjs +++ b/web/tests/lib/events.cjs @@ -266,13 +266,6 @@ exports.fixtures = { realm_id: 2, }, - realm__update__bot_creation_policy: { - type: "realm", - op: "update", - property: "bot_creation_policy", - value: 1, - }, - realm__update__default_code_block_language: { type: "realm", op: "update", @@ -363,6 +356,7 @@ exports.fixtures = { }, can_add_custom_emoji_group: 3, can_add_subscribers_group: 3, + can_create_bots_group: 3, can_create_public_channel_group: 3, can_invite_users_group: 3, can_move_messages_between_topics_group: 3, diff --git a/web/tests/settings_bots.test.cjs b/web/tests/settings_bots.test.cjs index f67f78efeb..e0e6b25ec3 100644 --- a/web/tests/settings_bots.test.cjs +++ b/web/tests/settings_bots.test.cjs @@ -95,15 +95,3 @@ test("generate_botserverrc_content", () => { assert.equal(content, expected); }); - -test("can_create_new_bots", ({override}) => { - override(current_user, "is_admin", true); - assert.ok(settings_bots.can_create_new_bots()); - - override(current_user, "is_admin", false); - override(realm, "realm_bot_creation_policy", 1); - assert.ok(settings_bots.can_create_new_bots()); - - override(realm, "realm_bot_creation_policy", 3); - assert.ok(!settings_bots.can_create_new_bots()); -}); diff --git a/web/tests/settings_org.test.cjs b/web/tests/settings_org.test.cjs index 9125646045..58c6a95d97 100644 --- a/web/tests/settings_org.test.cjs +++ b/web/tests/settings_org.test.cjs @@ -19,7 +19,6 @@ mock_esm("../src/loading", { mock_esm("../src/scroll_util", {scroll_element_into_container: noop}); set_global("document", "document-stub"); -const settings_bots = zrequire("settings_bots"); const settings_account = zrequire("settings_account"); const settings_components = zrequire("settings_components"); const settings_org = zrequire("settings_org"); @@ -100,7 +99,6 @@ function createSaveButtons(subsection) { function test_submit_settings_form(override, submit_form) { Object.assign(realm, { - realm_bot_creation_policy: settings_bots.bot_creation_policy_values.restricted.code, realm_waiting_period_threshold: 1, realm_default_language: '"es"', }); @@ -129,22 +127,7 @@ function test_submit_settings_form(override, submit_form) { $("#id_realm_waiting_period_threshold").val(10); - const $bot_creation_policy_elem = $("#id_realm_bot_creation_policy"); - $bot_creation_policy_elem.val("1"); - $bot_creation_policy_elem.attr("id", "id_realm_bot_creation_policy"); - $bot_creation_policy_elem.data = () => "number"; - let $subsection_elem = $(`#org-${CSS.escape(subsection)}`); - $subsection_elem.set_find_results(".prop-element", [$bot_creation_policy_elem]); - - patched = false; - submit_form.call({to_$: () => $(".save-discard-widget-button.save-button")}, ev); - assert.ok(patched); - - let expected_value = { - bot_creation_policy: 1, - }; - assert.deepEqual(data, expected_value); subsection = "user-defaults"; stubs = createSaveButtons(subsection); @@ -155,7 +138,6 @@ function test_submit_settings_form(override, submit_form) { const $realm_default_language_elem = $("#id_realm_default_language"); $realm_default_language_elem.val("en"); $realm_default_language_elem.attr("id", "id_realm_default_language"); - $realm_default_language_elem.data = () => "string"; $subsection_elem = $(`#org-${CSS.escape(subsection)}`); $subsection_elem.set_find_results(".prop-element", [$realm_default_language_elem]); @@ -163,7 +145,7 @@ function test_submit_settings_form(override, submit_form) { submit_form.call({to_$: () => $(".save-discard-widget-button.save-button")}, ev); assert.ok(patched); - expected_value = { + const expected_value = { default_language: "en", }; assert.deepEqual(data, expected_value); @@ -566,12 +548,7 @@ test("set_up", ({override, override_rewire}) => { // elements involved in disabling the can_create_groups input. override(realm, "zulip_plan_is_not_limited", true); - override_rewire(settings_components, "get_input_element_value", (elem) => { - if ($(elem).data() === "number") { - return Number.parseInt($(elem).val(), 10); - } - return $(elem).val(); - }); + override_rewire(settings_components, "get_input_element_value", (elem) => $(elem).val()); // TEST set_up() here, but this mostly just allows us to // get access to the click handlers. diff --git a/zerver/lib/event_types.py b/zerver/lib/event_types.py index d651c86ee1..15c2bb0853 100644 --- a/zerver/lib/event_types.py +++ b/zerver/lib/event_types.py @@ -512,10 +512,12 @@ class GroupSettingUpdateData(GroupSettingUpdateDataCore): can_access_all_users_group: int | AnonymousSettingGroupDict | None = None can_add_custom_emoji_group: int | AnonymousSettingGroupDict | None = None can_add_subscribers_group: int | AnonymousSettingGroupDict | None = None + can_create_bots_group: int | AnonymousSettingGroupDict | None = None can_create_groups: int | AnonymousSettingGroupDict | None = None can_create_public_channel_group: int | AnonymousSettingGroupDict | None = None can_create_private_channel_group: int | AnonymousSettingGroupDict | None = None can_create_web_public_channel_group: int | AnonymousSettingGroupDict | None = None + can_create_write_only_bots_group: int | AnonymousSettingGroupDict | None = None can_delete_any_message_group: int | AnonymousSettingGroupDict | None = None can_delete_own_message_group: int | AnonymousSettingGroupDict | None = None can_invite_users_group: int | AnonymousSettingGroupDict | None = None diff --git a/zerver/lib/users.py b/zerver/lib/users.py index 9c1cf25f83..b3ef6100ea 100644 --- a/zerver/lib/users.py +++ b/zerver/lib/users.py @@ -19,11 +19,7 @@ from zulip_bots.custom_exceptions import ConfigValidationError from zerver.lib.avatar import avatar_url, get_avatar_field, get_avatar_for_inaccessible_user from zerver.lib.cache import cache_with_key, get_cross_realm_dicts_key from zerver.lib.create_user import get_dummy_email_address_for_display_regex -from zerver.lib.exceptions import ( - JsonableError, - OrganizationAdministratorRequiredError, - OrganizationOwnerRequiredError, -) +from zerver.lib.exceptions import JsonableError, OrganizationOwnerRequiredError from zerver.lib.string_validation import check_string_is_printable from zerver.lib.timestamp import timestamp_to_datetime from zerver.lib.timezone import canonicalize_timezone @@ -41,7 +37,7 @@ from zerver.models import ( UserProfile, ) from zerver.models.groups import SystemGroups -from zerver.models.realms import BotCreationPolicyEnum, get_fake_email_domain, require_unique_names +from zerver.models.realms import get_fake_email_domain, require_unique_names from zerver.models.users import ( active_non_guest_user_ids, active_user_ids, @@ -188,20 +184,22 @@ def add_service( ) -def check_bot_creation_policy(user_profile: UserProfile, bot_type: int) -> None: - # Realm administrators can always add bot - if user_profile.is_realm_admin: +def check_can_create_bot(user_profile: UserProfile, bot_type: int) -> None: + if user_has_permission_for_group_setting( + user_profile.realm.can_create_bots_group, + user_profile, + Realm.REALM_PERMISSION_GROUP_SETTINGS["can_create_bots_group"], + ): return - if user_profile.realm.bot_creation_policy == BotCreationPolicyEnum.EVERYONE: - return - if user_profile.realm.bot_creation_policy == BotCreationPolicyEnum.ADMINS_ONLY: - raise OrganizationAdministratorRequiredError - if ( - user_profile.realm.bot_creation_policy == BotCreationPolicyEnum.LIMIT_GENERIC_BOTS - and bot_type == UserProfile.DEFAULT_BOT + if bot_type == UserProfile.INCOMING_WEBHOOK_BOT and user_has_permission_for_group_setting( + user_profile.realm.can_create_write_only_bots_group, + user_profile, + Realm.REALM_PERMISSION_GROUP_SETTINGS["can_create_write_only_bots_group"], ): - raise OrganizationAdministratorRequiredError + return + + raise JsonableError(_("Insufficient permission")) def check_valid_bot_type(user_profile: UserProfile, bot_type: int) -> None: diff --git a/zerver/migrations/0656_realm_can_create_bots_group_and_more.py b/zerver/migrations/0656_realm_can_create_bots_group_and_more.py new file mode 100644 index 0000000000..59d99c7b8e --- /dev/null +++ b/zerver/migrations/0656_realm_can_create_bots_group_and_more.py @@ -0,0 +1,33 @@ +# Generated by Django 5.0.10 on 2025-01-23 15:14 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0655_alter_stream_can_add_subscribers_group"), + ] + + operations = [ + migrations.AddField( + model_name="realm", + name="can_create_bots_group", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.RESTRICT, + related_name="+", + to="zerver.usergroup", + ), + ), + migrations.AddField( + model_name="realm", + name="can_create_write_only_bots_group", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.RESTRICT, + related_name="+", + to="zerver.usergroup", + ), + ), + ] diff --git a/zerver/migrations/0657_set_default_value_for_can_create_bots_group_and_more.py b/zerver/migrations/0657_set_default_value_for_can_create_bots_group_and_more.py new file mode 100644 index 0000000000..8f288ac449 --- /dev/null +++ b/zerver/migrations/0657_set_default_value_for_can_create_bots_group_and_more.py @@ -0,0 +1,65 @@ +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps +from django.db.models import OuterRef + + +def set_default_value_for_can_create_bots_group( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + Realm = apps.get_model("zerver", "Realm") + NamedUserGroup = apps.get_model("zerver", "NamedUserGroup") + + bot_creation_policy_to_group_name = { + 1: "role:members", + 2: "role:administrators", + 3: "role:administrators", + } + + for id, group_name in bot_creation_policy_to_group_name.items(): + Realm.objects.filter(can_create_bots_group=None, bot_creation_policy=id).update( + can_create_bots_group=NamedUserGroup.objects.filter( + name=group_name, realm=OuterRef("id"), is_system_group=True + ).values("pk") + ) + + +def set_default_value_for_can_create_write_only_bots_group( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + Realm = apps.get_model("zerver", "Realm") + NamedUserGroup = apps.get_model("zerver", "NamedUserGroup") + + bot_creation_policy_to_group_name = { + 1: "role:members", + 2: "role:members", + 3: "role:administrators", + } + + for id, group_name in bot_creation_policy_to_group_name.items(): + Realm.objects.filter(can_create_write_only_bots_group=None, bot_creation_policy=id).update( + can_create_write_only_bots_group=NamedUserGroup.objects.filter( + name=group_name, realm=OuterRef("id"), is_system_group=True + ).values("pk") + ) + + +class Migration(migrations.Migration): + atomic = False + + dependencies = [ + ("zerver", "0656_realm_can_create_bots_group_and_more"), + ] + + operations = [ + migrations.RunPython( + set_default_value_for_can_create_bots_group, + elidable=True, + reverse_code=migrations.RunPython.noop, + ), + migrations.RunPython( + set_default_value_for_can_create_write_only_bots_group, + elidable=True, + reverse_code=migrations.RunPython.noop, + ), + ] diff --git a/zerver/migrations/0658_alter_realm_can_create_bots_group_and_more.py b/zerver/migrations/0658_alter_realm_can_create_bots_group_and_more.py new file mode 100644 index 0000000000..db837f9272 --- /dev/null +++ b/zerver/migrations/0658_alter_realm_can_create_bots_group_and_more.py @@ -0,0 +1,31 @@ +# Generated by Django 5.0.10 on 2025-01-23 15:23 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0657_set_default_value_for_can_create_bots_group_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="realm", + name="can_create_bots_group", + field=models.ForeignKey( + on_delete=django.db.models.deletion.RESTRICT, + related_name="+", + to="zerver.usergroup", + ), + ), + migrations.AlterField( + model_name="realm", + name="can_create_write_only_bots_group", + field=models.ForeignKey( + on_delete=django.db.models.deletion.RESTRICT, + related_name="+", + to="zerver.usergroup", + ), + ), + ] diff --git a/zerver/models/realms.py b/zerver/models/realms.py index 8b2e61cac3..06673fe5c5 100644 --- a/zerver/models/realms.py +++ b/zerver/models/realms.py @@ -340,6 +340,16 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub "UserGroup", on_delete=models.RESTRICT, related_name="+" ) + # UserGroup which is allowed to create bots. + can_create_bots_group = models.ForeignKey( + "UserGroup", on_delete=models.RESTRICT, related_name="+" + ) + + # UserGroup which is allowed to create incoming webhooks. + can_create_write_only_bots_group = models.ForeignKey( + "UserGroup", on_delete=models.RESTRICT, related_name="+" + ) + # Global policy for who is allowed to use wildcard mentions in # streams with a large number of subscribers. Anyone can use # wildcard mentions in small streams regardless of this setting. @@ -697,6 +707,13 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub allow_everyone_group=False, default_group_name=SystemGroups.MEMBERS, ), + can_create_bots_group=GroupPermissionSetting( + require_system_group=False, + allow_internet_group=False, + allow_nobody_group=True, + allow_everyone_group=False, + default_group_name=SystemGroups.MEMBERS, + ), can_create_groups=GroupPermissionSetting( require_system_group=False, allow_internet_group=False, @@ -731,6 +748,13 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub SystemGroups.NOBODY, ], ), + can_create_write_only_bots_group=GroupPermissionSetting( + require_system_group=False, + allow_internet_group=False, + allow_nobody_group=True, + allow_everyone_group=False, + default_group_name=SystemGroups.MEMBERS, + ), can_delete_any_message_group=GroupPermissionSetting( require_system_group=False, allow_internet_group=False, @@ -1180,6 +1204,8 @@ def get_realm_with_settings(realm_id: int) -> Realm: "can_add_custom_emoji_group__named_user_group", "can_add_subscribers_group", "can_add_subscribers_group__named_user_group", + "can_create_bots_group", + "can_create_bots_group__named_user_group", "can_create_groups", "can_create_groups__named_user_group", "can_create_public_channel_group", @@ -1188,6 +1214,8 @@ def get_realm_with_settings(realm_id: int) -> Realm: "can_create_private_channel_group__named_user_group", "can_create_web_public_channel_group", "can_create_web_public_channel_group__named_user_group", + "can_create_write_only_bots_group", + "can_create_write_only_bots_group__named_user_group", "can_delete_any_message_group", "can_delete_any_message_group__named_user_group", "can_delete_own_message_group", diff --git a/zerver/models/users.py b/zerver/models/users.py index 83bc572e5d..f433b2f701 100644 --- a/zerver/models/users.py +++ b/zerver/models/users.py @@ -792,20 +792,19 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): @property def allowed_bot_types(self) -> list[int]: - from zerver.models.realms import BotCreationPolicyEnum - allowed_bot_types = [] - if ( - self.is_realm_admin - or self.realm.bot_creation_policy != BotCreationPolicyEnum.LIMIT_GENERIC_BOTS - ): - allowed_bot_types.append(UserProfile.DEFAULT_BOT) - allowed_bot_types += [ - UserProfile.INCOMING_WEBHOOK_BOT, - UserProfile.OUTGOING_WEBHOOK_BOT, - ] - if settings.EMBEDDED_BOTS_ENABLED: - allowed_bot_types.append(UserProfile.EMBEDDED_BOT) + if self.has_permission("can_create_bots_group"): + allowed_bot_types.extend( + [ + UserProfile.DEFAULT_BOT, + UserProfile.INCOMING_WEBHOOK_BOT, + UserProfile.OUTGOING_WEBHOOK_BOT, + ] + ) + if settings.EMBEDDED_BOTS_ENABLED: + allowed_bot_types.append(UserProfile.EMBEDDED_BOT) + elif self.has_permission("can_create_write_only_bots_group"): + allowed_bot_types.append(UserProfile.INCOMING_WEBHOOK_BOT) return allowed_bot_types def email_address_is_realm_public(self) -> bool: diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index cf30d9e3d7..b5b3196da8 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -4480,6 +4480,29 @@ paths: **Changes**: New in Zulip 10.0 (feature level 299). Previously `user_group_edit_policy` field used to control the permission to create user groups. + can_create_bots_group: + allOf: + - $ref: "#/components/schemas/GroupSettingValue" + - description: | + A [group-setting value](/api/group-setting-values) defining + the set of users who have permission to create all types of bot users + in the organization. See also `can_create_write_only_bots_group`. + + **Changes**: New in Zulip 10.0 (feature level 344). Previously, this + permission was controlled by the enum `bot_creation_policy`. Values + were 1=Members, 2=Generic bots limited to administrators, 3=Administrators. + can_create_write_only_bots_group: + allOf: + - $ref: "#/components/schemas/GroupSettingValue" + - description: | + A [group-setting value](/api/group-setting-values) defining + the set of users who have permission to create bot users that + can only send messages in the organization, i.e. incoming webhooks, + in addition to the users who are present in `can_create_bots_group`. + + **Changes**: New in Zulip 10.0 (feature level 344). Previously, this + permission was controlled by the enum `bot_creation_policy`. Values + were 1=Members, 2=Generic bots limited to administrators, 3=Administrators. can_create_public_channel_group: allOf: - $ref: "#/components/schemas/GroupSettingValue" @@ -16730,6 +16753,29 @@ paths: **Changes**: New in Zulip 10.0 (feature level 299). Previously `realm_user_group_edit_policy` field used to control the permission to create user groups. + realm_can_create_bots_group: + allOf: + - $ref: "#/components/schemas/GroupSettingValue" + - description: | + A [group-setting value](/api/group-setting-values) defining + the set of users who have permission to create all types of bot users + in the organization. See also `can_create_write_only_bots_group`. + + **Changes**: New in Zulip 10.0 (feature level 344). Previously, this + permission was controlled by the enum `bot_creation_policy`. Values + were 1=Members, 2=Generic bots limited to administrators, 3=Administrators. + realm_can_create_write_only_bots_group: + allOf: + - $ref: "#/components/schemas/GroupSettingValue" + - description: | + A [group-setting value](/api/group-setting-values) defining + the set of users who have permission to create bot users that + can only send messages in the organization, i.e. incoming webhooks, + in addition to the users who are present in `can_create_bots_group`. + + **Changes**: New in Zulip 10.0 (feature level 344). Previously, this + permission was controlled by the enum `bot_creation_policy`. Values + were 1=Members, 2=Generic bots limited to administrators, 3=Administrators. realm_can_manage_all_groups: allOf: - $ref: "#/components/schemas/GroupSettingValue" diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index 1e1a461537..0bb8337d6b 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -1,6 +1,6 @@ import filecmp import os -from typing import Any +from typing import Any, Optional from unittest.mock import MagicMock, patch import orjson @@ -9,8 +9,12 @@ from django.test import override_settings from zulip_bots.custom_exceptions import ConfigValidationError from zerver.actions.bots import do_change_bot_owner, do_change_default_sending_stream -from zerver.actions.realm_settings import do_set_realm_user_default_setting +from zerver.actions.realm_settings import ( + do_change_realm_permission_group_setting, + do_set_realm_user_default_setting, +) from zerver.actions.streams import do_change_stream_permission +from zerver.actions.user_groups import check_add_user_group 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 @@ -21,7 +25,8 @@ from zerver.lib.utils import assert_is_not_none from zerver.lib.webhooks.common import WebhookConfigOption from zerver.models import RealmUserDefault, Service, Subscription, UserProfile from zerver.models.bots import get_bot_services -from zerver.models.realms import BotCreationPolicyEnum, get_realm +from zerver.models.groups import NamedUserGroup, SystemGroups +from zerver.models.realms import get_realm from zerver.models.streams import get_stream from zerver.models.users import bot_owner_user_ids, get_user, is_cross_realm_bot_email @@ -808,101 +813,241 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): self.assert_num_bots_equal(0) self.assert_json_error(result, "Invalid bot type") - def test_no_generic_bots_allowed_for_non_admins(self) -> None: + def check_user_can_create_bot( + self, user: str, bot_name: str, bot_type: int, error_msg: Optional["str"] = None + ) -> None: bot_info = { - "full_name": "The Bot of Hamlet", - "short_name": "hambot", - "bot_type": 1, + "full_name": bot_name, + "short_name": bot_name, + "bot_type": bot_type, } - bot_email = "hambot-bot@zulip.testserver" + bot_email = f"{bot_name}-bot@zulip.testserver" bot_realm = get_realm("zulip") - bot_realm.bot_creation_policy = BotCreationPolicyEnum.LIMIT_GENERIC_BOTS - bot_realm.save(update_fields=["bot_creation_policy"]) - - # A regular user cannot create a generic bot - self.login("hamlet") - self.assert_num_bots_equal(0) + self.login(user) result = self.client_post("/json/bots", bot_info) - self.assert_num_bots_equal(0) - self.assert_json_error(result, "Must be an organization administrator") + if error_msg is not None: + self.assert_json_error(result, error_msg) + else: + self.assert_json_success(result) + profile = get_user(bot_email, bot_realm) + self.assertEqual(profile.bot_type, bot_type) + self.assertEqual(profile.bot_owner, self.example_user(user)) - # But can create an incoming webhook - self.assert_num_bots_equal(0) - self.create_bot(bot_type=UserProfile.INCOMING_WEBHOOK_BOT) - self.assert_num_bots_equal(1) - profile = get_user(bot_email, bot_realm) - self.assertEqual(profile.bot_type, UserProfile.INCOMING_WEBHOOK_BOT) + def test_can_create_write_only_bots_group(self) -> None: + """ + The `can_create_write_only_bots_group` realm setting works properly. + """ + realm = get_realm("zulip") + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") - def test_no_generic_bot_reactivation_allowed_for_non_admins(self) -> None: - self.login("hamlet") - self.create_bot(bot_type=UserProfile.DEFAULT_BOT) + administrators_system_group = NamedUserGroup.objects.get( + name=SystemGroups.ADMINISTRATORS, realm=realm, is_system_group=True + ) + moderators_system_group = NamedUserGroup.objects.get( + name=SystemGroups.MODERATORS, realm=realm, is_system_group=True + ) + members_system_group = NamedUserGroup.objects.get( + name=SystemGroups.MEMBERS, realm=realm, is_system_group=True + ) - bot_realm = get_realm("zulip") - bot_realm.bot_creation_policy = BotCreationPolicyEnum.LIMIT_GENERIC_BOTS - bot_realm.save(update_fields=["bot_creation_policy"]) + do_change_realm_permission_group_setting( + realm, + "can_create_bots_group", + administrators_system_group, + acting_user=None, + ) - bot_email = "hambot-bot@zulip.testserver" - bot_user = get_user(bot_email, bot_realm) - do_deactivate_user(bot_user, acting_user=None) + do_change_realm_permission_group_setting( + realm, + "can_create_write_only_bots_group", + members_system_group, + acting_user=None, + ) - # A regular user cannot reactivate a generic bot - self.assert_num_bots_equal(0) - result = self.client_post(f"/json/users/{bot_user.id}/reactivate") - self.assert_json_error(result, "Must be an organization administrator") - self.assert_num_bots_equal(0) + self.check_user_can_create_bot("hamlet", "testbot-1", UserProfile.INCOMING_WEBHOOK_BOT) + self.check_user_can_create_bot( + "polonius", "testbot-2", UserProfile.INCOMING_WEBHOOK_BOT, "Not allowed for guest users" + ) - def test_no_generic_bots_allowed_for_admins(self) -> None: - bot_email = "hambot-bot@zulip.testserver" - bot_realm = get_realm("zulip") - bot_realm.bot_creation_policy = BotCreationPolicyEnum.LIMIT_GENERIC_BOTS - bot_realm.save(update_fields=["bot_creation_policy"]) + do_change_realm_permission_group_setting( + realm, + "can_create_write_only_bots_group", + moderators_system_group, + acting_user=None, + ) + self.check_user_can_create_bot("shiva", "testbot-3", UserProfile.INCOMING_WEBHOOK_BOT) + self.check_user_can_create_bot("iago", "testbot-4", UserProfile.INCOMING_WEBHOOK_BOT) + self.check_user_can_create_bot( + "hamlet", "testbot-5", UserProfile.INCOMING_WEBHOOK_BOT, "Insufficient permission" + ) - # An administrator can create any type of bot - self.login("iago") - self.assert_num_bots_equal(0) - self.create_bot(bot_type=UserProfile.DEFAULT_BOT) - self.assert_num_bots_equal(1) - profile = get_user(bot_email, bot_realm) - self.assertEqual(profile.bot_type, UserProfile.DEFAULT_BOT) + do_change_realm_permission_group_setting( + realm, + "can_create_write_only_bots_group", + administrators_system_group, + acting_user=None, + ) + self.check_user_can_create_bot("iago", "testbot-5", UserProfile.INCOMING_WEBHOOK_BOT) + self.check_user_can_create_bot( + "shiva", "testbot-6", UserProfile.INCOMING_WEBHOOK_BOT, "Insufficient permission" + ) - def test_no_bots_allowed_for_non_admins(self) -> None: - bot_info = { - "full_name": "The Bot of Hamlet", - "short_name": "hambot", - "bot_type": 1, - } - bot_realm = get_realm("zulip") - bot_realm.bot_creation_policy = BotCreationPolicyEnum.ADMINS_ONLY - bot_realm.save(update_fields=["bot_creation_policy"]) + # Test for checking setting for non-system user group. + user_group = check_add_user_group( + realm, "new_group", [hamlet, cordelia], acting_user=hamlet + ) + do_change_realm_permission_group_setting( + realm, "can_create_write_only_bots_group", user_group, acting_user=None + ) - # A regular user cannot create a generic bot - self.login("hamlet") - self.assert_num_bots_equal(0) - result = self.client_post("/json/bots", bot_info) - self.assert_num_bots_equal(0) - self.assert_json_error(result, "Must be an organization administrator") + # Hamlet and Cordelia are in the allowed user group, so can create bots. + self.check_user_can_create_bot("hamlet", "testbot-6", UserProfile.INCOMING_WEBHOOK_BOT) + self.check_user_can_create_bot("cordelia", "testbot-7", UserProfile.INCOMING_WEBHOOK_BOT) - # Also, a regular user cannot create a incoming bot - bot_info["bot_type"] = 2 - self.login("hamlet") - self.assert_num_bots_equal(0) - result = self.client_post("/json/bots", bot_info) - self.assert_num_bots_equal(0) - self.assert_json_error(result, "Must be an organization administrator") + # Shiva is not in the allowed user group, so cannot create bots. + self.check_user_can_create_bot( + "shiva", "testbot-8", UserProfile.INCOMING_WEBHOOK_BOT, "Insufficient permission" + ) + # Iago is not in `can_create_write_only_bots_group` but is in `can_create_bots_group`, + # so can create any bot. + self.check_user_can_create_bot("iago", "testbot-8", UserProfile.INCOMING_WEBHOOK_BOT) - def test_no_bots_allowed_for_admins(self) -> None: - bot_email = "hambot-bot@zulip.testserver" - bot_realm = get_realm("zulip") - bot_realm.bot_creation_policy = BotCreationPolicyEnum.ADMINS_ONLY - bot_realm.save(update_fields=["bot_creation_policy"]) + # Test for checking the setting for anonymous user group. + anonymous_user_group = self.create_or_update_anonymous_group_for_setting( + [hamlet], + [administrators_system_group], + ) + do_change_realm_permission_group_setting( + realm, + "can_create_write_only_bots_group", + anonymous_user_group, + acting_user=None, + ) - # An administrator can create any type of bot - self.login("iago") - self.assert_num_bots_equal(0) - self.create_bot(bot_type=UserProfile.DEFAULT_BOT) - self.assert_num_bots_equal(1) - profile = get_user(bot_email, bot_realm) - self.assertEqual(profile.bot_type, UserProfile.DEFAULT_BOT) + # Hamlet is the direct member of the anonymous user group, so can create bots. + self.check_user_can_create_bot("hamlet", "testbot-9", UserProfile.INCOMING_WEBHOOK_BOT) + # Iago is in the `administrators_system_group` subgroup, so can create bots. + self.check_user_can_create_bot("iago", "testbot-10", UserProfile.INCOMING_WEBHOOK_BOT) + # Shiva is not in the anonymous user group, so cannot create bots. + self.check_user_can_create_bot( + "shiva", "testbot-11", UserProfile.INCOMING_WEBHOOK_BOT, "Insufficient permission" + ) + + do_change_realm_permission_group_setting( + realm, + "can_create_write_only_bots_group", + moderators_system_group, + acting_user=None, + ) + + # Iago is in `can_create_bots_group`, so can create any bot. + self.check_user_can_create_bot("iago", "testbot-11", UserProfile.DEFAULT_BOT) + self.check_user_can_create_bot("iago", "testbot-12", UserProfile.INCOMING_WEBHOOK_BOT) + # Shiva is only in `can_create_write_only_bots_group`, so can only create + # "INCOMING_WEBHOOK_BOT". + self.check_user_can_create_bot( + "shiva", "testbot-13", UserProfile.DEFAULT_BOT, "Insufficient permission" + ) + self.check_user_can_create_bot("shiva", "testbot-13", UserProfile.INCOMING_WEBHOOK_BOT) + # Hamlet is in neither of the group, so cannot create any bot. + self.check_user_can_create_bot( + "hamlet", "testbot-14", UserProfile.DEFAULT_BOT, "Insufficient permission" + ) + self.check_user_can_create_bot( + "hamlet", "testbot-14", UserProfile.INCOMING_WEBHOOK_BOT, "Insufficient permission" + ) + + def test_can_create_bots_group(self) -> None: + """ + The `can_create_bots_group` realm setting works properly. + """ + realm = get_realm("zulip") + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + + administrators_system_group = NamedUserGroup.objects.get( + name=SystemGroups.ADMINISTRATORS, realm=realm, is_system_group=True + ) + moderators_system_group = NamedUserGroup.objects.get( + name=SystemGroups.MODERATORS, realm=realm, is_system_group=True + ) + members_system_group = NamedUserGroup.objects.get( + name=SystemGroups.MEMBERS, realm=realm, is_system_group=True + ) + + do_change_realm_permission_group_setting( + realm, + "can_create_bots_group", + members_system_group, + acting_user=None, + ) + + self.check_user_can_create_bot("hamlet", "testbot-1", UserProfile.DEFAULT_BOT) + self.check_user_can_create_bot( + "polonius", "testbot-2", UserProfile.DEFAULT_BOT, "Not allowed for guest users" + ) + + do_change_realm_permission_group_setting( + realm, + "can_create_bots_group", + moderators_system_group, + acting_user=None, + ) + self.check_user_can_create_bot("shiva", "testbot-3", UserProfile.DEFAULT_BOT) + self.check_user_can_create_bot("iago", "testbot-4", UserProfile.DEFAULT_BOT) + self.check_user_can_create_bot( + "hamlet", "testbot-5", UserProfile.DEFAULT_BOT, "Insufficient permission" + ) + + do_change_realm_permission_group_setting( + realm, + "can_create_bots_group", + administrators_system_group, + acting_user=None, + ) + self.check_user_can_create_bot("iago", "testbot-5", UserProfile.DEFAULT_BOT) + self.check_user_can_create_bot( + "shiva", "testbot-6", UserProfile.DEFAULT_BOT, "Insufficient permission" + ) + + # Test for checking setting for non-system user group. + user_group = check_add_user_group( + realm, "new_group", [hamlet, cordelia], acting_user=hamlet + ) + do_change_realm_permission_group_setting( + realm, "can_create_bots_group", user_group, acting_user=None + ) + + # Hamlet and Cordelia are in the allowed user group, so can create bots. + self.check_user_can_create_bot("hamlet", "testbot-6", UserProfile.DEFAULT_BOT) + self.check_user_can_create_bot("cordelia", "testbot-7", UserProfile.DEFAULT_BOT) + + # Iago is not in the allowed user group, so cannot create bots. + self.check_user_can_create_bot( + "iago", "testbot-8", UserProfile.DEFAULT_BOT, "Insufficient permission" + ) + + # Test for checking the setting for anonymous user group. + anonymous_user_group = self.create_or_update_anonymous_group_for_setting( + [hamlet], + [administrators_system_group], + ) + do_change_realm_permission_group_setting( + realm, + "can_create_bots_group", + anonymous_user_group, + acting_user=None, + ) + + # Hamlet is the direct member of the anonymous user group, so can create bots. + self.check_user_can_create_bot("hamlet", "testbot-8", UserProfile.DEFAULT_BOT) + # Iago is in the `administrators_system_group` subgroup, so can create bots. + self.check_user_can_create_bot("iago", "testbot-9", UserProfile.DEFAULT_BOT) + # Shiva is not in the anonymous user group, so cannot create bots. + self.check_user_can_create_bot( + "shiva", "testbot-10", UserProfile.DEFAULT_BOT, "Insufficient permission" + ) def test_reactivating_bot_with_deactivated_owner(self) -> None: self.login("hamlet") diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 6a0b924067..12ec5e89b8 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -131,10 +131,12 @@ class HomeTest(ZulipTestCase): "realm_can_access_all_users_group", "realm_can_add_custom_emoji_group", "realm_can_add_subscribers_group", + "realm_can_create_bots_group", "realm_can_create_groups", "realm_can_create_private_channel_group", "realm_can_create_public_channel_group", "realm_can_create_web_public_channel_group", + "realm_can_create_write_only_bots_group", "realm_can_delete_any_message_group", "realm_can_delete_own_message_group", "realm_can_invite_users_group", @@ -275,7 +277,7 @@ class HomeTest(ZulipTestCase): # Verify succeeds once logged-in with ( - self.assert_database_query_count(54), + self.assert_database_query_count(59), patch("zerver.lib.cache.cache_set") as cache_mock, ): result = self._get_home_page(stream="Denmark") @@ -580,7 +582,7 @@ class HomeTest(ZulipTestCase): # Verify number of queries for Realm admin isn't much higher than for normal users. self.login("iago") with ( - self.assert_database_query_count(54), + self.assert_database_query_count(59), patch("zerver.lib.cache.cache_set") as cache_mock, ): result = self._get_home_page() @@ -612,7 +614,7 @@ class HomeTest(ZulipTestCase): self._get_home_page() # Then for the second page load, measure the number of queries. - with self.assert_database_query_count(49): + with self.assert_database_query_count(54): result = self._get_home_page() # Do a sanity check that our new streams were in the payload. diff --git a/zerver/views/realm.py b/zerver/views/realm.py index c48052b826..0bdd1950b1 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -132,10 +132,12 @@ def update_realm( digest_emails_enabled: Json[bool] | None = None, message_content_allowed_in_email_notifications: Json[bool] | None = None, bot_creation_policy: Json[BotCreationPolicyEnum] | None = None, + can_create_bots_group: Json[GroupSettingChangeRequest] | None = None, can_create_groups: Json[GroupSettingChangeRequest] | None = None, can_create_public_channel_group: Json[GroupSettingChangeRequest] | None = None, can_create_private_channel_group: Json[GroupSettingChangeRequest] | None = None, can_create_web_public_channel_group: Json[GroupSettingChangeRequest] | None = None, + can_create_write_only_bots_group: Json[GroupSettingChangeRequest] | None = None, can_invite_users_group: Json[GroupSettingChangeRequest] | None = None, can_manage_all_groups: Json[GroupSettingChangeRequest] | None = None, can_move_messages_between_channels_group: Json[GroupSettingChangeRequest] | None = None, diff --git a/zerver/views/users.py b/zerver/views/users.py index b0082acbd4..c62460be31 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -71,9 +71,9 @@ from zerver.lib.users import ( access_user_by_email, access_user_by_id, add_service, - check_bot_creation_policy, check_bot_name_available, check_can_access_user, + check_can_create_bot, check_full_name, check_short_name, check_valid_bot_config, @@ -189,7 +189,7 @@ def reactivate_user_backend( ) if target.is_bot: assert target.bot_type is not None - check_bot_creation_policy(user_profile, target.bot_type) + check_can_create_bot(user_profile, target.bot_type) check_bot_name_available(user_profile.realm_id, target.full_name, is_activation=True) do_reactivate_user(target, acting_user=user_profile) return json_success(request) @@ -614,7 +614,7 @@ def add_bot_backend( is_activation=False, ) - check_bot_creation_policy(user_profile, bot_type) + check_can_create_bot(user_profile, bot_type) check_valid_bot_type(user_profile, bot_type) check_valid_interface_type(interface_type)