From 9c1cdd330e51d28fd935e3a285d4ceacf1fb165e Mon Sep 17 00:00:00 2001 From: std-odoo Date: Wed, 12 May 2021 11:40:40 +0000 Subject: [PATCH] [IMP] mail: avoid email loops when Odoo reply automatically Purpose ======= In this commit, we will fix the loops that occur when you send an email to an alias (to create a ticket e.g.). In that case Odoo can reply "Your ticket has been created" and then the auto-replier of the user can reply to this email and the loop occurs. Specifications ============== To solve this issues, we add 2 system parameters - , 120 minutes by default - , 20 by default When an email is sent to an alias, we look on the last records created minutes ago. If we overcome the limit the email is ignored. Alias creation detection ======================== To detect the number of records created by a specific email address we use the `_primary_email` attribute set on the model. Task-2294034 closes odoo/odoo#78597 Related: odoo/enterprise#21772 Signed-off-by: Thibault Delavallee (tde) --- .../hr_recruitment/models/hr_recruitment.py | 1 + addons/mail/__manifest__.py | 1 + addons/mail/data/mail_templates_chatter.xml | 20 --- .../mail/data/mail_templates_mailgateway.xml | 33 +++++ addons/mail/models/mail_thread.py | 122 ++++++++++++++++++ addons/mail/models/mail_thread_blacklist.py | 8 ++ addons/mail/tests/common.py | 6 +- .../mail_group/models/mail_group_message.py | 1 + addons/project/models/project.py | 2 +- addons/test_mail/data/test_mail_data.py | 2 +- addons/test_mail/tests/test_mail_gateway.py | 81 ++++++++++++ .../models/mailing_models.py | 1 + 12 files changed, 254 insertions(+), 24 deletions(-) create mode 100644 addons/mail/data/mail_templates_mailgateway.xml diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py index b86e78e8ac9..d1c2f3a1b30 100644 --- a/addons/hr_recruitment/models/hr_recruitment.py +++ b/addons/hr_recruitment/models/hr_recruitment.py @@ -123,6 +123,7 @@ class Applicant(models.Model): _order = "priority desc, id desc" _inherit = ['mail.thread.cc', 'mail.activity.mixin', 'utm.mixin'] _mailing_enabled = True + _primary_email = 'email_from' name = fields.Char("Subject / Application", required=True, help="Email subject for applications sent via email", index='trigram') active = fields.Boolean("Active", default=True, help="If the active field is set to false, it will allow you to hide the case without removing it.") diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index 7f7894f143b..ebe076efca2 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -32,6 +32,7 @@ 'data/mail_message_subtype_data.xml', 'data/mail_templates_chatter.xml', 'data/mail_templates_email_layouts.xml', + 'data/mail_templates_mailgateway.xml', 'data/mail_channel_data.xml', 'data/mail_activity_data.xml', 'data/ir_cron_data.xml', diff --git a/addons/mail/data/mail_templates_chatter.xml b/addons/mail/data/mail_templates_chatter.xml index 6335f22eccd..3d86bdfe9ec 100644 --- a/addons/mail/data/mail_templates_chatter.xml +++ b/addons/mail/data/mail_templates_chatter.xml @@ -59,25 +59,5 @@

- - - - - - - diff --git a/addons/mail/data/mail_templates_mailgateway.xml b/addons/mail/data/mail_templates_mailgateway.xml new file mode 100644 index 00000000000..17b4ddc275f --- /dev/null +++ b/addons/mail/data/mail_templates_mailgateway.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 39114794ad1..44298548b4d 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -74,6 +74,7 @@ class MailThread(models.AbstractModel): _description = 'Email Thread' _mail_flat_thread = True # flatten the discussion history _mail_post_access = 'write' # access required on the document to post on it + _primary_email = 'email' # Must be set for the models that can be created by alias _Attachment = namedtuple('Attachment', ('fname', 'content', 'info')) message_is_follower = fields.Boolean( @@ -808,6 +809,116 @@ class MailThread(models.AbstractModel): for model in [bl_model for bl_model in bl_models if bl_model.model in self.env]: # transient test mode self.env[model.model].sudo().search([('message_bounce', '>', 0), ('email_normalized', '=', valid_email)])._message_reset_bounce(valid_email) + @api.model + def _check_primary_email(self): + """Check if the "_primary_email" model attribute is correctly set. + + If it is correctly set, return the _primary_email field name + Otherwise return None + """ + primary_email = getattr(self, '_primary_email', None) + if primary_email and primary_email in self._fields: + return primary_email + + @api.model + def _routing_detect_loop_from_records_domain(self, email_from_normalized): + """Return the domain to be used to detect duplicated records created by alias. + + :param email_from_normalized: FROM of the incoming email, normalized + """ + primary_email = self._check_primary_email() + if primary_email: + return [(primary_email, 'ilike', email_from_normalized)] + + _logger.info('Primary email missing on %s', self._name) + + @api.model + def _routing_detect_loop_from_records(self, message, message_dict, routes): + """This method returns True if the incoming email should be ignored. + + The goal of this method is to prevent loops which can occur if an auto-replier + send emails to Odoo. + """ + email_from = message_dict.get('email_from') + if not email_from: + return False + + email_from_normalized = tools.email_normalize(email_from) + + # Detect the email address sent to many emails + get_param = self.env['ir.config_parameter'].sudo().get_param + # Period in minutes in which we will look for + INCOMING_LIMIT_PERIOD = int(get_param('mail.gateway.loop.minutes', 120)) + INCOMING_LIMIT_ALIAS = int(get_param('mail.gateway.loop.threshold', 20)) + + create_date_limit = self.env.cr.now() - datetime.timedelta(minutes=INCOMING_LIMIT_PERIOD) + + # Search only once per model + models = { + self.env[model] + for model, thread_id, *__ in routes or [] + if not thread_id # Reply to an existing thread + } + + for model in models: + if not hasattr(model, '_routing_detect_loop_from_records_domain'): + continue + + domain = model._routing_detect_loop_from_records_domain(email_from_normalized) + + if not domain: + _logger.info('Domain missing for %s, could not find duplicated.', self._name) + continue + + mail_incoming_messages_count = model.sudo().search_count( + expression.AND([ + [('create_date', '>', create_date_limit)], + domain, + ]), + ) + + if mail_incoming_messages_count >= INCOMING_LIMIT_ALIAS: + _logger.info('Email address %s created too many <%s>.', email_from, model) + + body = self.env['ir.qweb']._render( + 'mail.message_notification_limit_email', + {'email': message_dict.get('to')}, + minimal_qcontext=True, + raise_if_not_found=False, + ) + + # Add a reference with a tag, to be able to ignore response to this email + references = ( + message_dict.get('message_id', '') + ' ' + + tools.generate_tracking_message_id('loop-detection-bounce-email') + ) + self._routing_create_bounce_email(email_from, body, message, references=references) + return True + + return False + + @api.model + def _routing_detect_loop_from_headers(self, message, msg_dict): + """Detect common headers set by the auto-repliers / mailing lists.""" + + # Detect if the email has been sent by an auto-replier or by a mailing list + for header in ('X-Autoreply', 'X-Autorespond', 'Feedback-ID', 'List-Id', 'List-Unsubscribe'): + if header in message: + _logger.info('Email sent by an auto-replier (%s), ignoring it.', header) + return True + + precedence = 'Precedence' in message and tools.decode_message_header(message, 'Precedence') + if precedence and precedence.lower() in ('bulk', 'auto_reply', 'list'): + _logger.info('Email sent by an auto-replier (Precedence), ignoring it.') + return True + + if '-loop-detection-bounce-email@' in msg_dict.get('references', '') \ + or '-loop-detection-bounce-email@' in msg_dict.get('in_reply_to', ''): + _logger.info('Email is a reply to the bounce notification, ignoring it.') + return True + + return False + @api.model def message_route(self, message, message_dict, model=None, thread_id=None, custom_values=None): """ Attempt to figure out the correct target model, thread_id, @@ -1091,8 +1202,14 @@ class MailThread(models.AbstractModel): msg_dict.get('email_from'), msg_dict.get('to'), msg_dict.get('message_id')) return False + if self._routing_detect_loop_from_headers(message, msg_dict): + return + # find possible routes for the message routes = self.message_route(message, msg_dict, model, thread_id, custom_values) + if self._routing_detect_loop_from_records(message, msg_dict, routes): + return + thread_id = self._message_route_process(message, msg_dict, routes) return thread_id @@ -1123,6 +1240,11 @@ class MailThread(models.AbstractModel): name_field = self._rec_name or 'name' if name_field in fields and not data.get('name'): data[name_field] = msg_dict.get('subject', '') + + primary_email = self._check_primary_email() + if primary_email and msg_dict.get('email_from'): + data[primary_email] = msg_dict['email_from'] + return self.create(data) def message_update(self, msg_dict, update_vals=None): diff --git a/addons/mail/models/mail_thread_blacklist.py b/addons/mail/models/mail_thread_blacklist.py index 48b9e734ab8..05915b983f7 100644 --- a/addons/mail/models/mail_thread_blacklist.py +++ b/addons/mail/models/mail_thread_blacklist.py @@ -124,3 +124,11 @@ class MailBlackListMixin(models.AbstractModel): } else: raise AccessError(_("You do not have the access right to unblacklist emails. Please contact your administrator.")) + + @api.model + def _routing_detect_loop_from_records_domain(self, email_from_normalized): + """Return the domain to be used to detect duplicated records created by alias. + + :param email_from_normalized: FROM of the incoming email, normalized + """ + return [('email_normalized', '=', email_from_normalized)] diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index b1035d43b78..b6acd739b87 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -114,11 +114,13 @@ class MockEmail(common.BaseCase, MockSmtplibCase): def format(self, template, to='groups@example.com, other@gmail.com', subject='Frogs', email_from='Sylvie Lelitre ', return_path='', cc='', extra='', msg_id='<1198923581.41972151344608186760.JavaMail@agrolait.com>', - **kwargs): + references='', **kwargs): + if not return_path: + return_path = '' return template.format( subject=subject, to=to, cc=cc, email_from=email_from, return_path=return_path, - extra=extra, msg_id=msg_id, + extra=extra, msg_id=msg_id, references=references, **kwargs) def format_and_process(self, template, email_from, to, subject='Frogs', cc='', diff --git a/addons/mail_group/models/mail_group_message.py b/addons/mail_group/models/mail_group_message.py index c76bc8ed5b5..09026ac20a5 100644 --- a/addons/mail_group/models/mail_group_message.py +++ b/addons/mail_group/models/mail_group_message.py @@ -21,6 +21,7 @@ class MailGroupMessage(models.Model): _description = 'Mailing List Message' _rec_name = 'subject' _order = 'create_date DESC' + _primary_email = 'email_from' # fields, can not be done with inherits because it will impact # the performance of the model (different cache, so the ORM will need diff --git a/addons/project/models/project.py b/addons/project/models/project.py index ebf80dbcb29..020d0cac990 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -941,6 +941,7 @@ class Task(models.Model): _inherit = ['portal.mixin', 'mail.thread.cc', 'mail.activity.mixin', 'rating.mixin'] _mail_post_access = 'read' _order = "priority desc, sequence, id desc" + _primary_email = 'email_from' _check_company_auto = True @api.model @@ -2155,7 +2156,6 @@ class Task(models.Model): custom_values = {} defaults = { 'name': msg.get('subject') or _("No Subject"), - 'email_from': msg.get('from'), 'planned_hours': 0.0, 'partner_id': msg.get('author_id'), 'description': msg.get('body'), diff --git a/addons/test_mail/data/test_mail_data.py b/addons/test_mail/data/test_mail_data.py index 81a8f456d7a..38c519aa4fc 100644 --- a/addons/test_mail/data/test_mail_data.py +++ b/addons/test_mail/data/test_mail_data.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -MAIL_TEMPLATE = """Return-Path: +MAIL_TEMPLATE = """Return-Path: {return_path} To: {to} cc: {cc} Received: by mail1.openerp.com (Postfix, from userid 10002) diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index dbaa8e7ee92..d416bb495d7 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -3,6 +3,8 @@ import socket +from datetime import datetime + from unittest.mock import DEFAULT from unittest.mock import patch @@ -12,6 +14,7 @@ from odoo.addons.test_mail.data import test_mail_data from odoo.addons.test_mail.data.test_mail_data import MAIL_TEMPLATE from odoo.addons.test_mail.models.test_mail_models import MailTestGateway from odoo.addons.test_mail.tests.common import TestMailCommon +from odoo.sql_db import Cursor from odoo.tests import tagged from odoo.tests.common import users from odoo.tools import email_split_and_format, formataddr, mute_logger @@ -1403,6 +1406,84 @@ class TestMailgateway(TestMailCommon): self.assertEqual(record.name, 'Spammy') self.assertEqual(record._name, 'mail.test.gateway') + # -------------------------------------------------- + # Emails loop detection + # -------------------------------------------------- + + @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.addons.mail.models.mail_mail') + @patch.object(Cursor, 'now', lambda *args, **kwargs: datetime(2022, 1, 1, 10, 0, 0)) + def test_routing_loop_alias(self): + """Test the limit on the number of record we can create by alias.""" + self.env['ir.config_parameter'].sudo().set_param('mail.gateway.loop.minutes', 30) + self.env['ir.config_parameter'].sudo().set_param('mail.gateway.loop.threshold', 5) + + alias = self.env['mail.alias'].create({ + 'alias_name': 'test', + 'alias_user_id': False, + 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, + 'alias_contact': 'everyone', + }) + + # Send an email 2 hours ago, should not have an impact on more recent emails + with patch.object(Cursor, 'now', lambda *args, **kwargs: datetime(2022, 1, 1, 8, 0, 0)): + self.format_and_process( + MAIL_TEMPLATE, + self.email_from, + f'{self.alias.alias_name}@{self.alias_domain}', + subject='Test alias loop old', + target_model=alias.alias_model_id.model, + ) + + for i in range(5): + self.format_and_process( + MAIL_TEMPLATE, + self.email_from, + f'{self.alias.alias_name}@{self.alias_domain}', + subject=f'Test alias loop {i}', + target_model=alias.alias_model_id.model, + ) + + records = self.env['mail.test.gateway'].search([('name', 'ilike', 'Test alias loop %')]) + self.assertEqual(len(records), 6, 'Should have created 6 ') + self.assertEqual(set(records.mapped('email_from')), set([self.email_from]), + msg='Should have automatically filled the email field') + + self.assertEqual(set(records.mapped('email_from')), {self.email_from}) + + for email_from in (self.email_from, self.email_from.upper()): + with self.mock_mail_gateway(): + self.format_and_process( + MAIL_TEMPLATE, + email_from, + f'{self.alias.alias_name}@{self.alias_domain}', + subject='Test alias loop X', + target_model=alias.alias_model_id.model, + return_path=email_from, + ) + + new_record = self.env['mail.test.gateway'].search([('name', '=', 'Test alias loop X')]) + self.assertFalse( + new_record, + msg='The loop should have been detected and the record should not have been created') + + self.assertSentEmail('"MAILER-DAEMON" ', [email_from]) + self.assertIn('-loop-detection-bounce-email@', self._mails[0]['references'], + msg='The "bounce email" tag must be in the reference') + + # The reply to the bounce email must be ignored + with self.mock_mail_gateway(): + self.format_and_process( + MAIL_TEMPLATE, + self.email_from, + f'{self.alias.alias_name}@{self.alias_domain}', + subject='Test alias loop X', + target_model=alias.alias_model_id.model, + return_path=self.email_from, + extra='References: ', + ) + + self.assertNotSentEmail() + class TestMailThreadCC(TestMailCommon): diff --git a/addons/test_mass_mailing/models/mailing_models.py b/addons/test_mass_mailing/models/mailing_models.py index cafda8d5cab..1b4d3bb78c4 100644 --- a/addons/test_mass_mailing/models/mailing_models.py +++ b/addons/test_mass_mailing/models/mailing_models.py @@ -10,6 +10,7 @@ class MailingSimple(models.Model): _description = 'Simple Mailing' _name = 'mailing.test.simple' _inherit = ['mail.thread'] + _primary_email = 'email_from' name = fields.Char() email_from = fields.Char()