From 019c6e3b5cbaf3c2d417eab665395818a5e6e35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 12 Sep 2023 10:26:42 +0200 Subject: [PATCH] [IMP] mail: check alias_domain_id / company usage PURPOSE Allow alias domains to be multiple, notably to be used in a multi company environment where each company has its own alias domain. SPECIFICATIONS Now that alias domains are multiple and can be linked to companies it is easy to end up with configuration where an alias is using a domain of CompanyA while the owner and/or target record belongs to CompanyB. We want to avoid that situation and strengthen company separation. When changing alias domain of an alias check that it is not used in another company than the one define on * the owner record (using owner fields like the 'project.project' for 'project.task' creating aliases); * the target record (using update fields like the 'mail.group' for group aliases that routes emails to a specific group); If the new alias domain is linked to a company that is different from the company of any related record, raise an error as it could lead to invalid multi-company setup and record creation. If the new alias domain is different from the company domain but is not used in any company it is a valid configuration. It is just flexibility offered by multi-domains aliases. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/mail_alias.py | 73 +++++++++++++++++++++++ addons/test_mail/tests/test_mail_alias.py | 58 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index 40f39d05240..91288442b06 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -99,6 +99,79 @@ class Alias(models.Model): ON mail_alias (alias_name, COALESCE(alias_domain_id, 0)) """) + @api.constrains('alias_domain_id', 'alias_force_thread_id', 'alias_parent_model_id', + 'alias_parent_thread_id', 'alias_model_id') + def _check_alias_domain_id_mc(self): + """ Check for invalid alias domains based on company configuration. + When having a parent record and/or updating an existing record alias + domain should match the one used on the related record. """ + tocheck = self.filtered(lambda domain: domain.alias_domain_id.company_ids) + if not tocheck: + return + + # helpers to find owner / target models + def _owner_model(alias): + return alias.alias_parent_model_id.model + def _owner_env(alias): + return self.env[_owner_model(alias)] + def _target_model(alias): + return alias.alias_model_id.model + def _target_env(alias): + return self.env[_target_model(alias)] + + # fetch impacted records, classify by model + recs_by_model = defaultdict(list) + for alias in tocheck: + # owner record (like 'project.project' for aliases creating new 'project.task') + if alias.alias_parent_model_id and alias.alias_parent_thread_id: + if _owner_env(alias)._mail_get_company_field(): + recs_by_model[_owner_model(alias)].append(alias.alias_parent_thread_id) + # target record (like 'mail.group' updating a given group) + if alias.alias_model_id and alias.alias_force_thread_id: + if _target_env(alias)._mail_get_company_field(): + recs_by_model[_target_model(alias)].append(alias.alias_force_thread_id) + + # helpers to fetch owner / target with prefetching + def _fetch_owner(alias): + if alias.alias_parent_thread_id in recs_by_model[alias.alias_parent_model_id.model]: + return _owner_env(alias).with_prefetch( + recs_by_model[_owner_model(alias)] + ).browse(alias.alias_parent_thread_id) + return None + def _fetch_target(alias): + if alias.alias_force_thread_id in recs_by_model[alias.alias_model_id.model]: + return _target_env(alias).with_prefetch( + recs_by_model[_target_model(alias)] + ).browse(alias.alias_force_thread_id) + return None + + # check company domains are compatible + for alias in tocheck: + if owner := _fetch_owner(alias): + company = owner[owner._mail_get_company_field()] + if company and company.alias_domain_id != alias.alias_domain_id and alias.alias_domain_id.company_ids: + raise ValidationError(_( + "We could not create alias %(alias_name)s because domain " + + "%(alias_domain_name)s belongs to company %(alias_company_names)s " + + "while the owner document belongs to company %(company_name)s.", + alias_company_names=','.join(alias.alias_domain_id.company_ids.mapped('name')), + alias_domain_name=alias.alias_domain_id.name, + alias_name=alias.display_name, + company_name=company.name, + )) + if target := _fetch_target(alias): + company = target[target._mail_get_company_field()] + if company and company.alias_domain_id != alias.alias_domain_id and alias.alias_domain_id.company_ids: + raise ValidationError(_( + "We could not create alias %(alias_name)s because domain " + + "%(alias_domain_name)s belongs to company %(alias_company_names)s " + + "while the target document belongs to company %(company_name)s.", + alias_company_names=','.join(alias.alias_domain_id.company_ids.mapped('name')), + alias_domain_name=alias.alias_domain_id.name, + alias_name=alias.display_name, + company_name=company.name, + )) + @api.constrains('alias_name') def _check_alias_is_ascii(self): """ The local-part ("display-name" ) of an diff --git a/addons/test_mail/tests/test_mail_alias.py b/addons/test_mail/tests/test_mail_alias.py index dc12cd8b447..7a43359aca7 100644 --- a/addons/test_mail/tests/test_mail_alias.py +++ b/addons/test_mail/tests/test_mail_alias.py @@ -90,6 +90,64 @@ class TestMailAlias(TestMailAliasCommon): self.assertEqual(ICP.get_param('mail.bounce.alias'), 'bounce+double.test') self.assertEqual(ICP.get_param('mail.catchall.alias'), 'catchall+double.test') + @users('erp_manager') + def test_alias_domain_company_check(self): + """ Check constraint trying to avoid ill-defined company setup aka + having an alias domain on parent record / record to update that does + not match the alias domain. """ + misc_alias_domain = self.env['mail.alias.domain'].create({'name': 'misc.com'}) + record_mc_c1, record_mc_c2 = self.env['mail.test.container.mc'].create([ + { + 'alias_name': 'Test1', + 'company_id': self.company_admin.id, + }, { + 'alias_name': 'Test2', + 'company_id': self.company_2.id, + } + ]) + alias_mc_c1, alias_mc_c2 = record_mc_c1.alias_id, record_mc_c2.alias_id + self.assertEqual( + (alias_mc_c1 + alias_mc_c2).alias_parent_model_id, + self.env['ir.model']._get('mail.test.container.mc')) + self.assertEqual( + (alias_mc_c1 + alias_mc_c2).mapped('alias_parent_thread_id'), + (record_mc_c1 + record_mc_c2).ids) + self.assertEqual(alias_mc_c1.alias_domain_id, self.mail_alias_domain) + self.assertEqual(alias_mc_c2.alias_domain_id, self.mail_alias_domain_c2) + + # mail_alias_domain_c2 is linked to a conflicting company + with self.assertRaises(exceptions.ValidationError): + record_mc_c1.alias_domain_id = self.mail_alias_domain_c2 + with self.assertRaises(exceptions.ValidationError): + alias_mc_c1.sudo().alias_domain_id = self.mail_alias_domain_c2 + # misc_alias_domain is not linked to any company, therefore ok + record_mc_c1.alias_domain_id = misc_alias_domain + + # alias updating records + record_upd_c1, record_upd_c2 = self.env['mail.test.alias.optional'].sudo().create([ + { + 'alias_name': 'Update C1', + 'company_id': self.company_admin.id, + }, { + 'alias_name': 'Update C2', + 'company_id': self.company_2.id, + } + ]) + alias_update_c1, alias_update_c2 = record_upd_c1.alias_id, record_upd_c2.alias_id + self.assertEqual( + (alias_update_c1 + alias_update_c2).mapped('alias_force_thread_id'), + (record_upd_c1 + record_upd_c2).ids) + self.assertEqual(alias_update_c1.alias_domain_id, self.mail_alias_domain) + self.assertEqual(alias_update_c2.alias_domain_id, self.mail_alias_domain_c2) + + # mail_alias_domain_c2 is linked to a conflicting company + with self.assertRaises(exceptions.ValidationError): + record_upd_c1.alias_domain_id = self.mail_alias_domain_c2 + with self.assertRaises(exceptions.ValidationError): + alias_update_c1.sudo().alias_domain_id = self.mail_alias_domain_c2 + # misc_alias_domain is not linked to any company, therefore ok + record_upd_c1.alias_domain_id = misc_alias_domain + @users('admin') def test_alias_name_unique(self): """ Check uniqueness constraint on alias names, at create and update.