From 5edb04d4914560796b5f3cd158dbbfd24ae17a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 2 Mar 2023 13:12:30 +0000 Subject: [PATCH] [IMP] test_mail: add posting tests in multi-company environment with attachments Purpose of this commit is to add tests to post in record user cannot access especially in multi company environment. Main observations * ACLs are checked when fetching record_name, then when fetching reply_to information, adding a bit of ACLs noise in the process; * check_access_rule of mail.message is called when creating the message (which is the real protection); * posting on a document user cannot see is possible when they have been notified of a parent message. Use case is: answer a ping, even on a document non readable, should always be doable. However it currently crashes notably due to attachments check, who always require the read access on the related document even if the message is authorized; In this commit we also move other tests about multi_company in the right file (and add a test flag) in order to have all of them in the same place easing debug and testing. Task-3213982 (Mail: fix 'multi-company' post support) X-original-commit: odoo/odoo@5656f4d20dca514b3935e001e82b51d1c205d1c1 Part-of: odoo/odoo#114511 --- .../test_mail/tests/test_mail_multicompany.py | 209 +++++++++++++++++- .../tests/test_mail_thread_internals.py | 65 +----- 2 files changed, 209 insertions(+), 65 deletions(-) diff --git a/addons/test_mail/tests/test_mail_multicompany.py b/addons/test_mail/tests/test_mail_multicompany.py index 657e2ecd740..393b3dfce7f 100644 --- a/addons/test_mail/tests/test_mail_multicompany.py +++ b/addons/test_mail/tests/test_mail_multicompany.py @@ -4,10 +4,14 @@ import socket from itertools import product +from unittest.mock import patch +from werkzeug.urls import url_parse, url_decode +from odoo.addons.mail.models.mail_message import Message from odoo.addons.test_mail.tests.common import TestMailCommon, TestRecipients +from odoo.exceptions import AccessError from odoo.tests import tagged -from odoo.tests.common import users +from odoo.tests.common import users, HttpCase from odoo.tools import formataddr, mute_logger @@ -26,6 +30,12 @@ class TestMultiCompanySetup(TestMailCommon, TestRecipients): 'name': 'Test', 'email_from': 'ignasse@example.com', }).with_context({}) + cls.test_records_mc = cls.env['mail.test.multi.company'].create([ + {'name': 'Test Company1', + 'company_id': cls.user_employee.company_id.id}, + {'name': 'Test Company2', + 'company_id': cls.user_employee_c2.company_id.id}, + ]) cls.partner_1 = cls.env['res.partner'].with_context(cls._test_context).create({ 'name': 'Valid Lelitre', @@ -49,6 +59,12 @@ class TestMultiCompanySetup(TestMailCommon, TestRecipients): 'message_id': '<123456-openerp-%s-mail.test.gateway@%s>' % (cls.test_record.id, socket.gethostname()), }) + def setUp(self): + super(TestMultiCompanySetup, self).setUp() + # patch registry to simulate a ready environment + self.patch(self.env.registry, 'ready', True) + self.flush_tracking() + @users('erp_manager') @mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') def test_mail_composer_wtpl_populate_new_recipient(self): @@ -159,3 +175,194 @@ class TestMultiCompanySetup(TestMailCommon, TestRecipients): "%s %s" % (self.user_employee_c2.company_id.name, test_record.name), "%s@%s" % (self.alias_catchall, self.alias_domain))) ) + + @users('employee_c2') + @mute_logger('odoo.addons.base.models.ir_rule') + def test_post_wo_access(self): + test_records_mc_c1, test_records_mc_c2 = self.test_records_mc.with_env(self.env) + attachments_data = [ + ('ReportLike1', 'AttContent1'), + ('ReportLike2', 'AttContent2'), + ] + + # ------------------------------------------------------------ + # Other company (no access) + # ------------------------------------------------------------ + + _original_car = Message.check_access_rule + with patch.object(Message, 'check_access_rule', + autospec=True, side_effect=_original_car) as mock_msg_car: + with self.assertRaises(AccessError): + test_records_mc_c1.message_post( + body='

Hello

', + message_type='comment', + record_name='CustomName', # avoid ACL on display_name + reply_to='custom.reply.to@test.example.com', # avoid ACL in notify_get_reply_to + subtype_xmlid='mail.mt_comment', + ) + self.assertEqual(mock_msg_car.call_count, 1, + 'Purpose is to raise at msg check access level') + with self.assertRaises(AccessError): + _name = test_records_mc_c1.name + + # no access to company1, access to post through being notified of parent + with self.assertRaises(AccessError): + _subject = test_records_mc_c1.message_ids.subject + self.assertEqual(len(self.test_records_mc[0].message_ids), 1) + initial_message = self.test_records_mc[0].message_ids + + self.env['mail.notification'].sudo().create({ + 'mail_message_id': initial_message.id, + 'notification_status': 'sent', + 'res_partner_id': self.user_employee_c2.partner_id.id, + }) + # additional: works only if in partner_ids, not notified via followers + initial_message.write({ + 'partner_ids': [(4, self.user_employee_c2.partner_id.id)], + }) + # now able to post as was notified of parent message + test_records_mc_c1.message_post( + body='

Hello

', + message_type='comment', + parent_id=initial_message.id, + record_name='CustomName', # avoid ACL on display_name + reply_to='custom.reply.to@test.example.com', # avoid ACL in notify_get_reply_to + subtype_xmlid='mail.mt_comment', + ) + + # now able to post as was notified of parent message + attachments = self.env['ir.attachment'].create( + self._generate_attachments_data( + 2, 'mail.compose.message', 0, + prefix='Other' + ) + ) + # record_name and reply_to may generate ACLs issues when computed by + # 'message_post' but should not, hence not specifying them to be sure + # testing the complete flow + test_records_mc_c1.message_post( + attachments=attachments_data, + attachment_ids=attachments.ids, + body='

Hello

', + message_type='comment', + parent_id=initial_message.id, + subtype_xmlid='mail.mt_comment', + ) + + # ------------------------------------------------------------ + # User company (access granted) + # ------------------------------------------------------------ + + # can effectively link attachments with message to record of writable record + attachments = self.env['ir.attachment'].create( + self._generate_attachments_data( + 2, 'mail.compose.message', 0, + prefix='Same' + ) + ) + message = test_records_mc_c2.message_post( + attachments=attachments_data, + attachment_ids=attachments.ids, + body='

Hello

', + message_type='comment', + record_name='CustomName', # avoid ACL on display_name + reply_to='custom.reply.to@test.example.com', # avoid ACL in notify_get_reply_to + subtype_xmlid='mail.mt_comment', + ) + self.assertTrue(attachments < message.attachment_ids) + self.assertEqual( + sorted(message.attachment_ids.mapped('name')), + sorted(['SameAttFileName_00.txt', 'SameAttFileName_01.txt', 'ReportLike1', 'ReportLike2']), + ) + self.assertEqual( + message.attachment_ids.mapped('res_id'), + [test_records_mc_c2.id] * 4, + ) + self.assertEqual( + message.attachment_ids.mapped('res_model'), + [test_records_mc_c2._name] * 4, + ) + + # cannot link attachments of unreachable records when posting on a document + # they can access (aka no access delegation through posting message) + attachments = self.env['ir.attachment'].sudo().create( + self._generate_attachments_data( + 1, + test_records_mc_c1._name, + test_records_mc_c1.id, + prefix='NoAccessMC' + ) + ) + with self.assertRaises(AccessError): + message = test_records_mc_c2.message_post( + attachments=attachments_data, + attachment_ids=attachments.ids, + body='

Hello

', + message_type='comment', + record_name='CustomName', # avoid ACL on display_name + reply_to='custom.reply.to@test.example.com', # avoid ACL in notify_get_reply_to + subtype_xmlid='mail.mt_comment', + ) + + +@tagged('-at_install', 'post_install', 'multi_company') +class TestMultiCompanyRedirect(TestMailCommon, HttpCase): + + @classmethod + def setUpClass(cls): + super(TestMultiCompanyRedirect, cls).setUpClass() + cls._activate_multi_company() + + def test_redirect_to_records(self): + """ Test mail/view redirection in MC environment, notably cids being + added in redirect if user has access to the record. """ + mc_record_c1, mc_record_c2 = self.env['mail.test.multi.company'].create([ + { + 'company_id': self.user_employee.company_id.id, + 'name': 'Multi Company Record', + }, + { + 'company_id': self.user_employee_c2.company_id.id, + 'name': 'Multi Company Record', + } + ]) + + for (login, password), mc_record in product( + ((None, None), # not logged: redirect to web/login + ('employee', 'employee'), # access only main company + ('admin', 'admin'), # access both companies + ), + (mc_record_c1, mc_record_c2), + ): + with self.subTest(login=login, mc_record=mc_record): + self.authenticate(login, password) + response = self.url_open( + f'/mail/view?model={mc_record._name}&res_id={mc_record.id}', + timeout=15 + ) + self.assertEqual(response.status_code, 200) + + if not login: + path = url_parse(response.url).path + self.assertEqual(path, '/web/login') + else: + user = self.env['res.users'].browse(self.session.uid) + self.assertEqual(user.login, login) + mc_error = login == 'employee' and mc_record == mc_record_c2 + if mc_error: + # Logged into company main, try accessing record in other + # company -> _redirect_to_record should redirect to + # messaging as the user doesn't have any access + fragment = url_parse(response.url).fragment + action = url_decode(fragment)['action'] + self.assertEqual(action, 'mail.action_discuss') + else: + # Logged into company main, try accessing record in same + # company -> _redirect_to_record should add company in + # allowed_company_ids + fragment = url_parse(response.url).fragment + cids = url_decode(fragment)['cids'] + if mc_record.company_id == user.company_id: + self.assertEqual(cids, f'{mc_record.company_id.id}') + else: + self.assertEqual(cids, f'{user.company_id.id},{mc_record.company_id.id}') diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py index 6b3ae5a8149..0f358a5cca9 100644 --- a/addons/test_mail/tests/test_mail_thread_internals.py +++ b/addons/test_mail/tests/test_mail_thread_internals.py @@ -3,12 +3,11 @@ from unittest.mock import patch from unittest.mock import DEFAULT -from werkzeug.urls import url_parse, url_decode from odoo import exceptions from odoo.addons.test_mail.models.test_mail_models import MailTestSimple from odoo.addons.test_mail.tests.common import TestMailCommon, TestRecipients -from odoo.tests.common import tagged, Form, HttpCase, users +from odoo.tests.common import tagged, Form, users from odoo.tools import mute_logger @@ -399,68 +398,6 @@ class TestDiscuss(TestMailCommon, TestRecipients): self.assertEqual(len(remaining_message), 0, "Test message should have been deleted") -@tagged('-at_install', 'post_install') -class TestMultiCompany(HttpCase): - - def test_redirect_to_records(self): - - self.company_A = self.env['res.company'].create({ - 'name': 'Company A', - 'user_ids': [(4, self.ref('base.user_admin'))], - }) - - self.company_B = self.env['res.company'].create({ - 'name': 'Company B', - }) - - self.multi_company_record = self.env['mail.test.multi.company'].create({ - 'name': 'Multi Company Record', - 'company_id': self.company_A.id, - }) - - # Test Case 0 - # Not logged, redirect to web/login - response = self.url_open('/mail/view?model=%s&res_id=%s' % ( - self.multi_company_record._name, - self.multi_company_record.id), timeout=15) - - path = url_parse(response.url).path - self.assertEqual(path, '/web/login') - - self.authenticate('admin', 'admin') - - # Test Case 1 - # Logged into company 1, try accessing record in company A - # _redirect_to_record should add company A in allowed_company_ids - response = self.url_open('/mail/view?model=%s&res_id=%s' % ( - self.multi_company_record._name, - self.multi_company_record.id), timeout=15) - - self.assertEqual(response.status_code, 200) - - fragment = url_parse(response.url).fragment - cids = url_decode(fragment)['cids'] - - self.assertEqual(cids, '1,%s' % (self.company_A.id)) - - # Test Case 2 - # Logged into company 1, try accessing record in company B - # _redirect_to_record should redirect to messaging as the user - # doesn't have any access for this company - self.multi_company_record.company_id = self.company_B - - response = self.url_open('/mail/view?model=%s&res_id=%s' % ( - self.multi_company_record._name, - self.multi_company_record.id), timeout=15) - - self.assertEqual(response.status_code, 200) - - fragment = url_parse(response.url).fragment - action = url_decode(fragment)['action'] - - self.assertEqual(action, 'mail.action_discuss') - - @tagged('mail_thread') class TestNoThread(TestMailCommon, TestRecipients): """ Specific tests for cross models thread features """