/json/bots: Replace email with user_id in API to update bots.

This commit is contained in:
Yashashvi Dave
2018-05-15 18:56:04 +05:30
committed by Tim Abbott
parent 3e04aa99fa
commit d6e2f9fc88
7 changed files with 104 additions and 51 deletions

View File

@@ -309,9 +309,10 @@ exports.set_up = function () {
});
$("#active_bots_list").on("click", "button.delete_bot", function (e) {
var email = $(e.currentTarget).data('email');
var bot_id = $(e.currentTarget).attr('data-user-id');
channel.del({
url: '/json/bots/' + encodeURIComponent(email),
url: '/json/bots/' + encodeURIComponent(bot_id),
success: function () {
var row = $(e.currentTarget).closest("li");
row.hide('slow', function () { row.remove(); });
@@ -387,7 +388,6 @@ exports.set_up = function () {
},
submitHandler: function () {
var bot_id = form.attr('data-user-id');
var email = form.attr('data-email');
var type = form.attr('data-type');
var full_name = form.find('.edit_bot_name').val();
@@ -419,7 +419,7 @@ exports.set_up = function () {
loading.make_indicator(spinner, {text: 'Editing bot'});
edit_button.hide();
channel.patch({
url: '/json/bots/' + encodeURIComponent(email),
url: '/json/bots/' + encodeURIComponent(bot_id),
data: formData,
cache: false,
processData: false,

View File

@@ -280,10 +280,10 @@ exports.on_load_success = function (realm_people_data) {
var row = $(e.target).closest(".user_row");
var email = get_email_for_user_row(row);
var bot_id = row.attr("data-user-id");
channel.del({
url: '/json/bots/' + encodeURIComponent(email),
url: '/json/bots/' + encodeURIComponent(bot_id),
error: function (xhr) {
ui_report.generic_row_button_error(xhr, $(e.target));
},
@@ -415,7 +415,7 @@ exports.on_load_success = function (realm_people_data) {
e.stopPropagation();
if (person.is_bot) {
url = "/json/bots/" + encodeURIComponent(person.email);
url = "/json/bots/" + encodeURIComponent(person.user_id);
data = {
full_name: full_name.val(),
};

View File

@@ -11,7 +11,7 @@
<a type="submit" download="{{zuliprc}}" class="btn download_bot_zuliprc" title="{{t 'Download zuliprc' }}" data-email="{{email}}">
<i class="icon-vector-download-alt sea-green"></i>
</a>
<button type="submit" class="btn delete_bot" title="{{t 'Delete bot' }}" data-email="{{email}}">
<button type="submit" class="btn delete_bot" title="{{t 'Delete bot' }}" data-user-id="{{user_id}}">
<i class="icon-vector-trash danger-red"></i>
</button>
</div>

View File

@@ -27,6 +27,11 @@ from zerver.lib.bot_lib import get_bot_handler
from zulip_bots.custom_exceptions import ConfigValidationError
class BotTest(ZulipTestCase, UploadSerializeMixin):
def get_bot_user(self, email: str) -> UserProfile:
realm = get_realm("zulip")
bot = get_user(email, realm)
return bot
def assert_num_bots_equal(self, count: int) -> None:
result = self.client_get("/json/bots")
self.assert_json_success(result)
@@ -54,7 +59,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assertEqual(realm.get_bot_domain(), 'zulip.testserver')
def deactivate_bot(self) -> None:
result = self.client_delete("/json/bots/hambot-bot@zulip.testserver")
email = 'hambot-bot@zulip.testserver'
result = self.client_delete("/json/bots/{}".format(self.get_bot_user(email).id))
self.assert_json_success(result)
def test_add_bot_with_bad_username(self) -> None:
@@ -125,8 +131,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(1)
email = 'hambot-bot@zulip.testserver'
realm = get_realm('zulip')
bot = get_user(email, realm)
bot = self.get_bot_user(email)
event = [e for e in events if e['event']['type'] == 'realm_bot'][0]
self.assertEqual(
@@ -155,7 +160,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assertEqual(len(bots), 1)
bot = bots[0]
self.assertEqual(bot['bot_owner'], self.example_email('hamlet'))
self.assertEqual(bot['user_id'], get_user(email, realm).id)
self.assertEqual(bot['user_id'], self.get_bot_user(email).id)
def test_add_bot_with_username_in_use(self) -> None:
self.login(self.example_email('hamlet'))
@@ -429,7 +434,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(0)
self.create_bot()
self.assert_num_bots_equal(1)
result = self.client_delete("/json/bots/bogus-bot@zulip.com")
invalid_user_id = 1000
result = self.client_delete("/json/bots/{}".format(invalid_user_id))
self.assert_json_error(result, 'No such bot')
self.assert_num_bots_equal(1)
@@ -466,6 +472,23 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bots = [bot for bot in all_bots]
self.assertEqual(len(bots), 0)
def test_cannot_deactivate_other_realm_bot(self) -> None:
realm = get_realm("zephyr")
self.login(self.mit_email("starnine"), realm=realm)
bot_info = {
'full_name': 'The Bot in zephyr',
'short_name': 'starn-bot',
'bot_type': '1',
}
result = self.client_post("/json/bots", bot_info, subdomain="zephyr")
self.assert_json_success(result)
result = self.client_get("/json/bots", subdomain="zephyr")
bot_email = result.json()['bots'][0]['username']
bot = get_user(bot_email, realm)
self.login(self.example_email("iago"))
result = self.client_delete("/json/bots/{}".format(bot.id))
self.assert_json_error(result, 'Insufficient permission')
def test_bot_deactivation_attacks(self) -> None:
"""You cannot deactivate somebody else's bot."""
self.login(self.example_email('hamlet'))
@@ -478,10 +501,11 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.login(self.example_email('othello'))
# Cannot deactivate a user as a bot
result = self.client_delete("/json/bots/" + self.example_email("hamlet"))
result = self.client_delete("/json/bots/{}".format(self.example_user("hamlet").id))
self.assert_json_error(result, 'No such bot')
result = self.client_delete("/json/bots/hambot-bot@zulip.testserver")
email = 'hambot-bot@zulip.testserver'
result = self.client_delete("/json/bots/{}".format(self.get_bot_user(email).id))
self.assert_json_error(result, 'Insufficient permission')
# But we don't actually deactivate the other person's bot.
@@ -489,7 +513,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(1)
# Cannot deactivate a bot as a user
result = self.client_delete("/json/users/hambot-bot@zulip.testserver")
result = self.client_delete("/json/users/{}".format(email))
self.assert_json_error(result, 'No such user')
self.assert_num_bots_equal(1)
@@ -501,6 +525,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
# Have Othello try to mess with Hamlet's bots.
self.login(self.example_email('othello'))
email = 'hambot-bot@zulip.testserver'
result = self.client_post("/json/bots/hambot-bot@zulip.testserver/api_key/regenerate")
self.assert_json_error(result, 'Insufficient permission')
@@ -508,7 +533,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'full_name': 'Fred',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, 'Insufficient permission')
def get_bot(self) -> Dict[str, Any]:
@@ -659,7 +684,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'full_name': 'Fred',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual('Fred', result.json()['full_name'])
@@ -672,7 +698,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'full_name': 'Fred',
}
result = self.client_patch("/json/bots/hamlet@zulip.com", bot_info)
result = self.client_patch("/json/bots/{}".format(self.example_user("hamlet").id), bot_info)
self.assert_json_error(result, "No such bot")
def test_patch_bot_owner(self) -> None:
@@ -686,7 +712,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'bot_owner': self.example_email('othello'),
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
# Test bot's owner has been changed successfully.
@@ -704,7 +731,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'bot_owner': '',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Failed to change owner, no such user")
profile = get_user('hambot-bot@zulip.testserver', get_realm('zulip'))
self.assertEqual(profile.bot_owner, self.example_user("hamlet"))
@@ -712,7 +740,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'bot_owner': 'Invalid name',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Failed to change owner, no such user")
profile = get_user('hambot-bot@zulip.testserver', get_realm('zulip'))
self.assertEqual(profile.bot_owner, self.example_user("hamlet"))
@@ -730,9 +758,10 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
'bot_owner': self.example_email('othello'),
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Failed to change owner, user is deactivated")
profile = get_user('hambot-bot@zulip.testserver', get_realm('zulip'))
profile = self.get_bot_user(email)
self.assertEqual(profile.bot_owner, self.example_user("hamlet"))
def test_patch_bot_owner_a_bot(self) -> None:
@@ -750,9 +779,10 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'bot_owner': 'hamelbot-bot@zulip.testserver',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Failed to change owner, bots can't own other bots")
profile = get_user('hambot-bot@zulip.testserver', get_realm('zulip'))
profile = get_user(email, get_realm('zulip'))
self.assertEqual(profile.bot_owner, self.example_user("hamlet"))
@override_settings(LOCAL_UPLOADS_DIR='var/bot_avatar')
@@ -770,11 +800,12 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
profile = get_user(bot_email, bot_realm)
self.assertEqual(profile.avatar_source, UserProfile.AVATAR_FROM_GRAVATAR)
email = 'hambot-bot@zulip.testserver'
# Try error case first (too many files):
with get_test_image_file('img.png') as fp1, \
get_test_image_file('img.gif') as fp2:
result = self.client_patch_multipart(
'/json/bots/hambot-bot@zulip.testserver',
'/json/bots/{}'.format(self.get_bot_user(email).id),
dict(file1=fp1, file2=fp2))
self.assert_json_error(result, 'You may only upload one file at a time')
@@ -784,7 +815,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
# HAPPY PATH
with get_test_image_file('img.png') as fp:
result = self.client_patch_multipart(
'/json/bots/hambot-bot@zulip.testserver',
'/json/bots/{}'.format(self.get_bot_user(email).id),
dict(file=fp))
profile = get_user(bot_email, bot_realm)
self.assertEqual(profile.avatar_version, 2)
@@ -808,7 +839,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_sending_stream': 'Denmark',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual('Denmark', result.json()['default_sending_stream'])
@@ -827,7 +859,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_sending_stream': 'Rome',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual('Rome', result.json()['default_sending_stream'])
@@ -846,7 +879,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_sending_stream': '',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
bot_email = "hambot-bot@zulip.testserver"
@@ -873,7 +907,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_sending_stream': 'Denmark',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual('Denmark', result.json()['default_sending_stream'])
@@ -898,7 +933,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_sending_stream': 'Denmark',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Invalid stream name 'Denmark'")
def test_patch_bot_to_stream_not_found(self) -> None:
@@ -912,7 +948,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_sending_stream': 'missing',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Invalid stream name 'missing'")
def test_patch_bot_events_register_stream(self) -> None:
@@ -926,7 +963,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_events_register_stream': 'Denmark',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual('Denmark', result.json()['default_events_register_stream'])
@@ -949,7 +987,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_events_register_stream': 'Denmark',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual('Denmark', result.json()['default_events_register_stream'])
@@ -973,7 +1012,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_events_register_stream': 'Denmark',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Invalid stream name 'Denmark'")
def test_patch_bot_events_register_stream_none(self) -> None:
@@ -987,7 +1027,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_events_register_stream': '',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
bot_email = "hambot-bot@zulip.testserver"
@@ -1009,7 +1050,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_events_register_stream': 'missing',
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_error(result, "Invalid stream name 'missing'")
def test_patch_bot_default_all_public_streams_true(self) -> None:
@@ -1023,7 +1065,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_all_public_streams': ujson.dumps(True),
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual(result.json()['default_all_public_streams'], True)
@@ -1042,7 +1085,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'default_all_public_streams': ujson.dumps(False),
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
self.assertEqual(result.json()['default_all_public_streams'], False)
@@ -1062,7 +1106,10 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
'full_name': 'Fred',
'method': 'PATCH'
}
result = self.client_post("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
# Important: We intentionally use the wrong method, post, here.
result = self.client_post("/json/bots/{}".format(self.get_bot_user(email).id),
bot_info)
self.assert_json_success(result)
self.assertEqual('Fred', result.json()['full_name'])
@@ -1077,7 +1124,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_info = {
'full_name': 'Fred',
}
result = self.client_patch("/json/bots/nonexistent-bot@zulip.com", bot_info)
invalid_user_id = 1000
result = self.client_patch("/json/bots/{}".format(invalid_user_id), bot_info)
self.assert_json_error(result, 'No such user')
self.assert_num_bots_equal(1)
@@ -1096,7 +1144,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
'service_payload_url': ujson.dumps("http://foo.bar2.com"),
'service_interface': Service.SLACK,
}
result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info)
email = 'hambot-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
service_interface = ujson.loads(result.content)['service_interface']
@@ -1113,7 +1162,8 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
service_name='giphy',
config_data=ujson.dumps({'key': '12345678'}))
bot_info = {'config_data': ujson.dumps({'key': '87654321'})}
result = self.client_patch("/json/bots/test-bot@zulip.testserver", bot_info)
email = 'test-bot@zulip.testserver'
result = self.client_patch("/json/bots/{}".format(self.get_bot_user(email).id), bot_info)
self.assert_json_success(result)
config_data = ujson.loads(result.content)['config_data']
self.assertEqual(config_data, ujson.loads(bot_info['config_data']))

View File

@@ -375,13 +375,16 @@ class ActivateTest(ZulipTestCase):
self.login(self.example_email("othello"))
# Cannot deactivate a user with the bot api
result = self.client_delete('/json/bots/hamlet@zulip.com')
result = self.client_delete('/json/bots/{}'.format(self.example_user("hamlet").id))
self.assert_json_error(result, 'No such bot')
# Cannot deactivate a nonexistent user.
result = self.client_delete('/json/users/nonexistent@zulip.com')
self.assert_json_error(result, 'No such user')
result = self.client_delete('/json/users/{}'.format(self.example_email("webhook_bot")))
self.assert_json_error(result, 'No such user')
result = self.client_delete('/json/users/iago@zulip.com')
self.assert_json_success(result)

View File

@@ -61,9 +61,9 @@ def check_last_admin(user_profile: UserProfile) -> bool:
return user_profile.is_realm_admin and len(admins) == 1
def deactivate_bot_backend(request: HttpRequest, user_profile: UserProfile,
email: str) -> HttpResponse:
bot_id: int) -> HttpResponse:
try:
target = get_user(email, user_profile.realm)
target = get_user_profile_by_id(bot_id)
except UserProfile.DoesNotExist:
return json_error(_('No such bot'))
if not target.is_bot:
@@ -158,7 +158,7 @@ def get_stream_name(stream: Optional[Stream]) -> Optional[str]:
@require_non_guest_human_user
@has_request_variables
def patch_bot_backend(
request: HttpRequest, user_profile: UserProfile, email: str,
request: HttpRequest, user_profile: UserProfile, bot_id: int,
full_name: Optional[str]=REQ(default=None),
bot_owner: Optional[str]=REQ(default=None),
config_data: Optional[Dict[str, str]]=REQ(default=None,
@@ -170,7 +170,7 @@ def patch_bot_backend(
default_all_public_streams: Optional[bool]=REQ(default=None, validator=check_bool)
) -> HttpResponse:
try:
bot = get_user(email, user_profile.realm)
bot = get_user_profile_by_id(bot_id)
except UserProfile.DoesNotExist:
return json_error(_('No such user'))

View File

@@ -138,7 +138,7 @@ v1_api_and_json_patterns = [
'POST': 'zerver.views.users.add_bot_backend'}),
url(r'^bots/(?!me/)(?P<email>[^/]*)/api_key/regenerate$', rest_dispatch,
{'POST': 'zerver.views.users.regenerate_bot_api_key'}),
url(r'^bots/(?!me/)(?P<email>[^/]*)$', rest_dispatch,
url(r'^bots/(?P<bot_id>[0-9]+)$', rest_dispatch,
{'PATCH': 'zerver.views.users.patch_bot_backend',
'DELETE': 'zerver.views.users.deactivate_bot_backend'}),