From 17bf3d91348d801e4db670bc8876894dfbd60e1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 2 Sep 2022 14:38:27 +0000 Subject: [PATCH] [FIX] portal, account: explicitly check access on record for chatter_post Currently using the ``chatter_post`` route to post a message implicitly checks for access rights on related record. It is done manually in ``message_post`` that is called by the route. It is done because record name is accessed as sudo and an explicit access has been added because of fear of information leak. However this is a bit out of scope for ``message_post`` to perform that check at that point. As the underlying record is accessed numerous times, adding an explicit check just adds queries. In portal, users may have a token and/or a hash and a partner_id, in which case they enter sudo mode if valid, and an error is raised otherwise. However when not using a token and/or a hash and a PID access check is now done directly in the controller helper. Moreover this allows to have the access error directly instead of being possibly hidden by another error, which was the case before this fix (was actually raising about missing email_from). In this commit we add an explicit check on record access before entering the post computation. That way access errors are directly raised. A fix in accounting is necessary to ensure the right error is raised. Indeed otherwise it currently raises due to non existing email_from, and should raise due to ACL instead. Tests are now more inlined with classic flows. Task-2710804 (Mail: Clean MailThread API) Part-of: odoo/odoo#99654 --- addons/account/tests/test_portal_attachment.py | 5 +++++ addons/portal/controllers/mail.py | 3 +++ 2 files changed, 8 insertions(+) diff --git a/addons/account/tests/test_portal_attachment.py b/addons/account/tests/test_portal_attachment.py index c6bc00733a8..eb42ad9b19a 100644 --- a/addons/account/tests/test_portal_attachment.py +++ b/addons/account/tests/test_portal_attachment.py @@ -31,6 +31,9 @@ class TestPortalAttachment(AccountTestInvoicingHttpCommon): @mute_logger('odoo.addons.http_routing.models.ir_http', 'odoo.http') def test_01_portal_attachment(self): """Test the portal chatter attachment route.""" + self.partner_a.write({ # ensure an email for message_post + 'email': 'partner.a@test.example.com', + }) self.authenticate(None, None) @@ -242,7 +245,9 @@ class TestPortalAttachment(AccountTestInvoicingHttpCommon): self.assertEqual(res.status_code, 200) self.out_invoice.invalidate_recordset(['message_ids']) self.assertEqual(len(self.out_invoice.message_ids), 2) + self.assertEqual(self.out_invoice.message_ids[0].author_id, self.partner_a) self.assertEqual(self.out_invoice.message_ids[0].body, "

test message 2

") + self.assertEqual(self.out_invoice.message_ids[0].email_from, self.partner_a.email_formatted) self.assertFalse(self.out_invoice.message_ids.attachment_ids) # Test attachment can be associated if all good (complete flow) diff --git a/addons/portal/controllers/mail.py b/addons/portal/controllers/mail.py index db417a34036..da6ce1404cf 100644 --- a/addons/portal/controllers/mail.py +++ b/addons/portal/controllers/mail.py @@ -60,6 +60,9 @@ def _message_post_helper(res_model, res_id, message, token='', _hash=False, pid= record = record.sudo() else: raise Forbidden() + else: # early check on access to avoid useless computation + record.check_access_rights('read') + record.check_access_rule('read') # deduce author of message author_id = request.env.user.partner_id.id if request.env.user.partner_id else False