Skip to content

Commit

Permalink
[FIX] base: start with fallbacks if email_from False
Browse files Browse the repository at this point in the history
Context:
A pesky bug has been flying under the radar since at least Odoo 15.0.
When passing `email_from=False` to the _find_mail_server, we will always have an early return at:
https://github.com/odoo/odoo/blob/f05626e14264cf3bb477c86ad82439726d778c8f/odoo/addons/base/models/ir_mail_server.py#L668C1-L676C1

This comes from the fact that we can generate False == False => True comparisons while filtering the `ir.mail.server` records, by :
* `email_from_normalize` is False (since `email_from` was also False) and mail server has no from_filter = > email_normalize(m.from_filter) == email_from_normalized => False == False
* `email_from_domain` is False (since `email_from` was also False)  and mail server has a full email address in the from_filter field => email_domain_normalize(m.from_filter) == email_from_domain => False == False

Both edge-cases leads to the first email server config to always be selected for the SMTP connection, even if a valid mail server exist matching for example the default notification email (i.e. notifications@custom.domain).

This can lead to notification emails failing on DB having multiple outgoing email servers setup, where the SMTP server does sender verification (Outlook, Gmail – you can only send as a specific sender email).

Proposed fix:
We wrap the first checkpoint in an if block with email_from. Logically if email_from is already False, we should skip to the second checkpoint (matching notifications default email) and only later try to fall back to the first `ir.mail.server`  record (while also triggering the warning log).
Added test for edge-case

How to reproduce:
Any workflow triggering an automatic notification email as Odoobot, while having the `email` field set to `False`.

1) Setup DB with website_sale installed
2) Set `email` field of Odoobot `res.partner` (by default id = 2) to False
3) Setup at least two outgoing email servers (`ir.mail.server`) with the first having no `from_filter` and the second one matching the default notifications email address (default = notifications@mycompany.example.com)
4) So to the Ecommerce shop and buy any random product and finalizing the transaction.
→ Triggered notification email will always be sent by mail server id = 1, even if it should match id = 2 in this case

OPW-4140192

closes odoo#181844

X-original-commit: 63e5444
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
  • Loading branch information
jorv-odoo committed Sep 29, 2024
1 parent 7ed4496 commit 1f7debc
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 6 deletions.
14 changes: 8 additions & 6 deletions odoo/addons/base/models/ir_mail_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,13 +746,15 @@ def _find_mail_server(self, email_from, mail_servers=None):
mail_servers = mail_servers.filtered('active')

# 1. Try to find a mail server for the right mail from
mail_server = mail_servers.filtered(lambda m: email_normalize(m.from_filter) == email_from_normalized)
if mail_server:
return mail_server[0], email_from
# Skip if passed email_from is False (example Odoobot has no email address)
if email_from:
mail_server = mail_servers.filtered(lambda m: email_normalize(m.from_filter) == email_from_normalized)
if mail_server:
return mail_server[0], email_from

mail_server = mail_servers.filtered(lambda m: email_domain_normalize(m.from_filter) == email_from_domain)
if mail_server:
return mail_server[0], email_from
mail_server = mail_servers.filtered(lambda m: email_domain_normalize(m.from_filter) == email_from_domain)
if mail_server:
return mail_server[0], email_from

# 2. Try to find a mail server for <notifications@domain.com>
if notifications_email:
Expand Down
6 changes: 6 additions & 0 deletions odoo/addons/base/tests/test_ir_mail_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ def test_mail_server_priorities(self):
self.assertEqual(mail_server, self.server_notification, 'Should take the notification email')
self.assertEqual(mail_from, 'notifications@test.com')

# test if notification server is selected if email_from = False
mail_server, mail_from = self.env['ir.mail_server']._find_mail_server(email_from=False)
self.assertEqual(mail_server, self.server_notification,
'Should select the notification email server if passed FROM address was False')
self.assertEqual(mail_from, 'notifications@test.com')

# remove the notifications email to simulate a mis-configured Odoo database
# so we do not have the choice, we have to spoof the FROM
# (otherwise we can not send the email)
Expand Down

0 comments on commit 1f7debc

Please sign in to comment.