[FIX] tools.mail: ignore original email during encapsulation

When the system broadcasts an email response to document followers,
if the config parameters `mail.force.smtp.from` or
`mail.dynamic.smtp.from` are defined, it will rewrite the `From`
address to avoid spoofing the sender's domain.
**NOTE**: As of 15.0, this is based on the `from_filter` setting on the
corresponding ir.mail_server, rather than the abovementioned config
parameters, but the rest of the discussion stands.

For example, if the `mail.catchall.domain` is set to `example.com` and
an email response comes from:

   "John D" <john@doe.com>

it will rewrite it to:

   "John D (john@doe.com)" <notifications@example.com>

This will make sure the system never sends outgoing email for an external
domain, as it has no authority for doing so, and that could
break mail filtering/authentication rules (SPF, DMARC, etc.)

During this "encapsulation rewrite step", both the original Sender name
and their email are preserved, and put into the quoted "name" field of
the rewritten address. It seems sensible to preserve as much information
as possible about the original sender.

Unfortunately, the inclusion of the Sender email in the final name makes
it appear to some inbox providers as if the message is trying to
deceptively impersonate another person (as many phishing schemes would).
As of November 2021 GMail at least does this, and will hide the name in
the UI when it happens. It will keep only the rewritten email, which is not
very useful in the case of a notification (even though it's more
technically correct, of course).

This patch removes the original email from the rewritten notification,
keeping only the name, considering that the email is not the most
important part, and it's better to have one of the two than none.

So after the patch, the rewritten address is now:

   "John D" <notifications@example.com>

When there is no name in the original address, we keep only the local
part of the email, to avoid the same display issue. The recipient will
have to identify the sender based on the context / past messages.

closes odoo/odoo#81807

X-original-commit: 3c65ec5a8191a392980ceb0a8c584767eae405f1
Signed-off-by: Olivier Dony <odo@odoo.com>

addons/test_mail/tests/test_mail_mail.py

@@ -124,9 +124,9 @@ class TestMailMail(TestMailCommon, MockSmtplibCase):
             any_order=True,
         )
 
-        self.assert_email_sent_smtp(message_from='"test@unknown_domain.com" <notifications@test.com>',
+        self.assert_email_sent_smtp(message_from='"test" <notifications@test.com>',
                                     emails_count=5, from_filter=self.server_notification.from_filter)
-        self.assert_email_sent_smtp(message_from='"test_2@unknown_domain.com" <notifications@test.com>',
+        self.assert_email_sent_smtp(message_from='"test_2" <notifications@test.com>',
                                     emails_count=5, from_filter=self.server_notification.from_filter)
         self.assert_email_sent_smtp(message_from='user_1@test_2.com', emails_count=5, from_filter=self.server_domain_2.from_filter)
         self.assert_email_sent_smtp(message_from='user_2@test_2.com', emails_count=5, from_filter=self.server_domain_2.from_filter)

addons/test_mass_mailing/tests/test_mailing_server.py

@@ -76,7 +76,7 @@ class TestMassMailingServer(TestMassMailCommon, MockSmtplibCase):
         self.assertEqual(self.find_mail_server_mocked.call_count, 1)
         self.assert_email_sent_smtp(
             smtp_from='notifications@test.com',
-            message_from='"Testing (unknow_email@unknow_domain.com)" <notifications@test.com>',
+            message_from='"Testing" <notifications@test.com>',
             from_filter=self.server_notification.from_filter,
             emails_count=8,
         )

odoo/addons/base/tests/test_ir_mail_server.py

@@ -7,7 +7,6 @@ from odoo.addons.base.tests.common import MockSmtplibCase
 from odoo.tests.common import TransactionCase
 from odoo.tools import mute_logger
 
-
 class TestIrMailServer(TransactionCase, MockSmtplibCase):
 
     def setUp(self):
@@ -132,7 +131,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase):
 
         self.assert_email_sent_smtp(
             smtp_from='notifications@test.com',
-            message_from='"Name (test@unknown_domain.com)" <notifications@test.com>',
+            message_from='"Name" <notifications@test.com>',
             from_filter='notifications@test.com',
         )
 
@@ -145,7 +144,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase):
 
         self.assert_email_sent_smtp(
             smtp_from='notifications@test.com',
-            message_from='"test@unknown_domain.com" <notifications@test.com>',
+            message_from='"test" <notifications@test.com>',
             from_filter='notifications@test.com',
         )
 
@@ -179,7 +178,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase):
 
         self.assert_email_sent_smtp(
             smtp_from=default_bounce_adress,
-            message_from='"Name (test@unknown_domain.com)" <notifications@test.com>',
+            message_from='"Name" <notifications@test.com>',
             from_filter='test.com',
         )
 
@@ -230,7 +229,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase):
         self.connect_mocked.assert_called_once()
         self.assert_email_sent_smtp(
             smtp_from='notifications@test.com',
-            message_from='"Name (test@unknown_domain.com)" <notifications@test.com>',
+            message_from='"Name" <notifications@test.com>',
             from_filter='notifications@test.com',
         )
 
@@ -262,7 +261,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase):
         self.connect_mocked.assert_called_once()
         self.assert_email_sent_smtp(
             smtp_from=default_bounce_adress,
-            message_from='"Name (test@unknown_domain.com)" <notifications@test.com>',
+            message_from='"Name" <notifications@test.com>',
             from_filter='test.com',
         )
 
@@ -314,7 +313,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase):
         self.connect_mocked.assert_called_once()
         self.assert_email_sent_smtp(
             smtp_from=default_bounce_adress,
-            message_from='"test@unknown_domain.com" <notifications@test.com>',
+            message_from='"test" <notifications@test.com>',
             from_filter='test.com',
         )
 
@@ -357,7 +356,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase):
         self.connect_mocked.assert_called_once()
         self.assert_email_sent_smtp(
             smtp_from=default_bounce_adress,
-            message_from='"test@unknown_domain.com" <notifications@test.com>',
+            message_from='"test" <notifications@test.com>',
             from_filter='test.com',
         )
 

odoo/tools/mail.py

@@ -606,7 +606,7 @@ def encapsulate_email(old_email, new_email):
     e.g.
     * Old From: "Admin" <admin@gmail.com>
     * New From: notifications@odoo.com
-    * Output:   "Admin (admin@gmail.com)" <notifications@odoo.com>
+    * Output: "Admin" <notifications@odoo.com>
     """
     old_email_split = getaddresses([old_email])
     if not old_email_split or not old_email_split[0]:
@@ -616,10 +616,11 @@ def encapsulate_email(old_email, new_email):
     if not new_email_split or not new_email_split[0]:
         return
 
-    if old_email_split[0][0]:
-        name_part = '%s (%s)' % old_email_split[0]
+    old_name, old_email = old_email_split[0]
+    if old_name:
+        name_part = old_name
     else:
-        name_part = old_email_split[0][1]
+        name_part = old_email.split("@")[0]
 
     return formataddr((
         name_part,