bots: Avoid multiple active bots with the same name.

Creating a bot with a name that is already in use
will raise an error. However, by deactivating
the existing bot, creating a new bot with the
same name, and then reactivating the original bot,
it is possible to have multiple bots with the same name.

To fix this, we check if the bot name is already
in use in the active bots list. If it is,
an error will be raised, prompting either the
name of the existing bot to be changed or
the bot to be deactivated.

Co-authored-by: Sujal Shah <sujalshah28092004@gmail.com>

zerver/actions/user_settings.py

@@ -272,6 +272,7 @@ def check_change_bot_full_name(
     check_bot_name_available(
         realm_id=user_profile.realm_id,
         full_name=new_full_name,
+        is_activation=False,
     )
     do_change_full_name(user_profile, new_full_name, acting_user)
 

zerver/lib/users.py

@@ -94,7 +94,7 @@ def check_full_name(
 # name (e.g. you can get there by reactivating a deactivated bot after
 # making a new bot with the same name).  This is just a check designed
 # to make it unlikely to happen by accident.
-def check_bot_name_available(realm_id: int, full_name: str) -> None:
+def check_bot_name_available(realm_id: int, full_name: str, *, is_activation: bool) -> None:
     dup_exists = UserProfile.objects.filter(
         realm_id=realm_id,
         full_name=full_name.strip(),
@@ -102,6 +102,11 @@ def check_bot_name_available(realm_id: int, full_name: str) -> None:
     ).exists()
 
     if dup_exists:
+        if is_activation:
+            raise JsonableError(
+                f'There is already an active bot named "{full_name}" in this organization. To reactivate this bot, you must rename or deactivate the other one first.'
+            )
+        else:
             raise JsonableError(_("Name is already in use!"))
 
 

zerver/tests/test_bots.py

@@ -669,6 +669,36 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
         self.assert_json_error(result, "No such user")
         self.assert_num_bots_equal(1)
 
+    def test_activate_bot_with_duplicate_name(self) -> None:
+        self.login("iago")
+        # Create a bot and then deactivate it
+        original_name = "Hamlet"
+        bot_info = {
+            "full_name": original_name,
+            "short_name": "hambot",
+        }
+        result = self.client_post("/json/bots", bot_info)
+        self.assert_json_success(result)
+        bot_email = "hambot-bot@zulip.testserver"
+        bot = self.get_bot_user(bot_email)
+        do_deactivate_user(bot, False, acting_user=None)
+        self.assertFalse(
+            UserProfile.objects.filter(is_bot=True, id=bot.id, is_active=True).exists()
+        )
+
+        # Create another bot with the same name
+        bot_info2 = {
+            "full_name": original_name,
+            "short_name": "hambot2",
+        }
+        result = self.client_post("/json/bots", bot_info2)
+        self.assert_json_success(result)
+        result = self.client_post(f"/json/users/{bot.id}/reactivate")
+        self.assert_json_error(
+            result,
+            'There is already an active bot named "Hamlet" in this organization. To reactivate this bot, you must rename or deactivate the other one first.',
+        )
+
     def test_bot_permissions(self) -> None:
         self.login("hamlet")
         self.assert_num_bots_equal(0)

zerver/views/users.py

@@ -184,6 +184,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_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)
 
@@ -526,6 +527,7 @@ def add_bot_backend(
     check_bot_name_available(
         realm_id=user_profile.realm_id,
         full_name=full_name,
+        is_activation=False,
     )
 
     check_bot_creation_policy(user_profile, bot_type)