From fd2ca5d8ec85e2b61f06e70a0e7c8f2b7c97b617 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Wed, 26 May 2021 10:07:06 +0000 Subject: [PATCH] [FW][IMP] mail: allow to remove plus addressing from bounce return addresses Since odoo/odoo@f4524f03c32a27b1899562a71cad6f491bfe44ce plus addressing is not used anymore for handling bounces. Indeed it relies on references / in reply to to find original message that bounced. It is therefore not necessary to enforce the use of plus addressing. As some provider do not support plus addressing as a way to contact left-part of email with sub-informations people should have a way to deactivate plus addressing used in bounce aliases. To preserve backwards compatibility for stable versions old behavior is retained unless a new `mail.bounce.alias.static` ICP is set with a truthy value. Fix https://github.com/odoo/odoo/issues/71242 by dropping requirement of plus addressing. @Tecnativa TT29827 Closes #71242 Task ID-2547347 PR odoo/odoo#72347 X-original-commit: df2d955bf41b01556e1b84bb1204aac045c95a63 --- addons/mail/models/mail_mail.py | 5 +- addons/mail/models/mail_thread.py | 4 ++ addons/test_mail/tests/test_mail_gateway.py | 43 +++++++++++++++++ addons/test_mail/tests/test_mail_mail.py | 53 ++++++++++++++++++++- 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index 13b0ce3308b..381982e7a17 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -314,9 +314,12 @@ class MailMail(models.Model): headers = {} ICP = self.env['ir.config_parameter'].sudo() bounce_alias = ICP.get_param("mail.bounce.alias") + bounce_alias_static = tools.str2bool(ICP.get_param("mail.bounce.alias.static", "False")) catchall_domain = ICP.get_param("mail.catchall.domain") if bounce_alias and catchall_domain: - if mail.mail_message_id.is_thread_message(): + if bounce_alias_static: + headers['Return-Path'] = '%s@%s' % (bounce_alias, catchall_domain) + elif mail.mail_message_id.is_thread_message(): headers['Return-Path'] = '%s+%d-%s-%d@%s' % (bounce_alias, mail.id, mail.model, mail.res_id, catchall_domain) else: headers['Return-Path'] = '%s+%d@%s' % (bounce_alias, mail.id, catchall_domain) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index b2ad14ca4c0..90009d7ca4a 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -873,6 +873,7 @@ class MailThread(models.AbstractModel): raise TypeError('message must be an email.message.EmailMessage at this point') catchall_alias = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") bounce_alias = self.env['ir.config_parameter'].sudo().get_param("mail.bounce.alias") + bounce_alias_static = tools.str2bool(self.env['ir.config_parameter'].sudo().get_param("mail.bounce.alias.static", "False")) fallback_model = model # get email.message.Message variables for future processing @@ -920,6 +921,9 @@ class MailThread(models.AbstractModel): if bounce_match: self._routing_handle_bounce(message, message_dict) return [] + if bounce_alias and bounce_alias_static and any(email == bounce_alias for email in email_to_localparts): + self._routing_handle_bounce(message, message_dict) + return [] if message.get_content_type() == 'multipart/report' or email_from_localpart == 'mailer-daemon': self._routing_handle_bounce(message, message_dict) return [] diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index e4b6e18e56e..61a4fd16fee 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -265,6 +265,7 @@ class TestMailgateway(TestMailCommon): set(message.attachment_ids.mapped('name')), set(['rosaçée.gif', 'verte!µ.gif', 'orangée.gif'])) + @mute_logger('odoo.addons.mail.models.mail_thread') def test_message_process_followers(self): """ Incoming email: recognized author not archived and not odoobot: added as follower """ with self.mock_mail_gateway(): @@ -773,6 +774,48 @@ class TestMailgateway(TestMailCommon): self.assertFalse(new_recs) self.assertNotSentEmail() + @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models') + def test_message_route_bounce_if_static_but_still_has_plus_addressing(self): + """Incoming email: bounce using bounce alias without plus addressing: keep old behavior.""" + self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', True) + with self.mock_mail_gateway(): + new_recs = self.format_and_process( + MAIL_TEMPLATE, self.partner_1.email_formatted, + '%s+%s-%s-%s@%s' % ( + self.alias_bounce, self.fake_email.id, + self.fake_email.model, self.fake_email.res_id, + self.alias_domain + ), + subject='Should bounce', + ) + self.assertFalse(new_recs) + self.assertEqual(len(self._mails), 0, 'message_process: incoming bounce produces no mails') + + @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models') + def test_message_route_bounce_if_static_without_plus_addressing(self): + """Incoming email: bounce using bounce alias without plus addressing: bounce it.""" + self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', True) + with self.mock_mail_gateway(): + new_recs = self.format_and_process( + MAIL_TEMPLATE, self.partner_1.email_formatted, + '%s@%s' % (self.alias_bounce, self.alias_domain), + subject='Should bounce', + ) + self.assertFalse(new_recs) + self.assertEqual(len(self._mails), 0, 'message_process: incoming bounce produces no mails') + + @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models') + def test_message_route_no_bounce_if_not_static_without_plus_addressing(self): + """Incoming email: bounce using bounce alias without plus addressing: raise as + considering as a direct write to bounce alias -> invalid """ + self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', False) + with self.assertRaises(ValueError): + self.format_and_process( + MAIL_TEMPLATE, self.partner_1.email_formatted, + '%s@%s' % (self.alias_bounce, self.alias_domain), + subject="Should fail because it is not a bounce and there's no alias", + ) + @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models') def test_message_route_bounce_other_recipients(self): """Incoming email: bounce processing: bounce should be computed even if not first recipient """ diff --git a/addons/test_mail/tests/test_mail_mail.py b/addons/test_mail/tests/test_mail_mail.py index 740a1c7f61c..7bed4bc0458 100644 --- a/addons/test_mail/tests/test_mail_mail.py +++ b/addons/test_mail/tests/test_mail_mail.py @@ -4,14 +4,24 @@ import psycopg2 from odoo import api -from odoo.addons.base.models.ir_mail_server import MailDeliveryException from odoo.addons.test_mail.tests.common import TestMailCommon -from odoo.tests import common +from odoo.tests import common, tagged from odoo.tools import mute_logger +@tagged('mail_mail') class TestMailMail(TestMailCommon): + @classmethod + def setUpClass(cls): + super(TestMailMail, cls).setUpClass() + cls._init_mail_gateway() + + cls.test_record = cls.env['mail.test.gateway'].with_context(cls._test_context).create({ + 'name': 'Test', + 'email_from': 'ignasse@example.com', + }).with_context({}) + @mute_logger('odoo.addons.mail.models.mail_mail') def test_mail_message_notify_from_mail_mail(self): # Due ot post-commit hooks, store send emails in every step @@ -25,6 +35,45 @@ class TestMailMail(TestMailCommon): self.assertSentEmail(mail.env.user.partner_id, ['test@example.com']) self.assertEqual(len(self._mails), 1) + def test_mail_mail_return_path(self): + # mail without thread-enabled record + base_values = { + 'body_html': '

Test

', + 'email_to': 'test@example.com', + } + + mail = self.env['mail.mail'].create(base_values) + with self.mock_mail_gateway(): + mail.send() + self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s+%d@%s' % (self.alias_bounce, mail.id, self.alias_domain)) + + # mail on thread-enabled record + mail = self.env['mail.mail'].create(dict(base_values, **{ + 'model': self.test_record._name, + 'res_id': self.test_record.id, + })) + with self.mock_mail_gateway(): + mail.send() + self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s+%d-%s-%s@%s' % (self.alias_bounce, mail.id, self.test_record._name, self.test_record.id, self.alias_domain)) + + # force static addressing on bounce alias + self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', True) + + # mail without thread-enabled record + mail = self.env['mail.mail'].create(base_values) + with self.mock_mail_gateway(): + mail.send() + self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s@%s' % (self.alias_bounce, self.alias_domain)) + + # mail on thread-enabled record + mail = self.env['mail.mail'].create(dict(base_values, **{ + 'model': self.test_record._name, + 'res_id': self.test_record.id, + })) + with self.mock_mail_gateway(): + mail.send() + self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s@%s' % (self.alias_bounce, self.alias_domain)) + class TestMailMailRace(common.TransactionCase):