From 7e6a4094484686b3392620255000aad84705ce05 Mon Sep 17 00:00:00 2001 From: David Gallay Date: Mon, 13 Jul 2026 09:45:43 +0200 Subject: [PATCH] [FIX] mail_composer_cc_bcc: prevent Bcc leak and Cc-only emails Due to X-Odoo-Bcc email header, the Bcc were leaking. Report in issue: https://github.com/OCA/mail/issues/142 X-Odoo-Bcc can still be enabled, but is disabled by default. Emails with no `To` were sent due to missing `lang` information. Odoo groups emails using `lang` -> we need to add it as well --- mail_composer_cc_bcc/models/ir_mail_server.py | 20 +- mail_composer_cc_bcc/models/mail_mail.py | 70 +++-- mail_composer_cc_bcc/models/mail_thread.py | 14 + mail_composer_cc_bcc/tests/__init__.py | 1 + .../tests/test_mail_cc_bcc.py | 11 +- .../tests/test_mail_cc_bcc_recipients.py | 272 ++++++++++++++++++ 6 files changed, 348 insertions(+), 40 deletions(-) create mode 100644 mail_composer_cc_bcc/tests/test_mail_cc_bcc_recipients.py diff --git a/mail_composer_cc_bcc/models/ir_mail_server.py b/mail_composer_cc_bcc/models/ir_mail_server.py index df6b1af39..f93389452 100644 --- a/mail_composer_cc_bcc/models/ir_mail_server.py +++ b/mail_composer_cc_bcc/models/ir_mail_server.py @@ -15,20 +15,22 @@ def _prepare_email_message(self, message, smtp_session): """ Define smtp_to based on context instead of To+Cc+Bcc """ - x_odoo_bcc_value = next( - (value for key, value in message._headers if key == "X-Odoo-Bcc"), None - ) - # Add Bcc field inside message to pass validation - if x_odoo_bcc_value: - message["Bcc"] = x_odoo_bcc_value - smtp_from, smtp_to_list, message = super()._prepare_email_message( message, smtp_session ) + # Each recipients gets its own email + # See method `_prepare_outgoing_list` is_from_composer = self.env.context.get("is_from_composer", False) - if is_from_composer and self.env.context.get("recipients", False): - smtp_to = self.env.context["recipients"].pop(0) + if is_from_composer: + # Empty recipients means there is a bug. + # => refuse to send, otherwise it would + # - send duplicate emails + # - leak Bcc + recipients = self.env.context.get("recipients") + if not recipients: + raise ValueError("Could not determine the recipient of this email") + smtp_to = recipients.pop(0) _logger.debug("smtp_to: %s", smtp_to) smtp_to_list = [smtp_to] diff --git a/mail_composer_cc_bcc/models/mail_mail.py b/mail_composer_cc_bcc/models/mail_mail.py index 8ceb59e36..2c98ae9fc 100644 --- a/mail_composer_cc_bcc/models/mail_mail.py +++ b/mail_composer_cc_bcc/models/mail_mail.py @@ -2,6 +2,8 @@ # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). +import os + from odoo import fields, models, tools from odoo.addons.base.models.ir_mail_server import extract_rfc2822_addresses @@ -25,6 +27,17 @@ class MailMail(models.Model): email_bcc = fields.Char("Bcc", help="Blind Cc message recipients") + def _expose_bcc_marker(self): + """Whether to also add the informational ``X-Odoo-Bcc`` marker header. + + Disabled by default: the marker survives sending and would expose the + bcc recipient on every copy. Enable it through the ``expose_x_odoo_bcc`` + context key or the ``EXPOSE_X_ODOO_BCC`` environment variable. + """ + if self.env.context.get("expose_x_odoo_bcc"): + return True + return tools.str2bool(os.environ.get("EXPOSE_X_ODOO_BCC") or "", False) + def _prepare_outgoing_list( self, mail_server=False, recipients_follower_status=None ): @@ -33,16 +46,23 @@ def _prepare_outgoing_list( mail_server=mail_server, recipients_follower_status=recipients_follower_status, ) - is_out_of_scope = len(self.ids) > 1 is_from_composer = self.env.context.get("is_from_composer", False) - if is_out_of_scope or not is_from_composer: + if not is_from_composer: return res - # Prepare values for To, Cc headers + # Every Cc partner is also a recipient and gets its own email, + # so Odoo's Cc-only email is always a duplicate here. + res = [m for m in res if m["email_to"]] + + # The To, Cc headers must be the same on every email, but no record + # holds the whole audience: partner_ids is empty for followers, and the + # mail.mail of the other langs are unlinked as they are sent. partners_cc_bcc = self.recipient_cc_ids + self.recipient_bcc_ids - partner_to_ids = [r.id for r in self.recipient_ids if r not in partners_cc_bcc] - partner_to = self.env["res.partner"].browse(partner_to_ids) + all_recipients = self.env["res.partner"].browse( + self.env.context.get("composer_recipient_ids") or [] + ) + partner_to = all_recipients - partners_cc_bcc email_to = format_emails(partner_to) email_to_raw = format_emails_raw(partner_to) email_cc = format_emails_str(self.recipient_cc_ids) @@ -50,27 +70,23 @@ def _prepare_outgoing_list( # Collect recipients (RCPT TO) and update all emails # with the same To, Cc headers (to be shown by email client as users expect) - recipients = set() + recipients = [] for m in res: - rcpt_to = None - if m["email_to"]: - rcpt_to = extract_rfc2822_addresses(m["email_to"][0])[0] - - # If the recipient is a Bcc, we had an explicit header X-Odoo-Bcc - # - It won't be shown by the email client, but can be useful for a recipient # noqa: E501 - # to understand why he received a given email - # - Also note that in python3, the smtp.send_message method does not - # transmit the Bcc field of a Message object - if rcpt_to in email_bcc: - m["headers"].update({"X-Odoo-Bcc": m["email_to"][0]}) - - # in the absence of self.email_to, Odoo creates one special mail for CC - # see https://github.com/odoo/odoo/commit/46bad8f0 - elif m["email_cc"]: - rcpt_to = extract_rfc2822_addresses(m["email_cc"][0])[0] - - if rcpt_to: - recipients.add(rcpt_to) + m_email_to = m["email_to"][0] + rcpt_to = extract_rfc2822_addresses(m_email_to)[0] + recipients.append(rcpt_to) + + # If the recipient is a Bcc, set a real Bcc header. + # _prepare_email_message uses it to build the envelope + # and then strips it, so it never leaks. + if rcpt_to in email_bcc: + # Avoid mutating the shared headers by making a copy + m["headers"] = {**m["headers"], "Bcc": m_email_to} + # Optional legacy marker. Unlike Bcc it survives sending, + # so only add it when explicitly enabled (it would expose + # the bcc recipient otherwise). + if self._expose_bcc_marker(): + m["headers"]["X-Odoo-Bcc"] = m_email_to m.update( { @@ -80,9 +96,7 @@ def _prepare_outgoing_list( } ) + # Propagate recipients to override smtp_to `_prepare_email_message` self.env.context = {**self.env.context, "recipients": list(recipients)} - if len(res) > len(recipients): - res.pop() - return res diff --git a/mail_composer_cc_bcc/models/mail_thread.py b/mail_composer_cc_bcc/models/mail_thread.py index 73d8ad70f..643c2d06f 100644 --- a/mail_composer_cc_bcc/models/mail_thread.py +++ b/mail_composer_cc_bcc/models/mail_thread.py @@ -93,6 +93,7 @@ def _notify_get_recipients(self, message, msg_vals, **kwargs): "notif": data.get("notif") and data.get("notif") or notif, "type": msg_type, "is_follower": data.get("is_follower"), + "lang": data.get("lang"), "uid": False, } rdata.append(pdata) @@ -123,6 +124,19 @@ def _notify_get_recipients_classify( customer_data["recipients"] += ids return [customer_data] + def _notify_thread_by_email(self, message, recipients_data, **kwargs): + # Pass the whole audience to `_prepare_outgoing_list` + # (only known here) + if self.env.context.get("is_from_composer") and not self.env.context.get( + "skip_adding_cc_bcc" + ): + self = self.with_context( + composer_recipient_ids=[ + data["id"] for data in recipients_data if data["notif"] == "email" + ] + ) + return super()._notify_thread_by_email(message, recipients_data, **kwargs) + def _notify_thread(self, message, msg_vals=False, **kwargs): if message.message_type == "notification": self = self.with_context(skip_adding_cc_bcc=True) diff --git a/mail_composer_cc_bcc/tests/__init__.py b/mail_composer_cc_bcc/tests/__init__.py index 7a8ae2177..e5edeec6e 100644 --- a/mail_composer_cc_bcc/tests/__init__.py +++ b/mail_composer_cc_bcc/tests/__init__.py @@ -1 +1,2 @@ from . import test_mail_cc_bcc +from . import test_mail_cc_bcc_recipients diff --git a/mail_composer_cc_bcc/tests/test_mail_cc_bcc.py b/mail_composer_cc_bcc/tests/test_mail_cc_bcc.py index 009bfe0a5..48e8ed6a0 100644 --- a/mail_composer_cc_bcc/tests/test_mail_cc_bcc.py +++ b/mail_composer_cc_bcc/tests/test_mail_cc_bcc.py @@ -159,12 +159,17 @@ def test_template_cc_bcc(self): expecting = self.partner_cc2 + self.partner_bcc self.assertEqual(composer.partner_bcc_ids, expecting) + def _set_parent_partner(self, parent, childs): + # Ensure assign works even when other modules are installed + # e.g. account: expect single record + for c in childs: + c.parent_id = parent + def test_template_cc_bcc_with_placeholders(self): """Test that template with placeholders for email_cc and email_bcc""" # Add child record to test_record - self.test_record.child_ids |= ( - self.partner_cc + self.partner_cc2 + self.partner_cc3 - ) + child_ids = self.partner_cc + self.partner_cc2 + self.partner_cc3 + self._set_parent_partner(self.test_record, child_ids) # Partner template values tmpl_model = self.env["ir.model"].search([("model", "=", "res.partner")]) diff --git a/mail_composer_cc_bcc/tests/test_mail_cc_bcc_recipients.py b/mail_composer_cc_bcc/tests/test_mail_cc_bcc_recipients.py new file mode 100644 index 000000000..a1dba6e09 --- /dev/null +++ b/mail_composer_cc_bcc/tests/test_mail_cc_bcc_recipients.py @@ -0,0 +1,272 @@ +# Copyright 2026 Camptocamp SA +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). + +from collections import Counter + +from odoo import Command +from odoo.tests.common import TransactionCase +from odoo.tools import email_normalize, email_split + +from odoo.addons.mail.tests.common import MailCase + + +class TestMailCcBccRecipients(TransactionCase, MailCase): + """Sending from the composer to To, Cc and Bcc recipients must always give: + + * exactly one email per recipient, + * the same To and Cc headers on every email, + * a Bcc header on the Bcc recipients' emails only. + + Odoo may split the notification into one mail.mail per lang to render the + layout in each lang. That is fine, but must not change any of the above: + each recipient still belongs to exactly one mail.mail. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + for code in ("fr_FR", "de_DE", "es_ES"): + cls.env["res.lang"]._activate_lang(code) + cls.record = cls.env["res.partner"].create( + {"name": "Document", "email": "document@example.com"} + ) + # us1 belongs to both lists, hence 7 partners for 2 lists of 4 + cls.us1 = cls._create_partner("us1", "en_US") + cls.us2 = cls._create_partner("us2", "en_US") + cls.us3 = cls._create_partner("us3", "en_US") + cls.us4 = cls._create_partner("us4", "en_US") + cls.fr = cls._create_partner("fr", "fr_FR") + cls.de = cls._create_partner("de", "de_DE") + cls.es = cls._create_partner("es", "es_ES") + + cls.partners_same_lang = cls.us1 + cls.us2 + cls.us3 + cls.us4 + cls.partners_different_langs = cls.us1 + cls.fr + cls.de + cls.es + + # ------------------------------------------------------------ + # HELPERS + # ------------------------------------------------------------ + + @classmethod + def _create_partner(cls, prefix, lang): + return cls.env["res.partner"].create( + { + "name": prefix.upper(), + "email": f"{prefix.lower()}@example.com", + "lang": lang, + } + ) + + def _send( + self, subject, partners_to, partners_cc, partners_bcc, mail_unlink_sent=False + ): + """Send one composer message, any number of To / Cc / Bcc partners""" + composer = ( + self.env["mail.compose.message"] + .with_context( + default_model=self.record._name, + default_res_ids=self.record.ids, + default_composition_mode="comment", + mail_notify_force_send=True, + ) + .create( + { + "subject": subject, + "body": "

Hello

", + "partner_ids": [Command.set(partners_to.ids)], + "partner_cc_ids": [Command.set(partners_cc.ids)], + "partner_bcc_ids": [Command.set(partners_bcc.ids)], + } + ) + ) + with self.mock_mail_gateway(mail_unlink_sent=mail_unlink_sent): + composer._action_send_mail() + return self.record.message_ids.filtered(lambda m: m.subject == subject)[:1] + + def _send_from_followers( + self, subject, partners_to, partners_cc, partners_bcc, mail_unlink_sent=False + ): + """Send with no explicit To: the recipients are followers of the record""" + self.record.message_subscribe(partner_ids=partners_to.ids) + composer = ( + self.env["mail.compose.message"] + .with_context( + default_model=self.record._name, + default_res_ids=self.record.ids, + default_composition_mode="comment", + mail_notify_force_send=True, + ) + .create( + { + "subject": subject, + "body": "

Hello

", + "partner_cc_ids": [Command.set(partners_cc.ids)], + "partner_bcc_ids": [Command.set(partners_bcc.ids)], + } + ) + ) + with self.mock_mail_gateway(mail_unlink_sent=mail_unlink_sent): + composer._action_send_mail() + return self.record.message_ids.filtered(lambda m: m.subject == subject)[:1] + + def _assert_emails_defined(self): + assert hasattr( + self, "emails" + ), "self.emails undefined. Did you use _send method ?" + assert self.emails, "No email was sent" + + def _envelope_recipients(self): + """Who each sent email is actually for (RCPT TO), one entry per email""" + # NOTE: `To` != `RCPT TO` + self._assert_emails_defined() + return [addr for email in self.emails for addr in email["smtp_to_list"]] + + def _assert_one_email_per_recipient(self, partners): + expected = Counter(partners.mapped("email")) + received = Counter(self._envelope_recipients()) + if received == expected: + return + report = ["Each recipient must get exactly 1 email:"] + for address in sorted(set(expected) | set(received)): + got = received.get(address, 0) + wanted = expected.get(address, 0) + flag = " <-- WRONG" if got != wanted else "" + report.append(f" {address}: got {got}, expected {wanted}{flag}") + report.append( + f" total sent: {sum(received.values())}, expected {len(partners)}" + ) + self.fail("\n".join(report)) + + def _assert_each_recipient_in_one_mail(self, message, partners): + """Whatever the lang split, a recipient belongs to a single mail.mail""" + received = Counter( + partner.email for mail in message.mail_ids for partner in mail.recipient_ids + ) + expected = Counter(partners.mapped("email")) + if received == expected: + return + report = ["Each recipient must belong to exactly 1 mail.mail:"] + for mail in message.mail_ids: + report.append(f" mail.mail: {mail.recipient_ids.mapped('email')}") + for address in sorted(set(expected) | set(received)): + report.append( + f" {address}: in {received.get(address, 0)} mail.mail, " + f"expected {expected.get(address, 0)}" + ) + self.fail("\n".join(report)) + + def _assert_same_to_cc_headers(self, partners_to, partners_cc): + self._assert_emails_defined() + # To is built from a list and Cc from a joined string, and the partners + # come out in no guaranteed order: compare addresses, not text. + for header, expected in (("msg_to", partners_to), ("msg_cc", partners_cc)): + values = { + tuple( + sorted(email_normalize(a) for a in email_split(email[header] or "")) + ) + for email in self.emails + } + self.assertEqual( + len(values), + 1, + f"Every email must show the same {header}, got {sorted(values)}", + ) + self.assertEqual(sorted(values.pop()), sorted(expected.mapped("email"))) + + def _assert_bcc_header_on_bcc_emails_only(self, partners_bcc): + values = [(mail.get("headers") or {}).get("Bcc") for mail in self._mails] + found = sorted(email_normalize(value) for value in values if value) + self.assertEqual( + found, + sorted(partners_bcc.mapped("email")), + "Only the Bcc recipients' emails carry a Bcc header, one each", + ) + + def _assert_headers(self, partners_to, partners_cc, partners_bcc): + """Assertions on the sent emails only, no mail.mail needed""" + self._assert_one_email_per_recipient(partners_to + partners_cc + partners_bcc) + self._assert_same_to_cc_headers(partners_to, partners_cc) + self._assert_bcc_header_on_bcc_emails_only(partners_bcc) + + def _assert_mails(self, message, partners_to, partners_cc, partners_bcc): + self._assert_each_recipient_in_one_mail( + message, partners_to + partners_cc + partners_bcc + ) + self._assert_one_email_per_recipient(partners_to + partners_cc + partners_bcc) + self._assert_same_to_cc_headers(partners_to, partners_cc) + self._assert_bcc_header_on_bcc_emails_only(partners_bcc) + + # ------------------------------------------------------------ + # TESTS + # ------------------------------------------------------------ + + def test_same_lang(self): + """4 recipients sharing one lang: a single mail.mail, 4 emails""" + partners_to, cc_1, cc_2, partners_bcc = self.partners_same_lang + partners_cc = cc_1 + cc_2 + message = self._send("same-lang", partners_to, partners_cc, partners_bcc) + + self.assertEqual(len(message.mail_ids), 1, "One lang means one mail.mail") + self._assert_mails(message, partners_to, partners_cc, partners_bcc) + + def test_several_to_in_different_langs(self): + """Two To in two langs still see each other in their To header""" + partners_to = self.us1 + self.fr + partners_cc = self.de + partners_bcc = self.es + message = self._send("many-to", partners_to, partners_cc, partners_bcc) + + self.assertEqual(len(message.mail_ids), 4, "One mail.mail per lang") + self._assert_mails(message, partners_to, partners_cc, partners_bcc) + + def test_different_langs(self): + """4 recipients, 4 langs: the split may happen, the emails must not change""" + partners_to, cc_1, cc_2, partners_bcc = self.partners_different_langs + partners_cc = cc_1 + cc_2 + message = self._send("different-langs", partners_to, partners_cc, partners_bcc) + + self._assert_mails(message, partners_to, partners_cc, partners_bcc) + + def test_followers_are_to_recipients(self): + """Followers get the email, though they never reach partner_ids""" + partners_to = self.us1 + self.fr + message = self._send_from_followers("followers", partners_to, self.de, self.es) + + self.assertFalse( + message.partner_ids, "Followers are not added to partner_ids by Odoo" + ) + self._assert_mails(message, partners_to, self.de, self.es) + + def test_to_header_survives_sent_mails_being_unlinked(self): + """Sent mails are unlinked: the last one must still show every To + + mail_unlink_sent is what production does, MailCase disables it. + """ + partners_to, cc_1, cc_2, partners_bcc = self.partners_different_langs + partners_cc = cc_1 + cc_2 + self._send( + "unlinked", + partners_to, + partners_cc, + partners_bcc, + mail_unlink_sent=True, + ) + # no _assert_mails here: the mail.mail are gone, only the emails remain + self._assert_headers(partners_to, partners_cc, partners_bcc) + + def test_no_recipient_left_refuses_to_send(self): + """Running out of recipients must raise, never fall back to To+Cc+Bcc + + The fallback would send to everyone at once and disclose the Bcc. + """ + mail_server = self.env["ir.mail_server"] + message = mail_server.build_email( + email_from="sender@example.com", + email_to=[self.us1.email], + subject="no-recipient-left", + body="

Hello

", + email_cc=self.de.email, + email_bcc=self.es.email, + ) + server = mail_server.with_context(is_from_composer=True, recipients=[]) + with self.assertRaises(ValueError): + server._prepare_email_message(message, None)