messages: Add support to allow bot-owners to delete messages.

This commit adds support to allow bot-owners to delete messages
sent by their bots if they are allowed to delete their own messages
as per "delete_own_message_policy" setting and the message delete
time limit has not passed.
This commit is contained in:
lapaz 2023-06-14 13:40:41 -04:00 committed by Tim Abbott
parent 59d025c570
commit 417b29638c
4 changed files with 148 additions and 5 deletions

View File

@ -32,6 +32,7 @@ import * as message_live_update from "./message_live_update";
import * as message_store from "./message_store"; import * as message_store from "./message_store";
import * as message_viewport from "./message_viewport"; import * as message_viewport from "./message_viewport";
import {page_params} from "./page_params"; import {page_params} from "./page_params";
import * as people from "./people";
import * as resize from "./resize"; import * as resize from "./resize";
import * as rows from "./rows"; import * as rows from "./rows";
import * as settings_data from "./settings_data"; import * as settings_data from "./settings_data";
@ -153,12 +154,23 @@ export function is_content_editable(message, edit_limit_seconds_buffer = 0) {
return false; return false;
} }
export function is_message_sent_by_my_bot(message) {
const user = people.get_by_user_id(message.sender_id);
if (user.bot_owner_id === undefined || user.bot_owner_id === null) {
// The message was not sent by a bot or the message was sent
// by a cross-realm bot which does not have an owner.
return false;
}
return people.is_my_user_id(user.bot_owner_id);
}
export function get_deletability(message) { export function get_deletability(message) {
if (page_params.is_admin) { if (page_params.is_admin) {
return true; return true;
} }
if (!message.sent_by_me) { if (!message.sent_by_me && !is_message_sent_by_my_bot(message)) {
return false; return false;
} }
if (message.locally_echoed) { if (message.locally_echoed) {

View File

@ -9,6 +9,7 @@ const {page_params} = require("./lib/zpage_params");
page_params.realm_move_messages_within_stream_limit_seconds = 259200; page_params.realm_move_messages_within_stream_limit_seconds = 259200;
const message_edit = zrequire("message_edit"); const message_edit = zrequire("message_edit");
const people = zrequire("people");
const is_content_editable = message_edit.is_content_editable; const is_content_editable = message_edit.is_content_editable;
@ -182,9 +183,25 @@ run_test("get_deletability", ({override}) => {
page_params.is_admin = true; page_params.is_admin = true;
override(settings_data, "user_can_delete_own_message", () => false); override(settings_data, "user_can_delete_own_message", () => false);
page_params.realm_message_content_delete_limit_seconds = null; page_params.realm_message_content_delete_limit_seconds = null;
const test_user = {
user_id: 1,
full_name: "Test user",
email: "test@zulip.com",
};
people.add_active_user(test_user);
const bot_user = {
user_id: 2,
full_name: "Test bot user",
email: "test-bot@zulip.com",
bot_owner_id: 1,
};
people.add_active_user(bot_user);
const message = { const message = {
sent_by_me: false, sent_by_me: false,
locally_echoed: true, locally_echoed: true,
sender_id: 1,
}; };
// Admin can always delete any message // Admin can always delete any message
@ -204,8 +221,12 @@ run_test("get_deletability", ({override}) => {
override(settings_data, "user_can_delete_own_message", () => true); override(settings_data, "user_can_delete_own_message", () => true);
assert.equal(message_edit.get_deletability(message), true); assert.equal(message_edit.get_deletability(message), true);
const now = new Date(); message.sent_by_me = false;
const current_timestamp = now / 1000; assert.equal(message_edit.get_deletability(message), false);
message.sent_by_me = true;
let now = new Date();
let current_timestamp = now / 1000;
message.timestamp = current_timestamp - 5; message.timestamp = current_timestamp - 5;
page_params.realm_message_content_delete_limit_seconds = 10; page_params.realm_message_content_delete_limit_seconds = 10;
@ -213,6 +234,24 @@ run_test("get_deletability", ({override}) => {
message.timestamp = current_timestamp - 60; message.timestamp = current_timestamp - 60;
assert.equal(message_edit.get_deletability(message), false); assert.equal(message_edit.get_deletability(message), false);
message.sender_id = 2;
message.sent_by_me = false;
people.initialize_current_user(test_user.user_id);
page_params.realm_message_content_delete_limit_seconds = null;
override(settings_data, "user_can_delete_own_message", () => true);
assert.equal(message_edit.get_deletability(message), true);
override(settings_data, "user_can_delete_own_message", () => false);
assert.equal(message_edit.get_deletability(message), false);
now = new Date();
current_timestamp = now / 1000;
page_params.realm_message_content_delete_limit_seconds = 10;
message.timestamp = current_timestamp - 60;
override(settings_data, "user_can_delete_own_message", () => true);
assert.equal(message_edit.get_deletability(message), false);
}); });
run_test("stream_and_topic_exist_in_edit_history", () => { run_test("stream_and_topic_exist_in_edit_history", () => {

View File

@ -4841,6 +4841,98 @@ class DeleteMessageTest(ZulipTestCase):
result = test_delete_message_by_owner(msg_id=msg_id) result = test_delete_message_by_owner(msg_id=msg_id)
self.assert_json_error(result, "Message already deleted") self.assert_json_error(result, "Message already deleted")
def test_delete_message_sent_by_bots(self) -> None:
iago = self.example_user("iago")
hamlet = self.example_user("hamlet")
cordelia = self.example_user("cordelia")
def set_message_deleting_params(
delete_own_message_policy: int, message_content_delete_limit_seconds: Union[int, str]
) -> None:
result = self.api_patch(
iago,
"/api/v1/realm",
{
"delete_own_message_policy": delete_own_message_policy,
"message_content_delete_limit_seconds": orjson.dumps(
message_content_delete_limit_seconds
).decode(),
},
)
self.assert_json_success(result)
def test_delete_message_by_admin(msg_id: int) -> "TestHttpResponse":
result = self.api_delete(iago, f"/api/v1/messages/{msg_id}")
return result
def test_delete_message_by_bot_owner(msg_id: int) -> "TestHttpResponse":
result = self.api_delete(hamlet, f"/api/v1/messages/{msg_id}")
return result
def test_delete_message_by_other_user(msg_id: int) -> "TestHttpResponse":
result = self.api_delete(cordelia, f"/api/v1/messages/{msg_id}")
return result
set_message_deleting_params(Realm.POLICY_ADMINS_ONLY, "unlimited")
hamlet = self.example_user("hamlet")
test_bot = self.create_test_bot("test-bot", hamlet)
msg_id = self.send_stream_message(test_bot, "Denmark")
result = test_delete_message_by_other_user(msg_id)
self.assert_json_error(result, "You don't have permission to delete this message")
result = test_delete_message_by_bot_owner(msg_id)
self.assert_json_error(result, "You don't have permission to delete this message")
result = test_delete_message_by_admin(msg_id)
self.assert_json_success(result)
msg_id = self.send_stream_message(test_bot, "Denmark")
set_message_deleting_params(Realm.POLICY_EVERYONE, "unlimited")
result = test_delete_message_by_other_user(msg_id)
self.assert_json_error(result, "You don't have permission to delete this message")
result = test_delete_message_by_bot_owner(msg_id)
self.assert_json_success(result)
msg_id = self.send_stream_message(test_bot, "Denmark")
set_message_deleting_params(Realm.POLICY_EVERYONE, 600)
message = Message.objects.get(id=msg_id)
message.date_sent = timezone_now() - datetime.timedelta(seconds=700)
message.save()
result = test_delete_message_by_other_user(msg_id)
self.assert_json_error(result, "You don't have permission to delete this message")
result = test_delete_message_by_bot_owner(msg_id)
self.assert_json_error(result, "The time limit for deleting this message has passed")
result = test_delete_message_by_admin(msg_id)
self.assert_json_success(result)
# Check that the bot can also delete the messages sent by them
# depending on the realm permissions for message deletion.
set_message_deleting_params(Realm.POLICY_ADMINS_ONLY, 600)
msg_id = self.send_stream_message(test_bot, "Denmark")
result = self.api_delete(test_bot, f"/api/v1/messages/{msg_id}")
self.assert_json_error(result, "You don't have permission to delete this message")
set_message_deleting_params(Realm.POLICY_EVERYONE, 600)
message = Message.objects.get(id=msg_id)
message.date_sent = timezone_now() - datetime.timedelta(seconds=700)
message.save()
result = self.api_delete(test_bot, f"/api/v1/messages/{msg_id}")
self.assert_json_error(result, "The time limit for deleting this message has passed")
message.date_sent = timezone_now() - datetime.timedelta(seconds=400)
message.save()
result = self.api_delete(test_bot, f"/api/v1/messages/{msg_id}")
self.assert_json_success(result)
def test_delete_message_according_to_delete_own_message_policy(self) -> None: def test_delete_message_according_to_delete_own_message_policy(self) -> None:
def check_delete_message_by_sender( def check_delete_message_by_sender(
sender_name: str, error_msg: Optional[str] = None sender_name: str, error_msg: Optional[str] = None

View File

@ -146,8 +146,8 @@ def validate_can_delete_message(user_profile: UserProfile, message: Message) ->
if user_profile.is_realm_admin: if user_profile.is_realm_admin:
# Admin can delete any message, any time. # Admin can delete any message, any time.
return return
if message.sender != user_profile: if message.sender != user_profile and message.sender.bot_owner_id != user_profile.id:
# Users can only delete messages sent by them. # Users can only delete messages sent by them or by their bots.
raise JsonableError(_("You don't have permission to delete this message")) raise JsonableError(_("You don't have permission to delete this message"))
if not user_profile.can_delete_own_message(): if not user_profile.can_delete_own_message():
# Only user with roles as allowed by delete_own_message_policy can delete message. # Only user with roles as allowed by delete_own_message_policy can delete message.