pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] mail: make file upload consistent between guest and portal

Browse Source 
opw-3370926

closes odoo/odoo#128290

X-original-commit: 2e5f8a3c9e7cff8c61f66e56da7b0f9a59f9bf2e
Signed-off-by: Vranckx Florian (flvr) <flvr@odoo.com>
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
This commit is contained in:
Sébastien Theys
2023-07-13 08:28:06 +02:00
parent 89e6beff36
commit d289d8a7d0
11 changed files with 333 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/controllers/thread.py
+7 -1
View File
@@ -63,6 +63,10 @@ class ThreadController(http.Controller):
@http.route("/mail/message/post", methods=["POST"], type="json", auth="public")
def mail_message_post(self, thread_model, thread_id, post_data, context=None):
guest = request.env["mail.guest"]._get_guest_from_request(request)
guest.env["ir.attachment"].browse(post_data.get("attachment_ids", []))._check_attachments_access(
post_data.get("attachment_tokens")
)
if context:
request.update_context(**context)
canned_response_ids = tuple(cid for cid in post_data.pop('canned_response_ids', []) if isinstance(cid, int))
@@ -90,8 +94,9 @@ class ThreadController(http.Controller):
return message_data
@http.route("/mail/message/update_content", methods=["POST"], type="json", auth="public")
def mail_message_update_content(self, message_id, body, attachment_ids, partner_ids=None):
def mail_message_update_content(self, message_id, body, attachment_ids, attachment_tokens=None, partner_ids=None):
guest = request.env["mail.guest"]._get_guest_from_request(request)
guest.env["ir.attachment"].browse(attachment_ids)._check_attachments_access(attachment_tokens)
message_sudo = guest.env["mail.message"].browse(message_id).sudo().exists()
if not message_sudo.is_current_user_or_guest_author and not guest.env.user._is_admin():
raise NotFound()
@@ -100,3 +105,4 @@ class ThreadController(http.Controller):
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]
addons/mail/models/ir_attachment.py
+24 -2
View File
@@ -3,14 +3,36 @@
import contextlib
from odoo import api, models
from odoo.exceptions import AccessError
from odoo import _, api, models, SUPERUSER_ID
from odoo.exceptions import AccessError, MissingError, UserError
from odoo.http import request
from odoo.tools import consteq
class IrAttachment(models.Model):
_inherit = 'ir.attachment'
def _check_attachments_access(self, attachment_tokens):
"""This method relies on access rules/rights and therefore it should not be called from a sudo env."""
self = self.sudo(False)
attachment_tokens = attachment_tokens or ([None] * len(self))
if len(attachment_tokens) != len(self):
raise UserError(_("An access token must be provided for each attachment."))
for attachment, access_token in zip(self, attachment_tokens):
try:
attachment_sudo = attachment.with_user(SUPERUSER_ID).exists()
if not attachment_sudo:
raise MissingError(_("The attachment %s does not exist.", attachment.id))
try:
attachment.check('write')
except AccessError:
if not access_token or not attachment_sudo.access_token or not consteq(attachment_sudo.access_token, access_token):
message_sudo = self.env['mail.message'].sudo().search([('attachment_ids', 'in', attachment_sudo.ids)], limit=1)
if not message_sudo or not message_sudo.is_current_user_or_guest_author:
raise
except (AccessError, MissingError):
raise UserError(_("The attachment %s does not exist or you do not have the rights to access it.", attachment.id))
def _post_add_create(self):
""" Overrides behaviour when the attachment is created through the controller
"""
addons/mail/models/mail_thread.py
+1 -1
View File
@@ -4093,7 +4093,7 @@ class MailThread(models.AbstractModel):
'Message': {
'id': message.id,
'body': message.body,
'attachment_ids': message.attachment_ids._attachment_format(),
'attachment_ids': message.attachment_ids.sorted("id")._attachment_format(),
'pinned_at': message.pinned_at,
'recipients': [{'id': p.id, 'name': p.name} for p in message.partner_ids],
'write_date': message.write_date,
addons/mail/static/src/core/common/message_service.js
+14 -3
View File
@@ -9,6 +9,8 @@ import { removeFromArrayWithPredicate, replaceArrayWithCompare } from "@mail/uti
import { convertBrToLineBreak, prettifyMessageContent } from "@mail/utils/common/format";
import { assignDefined, createLocalId } from "@mail/utils/common/misc";
import { markup } from "@odoo/owl";
import { _t } from "@web/core/l10n/translation";
import { registry } from "@web/core/registry";
import { sprintf } from "@web/core/utils/strings";
@@ -34,14 +36,22 @@ export class MessageService {
return;
}
const validMentions = this.getMentionsFromText(rawMentions, body);
await this.rpc("/mail/message/update_content", {
const messageData = await this.rpc("/mail/message/update_content", {
attachment_ids: attachments
.map(({ id }) => id)
.concat(message.attachments.map(({ id }) => id)),
.concat(message.attachments)
.map((attachment) => attachment.id),
attachment_tokens: attachments
.concat(message.attachments)
.map((attachment) => attachment.accessToken),
body: await prettifyMessageContent(body, validMentions),
message_id: message.id,
partner_ids: validMentions?.partners?.map((partner) => partner.id),
});
this.insert(
Object.assign(messageData, {
body: messageData.body ? markup(messageData.body) : messageData.body,
})
);
if (!message.isEmpty && this.store.hasLinkPreviewFeature) {
this.rpc(
"/mail/link_preview",
@@ -63,6 +73,7 @@ export class MessageService {
message.attachments = [];
await this.rpc("/mail/message/update_content", {
attachment_ids: [],
attachment_tokens: [],
body: "",
message_id: message.id,
});
addons/mail/static/src/core/common/thread_service.js
+1
View File
@@ -1006,6 +1006,7 @@ export class ThreadService {
post_data: {
body: await prettifyMessageContent(body, validMentions),
attachment_ids: attachments.map(({ id }) => id),
attachment_tokens: attachments.map((attachment) => attachment.accessToken),
canned_response_ids: cannedResponseIds,
message_type: "comment",
partner_ids,
addons/mail/static/tests/helpers/mock_server/controllers/discuss.js
+16 -7
View File
@@ -95,17 +95,26 @@ patch(MockServer.prototype, "mail/controllers/discuss", {
return this._mockRouteMailLoadMessageFailures();
}
if (route === "/mail/message/post") {
const finalData = {};
for (const allowedField of [
"attachment_ids",
"body",
"message_type",
"partner_ids",
"subtype_xmlid",
"parent_id",
]) {
if (args.post_data[allowedField] !== undefined) {
finalData[allowedField] = args.post_data[allowedField];
}
}
if (args.thread_model === "discuss.channel") {
return this._mockDiscussChannelMessagePost(
args.thread_id,
args.post_data,
args.context
);
return this._mockDiscussChannelMessagePost(args.thread_id, finalData, args.context);
}
return this._mockMailThreadMessagePost(
args.thread_model,
[args.thread_id],
args.post_data,
finalData,
args.context
);
}
@@ -124,7 +133,7 @@ patch(MockServer.prototype, "mail/controllers/discuss", {
attachment_ids: this._mockIrAttachment_attachmentFormat(args.attachment_ids),
},
});
return "dummy value for mock server";
return this._mockMailMessageMessageFormat([args.message_id])[0];
}
if (route === "/mail/read_subscription_data") {
const follower_id = args.follower_id;
addons/mail/static/tests/tours/discuss_public_tour.js
+56
View File
@@ -76,5 +76,61 @@ registry.category("web_tour.tours").add("mail/static/tests/tours/discuss_public_
trigger: '.o-mail-Message .o-mail-AttachmentCard:contains("text.txt")',
run() {},
},
{
content: "Click on more menu",
trigger: ".o-mail-Message [title='Expand']",
},
{
content: "Click on edit",
trigger: ".o-mail-Message [title='Edit']",
},
{
content: "Edit message",
trigger: ".o-mail-Message .o-mail-Composer-input",
run: "text vegetables",
},
{
content: "Add one more file in composer",
trigger: ".o-mail-Message .o-mail-Composer button[aria-label='Attach files']",
async run() {
const file = await createFile({
content: "hello 2",
contentType: "text/plain",
name: "extra.txt",
});
inputFiles(
document.querySelector(
".o-mail-Message .o-mail-Composer-coreMain .o_input_file"
),
[file]
);
},
},
{
content: "Check the earlier provided extra attachment is listed",
trigger: '.o-mail-Message .o-mail-Composer .o-mail-AttachmentCard[title="extra.txt"]',
extra_trigger:
".o-mail-Message .o-mail-Composer .o-mail-AttachmentCard:not(.o-isUploading)", // waiting the attachment to be uploaded
run() {},
},
{
content: "Save edited message",
trigger: ".o-mail-Message a:contains(save)",
},
{
content: "Check message is edited",
trigger: '.o-mail-Message-body:contains("vegetables")',
run() {},
},
{
content: "Check edited message contains the first attachment",
trigger: '.o-mail-Message .o-mail-AttachmentCard:contains("text.txt")',
run() {},
},
{
content: "Check edited message contains the extra attachment",
trigger: '.o-mail-Message .o-mail-AttachmentCard:contains("extra.txt")',
run() {},
},
],
});
addons/mail/static/tests/web/chatter_tests.js
+2
View File
@@ -52,6 +52,7 @@ QUnit.test("can post a message on a record thread", async (assert) => {
post_data: {
body: "hey",
attachment_ids: [],
attachment_tokens: [],
canned_response_ids: [],
message_type: "comment",
partner_ids: [],
@@ -89,6 +90,7 @@ QUnit.test("can post a note on a record thread", async (assert) => {
context: args.context,
post_data: {
attachment_ids: [],
attachment_tokens: [],
body: "hey",
canned_response_ids: [],
message_type: "comment",
addons/mail/tests/discuss/__init__.py
+1
View File
@@ -3,5 +3,6 @@
from . import test_discuss_channel
from . import test_discuss_channel_as_guest
from . import test_discuss_channel_member
from . import test_message_controller
from . import test_load_messages
from . import test_rtc
addons/mail/tests/discuss/test_message_controller.py
+208
View File
@@ -0,0 +1,208 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import json
import odoo
from odoo.tools import mute_logger
from odoo.tests import HttpCase
@odoo.tests.tagged("-at_install", "post_install")
class TestMessageController(HttpCase):
@classmethod
def setUpClass(cls):
super().setUpClass()
cls.channel = cls.env["discuss.channel"].create(
{
"group_public_id": None,
"name": "Test channel",
}
)
cls.public_user = cls.env.ref("base.public_user")
cls.attachments = (
cls.env["ir.attachment"]
.with_user(cls.public_user)
.sudo()
.create(
[
{
"access_token": cls.env["ir.attachment"]._generate_access_token(),
"name": "File 1",
"res_id": 0,
"res_model": "mail.compose.message",
},
{
"access_token": cls.env["ir.attachment"]._generate_access_token(),
"name": "File 2",
"res_id": 0,
"res_model": "mail.compose.message",
},
]
)
)
cls.guest = cls.env["mail.guest"].create({"name": "Guest"})
cls.channel.add_members(guest_ids=cls.guest.ids)
@mute_logger("odoo.addons.http_routing.models.ir_http", "odoo.http")
def test_channel_message_attachments(self):
self.authenticate(None, None)
self.opener.cookies[
self.guest._cookie_name
] = f"{self.guest.id}{self.guest._cookie_separator}{self.guest.access_token}"
# test message post: token error
res1 = self.url_open(
url="/mail/message/post",
data=json.dumps(
{
"params": {
"thread_model": self.channel._name,
"thread_id": self.channel.id,
"post_data": {
"body": "test",
"attachment_ids": [self.attachments[0].id],
"attachment_tokens": ["wrong token"],
},
},
}
),
headers={"Content-Type": "application/json"},
)
self.assertEqual(res1.status_code, 200)
self.assertIn(
f"The attachment {self.attachments[0].id} does not exist or you do not have the rights to access it",
res1.text,
"guest should not be allowed to add attachment without token when posting message",
)
# test message post: token ok
res2 = self.url_open(
url="/mail/message/post",
data=json.dumps(
{
"params": {
"thread_model": self.channel._name,
"thread_id": self.channel.id,
"post_data": {
"body": "test",
"attachment_ids": [self.attachments[0].id],
"attachment_tokens": [self.attachments[0].access_token],
"message_type": "comment",
},
},
}
),
headers={"Content-Type": "application/json"},
)
self.assertEqual(res2.status_code, 200)
message_format1 = res2.json()["result"]
self.assertEqual(
message_format1["attachment_ids"],
json.loads(json.dumps(self.attachments[0]._attachment_format())),
"guest should be allowed to add attachment with token when posting message",
)
# test message update: token error
res3 = self.url_open(
url="/mail/message/update_content",
data=json.dumps(
{
"params": {
"message_id": message_format1["id"],
"body": "test",
"attachment_ids": [self.attachments[1].id],
"attachment_tokens": ["wrong token"],
},
}
),
headers={"Content-Type": "application/json"},
)
self.assertEqual(res3.status_code, 200)
self.assertIn(
f"The attachment {self.attachments[1].id} does not exist or you do not have the rights to access it",
res3.text,
"guest should not be allowed to add attachment without token when updating message",
)
# test message update: token ok
res4 = self.url_open(
url="/mail/message/update_content",
data=json.dumps(
{
"params": {
"message_id": message_format1["id"],
"body": "test",
"attachment_ids": [self.attachments[1].id],
"attachment_tokens": [self.attachments[1].access_token],
},
}
),
headers={"Content-Type": "application/json"},
)
self.assertEqual(res4.status_code, 200)
message_format2 = res4.json()["result"]
self.assertEqual(
message_format2["attachment_ids"],
json.loads(json.dumps(self.attachments.sorted("id")._attachment_format())),
"guest should be allowed to add attachment with token when updating message",
)
# test message update: own attachment ok
res5 = self.url_open(
url="/mail/message/update_content",
data=json.dumps(
{
"params": {
"message_id": message_format2["id"],
"body": "test",
"attachment_ids": [self.attachments[1].id],
},
}
),
headers={"Content-Type": "application/json"},
)
self.assertEqual(res5.status_code, 200)
message_format3 = res5.json()["result"]
self.assertEqual(
message_format3["attachment_ids"],
json.loads(json.dumps(self.attachments.sorted("id")._attachment_format())),
"guest should be allowed to add own attachment without token when updating message",
)
@mute_logger("odoo.addons.http_routing.models.ir_http", "odoo.http")
def test_attachment_hijack(self):
att = self.env["ir.attachment"].create(
[
{
"name": "arguments_for_firing_marc_demo",
"res_id": 0,
"res_model": "mail.compose.message",
},
]
)
demo = self.authenticate("demo", "demo")
channel = self.env["discuss.channel"].create({"group_public_id": None, "name": "public_channel"})
channel.add_members(
self.env["res.users"].browse(demo.uid).partner_id.id
) # don't care, we just need a channel where demo is follower
no_access_request = self.url_open("/web/content/" + str(att.id))
self.assertFalse(
no_access_request.ok
) # if this test breaks, it might be due to a change in /web/content, or the default rules for accessing an attachment. This is not an issue but it makes this test irrelevant.
response = self.url_open(
url="/mail/message/post",
headers={"Content-Type": "application/json"}, # route called as demo
data=json.dumps(
{
"params": {
"post_data": {
"attachment_ids": [att.id], # demo does not have access to this attachment id
"body": "",
"message_type": "comment",
"partner_ids": [],
"subtype_xmlid": "mail.mt_comment",
},
"thread_id": channel.id,
"thread_model": "discuss.channel",
}
},
),
)
self.assertNotIn(
"arguments_for_firing_marc_demo", response.text
) # demo should not be able to see the name of the document
addons/portal/controllers/mail.py
+3 -10
View File
@@ -4,13 +4,12 @@
from werkzeug import urls
from werkzeug.exceptions import NotFound, Forbidden
from odoo import http, _
from odoo import http
from odoo.http import request
from odoo.osv import expression
from odoo.tools import consteq, plaintext2html
from odoo.addons.mail.controllers import mail
from odoo.addons.portal.controllers.portal import CustomerPortal
from odoo.exceptions import AccessError, MissingError, UserError
from odoo.exceptions import AccessError
def _check_special_access(res_model, res_id, token='', _hash='', pid=False):
@@ -107,13 +106,7 @@ class PortalChatter(http.Controller):
return ['token', 'pid']
def _portal_post_check_attachments(self, attachment_ids, attachment_tokens):
if len(attachment_tokens) != len(attachment_ids):
raise UserError(_("An access token must be provided for each attachment."))
for (attachment_id, access_token) in zip(attachment_ids, attachment_tokens):
try:
CustomerPortal._document_check_access(self, 'ir.attachment', attachment_id, access_token)
except (AccessError, MissingError):
raise UserError(_("The attachment %s does not exist or you do not have the rights to access it.", attachment_id))
request.env['ir.attachment'].browse(attachment_ids)._check_attachments_access(attachment_tokens)
def _portal_post_has_content(self, res_model, res_id, message, attachment_ids=None, **kw):
""" Tells if we can effectively post on the model based on content. """