settings: Limit the creation of generic bots.

This commit adds a setting to limit creation of generic bots
to admins for realms that want that restriction.  (Generic
bots, apart from being considered spammy on some realms,
have less locked down permissions than webhook bots).

Fixes #7066.
This commit is contained in:
Alena Volkova
2017-11-24 10:24:24 -05:00
committed by Steve Howell
parent 6f8792729d
commit 45f0c76c44
13 changed files with 118 additions and 17 deletions

View File

@@ -182,6 +182,13 @@ var event_fixtures = {
value: false, value: false,
}, },
realm__update__create_generic_bot_by_admins_only: {
type: 'realm',
op: 'update',
property: 'create_generic_bot_by_admins_only',
value: false,
},
realm__update_dict__default: { realm__update_dict__default: {
type: 'realm', type: 'realm',
op: 'update_dict', op: 'update_dict',
@@ -545,6 +552,9 @@ with_overrides(function (override) {
event = event_fixtures.realm__update__restricted_to_domain; event = event_fixtures.realm__update__restricted_to_domain;
test_realm_boolean(event, 'realm_restricted_to_domain'); test_realm_boolean(event, 'realm_restricted_to_domain');
event = event_fixtures.realm__update__create_stream_by_admins_only;
test_realm_boolean(event, 'realm_create_stream_by_admins_only');
event = event_fixtures.realm__update_dict__default; event = event_fixtures.realm__update_dict__default;
page_params.realm_allow_message_editing = false; page_params.realm_allow_message_editing = false;
page_params.realm_message_content_edit_limit_seconds = 0; page_params.realm_message_content_edit_limit_seconds = 0;

View File

@@ -41,6 +41,8 @@ function _setup_page() {
realm_email_changes_disabled: page_params.realm_email_changes_disabled, realm_email_changes_disabled: page_params.realm_email_changes_disabled,
realm_add_emoji_by_admins_only: page_params.realm_add_emoji_by_admins_only, realm_add_emoji_by_admins_only: page_params.realm_add_emoji_by_admins_only,
can_admin_emojis: page_params.is_admin || !page_params.realm_add_emoji_by_admins_only, can_admin_emojis: page_params.is_admin || !page_params.realm_add_emoji_by_admins_only,
realm_create_generic_bot_by_admins_only:
page_params.realm_create_generic_bot_by_admins_only,
realm_allow_message_deleting: page_params.realm_allow_message_deleting, realm_allow_message_deleting: page_params.realm_allow_message_deleting,
realm_allow_message_editing: page_params.realm_allow_message_editing, realm_allow_message_editing: page_params.realm_allow_message_editing,
realm_message_content_edit_limit_minutes: realm_message_content_edit_limit_minutes:

View File

@@ -56,6 +56,7 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
allow_edit_history: noop, allow_edit_history: noop,
allow_message_deleting: noop, allow_message_deleting: noop,
allow_message_editing: noop, allow_message_editing: noop,
create_generic_bot_by_admins_only: noop,
create_stream_by_admins_only: noop, create_stream_by_admins_only: noop,
default_language: settings_org.reset_realm_default_language, default_language: settings_org.reset_realm_default_language,
description: settings_org.update_realm_description, description: settings_org.update_realm_description,

View File

@@ -316,6 +316,11 @@ function _set_up() {
type: 'integer', type: 'integer',
msg: i18n.t("Waiting period threshold changed!"), msg: i18n.t("Waiting period threshold changed!"),
}, },
create_generic_bot_by_admins_only: {
type: 'bool',
checked_msg: i18n.t("Only administrators may now create new generic bots!"),
unchecked_msg: i18n.t("Any user may now create new generic bots!"),
},
}, },
}; };

View File

@@ -10,6 +10,7 @@
<div class="alert" id="admin-realm-add-emoji-by-admins-only-status"></div> <div class="alert" id="admin-realm-add-emoji-by-admins-only-status"></div>
<div class="alert" id="admin-realm-create-stream-by-admins-only-status"></div> <div class="alert" id="admin-realm-create-stream-by-admins-only-status"></div>
<div class="alert" id="admin-realm-waiting-period-threshold-status"></div> <div class="alert" id="admin-realm-waiting-period-threshold-status"></div>
<div class="alert" id="admin-realm-create-generic-bot-by-admins-only-status"></div>
<div class="side-padded-container m-10 inline-block organization-permissions-parent"> <div class="side-padded-container m-10 inline-block organization-permissions-parent">
<div class="input-group admin-restricted-to-domain"> <div class="input-group admin-restricted-to-domain">
@@ -115,6 +116,21 @@
</div> </div>
</div> </div>
<h3 class="light">{{t "Interactive bots" }}</h3>
<div class="side-padded-container m-10 inline-block organization-permissions-parent">
<div class="input-group">
<label class="checkbox">
<input type="checkbox" id="id_realm_create_generic_bot_by_admins_only" name="realm_create_generic_bot_by_admins_only"
{{#if realm_create_generic_bot_by_admins_only}}checked="checked"{{/if}} />
<span></span>
</label>
<label for="id_realm_create_generic_bot_by_admins_only" id="id_realm_create_generic_bot_by_admins_only_label" class="inline-block"
title="{{t 'If checked, only administrators may create new generic bots.' }}">
{{t "Prevent users from creating generic bots" }}
</label>
</div>
</div>
{{#if is_admin }} {{#if is_admin }}
<div class="input-group organization-submission"> <div class="input-group organization-submission">
<button type="submit" class="button rounded sea-green"> <button type="submit" class="button rounded sea-green">

View File

@@ -24,8 +24,8 @@ def check_short_name(short_name_raw: Text) -> Text:
raise JsonableError(_("Bad name or username")) raise JsonableError(_("Bad name or username"))
return short_name return short_name
def check_valid_bot_type(bot_type: int) -> None: def check_valid_bot_type(user_profile: UserProfile, bot_type: int) -> None:
if bot_type not in UserProfile.ALLOWED_BOT_TYPES: if bot_type not in user_profile.allowed_bot_types:
raise JsonableError(_('Invalid bot type')) raise JsonableError(_('Invalid bot type'))
def check_valid_interface_type(interface_type: int) -> None: def check_valid_interface_type(interface_type: int) -> None:

View File

@@ -0,0 +1,20 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.6 on 2017-11-30 20:05
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('zerver', '0130_text_choice_in_emojiset'),
]
operations = [
migrations.AddField(
model_name='realm',
name='create_generic_bot_by_admins_only',
field=models.BooleanField(default=False),
),
]

View File

@@ -144,6 +144,7 @@ class Realm(models.Model):
inline_url_embed_preview = models.BooleanField(default=True) # type: bool inline_url_embed_preview = models.BooleanField(default=True) # type: bool
create_stream_by_admins_only = models.BooleanField(default=False) # type: bool create_stream_by_admins_only = models.BooleanField(default=False) # type: bool
add_emoji_by_admins_only = models.BooleanField(default=False) # type: bool add_emoji_by_admins_only = models.BooleanField(default=False) # type: bool
create_generic_bot_by_admins_only = models.BooleanField(default=False) # type: bool
mandatory_topics = models.BooleanField(default=False) # type: bool mandatory_topics = models.BooleanField(default=False) # type: bool
show_digest_email = models.BooleanField(default=True) # type: bool show_digest_email = models.BooleanField(default=True) # type: bool
name_changes_disabled = models.BooleanField(default=False) # type: bool name_changes_disabled = models.BooleanField(default=False) # type: bool
@@ -178,6 +179,7 @@ class Realm(models.Model):
add_emoji_by_admins_only=bool, add_emoji_by_admins_only=bool,
allow_edit_history=bool, allow_edit_history=bool,
allow_message_deleting=bool, allow_message_deleting=bool,
create_generic_bot_by_admins_only=bool,
create_stream_by_admins_only=bool, create_stream_by_admins_only=bool,
default_language=Text, default_language=Text,
description=Text, description=Text,
@@ -496,16 +498,6 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
EMBEDDED_BOT: 'Embedded bot', EMBEDDED_BOT: 'Embedded bot',
} }
# For now, don't allow creating other bot types via the UI
ALLOWED_BOT_TYPES = [
DEFAULT_BOT,
INCOMING_WEBHOOK_BOT,
OUTGOING_WEBHOOK_BOT,
]
if settings.DEVELOPMENT:
# Embedded bots are not yet enabled in production.
ALLOWED_BOT_TYPES.append(EMBEDDED_BOT)
SERVICE_BOT_TYPES = [ SERVICE_BOT_TYPES = [
OUTGOING_WEBHOOK_BOT, OUTGOING_WEBHOOK_BOT,
EMBEDDED_BOT, EMBEDDED_BOT,
@@ -716,6 +708,20 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
def is_service_bot(self) -> bool: def is_service_bot(self) -> bool:
return self.is_bot and self.bot_type in UserProfile.SERVICE_BOT_TYPES return self.is_bot and self.bot_type in UserProfile.SERVICE_BOT_TYPES
@property
def allowed_bot_types(self):
# type: () -> List[int]
allowed_bot_types = []
if self.is_realm_admin or not self.realm.create_generic_bot_by_admins_only:
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)
return allowed_bot_types
@staticmethod @staticmethod
def emojiset_choices() -> Dict[Text, Text]: def emojiset_choices() -> Dict[Text, Text]:
return OrderedDict((emojiset[0], emojiset[1]) for emojiset in UserProfile.EMOJISET_CHOICES) return OrderedDict((emojiset[0], emojiset[1]) for emojiset in UserProfile.EMOJISET_CHOICES)

View File

@@ -526,6 +526,45 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(0) self.assert_num_bots_equal(0)
self.assert_json_error(result, 'Invalid bot type') self.assert_json_error(result, 'Invalid bot type')
def test_add_bot_with_bot_type_not_allowed(self) -> None:
bot_info = {
'full_name': 'The Bot of Hamlet',
'short_name': 'hambot',
'bot_type': 1,
}
bot_email = 'hambot-bot@zulip.testserver'
bot_realm = get_realm('zulip')
bot_realm.create_generic_bot_by_admins_only = True
bot_realm.save(update_fields=['create_generic_bot_by_admins_only'])
# A regular user cannot create a generic bot
self.login(self.example_email('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, 'Invalid bot type')
# 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_add_bot_with_bot_type_not_allowed_admin(self) -> None:
bot_email = 'hambot-bot@zulip.testserver'
bot_realm = get_realm('zulip')
bot_realm.create_generic_bot_by_admins_only = True
bot_realm.save(update_fields=['create_generic_bot_by_admins_only'])
# An administrator can create any type of bot
self.login(self.example_email('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)
def test_patch_bot_full_name(self) -> None: def test_patch_bot_full_name(self) -> None:
self.login(self.example_email('hamlet')) self.login(self.example_email('hamlet'))
bot_info = { bot_info = {

View File

@@ -109,6 +109,7 @@ class HomeTest(ZulipTestCase):
"realm_authentication_methods", "realm_authentication_methods",
"realm_bot_domain", "realm_bot_domain",
"realm_bots", "realm_bots",
"realm_create_generic_bot_by_admins_only",
"realm_create_stream_by_admins_only", "realm_create_stream_by_admins_only",
"realm_default_language", "realm_default_language",
"realm_default_stream_groups", "realm_default_stream_groups",

View File

@@ -64,14 +64,14 @@ def sent_time_in_epoch_seconds(user_message: Optional[UserMessage]) -> Optional[
# Return the epoch seconds in UTC. # Return the epoch seconds in UTC.
return calendar.timegm(user_message.message.pub_date.utctimetuple()) return calendar.timegm(user_message.message.pub_date.utctimetuple())
def get_bot_types(): def get_bot_types(user_profile):
# type: () -> List[Dict[Text, object]] # type: (UserProfile) -> List[Dict[Text, object]]
bot_types = [] bot_types = []
for type_id, name in UserProfile.BOT_TYPES.items(): for type_id, name in UserProfile.BOT_TYPES.items():
bot_types.append({ bot_types.append({
'type_id': type_id, 'type_id': type_id,
'name': name, 'name': name,
'allowed': type_id in UserProfile.ALLOWED_BOT_TYPES 'allowed': type_id in user_profile.allowed_bot_types
}) })
return bot_types return bot_types
@@ -200,7 +200,7 @@ def home_real(request: HttpRequest) -> HttpResponse:
prompt_for_invites = prompt_for_invites, prompt_for_invites = prompt_for_invites,
furthest_read_time = sent_time_in_epoch_seconds(latest_read), furthest_read_time = sent_time_in_epoch_seconds(latest_read),
has_mobile_devices = num_push_devices_for_user(user_profile) > 0, has_mobile_devices = num_push_devices_for_user(user_profile) > 0,
bot_types = get_bot_types(), bot_types = get_bot_types(user_profile),
) )
undesired_register_ret_fields = [ undesired_register_ret_fields = [

View File

@@ -34,6 +34,7 @@ def update_realm(
inline_url_embed_preview: Optional[bool]=REQ(validator=check_bool, default=None), inline_url_embed_preview: Optional[bool]=REQ(validator=check_bool, default=None),
create_stream_by_admins_only: Optional[bool]=REQ(validator=check_bool, default=None), create_stream_by_admins_only: Optional[bool]=REQ(validator=check_bool, default=None),
add_emoji_by_admins_only: Optional[bool]=REQ(validator=check_bool, default=None), add_emoji_by_admins_only: Optional[bool]=REQ(validator=check_bool, default=None),
create_generic_bot_by_admins_only: Optional[bool]=REQ(validator=check_bool, default=None),
allow_message_deleting: Optional[bool]=REQ(validator=check_bool, default=None), allow_message_deleting: Optional[bool]=REQ(validator=check_bool, default=None),
allow_message_editing: Optional[bool]=REQ(validator=check_bool, default=None), allow_message_editing: Optional[bool]=REQ(validator=check_bool, default=None),
mandatory_topics: Optional[bool]=REQ(validator=check_bool, default=None), mandatory_topics: Optional[bool]=REQ(validator=check_bool, default=None),

View File

@@ -273,7 +273,7 @@ def add_bot_backend(
return json_error(_("Username already in use")) return json_error(_("Username already in use"))
except UserProfile.DoesNotExist: except UserProfile.DoesNotExist:
pass pass
check_valid_bot_type(bot_type) check_valid_bot_type(user_profile, bot_type)
check_valid_interface_type(interface_type) check_valid_interface_type(interface_type)
if len(request.FILES) == 0: if len(request.FILES) == 0: