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

Since odoo/odoo@f4524f03c3 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

@@ -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)

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 []

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 """

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': '<p>Test</p>',
+            '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):