From 758d7b9146f142ce14db9cd7a0a5f531ab06de4c Mon Sep 17 00:00:00 2001
From: Tim Abbott <tabbott@zulipchat.com>
Date: Mon, 2 Apr 2018 18:55:51 -0700
Subject: [PATCH] bots: Clean up editing bots impacting non-bot users.

This fixes a bug where the endpoint for editing bot users would allow
an organization administrator to edit the full name of a bot user.

A combination of this an another recently fixed bug made it possible
for this process to set a `bot_owner` for a non-bot user; so we also
include a migration to fix that for any users that might have had our
model invariants corrupted in that way.
---
 .../migrations/0154_fix_invalid_bot_owner.py  | 24 +++++++++++++++++++
 zerver/tests/test_bots.py                     |  8 +++++++
 zerver/views/users.py                         |  2 ++
 3 files changed, 34 insertions(+)
 create mode 100644 zerver/migrations/0154_fix_invalid_bot_owner.py

diff --git a/zerver/migrations/0154_fix_invalid_bot_owner.py b/zerver/migrations/0154_fix_invalid_bot_owner.py
new file mode 100644
index 0000000000..a40132a65c
--- /dev/null
+++ b/zerver/migrations/0154_fix_invalid_bot_owner.py
@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.6 on 2018-04-03 01:52
+from __future__ import unicode_literals
+
+from django.db import migrations
+from django.db.backends.postgresql_psycopg2.schema import DatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+
+def migrate_fix_invalid_bot_owner_values(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
+    """Fixes UserProfile objects that incorrectly had a bot_owner set"""
+    UserProfile = apps.get_model('zerver', 'UserProfile')
+    UserProfile.objects.filter(is_bot=False).exclude(bot_owner=None).update(bot_owner=None)
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('zerver', '0153_remove_int_float_custom_fields'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            migrate_fix_invalid_bot_owner_values,
+            reverse_code=migrations.RunPython.noop),
+    ]
diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py
index cd56e69a57..01693658ef 100644
--- a/zerver/tests/test_bots.py
+++ b/zerver/tests/test_bots.py
@@ -667,6 +667,14 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
         bot = self.get_bot()
         self.assertEqual('Fred', bot['full_name'])
 
+    def test_patch_bot_full_name_non_bot(self) -> None:
+        self.login(self.example_email('iago'))
+        bot_info = {
+            'full_name': 'Fred',
+        }
+        result = self.client_patch("/json/bots/hamlet@zulip.com", bot_info)
+        self.assert_json_error(result, "No such bot")
+
     def test_patch_bot_owner(self) -> None:
         self.login(self.example_email('hamlet'))
         bot_info = {
diff --git a/zerver/views/users.py b/zerver/views/users.py
index 76c2ab1fe3..26015079bf 100644
--- a/zerver/views/users.py
+++ b/zerver/views/users.py
@@ -172,6 +172,8 @@ def patch_bot_backend(
     except UserProfile.DoesNotExist:
         return json_error(_('No such user'))
 
+    if not bot.is_bot:
+        return json_error(_('No such bot'))
     if not user_profile.can_admin_user(bot):
         return json_error(_('Insufficient permission'))