Skip to content

Commit

Permalink
[IMP] mail: avoid broadcasting partner lists and tracking values
Browse files Browse the repository at this point in the history
Before this PR, `_message_format` function always included data that were only
relevant to the logged user. It also `leak partner_ids`.

This PR condition the `needaction_partner_ids`, `history_partner_ids`,
`starredPersonas` and `trackingValues` values to a new param.
This allows us to control when those data should be sent to the user.

PR enterprise: odoo/enterprise#61353
  • Loading branch information
phenix-factory committed Apr 29, 2024
1 parent b1666fc commit 44f86cf
Show file tree
Hide file tree
Showing 21 changed files with 88 additions and 73 deletions.
6 changes: 3 additions & 3 deletions addons/im_livechat/controllers/chatbot.py
Expand Up @@ -15,7 +15,7 @@ def chatbot_restart(self, channel_id, chatbot_script_id):
if not discuss_channel or not chatbot.exists():
return None
chatbot_language = self._get_chatbot_language()
return discuss_channel.with_context(lang=chatbot_language)._chatbot_restart(chatbot)._message_format()[0]
return discuss_channel.with_context(lang=chatbot_language)._chatbot_restart(chatbot)._message_format(for_current_user=True)[0]

@http.route("/chatbot/answer/save", type="json", auth="public")
@add_guest_to_context
Expand Down Expand Up @@ -73,7 +73,7 @@ def chatbot_trigger_step(self, channel_id, chatbot_script_id=None):
'message': plaintext2html(next_step.message) if not is_html_empty(next_step.message) else False,
'type': next_step.step_type,
},
'message': posted_message._message_format()[0] if posted_message else None,
'message': posted_message._message_format(for_current_user=True)[0] if posted_message else None,
'operatorFound': next_step.step_type == 'forward_operator' and len(
discuss_channel.channel_member_ids) > 2,
}
Expand All @@ -98,7 +98,7 @@ def chatbot_validate_email(self, channel_id):
result = chatbot._validate_email(user_answer.body, discuss_channel)

if result['posted_message']:
result['posted_message'] = result['posted_message']._message_format()[0]
result['posted_message'] = result['posted_message']._message_format(for_current_user=True)[0]

return result

Expand Down
4 changes: 2 additions & 2 deletions addons/im_livechat/models/mail_message.py
Expand Up @@ -21,7 +21,7 @@ def _compute_parent_body(self):
for message in self:
message.parent_body = message.parent_id.body if message.parent_id else False

def _message_format(self, format_reply=True, msg_vals=None):
def _message_format(self, format_reply=True, msg_vals=None, for_current_user=False):
"""Override to remove email_from and to return the livechat username if applicable.
A third param is added to the author_id tuple in this case to be able to differentiate it
from the normal name in client code.
Expand All @@ -31,7 +31,7 @@ def _message_format(self, format_reply=True, msg_vals=None):
This allows the frontend display to include the additional features
(e.g: Show additional buttons with the available answers for this step). """

vals_list = super()._message_format(format_reply=format_reply, msg_vals=msg_vals)
vals_list = super()._message_format(format_reply=format_reply, msg_vals=msg_vals, for_current_user=for_current_user)
for vals in vals_list:
message_sudo = self.browse(vals['id']).sudo().with_prefetch(self.ids)
discuss_channel = self.env['discuss.channel'].browse(message_sudo.res_id) if message_sudo.model == 'discuss.channel' else self.env['discuss.channel']
Expand Down
2 changes: 1 addition & 1 deletion addons/im_livechat/tests/test_message.py
Expand Up @@ -66,7 +66,7 @@ def test_message_format(self):
% (record_rating.rating_image_url, record_rating.rating, record_rating.feedback),
rating_id=record_rating.id,
)
self.assertEqual(message._message_format(), [{
self.assertEqual(message._message_format(for_current_user=True), [{
'attachments': [],
'author': {
'id': self.users[1].partner_id.id,
Expand Down
6 changes: 3 additions & 3 deletions addons/mail/controllers/discuss/channel.py
Expand Up @@ -19,7 +19,7 @@ def _process_request_for_all(self, store, **kwargs):
# fetch channels data before messages to benefit from prefetching (channel info might
# prefetch a lot of data that message format could use)
store.add({"Thread": channels._channel_info()})
store.add({"Message": channels._get_last_messages()._message_format()})
store.add({"Message": channels._get_last_messages()._message_format(for_current_user=True)})


class ChannelController(http.Controller):
Expand Down Expand Up @@ -62,15 +62,15 @@ def discuss_channel_messages(self, channel_id, search_term=None, before=None, af
)
if not request.env.user._is_public() and not around:
res["messages"].set_message_done()
return {**res, "messages": res["messages"]._message_format()}
return {**res, "messages": res["messages"]._message_format(for_current_user=True)}

@http.route("/discuss/channel/pinned_messages", methods=["POST"], type="json", auth="public")
@add_guest_to_context
def discuss_channel_pins(self, channel_id):
channel = request.env["discuss.channel"].search([("id", "=", channel_id)])
if not channel:
raise NotFound()
return channel.pinned_message_ids.sorted(key="pinned_at", reverse=True)._message_format()
return channel.pinned_message_ids.sorted(key="pinned_at", reverse=True)._message_format(for_current_user=True)

@http.route("/discuss/channel/mute", methods=["POST"], type="json", auth="user")
def discuss_channel_mute(self, channel_id, minutes):
Expand Down
4 changes: 2 additions & 2 deletions addons/mail/controllers/mailbox.py
Expand Up @@ -16,10 +16,10 @@ def discuss_inbox_messages(self, search_term=None, before=None, after=None, limi
def discuss_history_messages(self, search_term=None, before=None, after=None, limit=30, around=None):
domain = [("needaction", "=", False)]
res = request.env["mail.message"]._message_fetch(domain, search_term=search_term, before=before, after=after, around=around, limit=limit)
return {**res, "messages": res["messages"]._message_format()}
return {**res, "messages": res["messages"]._message_format(for_current_user=True)}

@http.route("/mail/starred/messages", methods=["POST"], type="json", auth="user")
def discuss_starred_messages(self, search_term=None, before=None, after=None, limit=30, around=None):
domain = [("starred_partner_ids", "in", [request.env.user.partner_id.id])]
res = request.env["mail.message"]._message_fetch(domain, search_term=search_term, before=before, after=after, around=around, limit=limit)
return {**res, "messages": res["messages"]._message_format()}
return {**res, "messages": res["messages"]._message_format(for_current_user=True)}
6 changes: 3 additions & 3 deletions addons/mail/controllers/thread.py
Expand Up @@ -25,7 +25,7 @@ def mail_thread_messages(self, thread_model, thread_id, search_term=None, before
res = request.env["mail.message"]._message_fetch(domain, search_term=search_term, before=before, after=after, around=around, limit=limit)
if not request.env.user._is_public():
res["messages"].set_message_done()
return {**res, "messages": res["messages"]._message_format()}
return {**res, "messages": res["messages"]._message_format(for_current_user=True)}

@http.route("/mail/partner/from_email", methods=["POST"], type="json", auth="user")
def mail_thread_partner_from_email(self, emails, additional_values=None):
Expand Down Expand Up @@ -107,7 +107,7 @@ def mail_message_post(self, thread_model, thread_id, post_data, context=None, **
post_data["partner_ids"] = list(set((post_data.get("partner_ids", [])) + new_partners))
message_data = thread.message_post(
**{key: value for key, value in post_data.items() if key in self._get_allowed_message_post_params()}
)._message_format()[0]
)._message_format(for_current_user=True)[0]
if "temporary_id" in request.context:
message_data["temporary_id"] = request.context["temporary_id"]
return message_data
Expand All @@ -126,4 +126,4 @@ def mail_message_update_content(self, message_id, body, attachment_ids, attachme
guest.env[message_sudo.model].browse([message_sudo.res_id])._message_update_content(
message_sudo, body, attachment_ids=attachment_ids, partner_ids=partner_ids
)
return message_sudo._message_format()[0]
return message_sudo._message_format(for_current_user=True)[0]
21 changes: 11 additions & 10 deletions addons/mail/models/mail_message.py
Expand Up @@ -897,7 +897,7 @@ def _record_by_message(self):
record_by_message[message] = self.env[message.model].browse(message.res_id).with_prefetch(record_ids_by_model_name[message.model])
return record_by_message

def _message_format(self, format_reply=True, msg_vals=None):
def _message_format(self, format_reply=True, msg_vals=None, for_current_user=False):
""" Get the message values in the format for web client. Since message
values can be broadcasted, computed fields MUST NOT BE READ and
broadcasted.
Expand Down Expand Up @@ -995,27 +995,19 @@ def _message_format(self, format_reply=True, msg_vals=None):
'personas': [{'id': guest.id, 'name': guest.name, 'type': "guest"} for guest in reactions.guest_id] + [{'id': partner.id, 'name': partner.name, 'type': "partner"} for partner in reactions.partner_id],
'message': {'id': message_sudo.id},
} for content, reactions in reactions_per_content.items()]
allowed_tracking_ids = message_sudo.tracking_value_ids._filter_tracked_field_access(self.env)
displayed_tracking_ids = allowed_tracking_ids
if record and hasattr(record, '_track_filter_for_display'):
displayed_tracking_ids = record._track_filter_for_display(displayed_tracking_ids)
notifs = message_sudo.notification_ids.filtered("res_partner_id")
vals.update(message_sudo._message_format_extras(format_reply))
vals.pop("starred_partner_ids", None)
vals.update({
'author': author,
'default_subject': default_subject,
'notifications': message_sudo.notification_ids._filtered_for_web_client()._notification_format(),
'attachments': sorted(message_sudo.attachment_ids._attachment_format(), key=lambda a: a["id"]),
'trackingValues': displayed_tracking_ids._tracking_value_format(),
'linkPreviews': message_sudo.link_preview_ids.filtered(lambda preview: not preview.is_hidden)._link_preview_format(),
'reactions': reaction_groups,
'pinned_at': message_sudo.pinned_at,
'record_name': record_name,
'create_date': message_sudo.create_date,
'write_date': message_sudo.write_date,
"needaction_partner_ids": notifs.filtered(lambda n: not n.is_read).res_partner_id.ids,
"starredPersonas": [{"id": partner_id, "type": "partner"} for partner_id in message_sudo.starred_partner_ids.ids],
"is_note": message_sudo.subtype_id.id == note_id,
"is_discussion": message_sudo.subtype_id.id == com_id,
"subtype_description": message_sudo.subtype_id.description,
Expand All @@ -1029,6 +1021,15 @@ def _message_format(self, format_reply=True, msg_vals=None):
if self.env[message_sudo.model]._original_module:
thread["module_icon"] = modules.module.get_module_icon(self.env[message_sudo.model]._original_module)
vals["thread"] = thread
if for_current_user:
notifs = message_sudo.notification_ids.filtered("res_partner_id")
allowed_tracking_ids = message_sudo.tracking_value_ids._filter_tracked_field_access(self.env)
displayed_tracking_ids = allowed_tracking_ids
if record and hasattr(record, '_track_filter_for_display'):
displayed_tracking_ids = record._track_filter_for_display(displayed_tracking_ids)
vals["needaction_partner_ids"] = notifs.filtered(lambda n: not n.is_read).res_partner_id.ids
vals["starredPersonas"] = [{"id": partner_id, "type": "partner"} for partner_id in message_sudo.starred_partner_ids.ids]
vals["trackingValues"] = displayed_tracking_ids._tracking_value_format()
return vals_list

def _message_format_extras(self, format_reply):
Expand Down Expand Up @@ -1120,7 +1121,7 @@ def _message_format_personalize(self, partner_id, messages_formatted=None, forma
:return: list of messages_formatted personalized for the partner
"""
if not messages_formatted:
messages_formatted = self._message_format(format_reply=format_reply, msg_vals=msg_vals)
messages_formatted = self._message_format(format_reply=format_reply, msg_vals=msg_vals, for_current_user=True)
self._message_format_personalized_prepare(messages_formatted, [partner_id])
for vals in messages_formatted:
# set value for user being a follower, fallback to False if not prepared
Expand Down
2 changes: 1 addition & 1 deletion addons/mail/models/mail_thread.py
Expand Up @@ -3170,7 +3170,7 @@ def _notify_thread_by_inbox(self, message, recipients_data, msg_vals=False, **kw

MailMessage = self.env['mail.message']
messages_format_prepared = MailMessage._message_format_personalized_prepare(
message._message_format(msg_vals=msg_vals), partner_ids=inbox_pids)
message._message_format(msg_vals=msg_vals, for_current_user=True), partner_ids=inbox_pids)
for partner_id in inbox_pids:
bus_notifications.append(
(self.env['res.partner'].browse(partner_id),
Expand Down
4 changes: 2 additions & 2 deletions addons/mail/static/tests/discuss_app/discuss.test.js
Expand Up @@ -453,7 +453,7 @@ test("receive new needaction messages", async () => {
notification_type: "inbox",
res_partner_id: serverState.partnerId,
});
const [message1] = pyEnv["mail.message"]._message_format(messageId_1);
const [message1] = pyEnv["mail.message"]._message_format(messageId_1, true);
const [partner] = pyEnv["res.partner"].read(serverState.partnerId);
pyEnv["bus.bus"]._sendone(partner, "mail.message/inbox", message1);
await contains("button", { text: "Inbox", contains: [".badge", { text: "1" }] });
Expand All @@ -474,7 +474,7 @@ test("receive new needaction messages", async () => {
notification_type: "inbox",
res_partner_id: serverState.partnerId,
});
const [message2] = pyEnv["mail.message"]._message_format(messageId_2);
const [message2] = pyEnv["mail.message"]._message_format(messageId_2, true);
pyEnv["bus.bus"]._sendone(partner, "mail.message/inbox", message2);
await contains("button", { text: "Inbox", contains: [".badge", { text: "2" }] });
await contains(".o-mail-Message", { count: 2 });
Expand Down
Expand Up @@ -1079,7 +1079,7 @@ test("messaging menu should show new needaction messages from chatter", async ()
notification_type: "inbox",
res_partner_id: serverState.partnerId,
});
const [formattedMessage] = pyEnv["mail.message"]._message_format(messageId);
const [formattedMessage] = pyEnv["mail.message"]._message_format(messageId, true);
const [partner] = pyEnv["res.partner"].read(serverState.partnerId);
pyEnv["bus.bus"]._sendone(partner, "mail.message/inbox", formattedMessage);
await contains(".o-mail-NotificationItem-text", { text: "Frodo Baggins: @Mitchel Admin" });
Expand Down
26 changes: 19 additions & 7 deletions addons/mail/static/tests/mock_server/mail_mock_server.js
Expand Up @@ -339,7 +339,10 @@ async function discuss_channel_messages(request) {
}
return {
...res,
messages: MailMessage._message_format(res.messages.map((message) => message.id)),
messages: MailMessage._message_format(
res.messages.map((message) => message.id),
true
),
};
}

Expand Down Expand Up @@ -408,7 +411,7 @@ async function discuss_channel_pins(request) {
["res_id", "=", channel_id],
["pinned_at", "!=", false],
]);
return MailMessage._message_format(messageIds);
return MailMessage._message_format(messageIds, true);
}

registerRoute("/discuss/channel/set_last_seen_message", discuss_channel_mark_as_seen);
Expand Down Expand Up @@ -450,7 +453,8 @@ async function discuss_history_messages(request) {
return {
...res,
messages: MailMessage._message_format(
messagesWithNotification.map((message) => message.id)
messagesWithNotification.map((message) => message.id),
true
),
};
}
Expand Down Expand Up @@ -631,7 +635,7 @@ async function mail_message_update_content(request) {
pinned_at: message.pinned_at,
},
});
return MailMessage._message_format([message_id])[0];
return MailMessage._message_format([message_id], true)[0];
}

registerRoute("/discuss/channel/:cid/partner/:pid/avatar_128", partnerAvatar128);
Expand Down Expand Up @@ -727,7 +731,10 @@ async function discuss_starred_messages(request) {
const res = MailMessage._message_fetch(domain, search_term, before, after, false, limit);
return {
...res,
messages: MailMessage._message_format(res.messages.map((message) => message.id)),
messages: MailMessage._message_format(
res.messages.map((message) => message.id),
true
),
};
}

Expand Down Expand Up @@ -758,7 +765,10 @@ async function mail_thread_messages(request) {
MailMessage.set_message_done(res.messages.map((message) => message.id));
return {
...res,
messages: MailMessage._message_format(res.messages.map((message) => message.id)),
messages: MailMessage._message_format(
res.messages.map((message) => message.id),
true
),
};
}

Expand Down Expand Up @@ -870,7 +880,9 @@ async function processRequest(request) {
}
return lastMessage;
}, channelMessages[0]);
return lastMessage ? MailMessage._message_format([lastMessage.id])[0] : false;
return lastMessage
? MailMessage._message_format([lastMessage.id], true)[0]
: false;
})
.filter((lastMessage) => lastMessage),
Thread: DiscussChannel._channel_info(channels.map((channel) => channel.id)),
Expand Down
Expand Up @@ -318,7 +318,7 @@ export class DiscussChannel extends models.ServerModel {
return {
id: channel.id,
last_message: lastMessage
? MailMessage._message_format([lastMessage.id])[0]
? MailMessage._message_format([lastMessage.id], true)[0]
: false,
};
})
Expand Down

0 comments on commit 44f86cf

Please sign in to comment.