[MERGE] tools, mail: ease bounce email content update based on alias security

PURPOSE When incoming emails bounce due to alias security bounce email is quite generic. Purpose of this task is to ease its customization and update. SPECIFICATIONS In order to improve the flexibility of alias, add a customizable html field on the alias model. This html content will be send as bounce email core content in case of bounced/unauthorized mail received for this alias, Obviously it has no effect on 'everyone' security setting as no email will bounce due to that issue. If it is not set a default generic mail will be send depending on security setting. It allows to keep void html fields when no specific bounce content is required In HR, an old template allowing some light customization for employee based security option is removed as it is completely replaced by the new feature. Also add references message-id of the mail received to the answer so that threads are correctly set. LINKS Task ID 2126509 Community PR odoo/odoo#41827 Upgrade PR odoo/upgrade#936 Related: odoo/upgrade#936 Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
2020-03-12 15:27:06 +01:00
parent b1037eb2d1 c5e6511e8d
commit ee8c1b09cd
10 changed files with 127 additions and 81 deletions
@@ -60,44 +60,6 @@
                ])]"/>
        </record>

-        <record id="mail_template_data_unknown_employee_email_address" model="mail.template">
-            <field name="name">HR: mailgateway unknown employee bounce</field>
-            <field name="model_id" ref="base.model_ir_module_module"/>
-            <field name="subject">Your document has not been created</field>
-            <field name="email_from">${user.email_formatted | safe}</field>
-            <field name="email_to">${ctx['email_to']|safe}</field>
-            <field name="body_html" type="html">
-<table border="0" cellpadding="0" cellspacing="0" style="padding-top: 16px; background-color: #F1F1F1; font-family:Verdana, Arial,sans-serif; color: #454748; width: 100%; border-collapse:separate;"><tr><td align="center">
-<table border="0" cellpadding="0" cellspacing="0" width="590" style="padding: 16px; background-color: white; color: #454748; border-collapse:separate;">
-<tbody>
-    <tr>
-        <td align="center" style="min-width: 590px;">
-            <table border="0" cellpadding="0" cellspacing="0" width="590" style="min-width: 590px; background-color: white; padding: 0px 8px 0px 8px; border-collapse:separate;">
-                <tr><td valign="top" style="font-size: 13px;">
-                    <div>
-                        Hi,<br/>
-                        Your document has not been created because your email address is not recognized. Please send emails with the email address recorded on your employee information, or contact your HR manager.
-                    </div>
-                </td></tr>
-            </table>
-    </td></tr>
-</tbody>
-</table>
-</td></tr>
-<!-- POWERED BY -->
-<tr><td align="center" style="min-width: 590px;">
-    <table border="0" cellpadding="0" cellspacing="0" width="590" style="min-width: 590px; background-color: #F1F1F1; color: #454748; padding: 8px; border-collapse:separate;">
-      <tr><td style="text-align: center; font-size: 13px;">
-        Powered by <a target="_blank" href="https://www.odoo.com?utm_source=db&amp;utm_medium=hr" style="color: #875A7B;">Odoo</a>
-      </td></tr>
-    </table>
-</td></tr>
-</table>
-            </field>
-            <field name="lang">${user.lang}</field>
-            <field name="auto_delete" eval="True"/>
-        </record>
-
        <record model="ir.config_parameter" id="hr_presence_control_login" forcecreate="False">
            <field name="key">hr.hr_presence_control_login</field>
            <field name="value">True</field>
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.

-from odoo import fields, models, tools
+from odoo import fields, models, tools, _


 class Alias(models.Model):
@@ -9,8 +9,16 @@ class Alias(models.Model):

    alias_contact = fields.Selection(selection_add=[('employees', 'Authenticated Employees')])

+    def _get_alias_bounced_body_fallback(self, message_dict):
+        if self.alias_contact == 'employees':
+            return _("""Hi,<br/>
+Your document has not been created because your email address is not recognized.<br/>
+Please send emails with the email address recorded on your employee information, or contact your HR manager.""")
+        else:
+            return super(Alias, self)._get_alias_bounced_body_fallback(message_dict)

-class MailAlias(models.AbstractModel):
+
+class AliasMixin(models.AbstractModel):
    _inherit = 'mail.alias.mixin'

    def _alias_check_contact_on_record(self, record, message, message_dict, alias):
@@ -21,9 +29,6 @@ class MailAlias(models.AbstractModel):
            if not employee:
                employee = self.env['hr.employee'].search([('user_id.email', 'ilike', email_address)], limit=1)
            if not employee:
-                return {
-                    'error_message': 'restricted to employees',
-                    'error_template': self.env.ref('hr.mail_template_data_unknown_employee_email_address').body_html,
-                }
+                return _('restricted to employees')
            return True
-        return super(MailAlias, self)._alias_check_contact_on_record(record, message, message_dict, alias)
+        return super(AliasMixin, self)._alias_check_contact_on_record(record, message, message_dict, alias)
@@ -363,6 +363,12 @@
 <blockquote><t t-esc="message['body']"/></blockquote>
        </template>

+        <!-- Mail bounce alias mail template -->
+        <template id="mail_bounce_alias_security">
+<div><t t-raw="body"/></div>
+<blockquote><t t-raw="message['body']"/></blockquote>
+        </template>
+
        <!-- Channel and moderation related data -->
        <template id="mail_channel_notify_moderation">
 <div>
@@ -7,7 +7,7 @@ import re

 from odoo import _, api, fields, models
 from odoo.exceptions import ValidationError
-from odoo.tools import remove_accents
+from odoo.tools import remove_accents, is_html_empty

 _logger = logging.getLogger(__name__)

@@ -68,6 +68,9 @@ class Alias(models.Model):
             "- everyone: everyone can post\n"
             "- partners: only authenticated partners\n"
             "- followers: only followers of the related document or members of following channels\n")
+    alias_bounced_content = fields.Html(
+        "Custom Bounced Message", translate=True,
+        help="If set, this content will automatically be sent out to unauthorized users instead of the default message.")

    _sql_constraints = [
        ('alias_unique', 'UNIQUE(alias_name)', 'Unfortunately this email alias is already used, please choose a unique one')
@@ -171,6 +174,36 @@ class Alias(models.Model):
            'type': 'ir.actions.act_window',
        }

+    def _get_alias_bounced_body_fallback(self, message_dict):
+        return _("""Hi,<br/>
+The following email sent to %s cannot be accepted because this is a private email address.
+Only allowed people can contact us at this address.""" % message_dict.get('to'))
+
+    def _get_alias_bounced_body(self, message_dict):
+        """Get the body of the email return in case of bounced email.
+
+        :param message_dict: dictionary of mail values
+        """
+        lang_author = False
+        if message_dict.get('author_id'):
+            try:
+                lang_author = self.env['res.partner'].browse(message_dict['author_id']).lang
+            except:
+                pass
+
+        if lang_author:
+            self = self.with_context(lang=lang_author)
+
+        if not is_html_empty(self.alias_bounced_content):
+            body = self.alias_bounced_content
+        else:
+            body = self._get_alias_bounced_body_fallback(message_dict)
+        template = self.env.ref('mail.mail_bounce_alias_security', raise_if_not_found=True)
+        return template.render({
+            'body': body,
+            'message': message_dict
+        }, engine='ir.qweb', minimal_qcontext=True)
+

 class AliasMixin(models.AbstractModel):
    """ A mixin for models that inherits mail.alias. This mixin initializes the
@@ -255,20 +288,12 @@ class AliasMixin(models.AbstractModel):
        author = self.env['res.partner'].browse(message_dict.get('author_id', False))
        if alias.alias_contact == 'followers':
            if not record.ids:
-                return {
-                    'error_message': _('incorrectly configured alias (unknown reference record)'),
-                }
+                return _('incorrectly configured alias (unknown reference record)')
            if not hasattr(record, "message_partner_ids") or not hasattr(record, "message_channel_ids"):
-                return {
-                    'error_message': _('incorrectly configured alias'),
-                }
+                return _('incorrectly configured alias')
            accepted_partner_ids = record.message_partner_ids | record.message_channel_ids.mapped('channel_partner_ids')
            if not author or author not in accepted_partner_ids:
-                return {
-                    'error_message': _('restricted to followers'),
-                }
+                return _('restricted to followers')
        elif alias.alias_contact == 'partners' and not author:
-            return {
-                'error_message': _('restricted to known authors')
-            }
+            return _('restricted to known authors')
        return True
@@ -383,9 +383,7 @@ class Channel(models.Model):
        if alias.alias_contact == 'followers' and self.ids:
            author = self.env['res.partner'].browse(message_dict.get('author_id', False))
            if not author or author not in self.channel_partner_ids:
-                return {
-                    'error_message': _('restricted to channel members'),
-                }
+                return _('restricted to channel members')
            return True
        return super(Channel, self)._alias_check_contact(message, message_dict, alias)

@@ -30,7 +30,7 @@ from odoo import _, api, exceptions, fields, models, tools, registry, SUPERUSER_
 from odoo.exceptions import MissingError
 from odoo.osv import expression

-from odoo.tools import pycompat, ustr
+from odoo.tools import ustr
 from odoo.tools.misc import clean_context, split_every

 _logger = logging.getLogger(__name__)
@@ -820,7 +820,6 @@ class MailThread(models.AbstractModel):
                fallback on a message_new by resetting thread_id
            2 - check that message_update exists if thread_id is set; or at least
                that message_new exist
-            [ - find author_id if udpate_author is set]
            3 - if there is an alias, check alias_contact:
                'followers' and thread_id:
                    check on target document that the author is in the followers
@@ -848,12 +847,6 @@ class MailThread(models.AbstractModel):
        model, thread_id, alias = route[0], route[1], route[4]
        record_set = None

-        _generic_bounce_body_html = """<div>
-<p>Hello,</p>
-<p>The following email sent to %s cannot be accepted because this is a private email address.
-   Only allowed people can contact us at this address.</p>
-</div><blockquote>%s</blockquote>""" % (message_dict.get('to'), message_dict.get('body'))
-
        # Wrong model
        if not model:
            self._routing_warn(_('target model unspecified'), message_id, route, raise_exception)
@@ -877,7 +870,7 @@ class MailThread(models.AbstractModel):
            self._routing_warn(_('model %s does not accept document creation') % model, message_id, route, raise_exception)
            return ()

-        # Update message author if asked. We do it now because we need it for aliases (contact settings)
+        # Update message author. We do it now because we need it for aliases (contact settings)
        if not author_id:
            if record_set:
                authors = self._mail_find_partner_from_emails([email_from], records=record_set)
@@ -898,12 +891,13 @@ class MailThread(models.AbstractModel):
            else:
                obj = self.env[model]
            if hasattr(obj, '_alias_check_contact'):
-                check_result = obj._alias_check_contact(message, message_dict, alias)
+                error_code = obj._alias_check_contact(message, message_dict, alias)
            else:
-                check_result = self.env['mail.alias.mixin']._alias_check_contact_on_record(obj, message, message_dict, alias)
-            if check_result is not True:
-                self._routing_warn(_('alias %s: %s') % (alias.alias_name, check_result.get('error_message', _('unknown error'))), message_id, route, False)
-                self._routing_create_bounce_email(email_from, check_result.get('error_template', _generic_bounce_body_html), message)
+                error_code = self.env['mail.alias.mixin']._alias_check_contact_on_record(obj, message, message_dict, alias)
+            if error_code is not True:
+                self._routing_warn(_('alias %s: %s') % (alias.alias_name, error_code or _('unknown error')), message_id, route, False)
+                body = alias._get_alias_bounced_body(message_dict)
+                self._routing_create_bounce_email(email_from, body, message, references=message_id)
                return False

        return (model, thread_id, route[2], route[3], route[4])
@@ -1055,7 +1049,7 @@ class MailThread(models.AbstractModel):
                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.company.email)
+                self._routing_create_bounce_email(email_from, body, message, references=message_id, reply_to=self.env.company.email)
                return []

            dest_aliases = self.env['mail.alias'].search([('alias_name', 'in', rcpt_tos_valid_localparts)])
@@ -26,6 +26,7 @@
                            <field name="alias_user_id"/>
                            <field name="alias_parent_model_id"/>
                            <field name="alias_parent_thread_id"/>
+                            <field name="alias_bounced_content" attrs="{'invisible':[('alias_contact', '=', 'everyone')]}"/>
                        </group>
                    </sheet>
                </form>
@@ -235,6 +235,32 @@ class TestMailgateway(TestMailCommon):
    # Alias configuration
    # --------------------------------------------------

+    @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models.unlink', 'odoo.addons.mail.models.mail_mail')
+    def test_message_process_alias_config_bounced_content(self):
+        """ Custom bounced message for the alias => Received this custom message """
+        self.alias.write({
+            'alias_contact': 'partners',
+            'alias_bounced_content': '<p>What Is Dead May Never Die</p>'
+        })
+
+        # Test: custom bounced content
+        with self.mock_mail_gateway():
+            record = self.format_and_process(MAIL_TEMPLATE, self.email_from, 'groups@test.com', subject='Should Bounce')
+        self.assertFalse(record, 'message_process: should have bounced')
+        self.assertSentEmail('"MAILER-DAEMON" <bounce.test@test.com>', ['whatever-2a840@postmaster.twitter.com'], body_content='<p>What Is Dead May Never Die</p>')
+
+        self.alias.write({
+            'alias_contact': 'partners',
+            'alias_bounced_content': '<p></br></p>'
+        })
+
+        # Test: with "empty" bounced content (simulate view, putting always '<p></br></p>' in html field)
+        with self.mock_mail_gateway():
+            record = self.format_and_process(MAIL_TEMPLATE, self.email_from, 'groups@test.com', subject='Should Bounce')
+        self.assertFalse(record, 'message_process: should have bounced')
+        # Check if default (hardcoded) value is in the mail content
+        self.assertSentEmail('"MAILER-DAEMON" <bounce.test@test.com>', ['whatever-2a840@postmaster.twitter.com'], body_content='The following email sent to')
+
    @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
    def test_message_process_alias_user_id(self):
        """ Test alias ownership """
@@ -9,7 +9,8 @@ import threading

 from odoo.tests.common import BaseCase, SavepointCase, TransactionCase
 from odoo.tools import (
-    html_sanitize, append_content_to_html, plaintext2html, email_split,
+    is_html_empty, html_sanitize, append_content_to_html, plaintext2html,
+    email_split,
    misc, formataddr,
 )

@@ -103,10 +104,10 @@ class TestSanitizer(BaseCase):
        not_emails = [
            '<blockquote cite="mid:CAEJSRZvWvud8c6Qp=wfNG6O1+wK3i_jb33qVrF7XyrgPNjnyUA@mail.gmail.com" type="cite">cat</blockquote>',
            '<img alt="@github-login" class="avatar" src="/web/image/pi" height="36" width="36">']
-        for email in not_emails:
-            sanitized = html_sanitize(email)
-            left_part = email.split('>')[0]  # take only left part, as the sanitizer could add data information on node
-            self.assertNotIn(misc.html_escape(email), sanitized, 'html_sanitize stripped emails of original html')
+        for not_email in not_emails:
+            sanitized = html_sanitize(not_email)
+            left_part = not_email.split('>')[0]  # take only left part, as the sanitizer could add data information on node
+            self.assertNotIn(misc.html_escape(not_email), sanitized, 'html_sanitize stripped emails of original html')
            self.assertIn(left_part, sanitized)

    def test_style_parsing(self):
@@ -318,6 +319,19 @@ class TestHtmlTools(BaseCase):
        for html, content, plaintext_flag, preserve_flag, container_tag, expected in test_samples:
            self.assertEqual(append_content_to_html(html, content, plaintext_flag, preserve_flag, container_tag), expected, 'append_content_to_html is broken')

+    def test_is_html_empty(self):
+        void_strings_samples = ['', False, ' ']
+        for content in void_strings_samples:
+            self.assertTrue(is_html_empty(content))
+
+        void_html_samples = ['<p><br></p>', '<p><br> </p>']
+        for content in void_html_samples:
+            self.assertTrue(is_html_empty(content), 'Failed with %s' % content)
+
+        valid_html_samples = ['<p><br>1</p>', '<p>1<br > </p>']
+        for content in valid_html_samples:
+            self.assertFalse(is_html_empty(content))
+

 class TestEmailTools(BaseCase):
    """ Test some of our generic utility functions for emails """
@@ -254,6 +254,21 @@ def html_sanitize(src, silent=True, sanitize_tags=True, sanitize_attributes=Fals
 # HTML/Text management
 #----------------------------------------------------------

+
+def is_html_empty(html_content):
+    """Check if a html content is empty. If there are only formatting tags or
+    a void content return True. Famous use case if a '<p><br></p>' added by
+    some web editor.
+
+    :param str html_content: html content, coming from example from an HTML field
+    :returns: bool, True if no content found or if containing only void formatting tags
+    """
+    if not html_content:
+        return True
+    tag_re = re.compile(r'\<\s*\/?(?:p|div|span|br|b|i)\s*\>')
+    return not bool(re.sub(tag_re, '', html_content).strip())
+
+
 def html_keep_url(text):
    """ Transform the url into clickable link with <a/> tag """
    idx = 0