From 313c5c10cd7c0a49c9a78ca0b1f67b0d082f0d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 17 Jan 2024 14:41:46 +0000 Subject: [PATCH] [FIX] mail: do not crash when redirecting to non thread records '_redirect_to_record' is used notably when accessing 'mail/view' controller used notably with email notifications ('See Document' link). In order to try to avoid multi company issues a call to '_get_mail_redirect_suggested_company' is performed in order to find the record's preferred company. However it is defined on 'mail.thread' which means the redirect crashes if invoked using a non thread record. This is quite rare but may happen with models using a mailgateway implementation similar to 'mail.thread' without inheriting from the mixin (see 'mail.group' model added in Odoo 16). This limitation is present since a long time but is now more present since odoo/odoo@f4523fcd9219fd781a984dab11a0b8689e236b77 . Task-3689459 closes odoo/odoo#150309 X-original-commit: 9a366dcdb789d0f160871ee4d29a463e2641ca26 Signed-off-by: Thibault Delavallee (tde) --- addons/mail/controllers/mail.py | 6 +-- addons/mail/models/mail_thread.py | 14 ------ addons/mail/models/models.py | 18 ++++++++ .../test_mail/tests/test_mail_multicompany.py | 46 +++++++++++++++++++ 4 files changed, 67 insertions(+), 17 deletions(-) diff --git a/addons/mail/controllers/mail.py b/addons/mail/controllers/mail.py index b90d1f2e740..79eae7eb27f 100644 --- a/addons/mail/controllers/mail.py +++ b/addons/mail/controllers/mail.py @@ -49,7 +49,7 @@ class MailController(http.Controller): # to give access to the record to a recipient that has normally no access. uid = request.session.uid user = request.env['res.users'].sudo().browse(uid) - cids = False + cids = [] # no model / res_id, meaning no possible record -> redirect to login if not model or not res_id or model not in request.env: @@ -71,8 +71,8 @@ class MailController(http.Controller): # We need here to extend the "allowed_company_ids" to allow a redirection # to any record that the user can access, regardless of currently visible # records based on the "currently allowed companies". - cids = request.httprequest.cookies.get('cids', str(user.company_id.id)) - cids = [int(cid) for cid in cids.split(',')] + cids_str = request.httprequest.cookies.get('cids', str(user.company_id.id)) + cids = [int(cid) for cid in cids_str.split(',')] try: record_sudo.with_user(uid).with_context(allowed_company_ids=cids).check_access_rule('read') except AccessError: diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index d2e2ff22578..2467daa741d 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -4245,20 +4245,6 @@ class MailThread(models.AbstractModel): # CONTROLLERS # ------------------------------------------------------ - def _get_mail_redirect_suggested_company(self): - """ Return the suggested company to be set on the context - in case of a mail redirection to the record. To avoid multi - company issues when clicking on a link sent by email, this - could be called to try setting the most suited company on - the allowed_company_ids in the context. This method can be - overridden, for example on the hr.leave model, where the - most suited company is the company of the leave type, as - specified by the ir.rule. - """ - if 'company_id' in self: - return self.company_id - return False - def _get_mail_thread_data_attachments(self): self.ensure_one() return self.env['ir.attachment'].search([('res_id', '=', self.id), ('res_model', '=', self._name)], order='id desc') diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 8fd779de946..3da323376b8 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -404,3 +404,21 @@ class BaseModel(models.AbstractModel): self.ensure_one() return Markup("%s") % ( self._name, self.id, title or self.display_name) + + # ------------------------------------------------------ + # CONTROLLERS + # ------------------------------------------------------ + + def _get_mail_redirect_suggested_company(self): + """ Return the suggested company to be set on the context + in case of a mail redirection to the record. To avoid multi + company issues when clicking on a link sent by email, this + could be called to try setting the most suited company on + the allowed_company_ids in the context. This method can be + overridden, for example on the hr.leave model, where the + most suited company is the company of the leave type, as + specified by the ir.rule. + """ + if 'company_id' in self: + return self.company_id + return False diff --git a/addons/test_mail/tests/test_mail_multicompany.py b/addons/test_mail/tests/test_mail_multicompany.py index 0c82ab2e121..8cdcb3f2055 100644 --- a/addons/test_mail/tests/test_mail_multicompany.py +++ b/addons/test_mail/tests/test_mail_multicompany.py @@ -364,3 +364,49 @@ class TestMultiCompanyRedirect(MailCommon, HttpCase): self.assertEqual(cids, f'{mc_record.company_id.id}') else: self.assertEqual(cids, f'{user.company_id.id},{mc_record.company_id.id}') + + def test_redirect_to_records_nothread(self): + """ Test no thread models and redirection """ + nothreads = self.env['mail.test.nothread'].create([ + { + 'company_id': company.id, + 'name': f'Test with {company.name}', + } + for company in (self.company_admin, self.company_2, self.env['res.company']) + ]) + + # when being logged, cids should be based on current user's company unless + # there is an access issue (not tested here, see 'test_redirect_to_records') + self.authenticate(self.user_admin.login, self.user_admin.login) + for test_record in nothreads: + for user_company in self.company_admin, self.company_2: + with self.subTest(record_name=test_record.name, user_company=user_company): + self.user_admin.write({'company_id': user_company.id}) + response = self.url_open( + f'/mail/view?model={test_record._name}&res_id={test_record.id}', + timeout=15 + ) + self.assertEqual(response.status_code, 200) + + decoded_fragment = url_decode(url_parse(response.url).fragment) + self.assertTrue("cids" in decoded_fragment) + self.assertEqual(decoded_fragment['cids'], str(user_company.id)) + + # when being not logged, cids should be added based on + # '_get_mail_redirect_suggested_company' + self.authenticate(None, None) + for test_record in nothreads: + with self.subTest(record_name=test_record.name, user_company=user_company): + self.user_admin.write({'company_id': user_company.id}) + response = self.url_open( + f'/mail/view?model={test_record._name}&res_id={test_record.id}', + timeout=15 + ) + self.assertEqual(response.status_code, 200) + + decoded_fragment = url_decode(url_parse(response.url).fragment) + if test_record.company_id: + self.assertIn('cids', decoded_fragment) + self.assertEqual(decoded_fragment['cids'], str(test_record.company_id.id)) + else: + self.assertNotIn('cids', decoded_fragment)