[IMP] mail: allow to remove plus addressing from bounce return addresses

Since odoo/odoo@f4524f0 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 odoo#71242 by dropping requirement of plus addressing.

@Tecnativa TT29827
Closes odoo#71242
Task ID-2547347
PR odoo#72347

addons/mail/models/mail_mail.py

@@ -301,9 +301,12 @@ def _send(self, auto_commit=False, raise_exception=False, smtp_session=None):
                 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)

addons/mail/models/mail_thread.py

@@ -902,6 +902,7 @@ def message_route(self, message, message_dict, model=None, thread_id=None, custo
             raise TypeError('message must be an email.message.Message 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
@@ -949,6 +950,9 @@ def message_route(self, message, message_dict, model=None, thread_id=None, custo
             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 []

addons/test_mail/tests/test_mail_gateway.py

@@ -582,6 +582,46 @@ def test_message_route_bounce(self):
         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_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)
+        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)
+        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 """

addons/test_mail/tests/test_mail_mail.py

@@ -7,12 +7,23 @@
 from odoo import api
 from odoo.addons.base.models.ir_mail_server import MailDeliveryException
 from odoo.addons.test_mail.tests import common as mail_common
-from odoo.tests import common
+from odoo.tests import common, tagged
 from odoo.tools import mute_logger
 
 
+@tagged('mail_mail')
 class TestMail(mail_common.BaseFunctionalTest, mail_common.MockEmails):
 
+    @classmethod
+    def setUpClass(cls):
+        super(TestMail, 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
@@ -39,6 +50,44 @@ def test_mail_message_values_unicode(self):
 
         self.assertRaises(MailDeliveryException, lambda: mail.send(raise_exception=True))
 
+    def test_mail_mail_return_path(self):
+        # mail without thread-enabled record
+        base_values = {
+            'body_html': '<p>Test</p>',
+            'email_to': 'test@example.com',
+        }
+
+        mail = self.env['mail.mail'].create(base_values)
+        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
+        self._mails[:] = []
+        mail = self.env['mail.mail'].create(dict(base_values, **{
+            'model': self.test_record._name,
+            'res_id': self.test_record.id,
+        }))
+        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
+        self._mails[:] = []
+        mail = self.env['mail.mail'].create(base_values)
+        mail.send()
+        self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s@%s' % (self.alias_bounce, self.alias_domain))
+
+        # mail on thread-enabled record
+        self._mails[:] = []
+        mail = self.env['mail.mail'].create(dict(base_values, **{
+            'model': self.test_record._name,
+            'res_id': self.test_record.id,
+        }))
+        mail.send()
+        self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s@%s' % (self.alias_bounce, self.alias_domain))
+
 
 class TestMailRace(common.TransactionCase, mail_common.MockEmails):