From 17e5bbb38b6dbfbdfc13a07a875ddf3a7a2dc324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 8 Aug 2023 12:10:33 +0000 Subject: [PATCH 1/4] [IMP] base, mail: improve and add tests about ir mail server Prepares the move of ICP to mail before replacing them by dynamic alias domains. Improve test coverage, notably for edge cases. Continue to make tests more explicit after odoo/odoo#131492. Some tests are also merged to lessen number of different tests when possible, notably when only a test parameter differs (like giving an SMTP session or not). Clean ICP and mail servers setup in test classes allowing to remove some unnecessary extra initialization. Cleanup a mock in mail. Task-3453347 (Mail: Move Mail ICP from Base to Mail) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#130750 --- .../account/tests/test_account_move_send.py | 11 +- addons/mail/tests/common.py | 38 +- addons/test_mail/tests/test_mail_composer.py | 12 +- addons/test_mail/tests/test_mail_mail.py | 39 +- .../tests/test_mailing_server.py | 15 +- odoo/addons/base/tests/common.py | 44 +- odoo/addons/base/tests/test_ir_mail_server.py | 446 +++++++++--------- 7 files changed, 292 insertions(+), 313 deletions(-) diff --git a/addons/account/tests/test_account_move_send.py b/addons/account/tests/test_account_move_send.py index 32d489b4140..cafe09c5fe6 100644 --- a/addons/account/tests/test_account_move_send.py +++ b/addons/account/tests/test_account_move_send.py @@ -29,7 +29,7 @@ class TestAccountComposerPerformance(AccountTestInvoicingCommon, MailCommon): 'body_html': '

TemplateBody for

', 'description': 'Sent to customers with their invoices in attachment', 'email_from': "{{ (object.invoice_user_id.email_formatted or user.email_formatted) }}", - 'mail_server_id': cls.mail_server_global.id, + 'mail_server_id': cls.mail_server_default.id, 'model_id': cls.env['ir.model']._get_id('account.move'), 'name': "Invoice: Test Sending", 'partner_to': "{{ object.partner_id.id }}", @@ -124,9 +124,6 @@ class TestAccountComposerPerformance(AccountTestInvoicingCommon, MailCommon): def setUp(self): super().setUp() - # setup mail gateway to simulate complete reply-to computation - self._init_mail_gateway() - # patch registry to simulate a ready environment self.patch(self.env.registry, 'ready', True) self.flush_tracking() @@ -200,7 +197,7 @@ class TestAccountComposerPerformance(AccountTestInvoicingCommon, MailCommon): 'auto_delete': True, 'email_from': self.user_account_other.email_formatted, 'is_notification': True, # should keep logs by default - 'mail_server_id': self.mail_server_global, + 'mail_server_id': self.mail_server_default, 'subject': _exp_subject, 'reply_to': formataddr(( f'{move.company_id.name} {_exp_move_name}', @@ -286,7 +283,7 @@ class TestAccountComposerPerformance(AccountTestInvoicingCommon, MailCommon): 'auto_delete': True, 'email_from': self.user_account_other.email_formatted, 'is_notification': True, # should keep logs by default - 'mail_server_id': self.mail_server_global, + 'mail_server_id': self.mail_server_default, 'subject': f'{self.env.user.company_id.name} Invoice (Ref {test_move.name})', 'reply_to': formataddr(( f'{test_move.company_id.name} {test_move.display_name}', @@ -373,7 +370,7 @@ class TestAccountComposerPerformance(AccountTestInvoicingCommon, MailCommon): 'auto_delete': True, 'email_from': self.user_account_other.email_formatted, 'is_notification': True, # should keep logs by default - 'mail_server_id': self.mail_server_global, + 'mail_server_id': self.mail_server_default, 'subject': f'SpanishSubject for {test_move.name}', # translated version 'reply_to': formataddr(( f'{test_move.company_id.name} {test_move.display_name}', diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 48f01f3cf88..2be12eb3a25 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -52,10 +52,15 @@ class MockEmail(common.BaseCase, MockSmtplibCase): self.mail_unlink_sent = mail_unlink_sent self._init_mail_mock() - def _ir_mail_server_build_email(model, *args, **kwargs): - self._mails.append(kwargs) - self._mails_args.append(args) - return build_email_origin(model, *args, **kwargs) + def _ir_mail_server_build_email(model, email_from, email_to, subject, body, **kwargs): + self._mails.append({ + 'email_from': email_from, + 'email_to': email_to, + 'subject': subject, + 'body': body, + **kwargs, + }) + return build_email_origin(model, email_from, email_to, subject, body, **kwargs) def _mail_mail_create(model, *args, **kwargs): res = mail_create_origin(model, *args, **kwargs) @@ -79,40 +84,25 @@ class MockEmail(common.BaseCase, MockSmtplibCase): def _init_mail_mock(self): self._mails = [] - self._mails_args = [] self._new_mails = self.env['mail.mail'].sudo() @classmethod def _init_mail_gateway(cls): + super()._init_mail_gateway() cls.alias_domain = 'test.mycompany.com' cls.alias_catchall = 'catchall.test' cls.alias_bounce = 'bounce.test' - cls.default_from = 'notifications' + cls.default_from = 'notifications.test' + cls.default_from_filter = False cls.env['ir.config_parameter'].set_param('mail.bounce.alias', cls.alias_bounce) cls.env['ir.config_parameter'].set_param('mail.catchall.domain', cls.alias_domain) cls.env['ir.config_parameter'].set_param('mail.catchall.alias', cls.alias_catchall) cls.env['ir.config_parameter'].set_param('mail.default.from', cls.default_from) + cls.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', cls.default_from_filter) # mailer daemon email preformatting cls.mailer_daemon_email = formataddr(('MAILER-DAEMON', '%s@%s' % (cls.alias_bounce, cls.alias_domain))) - @classmethod - def _init_outgoing_gateway(cls): - cls.env['ir.mail_server'].search([]).unlink() - cls.mail_server_domain, cls.mail_server_global = cls.env['ir.mail_server'].create([ - {'from_filter': cls.alias_domain, - 'name': 'Domain Based Server', - 'smtp_encryption': 'none', - 'smtp_host': 'smtp_host', - }, - {'from_filter': False, - 'name': 'No FromFilter Server', - 'smtp_encryption': 'none', - 'smtp_host': 'smtp_host', - } - ]) - cls.mail_servers = cls.mail_server_domain + cls.mail_server_global - # ------------------------------------------------------------ # GATEWAY TOOLS # ------------------------------------------------------------ @@ -1106,7 +1096,7 @@ class MailCommon(common.TransactionCase, MailCase): # give default values for all email aliases and domain cls._init_mail_gateway() - cls._init_outgoing_gateway() + cls._init_mail_servers() # by default avoid rendering restriction complexity cls.env['ir.config_parameter'].set_param('mail.restrict.template.rendering', False) diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index e2a3917cc94..ee654188e71 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -768,14 +768,14 @@ class TestComposerInternals(TestMailComposer): # 1. check without template + template update composer = self.env['mail.compose.message'].with_context(ctx).create({ 'body': '

Test Body />

') - self.assertEqual(composer.mail_server_id, self.mail_server_global) + self.assertEqual(composer.mail_server_id, self.mail_server_default) if composition_mode == 'comment' and not batch: self.assertEqual(composer.record_name, self.test_record.name) else: @@ -804,13 +804,13 @@ class TestComposerInternals(TestMailComposer): # manual values is kept over template composer.write({ 'body': '

Back to my amazing body />

') - self.assertEqual(composer.mail_server_id, self.mail_server_global) + self.assertEqual(composer.mail_server_id, self.mail_server_default) self.assertEqual(composer.record_name, 'Manual update') self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}') self.assertEqual(composer.subject, 'Back to my amazing subject for {{ record.name }}') @@ -821,13 +821,13 @@ class TestComposerInternals(TestMailComposer): if composition_mode == 'comment' and not batch: self.assertEqual(composer.body, '

Back to my amazing body />

') - self.assertEqual(composer.mail_server_id, self.mail_server_global) + self.assertEqual(composer.mail_server_id, self.mail_server_default) self.assertEqual(composer.record_name, 'Manual update') self.assertEqual(composer.scheduled_date, '{{ datetime.datetime(2023, 1, 10, 10, 0, 0) }}') self.assertEqual(composer.subject, 'Back to my amazing subject for {{ record.name }}') diff --git a/addons/test_mail/tests/test_mail_mail.py b/addons/test_mail/tests/test_mail_mail.py index f9740306bd2..a7f729f0c15 100644 --- a/addons/test_mail/tests/test_mail_mail.py +++ b/addons/test_mail/tests/test_mail_mail.py @@ -26,7 +26,6 @@ class TestMailMail(MailCommon): @classmethod def setUpClass(cls): super(TestMailMail, cls).setUpClass() - cls._init_mail_servers() cls.test_record = cls.env['mail.test.gateway'].with_context(cls._test_context).create({ 'name': 'Test', @@ -359,9 +358,6 @@ class TestMailMail(MailCommon): managed and stored at mail and notification level. """ mail, notification = self.test_mail, self.test_notification - self.env['ir.config_parameter'].set_param('mail.catchall.domain', self.alias_domain) - self.env['ir.config_parameter'].set_param('mail.default.from', self.default_from) - # MailServer.send_email(): _prepare_email_message: missing To for email_to in self.emails_falsy: self._reset_data() @@ -660,9 +656,8 @@ class TestMailMailServer(MailCommon): @classmethod def setUpClass(cls): super().setUpClass() - cls._init_mail_servers() - cls.server_domain_2 = cls.env['ir.mail_server'].create({ + cls.mail_server_domain_2 = cls.env['ir.mail_server'].create({ 'from_filter': 'test_2.com', 'name': 'Server 2', 'smtp_host': 'test_2.com', @@ -676,7 +671,7 @@ class TestMailMailServer(MailCommon): """ self.assertEqual( self.env['ir.mail_server']._get_default_from_address(), - f'notifications@{self.alias_domain}' + f'{self.default_from}@{self.alias_domain}' ) mail_values = { @@ -696,19 +691,19 @@ class TestMailMailServer(MailCommon): # Should use the test_2 mail server # Once with "user_1@test_2.com" as login # Once with "user_2@test_2.com" as login - mails |= self.env['mail.mail'].create([{ + mails += self.env['mail.mail'].create([{ **mail_values, 'email_from': 'user_1@test_2.com', - } for _ in range(5)]) | self.env['mail.mail'].create([{ + } for _ in range(5)]) + self.env['mail.mail'].create([{ **mail_values, 'email_from': 'user_2@test_2.com', } for _ in range(5)]) # Mail server is forced - mails |= self.env['mail.mail'].create([{ + mails += self.env['mail.mail'].create([{ **mail_values, 'email_from': 'user_1@test_2.com', - 'mail_server_id': self.server_domain.id, + 'mail_server_id': self.mail_server_domain.id, } for _ in range(5)]) with self.mock_smtplib_connection(): @@ -722,21 +717,21 @@ class TestMailMailServer(MailCommon): self.assertEqual(self.connect_mocked.call_count, 4, 'Must be called once per batch which share the same mail server and the same smtp from') self.connect_mocked.assert_has_calls( calls=[ - call(smtp_from=f'notifications@{self.alias_domain}', mail_server_id=self.server_notification.id), - call(smtp_from='user_1@test_2.com', mail_server_id=self.server_domain_2.id), - call(smtp_from='user_2@test_2.com', mail_server_id=self.server_domain_2.id), - call(smtp_from='user_1@test_2.com', mail_server_id=self.server_domain.id), + call(smtp_from=f'{self.default_from}@{self.alias_domain}', mail_server_id=self.mail_server_notification.id), + call(smtp_from='user_1@test_2.com', mail_server_id=self.mail_server_domain_2.id), + call(smtp_from='user_2@test_2.com', mail_server_id=self.mail_server_domain_2.id), + call(smtp_from='user_1@test_2.com', mail_server_id=self.mail_server_domain.id), ], any_order=True, ) - self.assertSMTPEmailsSent(message_from=f'"test" ', - emails_count=5, from_filter=self.server_notification.from_filter) - self.assertSMTPEmailsSent(message_from=f'"test_2" ', - emails_count=5, from_filter=self.server_notification.from_filter) - self.assertSMTPEmailsSent(message_from='user_1@test_2.com', emails_count=5, from_filter=self.server_domain_2.from_filter) - self.assertSMTPEmailsSent(message_from='user_2@test_2.com', emails_count=5, from_filter=self.server_domain_2.from_filter) - self.assertSMTPEmailsSent(message_from='user_1@test_2.com', emails_count=5, from_filter=self.server_domain.from_filter) + self.assertSMTPEmailsSent(message_from=f'"test" <{self.default_from}@{self.alias_domain}>', + emails_count=5, from_filter=self.mail_server_notification.from_filter) + self.assertSMTPEmailsSent(message_from=f'"test_2" <{self.default_from}@{self.alias_domain}>', + emails_count=5, from_filter=self.mail_server_notification.from_filter) + self.assertSMTPEmailsSent(message_from='user_1@test_2.com', emails_count=5, mail_server=self.mail_server_domain_2) + self.assertSMTPEmailsSent(message_from='user_2@test_2.com', emails_count=5, mail_server=self.mail_server_domain_2) + self.assertSMTPEmailsSent(message_from='user_1@test_2.com', emails_count=5, mail_server=self.mail_server_domain) @tagged('mail_mail') diff --git a/addons/test_mass_mailing/tests/test_mailing_server.py b/addons/test_mass_mailing/tests/test_mailing_server.py index b533d26db9e..fda438bf015 100644 --- a/addons/test_mass_mailing/tests/test_mailing_server.py +++ b/addons/test_mass_mailing/tests/test_mailing_server.py @@ -14,7 +14,6 @@ class TestMassMailingServer(TestMassMailCommon): @classmethod def setUpClass(cls): super(TestMassMailingServer, cls).setUpClass() - cls._init_mail_servers() cls.recipients = cls._create_mailing_test_records(model='mailing.test.optout', count=8) def test_mass_mailing_server_archived_usage_protection(self): @@ -98,23 +97,23 @@ class TestMassMailingServer(TestMassMailCommon): 'body_html': 'Body for ', 'email_from': 'unknow_email@unknow_domain.com', 'mailing_model_id': self.env['ir.model']._get_id('mailing.test.optout'), - 'mail_server_id': self.server_notification.id, + 'mail_server_id': self.mail_server_notification.id, }]) with self.mock_smtplib_connection(): mailings.action_send_mail() self.assertEqual(self.find_mail_server_mocked.call_count, 3, 'Must be called only once per mail from except when forced') - for (expected_smtp_from, expected_msg_from, expected_from_filter) in [ - ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', self.server_user.from_filter), - (f'{self.alias_bounce}@{self.alias_domain}', 'unknown_name@test.mycompany.com', self.server_domain.from_filter), + for (expected_smtp_from, expected_msg_from, expected_mail_server) in [ + ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', self.mail_server_user), + (f'{self.alias_bounce}@{self.alias_domain}', 'unknown_name@test.mycompany.com', self.mail_server_domain), # We do not have a mail server for this address email, so fall back to the "notifications@domain" email. - ('notifications@test.mycompany.com', '"Testing" ', self.server_notification.from_filter), + (f'{self.default_from}@{self.alias_domain}', f'"Testing" <{self.default_from}@{self.alias_domain}>', self.mail_server_notification), # forced sever - ('unknow_email@unknow_domain.com', 'unknow_email@unknow_domain.com', self.server_notification.from_filter), + ('unknow_email@unknow_domain.com', 'unknow_email@unknow_domain.com', self.mail_server_notification), ]: self.assertSMTPEmailsSent( smtp_from=expected_smtp_from, message_from=expected_msg_from, - from_filter=expected_from_filter, + mail_server=expected_mail_server, emails_count=8, ) diff --git a/odoo/addons/base/tests/common.py b/odoo/addons/base/tests/common.py index b2411e0eda4..ffe3649861f 100644 --- a/odoo/addons/base/tests/common.py +++ b/odoo/addons/base/tests/common.py @@ -341,11 +341,11 @@ class MockSmtplibCase: def _build_email(self, mail_from, return_path=None, **kwargs): return self.env['ir.mail_server'].build_email( - body=kwargs.pop('body', 'body'), - email_from=mail_from, - email_to='dest@example-é.com', + mail_from, + kwargs.pop('email_to', 'dest@example-é.com'), + kwargs.pop('subject', 'subject'), + kwargs.pop('body', 'body'), headers={'Return-Path': return_path} if return_path else None, - subject='subject', **kwargs, ) @@ -354,19 +354,26 @@ class MockSmtplibCase: self.env['ir.mail_server'].send_email(msg, smtp_session=smtp_session) return smtp_session.messages.pop() - def assertSMTPEmailsSent(self, smtp_from=None, smtp_to_list=None, message_from=None, from_filter=None, emails_count=1): - """Check that the given email has been sent. - - If one of the parameter is None, it's just ignored and not used to retrieve the email. + def assertSMTPEmailsSent(self, smtp_from=None, smtp_to_list=None, message_from=None, + mail_server=None, from_filter=None, + emails_count=1): + """Check that the given email has been sent. If one of the parameter is + None it is just ignored and not used to retrieve the email. :param smtp_from: FROM used for the authentication to the mail server :param smtp_to_list: List of destination email address :param message_from: FROM used in the SMTP headers - :param from_filter: from_filter of the used to send the email - Can use a lambda to check the value + :arap mail_server: used to compare the 'from_filter' as an alternative + to using the from_filter parameter + :param from_filter: from_filter of the used to send the + email. False means 'match everything';' :param emails_count: the number of emails which should match the condition :return: True if at least one email has been found with those parameters """ + if from_filter is not None and mail_server: + raise ValueError('Invalid usage: use either from_filter either mail_server') + if from_filter is None and mail_server is not None: + from_filter = mail_server.from_filter matching_emails = filter( lambda email: (smtp_from is None or smtp_from == email['smtp_from']) @@ -391,10 +398,10 @@ class MockSmtplibCase: ) @classmethod - def _init_mail_config(cls): + def _init_mail_gateway(cls): cls.alias_bounce = 'bounce.test' cls.alias_domain = 'test.mycompany.com' - cls.default_from = 'notifications' + cls.default_from = 'notifications.test' cls.default_from_filter = False cls.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', cls.alias_domain) cls.env['ir.config_parameter'].sudo().set_param('mail.default.from', cls.default_from) @@ -409,12 +416,7 @@ class MockSmtplibCase: 'smtp_host': 'smtp_host', 'smtp_encryption': 'none', } - ( - cls.server_domain, - cls.server_user, - cls.server_notification, - cls.server_default, - ) = cls.env['ir.mail_server'].create([ + cls.mail_servers = cls.env['ir.mail_server'].create([ { 'name': 'Domain based server', 'from_filter': 'test.mycompany.com', @@ -425,7 +427,7 @@ class MockSmtplibCase: ** ir_mail_server_values, }, { 'name': 'Server Notifications', - 'from_filter': 'notifications@test.mycompany.com', + 'from_filter': 'notifications.test@test.mycompany.com', ** ir_mail_server_values, }, { 'name': 'Server No From Filter', @@ -433,3 +435,7 @@ class MockSmtplibCase: ** ir_mail_server_values, }, ]) + ( + cls.mail_server_domain, cls.mail_server_user, + cls.mail_server_notification, cls.mail_server_default + ) = cls.mail_servers diff --git a/odoo/addons/base/tests/test_ir_mail_server.py b/odoo/addons/base/tests/test_ir_mail_server.py index 9d97f6e640b..ad822e4ae1b 100644 --- a/odoo/addons/base/tests/test_ir_mail_server.py +++ b/odoo/addons/base/tests/test_ir_mail_server.py @@ -3,14 +3,13 @@ import email.message import email.policy -import itertools from unittest.mock import patch from odoo import tools from odoo.addons.base.tests import test_mail_examples from odoo.addons.base.tests.common import MockSmtplibCase -from odoo.tests import tagged +from odoo.tests import tagged, users from odoo.tests.common import TransactionCase from odoo.tools import mute_logger from odoo.tools import config @@ -43,10 +42,14 @@ class EmailConfigCase(TransactionCase): for (icp_from, icp_domain), expected_from in zip( [ ('icp', 'test.mycompany.com'), + ('icp', False), ('icp@another.company.com', 'test.mycompany.com'), + ('icp@another.company.com', False), (False, 'test.mycompany.com'), ], [ "icp@test.mycompany.com", + "settings@example.com", + "icp@another.company.com", "icp@another.company.com", "settings@example.com", ], @@ -67,7 +70,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @classmethod def setUpClass(cls): super().setUpClass() - cls._init_mail_config() + cls._init_mail_gateway() cls._init_mail_servers() cls.default_bounce_address = f'{cls.alias_bounce}@{cls.alias_domain}' @@ -124,35 +127,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): "There should be 3 headers MIME-Version: one on the enveloppe, " "one on the html part, one on the text part") - def test_mail_server_match_from_filter(self): - """ Test the from_filter field on the "ir.mail_server". """ - # Should match - tests = [ - ('admin@mail.example.com', 'mail.example.com'), - ('admin@mail.example.com', 'mail.EXAMPLE.com'), - ('admin@mail.example.com', 'admin@mail.example.com'), - ('admin@mail.example.com', False), - ('"fake@test.mycompany.com" ', 'mail.example.com'), - ('"fake@test.mycompany.com" ', 'mail.example.com'), - ('"fake@test.mycompany.com" ', 'test.mycompany.com, mail.example.com, test2.com'), - ] - for email, from_filter in tests: - self.assertTrue(self.env['ir.mail_server']._match_from_filter(email, from_filter)) - - # Should not match - tests = [ - ('admin@mail.example.com', 'test@mail.example.com'), - ('admin@mail.example.com', 'test.mycompany.com'), - ('admin@mail.example.com', 'mail.éxample.com'), - ('admin@mmail.example.com', 'mail.example.com'), - ('admin@mail.example.com', 'mmail.example.com'), - ('"admin@mail.example.com" ', 'mail.example.com'), - ('"fake@test.mycompany.com" ', 'test.mycompany.com, wrong.mail.example.com, test3.com'), - ] - for email, from_filter in tests: - self.assertFalse(self.env['ir.mail_server']._match_from_filter(email, from_filter)) - - def test_mail_body(self): + def test_content_mail_body(self): bodies = [ 'content', '

content

', @@ -187,6 +162,34 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): body_alternative = body_alternative.strip('\n') self.assertEqual(body_alternative, expected) + def test_mail_server_match_from_filter(self): + """ Test the from_filter field on the "ir.mail_server". """ + # Should match + tests = [ + ('admin@mail.example.com', 'mail.example.com'), + ('admin@mail.example.com', 'mail.EXAMPLE.com'), + ('admin@mail.example.com', 'admin@mail.example.com'), + ('admin@mail.example.com', False), + ('"fake@test.mycompany.com" ', 'mail.example.com'), + ('"fake@test.mycompany.com" ', 'mail.example.com'), + ('"fake@test.mycompany.com" ', 'test.mycompany.com, mail.example.com, test2.com'), + ] + for email, from_filter in tests: + self.assertTrue(self.env['ir.mail_server']._match_from_filter(email, from_filter)) + + # Should not match + tests = [ + ('admin@mail.example.com', 'test@mail.example.com'), + ('admin@mail.example.com', 'test.mycompany.com'), + ('admin@mail.example.com', 'mail.éxample.com'), + ('admin@mmail.example.com', 'mail.example.com'), + ('admin@mail.example.com', 'mmail.example.com'), + ('"admin@mail.example.com" ', 'mail.example.com'), + ('"fake@test.mycompany.com" ', 'test.mycompany.com, wrong.mail.example.com, test3.com'), + ] + for email, from_filter in tests: + self.assertFalse(self.env['ir.mail_server']._match_from_filter(email, from_filter)) + @mute_logger('odoo.models.unlink') def test_mail_server_priorities(self): """ Test if we choose the right mail server to send an email. @@ -207,41 +210,40 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): self.assertTrue(self.env['ir.mail_server']._get_default_from_address(), 'Notifications email must be set for testing') self.assertTrue(self.env['ir.mail_server']._get_default_bounce_address(), 'Bounce email must be set for testing') # this mail server can not be used for a specific email address and 2 domain names - self.server_user.from_filter = "domain1.com, specific_user@test.mycompany.com, domain2.com" + self.mail_server_user.from_filter = "domain1.com, specific_user@test.mycompany.com, domain2.com" for email_from, (expected_mail_server, expected_email_from) in zip( [ + # matches user-specific server 'specific_user@test.mycompany.com', + # matches user-specific server (with formatting') -> should extract + # email from full name, must keep the given email_from '"Name name@strange.name" ', - 'specific_user@test.mycompany.com', + # case check + 'SPECIFIC_USER@test.mycompany.com', + 'specific_user@test.MYCOMPANY.com', + # matches domain-based server: domain is case insensitive 'unknown_email@test.mycompany.com', 'unknown_email@TEST.MYCOMPANY.COM', + '"Unknown" ', + # fallback on notification email '"Test" ', - # server_user multiple from filter check - '"Test" ', + # mail_server_user multiple from_filter check: can be used for a + # specific email and 2 domain names -> check other domains in filter '"Example" ', '"Example" ', - '"Test" ', ], [ - (self.server_user, 'specific_user@test.mycompany.com'), - # must extract email from full name, must keep the given email from - (self.server_user, '"Name name@strange.name" '), - # should not be case sensitive - (self.server_user, 'specific_user@test.mycompany.com'), - (self.server_domain, 'unknown_email@test.mycompany.com'), - # cover a different condition that the "email case insensitive" test: domain is case insensitive - (self.server_domain, 'unknown_email@TEST.MYCOMPANY.COM'), - # should take the notification email - (self.server_notification, 'notifications@test.mycompany.com'), - # server_user multipl from_filter check: can be used for a sepcific emial and 2 domain names - # 1. entire email matches, server should be used - (self.server_user, '"Test" '), - # 2. domain matches, server should be used - (self.server_user, '"Example" '), - # 3. other domain matches, server shold be used - (self.server_user, '"Example" '), - # check that we take the domain server and not the "user specific" server - (self.server_domain, '"Test" '), + (self.mail_server_user, 'specific_user@test.mycompany.com'), + (self.mail_server_user, '"Name name@strange.name" '), + (self.mail_server_user, 'SPECIFIC_USER@test.mycompany.com'), + (self.mail_server_user, 'specific_user@test.MYCOMPANY.com'), + (self.mail_server_domain, 'unknown_email@test.mycompany.com'), + (self.mail_server_domain, 'unknown_email@TEST.MYCOMPANY.COM'), + (self.mail_server_domain, '"Unknown" '), + (self.mail_server_notification, 'notifications.test@test.mycompany.com'), + # mail_server_user multiple from_filter check + (self.mail_server_user, '"Example" '), + (self.mail_server_user, '"Example" '), ], ): with self.subTest(email_from=email_from): @@ -256,70 +258,76 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): the FROM otherwise we cannot send any email. """ self.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', False) self.env['ir.config_parameter'].sudo().set_param('mail.default.from', False) - for email_from, (expected_mail_server, expected_from_filter, expected_email_from) in zip( + + for email_from, (expected_mail_server, expected_email_from) in zip( [ - 'test@unknown_domain.com', + 'specific_user@test.mycompany.com', + 'unknown_email@test.mycompany.com', + # no notification set, must be forced to spoof the FROM + '"Test" ', ], [ - (self.server_default, False, 'test@unknown_domain.com'), + (self.mail_server_user, 'specific_user@test.mycompany.com'), + (self.mail_server_domain, 'unknown_email@test.mycompany.com'), + (self.mail_server_default, '"Test" '), ], ): with self.subTest(email_from=email_from): - mail_server, mail_from = self.env['ir.mail_server']._find_mail_server(email_from='test@unknown_domain.com') - self.assertEqual(mail_server.from_filter, expected_from_filter, 'No notifications email set, must be forced to spoof the FROM') + mail_server, mail_from = self.env['ir.mail_server']._find_mail_server(email_from=email_from) self.assertEqual(mail_server, expected_mail_server) self.assertEqual(mail_from, expected_email_from) @mute_logger('odoo.models.unlink') def test_mail_server_send_email(self): + """ Test main 'send_email' usage: check mail_server choice based on from + filters, encapsulation, spoofing. """ IrMailServer = self.env['ir.mail_server'] - for (mail_from, (expected_smtp_from, expected_msg_from, expected_from_filter)), provide_smtp in itertools.product( - zip( - [ - 'specific_user@test.mycompany.com', - '"Name" ', - 'test@unknown_domain.com', - 'unknown_name@test.mycompany.com' - ], [ - # A mail server is configured for the email - ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', 'specific_user@test.mycompany.com'), - # No mail server are configured for the email address, so it will use the - # notifications email instead and encapsulate the old email - ('notifications@test.mycompany.com', '"Name" ', 'notifications@test.mycompany.com'), - # same situation, but the original email has no name part - ('notifications@test.mycompany.com', '"test" ', 'notifications@test.mycompany.com'), - # A mail server is configured for the entire domain name, so we can use the bounce - # email address because the mail server supports it - (self.default_bounce_address, 'unknown_name@test.mycompany.com', 'test.mycompany.com'), - ], - ), - [False, True], # provide SMTP session or not: should not impact sending + for mail_from, (expected_smtp_from, expected_msg_from, expected_mail_server) in zip( + [ + 'specific_user@test.mycompany.com', + '"Name" ', + 'test@unknown_domain.com', + '"Name" ' + ], [ + # A mail server is configured for the email + ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', self.mail_server_user), + # No mail server are configured for the email address, so it will use the + # notifications email instead and encapsulate the old email + ('notifications.test@test.mycompany.com', '"Name" ', self.mail_server_notification), + # same situation, but the original email has no name part + ('notifications.test@test.mycompany.com', '"test" ', self.mail_server_notification), + # A mail server is configured for the entire domain name, so we can use the bounce + # email address because the mail server supports it + (self.default_bounce_address, '"Name" ', self.mail_server_domain), + ] ): - with self.subTest(mail_from=mail_from): - with self.mock_smtplib_connection(): - if provide_smtp: - smtp_session = IrMailServer.connect(smtp_from=mail_from) - message = self._build_email(mail_from=mail_from) - IrMailServer.send_email(message, smtp_session=smtp_session) - else: - message = self._build_email(mail_from=mail_from) - IrMailServer.send_email(message) + # test with and without providing an SMTP session, which should not impact test + for provide_smtp in [False, True]: + with self.subTest(mail_from=mail_from, provide_smtp=provide_smtp): + with self.mock_smtplib_connection(): + if provide_smtp: + smtp_session = IrMailServer.connect(smtp_from=mail_from) + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message, smtp_session=smtp_session) + else: + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message) - self.connect_mocked.assert_called_once() - self.assertEqual(len(self.emails), 1) - self.assertSMTPEmailsSent( - smtp_from=expected_smtp_from, - message_from=expected_msg_from, - from_filter=expected_from_filter, - ) + self.connect_mocked.assert_called_once() + self.assertEqual(len(self.emails), 1) + self.assertSMTPEmailsSent( + smtp_from=expected_smtp_from, + message_from=expected_msg_from, + mail_server=expected_mail_server, + ) # remove the notification server - # so will use the mail server + # so will use the mail server # The mail server configured for the notifications email has been removed # but we can still use the mail server configured for test.mycompany.com # and so we will be able to use the bounce address # because we use the mail server for "test.mycompany.com" - self.server_notification.unlink() + self.mail_server_notification.unlink() for provide_smtp in [False, True]: with self.mock_smtplib_connection(): if provide_smtp: @@ -334,13 +342,13 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): self.assertEqual(len(self.emails), 1) self.assertSMTPEmailsSent( smtp_from=self.default_bounce_address, - message_from='"Name" ', - from_filter='test.mycompany.com', + message_from='"Name" ', + mail_server=self.mail_server_domain, ) @mute_logger('odoo.models.unlink') def test_mail_server_send_email_IDNA(self): - # Test that the mail from / recipient envelop are encoded using IDNA + """ Test that the mail from / recipient envelop are encoded using IDNA """ self.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', 'ééééééé.com') with self.mock_smtplib_connection(): message = self._build_email(mail_from='test@ééééééé.com') @@ -356,21 +364,30 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @mute_logger('odoo.models.unlink') def test_mail_server_send_email_default_from(self): - # Test the case when the "mail.default.from" contains a full email address and not just the local part - # the domain of this default email address can be different than the catchall domain - self.env['ir.config_parameter'].sudo().set_param('mail.default.from', 'test@custom_domain.com') - _custom_server = self.env['ir.mail_server'].create({ + """ Test the case when the "mail.default.from" contains a full email address + and not just the local part the domain of this default email address can be + different than the catchall domain """ + other_default_from = 'notifications.other.test@custom_domain.com' + self.env['ir.config_parameter'].sudo().set_param('mail.default.from', other_default_from) + custom_server = self.env['ir.mail_server'].create({ 'from_filter': 'custom_domain.com', 'name': 'Custom Domain Server', 'smtp_host': 'smtp_host', 'smtp_encryption': 'none', }) - for mail_from, (expected_smtp_from, expected_msg_from, expected_from_filter) in zip( + for mail_from, (expected_smtp_from, expected_msg_from, expected_mail_server) in zip( [ - '"Name" ', + # custom server domain + '"Custom Server Name" ', + # unknown domain + '"Name" ', + # notification server domain + '"Name" ' ], [ - ('test@custom_domain.com', '"Name" ', 'custom_domain.com'), + ('customer.test@custom_domain.com', '"Custom Server Name" ', custom_server), + (other_default_from, '"Name" ', custom_server), + (self.default_bounce_address, '"Name" ', self.mail_server_domain), ], ): with self.subTest(mail_from=mail_from): @@ -382,116 +399,85 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): smtp_from=expected_smtp_from, smtp_to_list=['dest@xn--example--i1a.com'], message_from=expected_msg_from, - from_filter=expected_from_filter, + mail_server=expected_mail_server, ) - @mute_logger('odoo.models.unlink') - @patch.dict(config.options, {"from_filter": "test.mycompany.com", "smtp_server": "example.com"}) - def test_mail_server_config_bin(self): - """Test the configuration provided in the odoo-bin arguments. - - This config is used when no mail server exists. - """ - IrMailServer = self.env['ir.mail_server'] - - # Remove all mail server so we will use the odoo-bin arguments - self.env['ir.mail_server'].search([]).unlink() - self.assertFalse(self.env['ir.mail_server'].search([])) - - for email_from, (expected_smtp_from, expected_msg_from) in zip( - [ - # inside "from_filter" domain - 'specific_user@test.mycompany.com', - # outside "from_filter" domain - 'test@unknown_domain.com', - ], [ - (self.default_bounce_address, 'specific_user@test.mycompany.com'), - # outside "from_filter" domain: we will use notifications emails in the - # headers, and bounce address in the envelope because the "from_filter" - # allows to use the entire domain - (self.default_bounce_address, '"test" '), - ] - ): - with self.subTest(email_from=email_from): - with self.mock_smtplib_connection(): - message = self._build_email(mail_from=email_from) - IrMailServer.send_email(message) - - self.connect_mocked.assert_called_once() - self.assertSMTPEmailsSent( - smtp_from=expected_smtp_from, - message_from=expected_msg_from, - from_filter='test.mycompany.com', - ) - - @mute_logger('odoo.models.unlink') - @patch.dict(config.options, {'from_filter': 'test.mycompany.com', 'smtp_server': 'example.com'}) - def test_mail_server_config_bin_default_from_filter(self): - """Test that the config parameter "mail.default.from_filter" overwrites - the odoo-bin argument "--from-filter" """ - self.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', 'example.com') - - IrMailServer = self.env['ir.mail_server'] - - # Remove all mail server so we will use the odoo-bin arguments - IrMailServer.search([]).unlink() - self.assertFalse(IrMailServer.search([])) - - # Use an email in the domain of the config parameter "mail.default.from_filter" - with self.mock_smtplib_connection(): - message = self._build_email(mail_from='specific_user@example.com') - IrMailServer.send_email(message) - - self.assertSMTPEmailsSent( - smtp_from='specific_user@example.com', - message_from='specific_user@example.com', - from_filter='example.com', - ) - @mute_logger('odoo.models.unlink') @patch.dict(config.options, { "from_filter": "dummy@example.com, test.mycompany.com, dummy2@example.com", "smtp_server": "example.com", }) - def test_mail_server_config_bin_smtp_session(self): - """Test the configuration provided in the odoo-bin arguments. + def test_mail_server_config_bin(self): + """ Test the configuration provided in the odoo-bin arguments. This config + is used when no mail server exists. Test with and without giving a + pre-configured SMTP session, should not impact results. - This config is used when no mail server exists. - Use a pre-configured SMTP session. + Also check "mail.default.from_filter" parameter usage that should overwrite + odoo-bin argument "--from-filter". """ IrMailServer = self.env['ir.mail_server'] # Remove all mail server so we will use the odoo-bin arguments - self.env['ir.mail_server'].search([]).unlink() - self.assertFalse(self.env['ir.mail_server'].search([])) + IrMailServer.search([]).unlink() + self.assertFalse(IrMailServer.search([])) - for email_from, (expected_smtp_from, expected_msg_from) in zip( + for mail_from, (expected_smtp_from, expected_msg_from) in zip( [ - # use an email in the domain of the "from_filter" + # inside "from_filter" domain 'specific_user@test.mycompany.com', - # Use an email outside of the domain of the "from_filter" + '"Formatted Name" ', + '"Formatted Name" ', + '"Formatted Name" ', + # outside "from_filter" domain 'test@unknown_domain.com', + '"Formatted Name" ', ], [ + # inside "from_filter" domain: no rewriting (self.default_bounce_address, 'specific_user@test.mycompany.com'), - # Use an email outside of the domain of the "from_filter" - # So we will use the notifications email in the headers and the bounce address - # in the envelop because the "from_filter" allows to use the entire domain - (self.default_bounce_address, '"test" ') + (self.default_bounce_address, '"Formatted Name" '), + (self.default_bounce_address, '"Formatted Name" '), + (self.default_bounce_address, '"Formatted Name" '), + # outside "from_filter" domain: we will use notifications emails in the + # headers, and bounce address in the envelope because the "from_filter" + # allows to use the entire domain + (self.default_bounce_address, '"test" '), + (self.default_bounce_address, '"Formatted Name" '), ] ): - with self.subTest(email_from=email_from): - with self.mock_smtplib_connection(): - smtp_session = IrMailServer.connect(smtp_from=email_from) - message = self._build_email(mail_from=email_from) - IrMailServer.send_email(message, smtp_session=smtp_session) + for provide_smtp in [False, True]: # providing smtp session should ont impact test + with self.subTest(mail_from=mail_from, provide_smtp=provide_smtp): + with self.mock_smtplib_connection(): + if provide_smtp: + smtp_session = IrMailServer.connect(smtp_from=mail_from) + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message, smtp_session=smtp_session) + else: + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message) - self.connect_mocked.assert_called_once() - self.assertSMTPEmailsSent( - smtp_from=expected_smtp_from, - message_from=expected_msg_from, - from_filter='dummy@example.com, test.mycompany.com, dummy2@example.com', - ) + self.connect_mocked.assert_called_once() + self.assertEqual(len(self.emails), 1) + self.assertSMTPEmailsSent( + smtp_from=expected_smtp_from, + message_from=expected_msg_from, + from_filter="dummy@example.com, test.mycompany.com, dummy2@example.com", + ) + # for from_filter in ICP, overwrite the one from odoo-bin + self.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', 'icp.example.com') + + # Use an email in the domain of the config parameter "mail.default.from_filter" + with self.mock_smtplib_connection(): + message = self._build_email(mail_from='specific_user@icp.example.com') + IrMailServer.send_email(message) + + self.assertSMTPEmailsSent( + smtp_from='specific_user@icp.example.com', + message_from='specific_user@icp.example.com', + from_filter='icp.example.com', + ) + + @users('admin') def test_mail_server_get_test_email_addresses(self): """Test the email used to test the mail server connection.""" test_server = self.env['ir.mail_server'].create({ @@ -509,12 +495,20 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): ('notifications@example.com', 'dummy.com, full_email@example_2.com, dummy2.com'), ('notifications', 'dummy.com, full_email@example_2.com, dummy2.com'), ('notifications@example.com', 'example.com'), + # default relies on "odoo" + (False, 'example.com'), + # fallback on user email + ('example_2.com', False), + (False, False), ], [ 'noreply@example_2.com', 'notifications@example_2.com', 'full_email@example_2.com', 'full_email@example_2.com', 'notifications@example.com', + 'odoo@example.com', + self.env.user.email, + self.env.user.email, ], ): with self.subTest(default_from=default_from, from_filter=from_filter): @@ -526,16 +520,16 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @mute_logger('odoo.models.unlink') @patch.dict(config.options, {'from_filter': 'fake.com', 'smtp_server': 'cli_example.com'}) def test_mail_server_config_cli(self): - """Check the mail server when the "smtp_authentication" is "cli". - - Should take the configuration from the odoo-bin argument. - The "from_filter" of the mail server should overwrite the one set - in the CLI arguments. + """ Test the mail server configuration when the "smtp_authentication" is + "cli". It should take the configuration from the odoo-bin argument. The + "from_filter" of the mail server should overwrite the one set in the CLI + arguments. """ + IrMailServer = self.env['ir.mail_server'] # should be ignored by the mail server self.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', 'fake.com') - self.env['ir.mail_server'].create([{ + server_other = IrMailServer.create([{ 'name': 'Server No From Filter', 'smtp_host': 'smtp_host', 'smtp_encryption': 'none', @@ -543,27 +537,25 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): 'from_filter': 'dummy@example.com, cli_example.com, dummy2@example.com', }]) - IrMailServer = self.env['ir.mail_server'] + for mail_from, (expected_smtp_from, expected_msg_from, expected_mail_server) in zip( + [ + # check that the CLI server take the configuration in the odoo-bin argument + # except the from_filter which is taken on the mail server + 'test@cli_example.com', + # other mail servers still work + 'specific_user@test.mycompany.com', + ], [ + ('test@cli_example.com', 'test@cli_example.com', server_other), + ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', self.mail_server_user), + ], + ): + with self.subTest(mail_from=mail_from): + with self.mock_smtplib_connection(): + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message) - # check that the CLI server take the configuration in the odoo-bin argument - # except the from_filter which is taken on the mail server - with self.mock_smtplib_connection(): - message = self._build_email(mail_from='test@cli_example.com') - IrMailServer.send_email(message) - - self.assertSMTPEmailsSent( - smtp_from='test@cli_example.com', - message_from='test@cli_example.com', - from_filter='dummy@example.com, cli_example.com, dummy2@example.com', - ) - - # other mail server still work - with self.mock_smtplib_connection(): - message = self._build_email(mail_from='specific_user@test.mycompany.com') - IrMailServer.send_email(message) - - self.assertSMTPEmailsSent( - smtp_from='specific_user@test.mycompany.com', - message_from='specific_user@test.mycompany.com', - from_filter='specific_user@test.mycompany.com', - ) + self.assertSMTPEmailsSent( + smtp_from=expected_smtp_from, + message_from=expected_msg_from, + mail_server=expected_mail_server, + ) From 79677134d77904a8bc9551c9c33cff59bd15636e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 21 Aug 2023 14:10:05 +0000 Subject: [PATCH 2/4] [IMP] base: split _get_test_email_addresses to distinguish from/to Split '_get_test_email_addresses' into two methods allowing to generate the 'from' and 'to' when testing SMTP connection. As 'email_to' is always the same better have a small method for it. Moreover it eases overrides if some code wants to tune the from / to by overriding only the necessary one. Task-3453347 (Mail: Move Mail ICP from Base to Mail) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#130750 --- odoo/addons/base/models/ir_mail_server.py | 19 +++-- odoo/addons/base/tests/test_ir_mail_server.py | 80 +++++++++---------- 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/odoo/addons/base/models/ir_mail_server.py b/odoo/addons/base/models/ir_mail_server.py index 72caaf5e17b..13098d212c7 100644 --- a/odoo/addons/base/models/ir_mail_server.py +++ b/odoo/addons/base/models/ir_mail_server.py @@ -200,31 +200,33 @@ class IrMailServer(models.Model): """ return dict() - def _get_test_email_addresses(self): + def _get_test_email_from(self): self.ensure_one() - email_to = "noreply@odoo.com" if self.from_filter: from_filter_parts = [part.strip() for part in self.from_filter.split(",")] if mail_from := next((email for email in from_filter_parts if "@" in email), None): # All emails will be sent from the same address - return mail_from, email_to + return mail_from # All emails will be sent from any address in the same domain default_from = self.env["ir.config_parameter"].sudo().get_param("mail.default.from", "odoo") if "@" not in default_from: - return f"{default_from}@{from_filter_parts[0]}", email_to + return f"{default_from}@{from_filter_parts[0]}" elif self._match_from_filter(default_from, self.from_filter): # the mail server is configured for a domain # that match the default email address - return default_from, email_to + return default_from # the from_filter is configured for a domain different that the one # of the full email configured in mail.default.from - return f"noreply@{from_filter_parts[0]}", email_to + return f"noreply@{from_filter_parts[0]}" # Fallback to current user email if there's no from filter email_from = self.env.user.email if not email_from: raise UserError(_('Please configure an email on the current user to simulate ' 'sending an email message via this outgoing server')) - return email_from, email_to + return email_from + + def _get_test_email_to(self): + return "noreply@odoo.com" def test_smtp_connection(self): for server in self: @@ -232,7 +234,8 @@ class IrMailServer(models.Model): try: smtp = self.connect(mail_server_id=server.id, allow_archived=True) # simulate sending an email from current user's address - without sending it! - email_from, email_to = server._get_test_email_addresses() + email_from = server._get_test_email_from() + email_to = server._get_test_email_to() # Testing the MAIL FROM step should detect sender filter problems (code, repl) = smtp.mail(email_from) if code != 250: diff --git a/odoo/addons/base/tests/test_ir_mail_server.py b/odoo/addons/base/tests/test_ir_mail_server.py index ad822e4ae1b..a670bba130e 100644 --- a/odoo/addons/base/tests/test_ir_mail_server.py +++ b/odoo/addons/base/tests/test_ir_mail_server.py @@ -162,6 +162,46 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): body_alternative = body_alternative.strip('\n') self.assertEqual(body_alternative, expected) + @users('admin') + def test_mail_server_get_test_email_from(self): + """ Test the email used to test the mail server connection. """ + test_server = self.env['ir.mail_server'].create({ + 'from_filter': 'example_2.com, example_3.com', + 'name': 'Test Server', + 'smtp_host': 'smtp_host', + 'smtp_encryption': 'none', + }) + + # check default.from / filter matching + for (default_from, from_filter), expected_test_email in zip( + [ + ('notifications@example.com', 'example_2.com, example_3.com'), + ('notifications', 'example_2.com, example_3.com'), + ('notifications@example.com', 'dummy.com, full_email@example_2.com, dummy2.com'), + ('notifications', 'dummy.com, full_email@example_2.com, dummy2.com'), + ('notifications@example.com', 'example.com'), + # default relies on "odoo" + (False, 'example.com'), + # fallback on user email + ('example_2.com', False), + (False, False), + ], [ + 'noreply@example_2.com', + 'notifications@example_2.com', + 'full_email@example_2.com', + 'full_email@example_2.com', + 'notifications@example.com', + 'odoo@example.com', + self.env.user.email, + self.env.user.email, + ], + ): + with self.subTest(default_from=default_from, from_filter=from_filter): + self.env['ir.config_parameter'].set_param('mail.default.from', default_from) + test_server.from_filter = from_filter + email_from = test_server._get_test_email_from() + self.assertEqual(email_from, expected_test_email) + def test_mail_server_match_from_filter(self): """ Test the from_filter field on the "ir.mail_server". """ # Should match @@ -477,46 +517,6 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): from_filter='icp.example.com', ) - @users('admin') - def test_mail_server_get_test_email_addresses(self): - """Test the email used to test the mail server connection.""" - test_server = self.env['ir.mail_server'].create({ - 'from_filter': 'example_2.com, example_3.com', - 'name': 'Test Server', - 'smtp_host': 'smtp_host', - 'smtp_encryption': 'none', - }) - - # check default.from / filter matching - for (default_from, from_filter), expected_test_email in zip( - [ - ('notifications@example.com', 'example_2.com, example_3.com'), - ('notifications', 'example_2.com, example_3.com'), - ('notifications@example.com', 'dummy.com, full_email@example_2.com, dummy2.com'), - ('notifications', 'dummy.com, full_email@example_2.com, dummy2.com'), - ('notifications@example.com', 'example.com'), - # default relies on "odoo" - (False, 'example.com'), - # fallback on user email - ('example_2.com', False), - (False, False), - ], [ - 'noreply@example_2.com', - 'notifications@example_2.com', - 'full_email@example_2.com', - 'full_email@example_2.com', - 'notifications@example.com', - 'odoo@example.com', - self.env.user.email, - self.env.user.email, - ], - ): - with self.subTest(default_from=default_from, from_filter=from_filter): - self.env['ir.config_parameter'].set_param('mail.default.from', default_from) - test_server.from_filter = from_filter - email_from = test_server._get_test_email_addresses()[0] - self.assertEqual(email_from, expected_test_email) - @mute_logger('odoo.models.unlink') @patch.dict(config.options, {'from_filter': 'fake.com', 'smtp_server': 'cli_example.com'}) def test_mail_server_config_cli(self): From 29f7e6d894347eb8ae0cacd8ed79ab2331bdce82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 22 Aug 2023 14:01:42 +0000 Subject: [PATCH 3/4] [FIX] base: be defensive when computing parts of 'from_filter' From filter could be ill-defined, like ' ' or ','. This commit just make some code more defensive against those values. Task-3453347 (Mail: Move Mail ICP from Base to Mail) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#130750 --- odoo/addons/base/models/ir_mail_server.py | 6 ++---- odoo/addons/base/tests/test_ir_mail_server.py | 6 +++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/odoo/addons/base/models/ir_mail_server.py b/odoo/addons/base/models/ir_mail_server.py index 13098d212c7..72ec2917507 100644 --- a/odoo/addons/base/models/ir_mail_server.py +++ b/odoo/addons/base/models/ir_mail_server.py @@ -202,8 +202,7 @@ class IrMailServer(models.Model): def _get_test_email_from(self): self.ensure_one() - if self.from_filter: - from_filter_parts = [part.strip() for part in self.from_filter.split(",")] + if from_filter_parts := [part.strip() for part in (self.from_filter or '').split(",") if part.strip()]: if mail_from := next((email for email in from_filter_parts if "@" in email), None): # All emails will be sent from the same address return mail_from @@ -797,8 +796,7 @@ class IrMailServer(models.Model): normalized_mail_from = email_normalize(email_from) normalized_domain = email_domain_extract(normalized_mail_from) - for email_filter in from_filter.split(','): - email_filter = email_filter.strip() + for email_filter in [part.strip() for part in (from_filter or '').split(',') if part.strip()]: if '@' in email_filter and email_normalize(email_filter) == normalized_mail_from: return True if '@' not in email_filter and email_domain_normalize(email_filter) == normalized_domain: diff --git a/odoo/addons/base/tests/test_ir_mail_server.py b/odoo/addons/base/tests/test_ir_mail_server.py index a670bba130e..f5666adac52 100644 --- a/odoo/addons/base/tests/test_ir_mail_server.py +++ b/odoo/addons/base/tests/test_ir_mail_server.py @@ -182,7 +182,9 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): ('notifications@example.com', 'example.com'), # default relies on "odoo" (False, 'example.com'), - # fallback on user email + # fallback on user email if no from_filter + ('example_2.com', ' '), + ('example_2.com', ','), ('example_2.com', False), (False, False), ], [ @@ -194,6 +196,8 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): 'odoo@example.com', self.env.user.email, self.env.user.email, + self.env.user.email, + self.env.user.email, ], ): with self.subTest(default_from=default_from, from_filter=from_filter): From a4fe51cb2e8877edac0a74c98eb2829b938aec11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 27 Jul 2023 15:43:12 +0000 Subject: [PATCH 4/4] [MOV] base, mail: move mail config parameters usage to mail RATIONALE As multi-company tolerant alias domains will soon replace the usage of configuration parameters, having them in base then replaced by more advanced models in mail would be complicated to handle and not useful. Move those ICP to 'mail' so that all mail configuration is done in that module. SPECIFICATIONS Move config parameter used for alias domains configuration in 'mail' module. Base should be as simple as possible and let mail deal with mail server complexity. Move 'mail.{bounce/catchall}.alias' used with 'mail.alias.domain' to make bounce and catchall emails. Move 'mail.default.from' as it will be integrated into alias domains in some form. Note that 'mail.default.from_filter' stays as an ICP in base as it is a more global default parameter. It is used as default value in 'connect' when no mail_server is used and no from_filter can be retrieved. Some tests in 'base' are either fixed, either moved directly into 'mail'. We now differentiate base behavior (without ICP) from configurable behavior (with ICP in mail). Task-3453347 (Mail: Move Mail ICP from Base to Mail) Prepares Task-36879 (Mail: Support MultiCompany Aliases) Part-of: odoo/odoo#130750 --- addons/mail/__manifest__.py | 1 + .../mail}/data/ir_config_parameter_data.xml | 0 addons/mail/models/ir_mail_server.py | 44 ++- addons/mail/tests/__init__.py | 1 + addons/mail/tests/test_ir_mail_server.py | 338 ++++++++++++++++++ odoo/addons/base/__manifest__.py | 1 - odoo/addons/base/models/ir_mail_server.py | 68 +--- odoo/addons/base/tests/common.py | 6 - odoo/addons/base/tests/test_ir_mail_server.py | 223 +++--------- 9 files changed, 441 insertions(+), 241 deletions(-) rename {odoo/addons/base => addons/mail}/data/ir_config_parameter_data.xml (100%) create mode 100644 addons/mail/tests/test_ir_mail_server.py diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index 3706a601b20..0d3fb705dda 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -86,6 +86,7 @@ For more specific needs, you may also assign custom-defined actions 'views/mail_shortcode_views.xml', 'views/mail_activity_views.xml', 'views/res_config_settings_views.xml', + 'data/ir_config_parameter_data.xml', 'data/res_partner_data.xml', 'data/mail_message_subtype_data.xml', 'data/mail_templates_chatter.xml', diff --git a/odoo/addons/base/data/ir_config_parameter_data.xml b/addons/mail/data/ir_config_parameter_data.xml similarity index 100% rename from odoo/addons/base/data/ir_config_parameter_data.xml rename to addons/mail/data/ir_config_parameter_data.xml diff --git a/addons/mail/models/ir_mail_server.py b/addons/mail/models/ir_mail_server.py index 316b46ed58e..3c7f52dcba9 100644 --- a/addons/mail/models/ir_mail_server.py +++ b/addons/mail/models/ir_mail_server.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import _, fields, models +from odoo import _, api, fields, models class IrMailServer(models.Model): @@ -21,3 +21,45 @@ class IrMailServer(models.Model): map(lambda t: _('%s (Email Template)', t.display_name), record.mail_template_ids) ) return usages_super + + @api.model + def _get_default_bounce_address(self): + """ Compute the default bounce address. Try to use mail-defined config + parameter bounce alias if set. """ + ICP = self.env['ir.config_parameter'].sudo() + bounce_alias = ICP.get_param('mail.bounce.alias') + domain = ICP.get_param('mail.catchall.domain') + if bounce_alias and domain: + return f'{bounce_alias}@{domain}' + return super()._get_default_from_address() + + @api.model + def _get_default_from_address(self): + """ Default from: try to use ``mail.default.from`` ICP either as a + full email address, either completed with ``mail.catchall.domain`` + to form a alias-based default from. """ + get_param = self.env['ir.config_parameter'].sudo().get_param + email_from = get_param("mail.default.from") + if email_from and "@" in email_from: + return email_from + if email_from: + if domain := get_param("mail.catchall.domain"): + return f"{email_from}@{domain}" + return super()._get_default_from_address() + + def _get_test_email_from(self): + self.ensure_one() + if from_filter_parts := [part.strip() for part in (self.from_filter or '').split(",") if part.strip()]: + # find first found complete email in filter parts + if mail_from := next((email for email in from_filter_parts if "@" in email), None): + return mail_from + # try to complete with default.from in same domain of first found filter parts + default_from = self.env["ir.config_parameter"].sudo().get_param("mail.default.from", "odoo") + if "@" not in default_from: + return f"{default_from}@{from_filter_parts[0]}" + # the mail server is configured for a domain that matches the default email address + if self._match_from_filter(default_from, self.from_filter): + return default_from + # no from_filter or from_filter is configured for a domain different that + # the one of the full email configured in mail.default.from -> fallback + return super()._get_test_email_from() diff --git a/addons/mail/tests/__init__.py b/addons/mail/tests/__init__.py index 47b456a3f75..43b2f48dd5c 100644 --- a/addons/mail/tests/__init__.py +++ b/addons/mail/tests/__init__.py @@ -1,5 +1,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. +from . import test_ir_mail_server from . import test_link_preview from . import test_mail_activity from . import test_mail_composer diff --git a/addons/mail/tests/test_ir_mail_server.py b/addons/mail/tests/test_ir_mail_server.py new file mode 100644 index 00000000000..ffc1324eda6 --- /dev/null +++ b/addons/mail/tests/test_ir_mail_server.py @@ -0,0 +1,338 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from unittest.mock import patch + +from odoo.addons.mail.tests.common import MailCommon +from odoo.tests import tagged, users +from odoo.tools import config, mute_logger + + +@tagged('mail_server') +class TestIrMailServer(MailCommon): + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.default_bounce_address = f'{cls.alias_bounce}@{cls.alias_domain}' + cls.default_from_address = f'{cls.default_from}@{cls.alias_domain}' + + def test_assert_base_values(self): + self.assertEqual( + self.env['ir.mail_server']._get_default_bounce_address(), + self.default_bounce_address) + self.assertEqual( + self.env['ir.mail_server']._get_default_from_address(), + self.default_from_address) + + @patch.dict(config.options, {"email_from": "settings@example.com"}) + def test_default_email_from(self, *args): + """ Check that 'mail.default.from' setting is respected. """ + ICP = self.env["ir.config_parameter"].sudo() + for (icp_from, icp_domain), expected_from in zip( + [ + ('icp', 'test.mycompany.com'), + ('icp', False), + ('icp@another.company.com', 'test.mycompany.com'), + ('icp@another.company.com', False), + (False, 'test.mycompany.com'), + ], [ + "icp@test.mycompany.com", + "settings@example.com", + "icp@another.company.com", + "icp@another.company.com", + "settings@example.com", + ], + ): + with self.subTest(icp_from=icp_from, icp_domain=icp_domain): + ICP.set_param("mail.default.from", icp_from) + ICP.set_param("mail.catchall.domain", icp_domain) + message = self.env["ir.mail_server"].build_email( + False, "recipient@example.com", "Subject", + "The body of an email", + ) + self.assertEqual(message["From"], expected_from) + + @mute_logger('odoo.models.unlink') + @patch.dict(config.options, { + "from_filter": "dummy@example.com, test.mycompany.com, dummy2@example.com", + "smtp_server": "example.com", + }) + def test_mail_server_config_bin(self): + """ Test the configuration provided in the odoo-bin arguments. This config + is used when no mail server exists. Test with and without giving a + pre-configured SMTP session, should not impact results. + + Also check "mail.default.from_filter" parameter usage that should overwrite + odoo-bin argument "--from-filter". + """ + IrMailServer = self.env['ir.mail_server'] + + # Remove all mail server so we will use the odoo-bin arguments + IrMailServer.search([]).unlink() + self.assertFalse(IrMailServer.search([])) + + for mail_from, (expected_smtp_from, expected_msg_from) in zip( + [ + # inside "from_filter" domain + 'specific_user@test.mycompany.com', + '"Formatted Name" ', + '"Formatted Name" ', + '"Formatted Name" ', + # outside "from_filter" domain + 'test@unknown_domain.com', + '"Formatted Name" ', + ], [ + # inside "from_filter" domain: no rewriting + (self.default_bounce_address, 'specific_user@test.mycompany.com'), + (self.default_bounce_address, '"Formatted Name" '), + (self.default_bounce_address, '"Formatted Name" '), + (self.default_bounce_address, '"Formatted Name" '), + # outside "from_filter" domain: we will use notifications emails in the + # headers, and bounce address in the envelope because the "from_filter" + # allows to use the entire domain + (self.default_bounce_address, '"test" '), + (self.default_bounce_address, '"Formatted Name" '), + ] + ): + for provide_smtp in [False, True]: # providing smtp session should ont impact test + with self.subTest(mail_from=mail_from, provide_smtp=provide_smtp): + with self.mock_smtplib_connection(): + if provide_smtp: + smtp_session = IrMailServer.connect(smtp_from=mail_from) + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message, smtp_session=smtp_session) + else: + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message) + + self.connect_mocked.assert_called_once() + self.assertEqual(len(self.emails), 1) + self.assertSMTPEmailsSent( + smtp_from=expected_smtp_from, + message_from=expected_msg_from, + from_filter="dummy@example.com, test.mycompany.com, dummy2@example.com", + ) + + # for from_filter in ICP, overwrite the one from odoo-bin + self.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', 'icp.example.com') + + # Use an email in the domain of the config parameter "mail.default.from_filter" + with self.mock_smtplib_connection(): + message = self._build_email(mail_from='specific_user@icp.example.com') + IrMailServer.send_email(message) + + self.assertSMTPEmailsSent( + smtp_from='specific_user@icp.example.com', + message_from='specific_user@icp.example.com', + from_filter='icp.example.com', + ) + + @users('admin') + def test_mail_server_get_test_email_from(self): + """ Test the email used to test the mail server connection. Check + from_filter parsing / mail.default.from support. """ + test_server = self.env['ir.mail_server'].create({ + 'from_filter': 'example_2.com, example_3.com', + 'name': 'Test Server', + 'smtp_host': 'smtp_host', + 'smtp_encryption': 'none', + }) + + # check default.from / filter matching + for (default_from, from_filter), expected_test_email in zip( + [ + ('notifications@example.com', 'example_2.com, example_3.com'), + ('notifications', 'example_2.com, example_3.com'), + ('notifications@example.com', 'dummy.com, full_email@example_2.com, dummy2.com'), + ('notifications', 'dummy.com, full_email@example_2.com, dummy2.com'), + ('notifications@example.com', 'example.com'), + # default relies on "odoo" + (False, 'example.com'), + # fallback on user email if no from_filter + ('example_2.com', ' '), + ('example_2.com', ','), + ('example_2.com', False), + (False, False), + ], [ + 'noreply@example_2.com', + 'notifications@example_2.com', + 'full_email@example_2.com', + 'full_email@example_2.com', + 'notifications@example.com', + 'odoo@example.com', + self.env.user.email, + self.env.user.email, + self.env.user.email, + self.env.user.email, + ], + ): + with self.subTest(default_from=default_from, from_filter=from_filter): + self.env['ir.config_parameter'].set_param('mail.default.from', default_from) + test_server.from_filter = from_filter + email_from = test_server._get_test_email_from() + self.assertEqual(email_from, expected_test_email) + + @mute_logger('odoo.models.unlink') + def test_mail_server_priorities(self): + """ Test if we choose the right mail server to send an email. + Priorities are + 1. Forced mail server (e.g.: in mass mailing) + - If the "from_filter" of the mail server match the notification email + use the notifications email in the "From header" + - Otherwise spoof the "From" (because we force the mail server but we don't + know which email use to send it) + 2. A mail server for which the "from_filter" match the "From" header + 3. A mail server for which the "from_filter" match the domain of the "From" header + 4. The mail server used for notifications + 5. A mail server without "from_filter" (and so spoof the "From" header because we + do not know for which email address it can be used) + """ + # this mail server can now be used for a specific email address and 2 domain names + self.mail_server_user.from_filter = "domain1.com, specific_user@test.mycompany.com, domain2.com" + + for email_from, (expected_mail_server, expected_email_from) in zip( + [ + # matches user-specific server + 'specific_user@test.mycompany.com', + # matches user-specific server (with formatting') -> should extract + # email from full name, must keep the given email_from + '"Name name@strange.name" ', + # case check + 'SPECIFIC_USER@test.mycompany.com', + 'specific_user@test.MYCOMPANY.com', + # matches domain-based server: domain is case insensitive + 'unknown_email@test.mycompany.com', + 'unknown_email@TEST.MYCOMPANY.COM', + '"Unknown" ', + # fallback on notification email + '"Test" ', + # mail_server_user multiple from_filter check: can be used for a + # specific email and 2 domain names -> check other domains in filter + '"Example" ', + '"Example" ', + ], [ + (self.mail_server_user, 'specific_user@test.mycompany.com'), + (self.mail_server_user, '"Name name@strange.name" '), + (self.mail_server_user, 'SPECIFIC_USER@test.mycompany.com'), + (self.mail_server_user, 'specific_user@test.MYCOMPANY.com'), + (self.mail_server_domain, 'unknown_email@test.mycompany.com'), + (self.mail_server_domain, 'unknown_email@TEST.MYCOMPANY.COM'), + (self.mail_server_domain, '"Unknown" '), + (self.mail_server_notification, 'notifications.test@test.mycompany.com'), + # mail_server_user multiple from_filter check + (self.mail_server_user, '"Example" '), + (self.mail_server_user, '"Example" '), + ], + ): + with self.subTest(email_from=email_from): + mail_server, mail_from = self.env['ir.mail_server']._find_mail_server(email_from=email_from) + self.assertEqual(mail_server, expected_mail_server) + self.assertEqual(mail_from, expected_email_from) + + @mute_logger('odoo.models.unlink') + def test_mail_server_send_email(self): + """ Test main 'send_email' usage: check mail_server choice based on from + filters, encapsulation, spoofing. """ + IrMailServer = self.env['ir.mail_server'] + + for mail_from, (expected_smtp_from, expected_msg_from, expected_mail_server) in zip( + [ + 'specific_user@test.mycompany.com', + '"Name" ', + 'test@unknown_domain.com', + '"Name" ' + ], [ + # A mail server is configured for the email + ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', self.mail_server_user), + # No mail server are configured for the email address, so it will use the + # notifications email instead and encapsulate the old email + ('notifications.test@test.mycompany.com', '"Name" ', self.mail_server_notification), + # same situation, but the original email has no name part + ('notifications.test@test.mycompany.com', '"test" ', self.mail_server_notification), + # A mail server is configured for the entire domain name, so we can use the bounce + # email address because the mail server supports it + (self.default_bounce_address, '"Name" ', self.mail_server_domain), + ], + ): + # test with and without providing an SMTP session, which should not impact test + for provide_smtp in [True, False]: + with self.subTest(mail_from=mail_from): + with self.mock_smtplib_connection(): + if provide_smtp: + smtp_session = IrMailServer.connect(smtp_from=mail_from) + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message, smtp_session=smtp_session) + else: + message = self._build_email(mail_from=mail_from) + IrMailServer.send_email(message) + self.connect_mocked.assert_called_once() + self.assertEqual(len(self.emails), 1) + self.assertSMTPEmailsSent( + smtp_from=expected_smtp_from, + message_from=expected_msg_from, + mail_server=expected_mail_server, + ) + + # remove the notification server + # so will use the mail server + # The mail server configured for the notifications email has been removed + # but we can still use the mail server configured for test.mycompany.com + # and so we will be able to use the bounce address + # because we use the mail server for "test.mycompany.com" + self.mail_server_notification.unlink() + for provide_smtp in [False, True]: + with self.mock_smtplib_connection(): + if provide_smtp: + smtp_session = IrMailServer.connect(smtp_from='"Name" ') + message = self._build_email(mail_from='"Name" ') + IrMailServer.send_email(message, smtp_session=smtp_session) + else: + message = self._build_email(mail_from='"Name" ') + IrMailServer.send_email(message) + self.connect_mocked.assert_called_once() + self.assertEqual(len(self.emails), 1) + self.assertSMTPEmailsSent( + smtp_from=self.default_bounce_address, + message_from='"Name" ', + mail_server=self.mail_server_domain, + ) + + def test_mail_server_send_email_default_from(self): + """ Test the case when the "mail.default.from" contains a full email address + and not just the local part the domain of this default email address can be + different than the catchall domain """ + other_default_from = 'notifications.other.test@custom_domain.com' + self.env['ir.config_parameter'].sudo().set_param('mail.default.from', other_default_from) + custom_server = self.env['ir.mail_server'].create({ + 'from_filter': 'custom_domain.com', + 'name': 'Custom Domain Server', + 'smtp_host': 'smtp_host', + 'smtp_encryption': 'none', + }) + + for mail_from, (expected_smtp_from, expected_msg_from, expected_mail_server) in zip( + [ + # custom server domain + '"Custom Server Name" ', + # unknown domain + '"Name" ', + # notification server domain + '"Name" ' + ], [ + ('customer.test@custom_domain.com', '"Custom Server Name" ', custom_server), + (other_default_from, '"Name" ', custom_server), + (self.default_bounce_address, '"Name" ', self.mail_server_domain), + ], + ): + with self.subTest(mail_from=mail_from): + with self.mock_smtplib_connection(): + message = self._build_email(mail_from=mail_from) + self.env['ir.mail_server'].send_email(message) + + self.assertSMTPEmailsSent( + smtp_from=expected_smtp_from, + smtp_to_list=['dest@xn--example--i1a.com'], + message_from=expected_msg_from, + mail_server=expected_mail_server, + ) diff --git a/odoo/addons/base/__manifest__.py b/odoo/addons/base/__manifest__.py index d77e0f11e8e..f57cf22966b 100644 --- a/odoo/addons/base/__manifest__.py +++ b/odoo/addons/base/__manifest__.py @@ -21,7 +21,6 @@ The kernel of Odoo, needed for all installation. 'data/report_paperformat_data.xml', 'data/res_country_data.xml', 'data/ir_demo_data.xml', - 'data/ir_config_parameter_data.xml', 'security/base_groups.xml', 'security/base_security.xml', 'views/base_menus.xml', diff --git a/odoo/addons/base/models/ir_mail_server.py b/odoo/addons/base/models/ir_mail_server.py index 72ec2917507..1f3238b51eb 100644 --- a/odoo/addons/base/models/ir_mail_server.py +++ b/odoo/addons/base/models/ir_mail_server.py @@ -202,24 +202,17 @@ class IrMailServer(models.Model): def _get_test_email_from(self): self.ensure_one() + email_from = False if from_filter_parts := [part.strip() for part in (self.from_filter or '').split(",") if part.strip()]: - if mail_from := next((email for email in from_filter_parts if "@" in email), None): - # All emails will be sent from the same address - return mail_from - # All emails will be sent from any address in the same domain - default_from = self.env["ir.config_parameter"].sudo().get_param("mail.default.from", "odoo") - if "@" not in default_from: - return f"{default_from}@{from_filter_parts[0]}" - elif self._match_from_filter(default_from, self.from_filter): - # the mail server is configured for a domain - # that match the default email address - return default_from - # the from_filter is configured for a domain different that the one - # of the full email configured in mail.default.from - return f"noreply@{from_filter_parts[0]}" - # Fallback to current user email if there's no from filter - email_from = self.env.user.email + # find first found complete email in filter parts + email_from = next((email for email in from_filter_parts if "@" in email), False) + # no complete email -> consider noreply + if not email_from: + email_from = f"noreply@{from_filter_parts[0]}" if not email_from: + # Fallback to current user email if there's no from filter + email_from = self.env.user.email + if not email_from or "@" not in email_from: raise UserError(_('Please configure an email on the current user to simulate ' 'sending an email message via this outgoing server')) return email_from @@ -531,47 +524,20 @@ class IrMailServer(models.Model): @api.model def _get_default_bounce_address(self): - '''Compute the default bounce address. + """ Computes the default bounce address. It is used to set the envelop + address if no envelop address is provided in the message. - The default bounce address is used to set the envelop address if no - envelop address is provided in the message. It is formed by properly - joining the parameters "mail.bounce.alias" and - "mail.catchall.domain". - - If "mail.bounce.alias" is not set it defaults to "postmaster-odoo". - - If "mail.catchall.domain" is not set, return None. - - ''' - ICP = self.env['ir.config_parameter'].sudo() - bounce_alias = ICP.get_param('mail.bounce.alias') - domain = ICP.get_param('mail.catchall.domain') - if bounce_alias and domain: - return '%s@%s' % (bounce_alias, domain) - return + :return str/None: defaults to the ``--email-from`` CLI/config parameter. + """ + return tools.config.get("email_from") @api.model def _get_default_from_address(self): - """Compute the default from address. + """ Computes the default from address. It is used for the "header from" + address when no other has been received. - Used for the "header from" address when no other has been received. - - :return str/None: - If the config parameter ``mail.default.from`` contains - a full email address, return it. - Otherwise, combines config parameters ``mail.default.from`` and - ``mail.catchall.domain`` to generate a default sender address. - - If some of those parameters is not defined, it will default to the - ``--email-from`` CLI/config parameter. + :return str/None: defaults to the ``--email-from`` CLI/config parameter. """ - get_param = self.env['ir.config_parameter'].sudo().get_param - email_from = get_param("mail.default.from") - if email_from and "@" in email_from: - return email_from - domain = get_param("mail.catchall.domain") - if email_from and domain: - return "%s@%s" % (email_from, domain) return tools.config.get("email_from") def _prepare_email_message(self, message, smtp_session): diff --git a/odoo/addons/base/tests/common.py b/odoo/addons/base/tests/common.py index ffe3649861f..1e20d0416cf 100644 --- a/odoo/addons/base/tests/common.py +++ b/odoo/addons/base/tests/common.py @@ -399,13 +399,7 @@ class MockSmtplibCase: @classmethod def _init_mail_gateway(cls): - cls.alias_bounce = 'bounce.test' - cls.alias_domain = 'test.mycompany.com' - cls.default_from = 'notifications.test' cls.default_from_filter = False - cls.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', cls.alias_domain) - cls.env['ir.config_parameter'].sudo().set_param('mail.default.from', cls.default_from) - cls.env['ir.config_parameter'].sudo().set_param('mail.bounce.alias', cls.alias_bounce) cls.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', cls.default_from_filter) @classmethod diff --git a/odoo/addons/base/tests/test_ir_mail_server.py b/odoo/addons/base/tests/test_ir_mail_server.py index f5666adac52..7557421649a 100644 --- a/odoo/addons/base/tests/test_ir_mail_server.py +++ b/odoo/addons/base/tests/test_ir_mail_server.py @@ -36,32 +36,13 @@ class _FakeSMTP: class EmailConfigCase(TransactionCase): @patch.dict(config.options, {"email_from": "settings@example.com"}) - def test_default_email_from(self, *args): - """Email from setting is respected.""" - ICP = self.env["ir.config_parameter"].sudo() - for (icp_from, icp_domain), expected_from in zip( - [ - ('icp', 'test.mycompany.com'), - ('icp', False), - ('icp@another.company.com', 'test.mycompany.com'), - ('icp@another.company.com', False), - (False, 'test.mycompany.com'), - ], [ - "icp@test.mycompany.com", - "settings@example.com", - "icp@another.company.com", - "icp@another.company.com", - "settings@example.com", - ], - ): - with self.subTest(icp_from=icp_from, icp_domain=icp_domain): - ICP.set_param("mail.default.from", icp_from) - ICP.set_param("mail.catchall.domain", icp_domain) - message = self.env["ir.mail_server"].build_email( - False, "recipient@example.com", "Subject", - "The body of an email", - ) - self.assertEqual(message["From"], expected_from) + def test_default_email_from(self): + """ Email from setting is respected and comes from configuration. """ + message = self.env["ir.mail_server"].build_email( + False, "recipient@example.com", "Subject", + "The body of an email", + ) + self.assertEqual(message["From"], "settings@example.com") @tagged('mail_server') @@ -70,19 +51,16 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @classmethod def setUpClass(cls): super().setUpClass() - cls._init_mail_gateway() + # TEMPORARY + for icp in {'mail.default.from', 'mail.catchall.alias', 'mail.bounce.alias', 'mail.catchall.domain', 'mail.default.from_filter'}: + cls.env['ir.config_parameter'].sudo().set_param(icp, False) cls._init_mail_servers() - - cls.default_bounce_address = f'{cls.alias_bounce}@{cls.alias_domain}' - cls.default_from_address = f'{cls.default_from}@{cls.alias_domain}' + cls.default_bounce_address = 'CACA' + cls.default_from_address = 'PROUT' def test_assert_base_values(self): - self.assertEqual( - self.env['ir.mail_server']._get_default_bounce_address(), - self.default_bounce_address) - self.assertEqual( - self.env['ir.mail_server']._get_default_from_address(), - self.default_from_address) + self.assertFalse(self.env['ir.mail_server']._get_default_bounce_address()) + self.assertFalse(self.env['ir.mail_server']._get_default_from_address()) def test_bpo_34424_35805(self): """Ensure all email sent are bpo-34424 and bpo-35805 free""" @@ -164,44 +142,31 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @users('admin') def test_mail_server_get_test_email_from(self): - """ Test the email used to test the mail server connection. """ + """ Test the email used to test the mail server connection. Check + from_filter parsing / default fallback value. """ test_server = self.env['ir.mail_server'].create({ 'from_filter': 'example_2.com, example_3.com', 'name': 'Test Server', 'smtp_host': 'smtp_host', 'smtp_encryption': 'none', }) - - # check default.from / filter matching - for (default_from, from_filter), expected_test_email in zip( + for from_filter, expected_test_email in zip( [ - ('notifications@example.com', 'example_2.com, example_3.com'), - ('notifications', 'example_2.com, example_3.com'), - ('notifications@example.com', 'dummy.com, full_email@example_2.com, dummy2.com'), - ('notifications', 'dummy.com, full_email@example_2.com, dummy2.com'), - ('notifications@example.com', 'example.com'), - # default relies on "odoo" - (False, 'example.com'), - # fallback on user email if no from_filter - ('example_2.com', ' '), - ('example_2.com', ','), - ('example_2.com', False), - (False, False), + 'example_2.com, example_3.com', + 'dummy.com, full_email@example_2.com, dummy2.com', + # fallback on user's email + ' ', + ',', + False, ], [ 'noreply@example_2.com', - 'notifications@example_2.com', 'full_email@example_2.com', - 'full_email@example_2.com', - 'notifications@example.com', - 'odoo@example.com', - self.env.user.email, self.env.user.email, self.env.user.email, self.env.user.email, ], ): - with self.subTest(default_from=default_from, from_filter=from_filter): - self.env['ir.config_parameter'].set_param('mail.default.from', default_from) + with self.subTest(from_filter=from_filter): test_server.from_filter = from_filter email_from = test_server._get_test_email_from() self.assertEqual(email_from, expected_test_email) @@ -236,73 +201,9 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @mute_logger('odoo.models.unlink') def test_mail_server_priorities(self): - """ Test if we choose the right mail server to send an email. - - Priorities are - 1. Forced mail server (e.g.: in mass mailing) - - If the "from_filter" of the mail server match the notification email - use the notifications email in the "From header" - - Otherwise spoof the "From" (because we force the mail server but we don't - know which email use to send it) - 2. A mail server for which the "from_filter" match the "From" header - 3. A mail server for which the "from_filter" match the domain of the "From" header - 4. The mail server used for notifications - 5. A mail server without "from_filter" (and so spoof the "From" header because we - do not know for which email address it can be used) - """ - # sanity checks - self.assertTrue(self.env['ir.mail_server']._get_default_from_address(), 'Notifications email must be set for testing') - self.assertTrue(self.env['ir.mail_server']._get_default_bounce_address(), 'Bounce email must be set for testing') - # this mail server can not be used for a specific email address and 2 domain names - self.mail_server_user.from_filter = "domain1.com, specific_user@test.mycompany.com, domain2.com" - - for email_from, (expected_mail_server, expected_email_from) in zip( - [ - # matches user-specific server - 'specific_user@test.mycompany.com', - # matches user-specific server (with formatting') -> should extract - # email from full name, must keep the given email_from - '"Name name@strange.name" ', - # case check - 'SPECIFIC_USER@test.mycompany.com', - 'specific_user@test.MYCOMPANY.com', - # matches domain-based server: domain is case insensitive - 'unknown_email@test.mycompany.com', - 'unknown_email@TEST.MYCOMPANY.COM', - '"Unknown" ', - # fallback on notification email - '"Test" ', - # mail_server_user multiple from_filter check: can be used for a - # specific email and 2 domain names -> check other domains in filter - '"Example" ', - '"Example" ', - ], [ - (self.mail_server_user, 'specific_user@test.mycompany.com'), - (self.mail_server_user, '"Name name@strange.name" '), - (self.mail_server_user, 'SPECIFIC_USER@test.mycompany.com'), - (self.mail_server_user, 'specific_user@test.MYCOMPANY.com'), - (self.mail_server_domain, 'unknown_email@test.mycompany.com'), - (self.mail_server_domain, 'unknown_email@TEST.MYCOMPANY.COM'), - (self.mail_server_domain, '"Unknown" '), - (self.mail_server_notification, 'notifications.test@test.mycompany.com'), - # mail_server_user multiple from_filter check - (self.mail_server_user, '"Example" '), - (self.mail_server_user, '"Example" '), - ], - ): - with self.subTest(email_from=email_from): - mail_server, mail_from = self.env['ir.mail_server']._find_mail_server(email_from=email_from) - self.assertEqual(mail_server, expected_mail_server) - self.assertEqual(mail_from, expected_email_from) - - @mute_logger('odoo.models.unlink') - def test_mail_server_priorities_base_config(self): - """ Test if we choose the right mail server to send an email, without - ICP configuration. Simulates unconfigured Odoo DB so we have to spoof - the FROM otherwise we cannot send any email. """ - self.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', False) - self.env['ir.config_parameter'].sudo().set_param('mail.default.from', False) - + """ Test if we choose the right mail server to send an email. Simulates + simple Odoo DB so we have to spoof the FROM otherwise we cannot send + any email. """ for email_from, (expected_mail_server, expected_email_from) in zip( [ 'specific_user@test.mycompany.com', @@ -337,12 +238,12 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', self.mail_server_user), # No mail server are configured for the email address, so it will use the # notifications email instead and encapsulate the old email - ('notifications.test@test.mycompany.com', '"Name" ', self.mail_server_notification), + ('test@unknown_domain.com', '"Name" ', self.mail_server_default), # same situation, but the original email has no name part - ('notifications.test@test.mycompany.com', '"test" ', self.mail_server_notification), + ('test@unknown_domain.com', 'test@unknown_domain.com', self.mail_server_default), # A mail server is configured for the entire domain name, so we can use the bounce # email address because the mail server supports it - (self.default_bounce_address, '"Name" ', self.mail_server_domain), + ('unknown_name@test.mycompany.com', '"Name" ', self.mail_server_domain), ] ): # test with and without providing an SMTP session, which should not impact test @@ -385,9 +286,9 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): self.connect_mocked.assert_called_once() self.assertEqual(len(self.emails), 1) self.assertSMTPEmailsSent( - smtp_from=self.default_bounce_address, - message_from='"Name" ', - mail_server=self.mail_server_domain, + smtp_from='test@unknown_domain.com', + message_from='"Name" ', + from_filter=False, ) @mute_logger('odoo.models.unlink') @@ -400,52 +301,12 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): self.assertEqual(len(self.emails), 1) self.assertSMTPEmailsSent( - smtp_from=f'{self.alias_bounce}@xn--9caaaaaaa.com', + smtp_from='test@xn--9caaaaaaa.com', smtp_to_list=['dest@xn--example--i1a.com'], message_from='test@=?utf-8?b?w6nDqcOpw6nDqcOpw6k=?=.com', from_filter=False, ) - @mute_logger('odoo.models.unlink') - def test_mail_server_send_email_default_from(self): - """ Test the case when the "mail.default.from" contains a full email address - and not just the local part the domain of this default email address can be - different than the catchall domain """ - other_default_from = 'notifications.other.test@custom_domain.com' - self.env['ir.config_parameter'].sudo().set_param('mail.default.from', other_default_from) - custom_server = self.env['ir.mail_server'].create({ - 'from_filter': 'custom_domain.com', - 'name': 'Custom Domain Server', - 'smtp_host': 'smtp_host', - 'smtp_encryption': 'none', - }) - - for mail_from, (expected_smtp_from, expected_msg_from, expected_mail_server) in zip( - [ - # custom server domain - '"Custom Server Name" ', - # unknown domain - '"Name" ', - # notification server domain - '"Name" ' - ], [ - ('customer.test@custom_domain.com', '"Custom Server Name" ', custom_server), - (other_default_from, '"Name" ', custom_server), - (self.default_bounce_address, '"Name" ', self.mail_server_domain), - ], - ): - with self.subTest(mail_from=mail_from): - with self.mock_smtplib_connection(): - message = self._build_email(mail_from=mail_from) - self.env['ir.mail_server'].send_email(message) - - self.assertSMTPEmailsSent( - smtp_from=expected_smtp_from, - smtp_to_list=['dest@xn--example--i1a.com'], - message_from=expected_msg_from, - mail_server=expected_mail_server, - ) - @mute_logger('odoo.models.unlink') @patch.dict(config.options, { "from_filter": "dummy@example.com, test.mycompany.com, dummy2@example.com", @@ -477,15 +338,13 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): '"Formatted Name" ', ], [ # inside "from_filter" domain: no rewriting - (self.default_bounce_address, 'specific_user@test.mycompany.com'), - (self.default_bounce_address, '"Formatted Name" '), - (self.default_bounce_address, '"Formatted Name" '), - (self.default_bounce_address, '"Formatted Name" '), - # outside "from_filter" domain: we will use notifications emails in the - # headers, and bounce address in the envelope because the "from_filter" - # allows to use the entire domain - (self.default_bounce_address, '"test" '), - (self.default_bounce_address, '"Formatted Name" '), + ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com'), + ('specific_user@test.mycompany.com', '"Formatted Name" '), + ('specific_user@test.MYCOMPANY.com', '"Formatted Name" '), + ('SPECIFIC_USER@test.mycompany.com', '"Formatted Name" '), + # outside "from_filter" domain: spoofing, as fallback email can be found + ('test@unknown_domain.com', 'test@unknown_domain.com'), + ('test@unknown_domain.com', '"Formatted Name" '), ] ): for provide_smtp in [False, True]: # providing smtp session should ont impact test