Skip to content
Permalink
Browse files

[FIX] mail: Ignore aliases for other domains

Imagine this scenario A:

1. In you DB, there's an alias called "sales" and another one called "purchases".
2. Your `mail.catchall.domain` is `alice.example.com`.
3. Bob sends a new email with `To: sales@bob.example.com, purchases@alice.example.com`.
4. Odoo routes wrongly routes the incoming email to the "sales" alias, which should have been ignored because it doesn't even belong to your domain.

Another possible scenario B:

1. In you DB, there's an alias called "sales" and another one called "purchases".
2. Your `mail.catchall.domain` is `alice.example.com`.
3. Bob sends an email, as a reply to a sale, but he's a joker and sends it with `To: purchases@alice.example.com, sales@alice.example.com`.
4. Odoo interprets wrongly that this email is a new record for "purchases", when it could have safely interpreted it as a tricky-but-valid response to an existing "sales" record.

Note: without the patch for scenario A, scenario B can happen also if Bob replies with `To: purchases@bob.example.com, sales@alice.example.com`.

With this patch, these problems won't happen anymore.

@Tecnativa TT20034
  • Loading branch information
Yajo committed Nov 6, 2019
1 parent 74f7aab commit 54d1c9a0db7e878795ff2d38eed72974aa0b39da
@@ -1132,6 +1132,7 @@ def message_route(self, message, message_dict, model=None, thread_id=None, custo
Alias, dest_aliases = self.env['mail.alias'], self.env['mail.alias']
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")
alias_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain")
fallback_model = model

# get email.message.Message variables for future processing
@@ -1148,7 +1149,11 @@ def message_route(self, message, message_dict, model=None, thread_id=None, custo
email_from = tools.decode_message_header(message, 'From')
email_from_localpart = (tools.email_split(email_from) or [''])[0].split('@', 1)[0].lower()
email_to = tools.decode_message_header(message, 'To')
email_to_localpart = (tools.email_split(email_to) or [''])[0].split('@', 1)[0].lower()
email_to_localparts = [
e.split('@', 1)[0].lower()
for e in (tools.email_split(email_to) or [''])
if not alias_domain or e.endswith('@%s' % alias_domain)
]

# Delivered-To is a safe bet in most modern MTAs, but we have to fallback on To + Cc values
# for all the odd MTAs out there, as there is no standard header for the envelope's `rcpt_to` value.
@@ -1158,10 +1163,14 @@ def message_route(self, message, message_dict, model=None, thread_id=None, custo
tools.decode_message_header(message, 'Cc'),
tools.decode_message_header(message, 'Resent-To'),
tools.decode_message_header(message, 'Resent-Cc')])
rcpt_tos_localparts = [e.split('@')[0].lower() for e in tools.email_split(rcpt_tos)]
rcpt_tos_localparts = [
e.split('@')[0].lower()
for e in tools.email_split(rcpt_tos)
if not alias_domain or e.endswith('@%s' % alias_domain)
]

# 0. Verify whether this is a bounced email and use it to collect bounce data and update notifications for customers
if bounce_alias and bounce_alias in email_to_localpart:
if bounce_alias and any(email.startswith(bounce_alias) for email in email_to_localparts):
# Bounce regex: typical form of bounce is bounce_alias+128-crm.lead-34@domain
# group(1) = the mail ID; group(2) = the model (if any); group(3) = the record ID
bounce_re = re.compile("%s\+(\d+)-?([\w.]+)?-?(\d+)?" % re.escape(bounce_alias), re.UNICODE)
@@ -1217,21 +1226,27 @@ def message_route(self, message, message_dict, model=None, thread_id=None, custo
msg_references = [ref for ref in tools.mail_header_msgid_re.findall(thread_references) if 'reply_to' not in ref]
mail_messages = MailMessage.sudo().search([('message_id', 'in', msg_references)], limit=1, order='id desc, message_id')
is_a_reply = bool(mail_messages)
alias_domain = [('alias_name', 'in', rcpt_tos_localparts)]

# 1.1 Handle forward to an alias with a different model: do not consider it as a reply
if reply_model and reply_thread_id:
other_alias = Alias.search([
other_aliases = Alias.search([
'&',
('alias_name', '!=', False),
('alias_name', '=', email_to_localpart)
('alias_name', 'in', email_to_localparts),
])
if other_alias and other_alias.alias_model_id.model != reply_model:
is_a_reply = False
for other_alias in other_aliases:
if other_alias.alias_model_id.model == reply_model:
is_a_reply = bool(mail_messages)
alias_domain.append(("alias_model_id.model", "=", reply_model))
break
if other_alias.alias_model_id.model != reply_model:
is_a_reply = False

if is_a_reply:
model, thread_id = mail_messages.model, mail_messages.res_id
if not reply_private: # TDE note: not sure why private mode as no alias search, copying existing behavior
dest_aliases = Alias.search([('alias_name', 'in', rcpt_tos_localparts)], limit=1)
dest_aliases = Alias.search(alias_domain, limit=1)

route = self.message_route_verify(
message, message_dict,
@@ -1252,15 +1267,15 @@ def message_route(self, message, message_dict, model=None, thread_id=None, custo
message_dict.pop('parent_id', None)

# check it does not directly contact catchall
if catchall_alias and catchall_alias in email_to_localpart:
if catchall_alias and any(email.startswith(catchall_alias) for email in email_to_localparts):
_logger.info('Routing mail from %s to %s with Message-Id %s: direct write to catchall, bounce', email_from, email_to, message_id)
body = self.env.ref('mail.mail_bounce_catchall').render({
'message': message,
}, engine='ir.qweb')
self._routing_create_bounce_email(email_from, body, message, reply_to=self.env.user.company_id.email)
return []

dest_aliases = Alias.search([('alias_name', 'in', rcpt_tos_localparts)])
dest_aliases = Alias.search(alias_domain)
if dest_aliases:
routes = []
for alias in dest_aliases:
@@ -97,6 +97,7 @@ class MailTestAlias(models.Model):
alias_id = fields.Many2one(
'mail.alias', 'Alias',
delegate=True)
message_bounce = fields.Integer(default=0)

def get_alias_model_name(self, vals):
return vals.get('alias_model', 'mail.test')
@@ -179,6 +179,52 @@ def test_message_process_alias_everyone(self):
self.assertEqual(len(record), 1)
self.assertEqual(len(record.message_ids), 1)

@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_process_received_bounce(self):
"""Incoming bounce is properly processed."""
self.env['ir.config_parameter'].set_param('mail.bounce.alias', 'oops')
self.env['ir.config_parameter'].set_param('mail.catchall.domain', 'example.com')

# Test: No group created, incoming bounce is counted
self.assertEqual(self.test_public.message_bounce, 0, 'The bounced thread should have no bounced messages by default')
new_groups = self.format_and_process(
MAIL_TEMPLATE,
email_from='Valid Lelitre <valid.lelitre@agrolait.com>',
to='valid.other@gmail.com, oops+{msg_id}-{model}-{res_id}@example.com'.format(
msg_id=self.fake_email.id,
model=self.fake_email.model,
res_id=self.fake_email.res_id,
),
subject='Your email bounced, be more careful next time plea se',
)
self.assertFalse(new_groups)
self.assertEqual(len(self._mails), 0, 'message_process: incoming bounce produces no mails')
self.assertEqual(self.test_public.message_bounce, 1, 'The bounced thread should have 1 bounced message')

@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_process_received_bounce_no_domain_confusion(self):
"""Incoming bounce-like mails are not confused when in alien domains."""
self.env['ir.config_parameter'].set_param('mail.bounce.alias', 'oops')
self.env['ir.config_parameter'].set_param('mail.catchall.domain', 'example.com')
# A partner with an address that seems like our bounce address
alien_bounce_partner = self.env["res.partner"].create({
"name": "Alien bounce address",
"email": 'oops+{msg_id}-{model}-{res_id}@alien.com'.format(
msg_id=self.fake_email.id,
model=self.fake_email.model,
res_id=self.fake_email.res_id,
),
})
# Test: group created, bounce-similar email is treated as a normal address
new_groups = self.format_and_process(
MAIL_TEMPLATE,
email_from='Valid Lelitre <valid.lelitre@agrolait.com>',
to=alien_bounce_partner.email + ', groups@example.com',
)
self.assertEqual(new_groups.message_ids[0].author_id, self.partner_1, 'message_process: recognized email -> author_id')
self.assertIn('Valid Lelitre <valid.lelitre@agrolait.com>', new_groups.message_ids[0].email_from, 'message_process: recognized email -> email_from')
self.assertEqual(new_groups.message_ids.partner_ids, alien_bounce_partner, 'message_process: alien bounce-like address should be subscribed as a normal partner')

@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models.unlink', 'odoo.addons.mail.models.mail_mail')
def test_message_process_alias_partners_bounce(self):
""" Incoming email from an unknown partner on a Partners only alias -> bounce + test bounce email """
@@ -462,6 +508,17 @@ def test_message_process_crash_no_data(self):
to='noone@example.com', subject='spam', extra='',
msg_id='<1198923581.41972151344608186760.JavaMail.new5@agrolait.com>')

@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_process_crash_alien_domain_same_alias(self):
"""Incoming email to the same address in an alien domain must raise."""
self.env['ir.config_parameter'].set_param('mail.catchall.domain', 'example.com')
with self.assertRaises(ValueError):
self.format_and_process(
MAIL_TEMPLATE,
email_from='Valid Lelitre <valid.lelitre@agrolait.com>',
to='groups@alien.com, other@gmail.com',
)

@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_process_fallback(self):
""" Incoming email with model that accepting incoming emails as fallback """

0 comments on commit 54d1c9a

Please sign in to comment.
You can’t perform that action at this time.