From c03654896febf192b43f6482e830e2e8a006c668 Mon Sep 17 00:00:00 2001 From: Katherine Zaoral Date: Tue, 25 Aug 2020 13:59:39 -0300 Subject: [PATCH 1/2] [ADD] mail_outbound_static: Domain whitelist based logic. --- mail_outbound_static/README.rst | 32 ++- mail_outbound_static/__manifest__.py | 2 +- mail_outbound_static/i18n/es.po | 63 +++++ .../i18n/mail_outbound_static.pot | 30 ++- mail_outbound_static/models/ir_mail_server.py | 98 +++++++- mail_outbound_static/readme/CONTRIBUTORS.rst | 2 + mail_outbound_static/readme/DESCRIPTION.rst | 25 +- mail_outbound_static/readme/ROADMAP.rst | 3 +- mail_outbound_static/readme/USAGE.rst | 1 + .../static/description/index.html | 31 ++- .../tests/test_ir_mail_server.py | 215 +++++++++++++++++- .../views/ir_mail_server_view.xml | 1 + 12 files changed, 478 insertions(+), 25 deletions(-) create mode 100644 mail_outbound_static/i18n/es.po diff --git a/mail_outbound_static/README.rst b/mail_outbound_static/README.rst index 8f75c5515..bda2333b3 100644 --- a/mail_outbound_static/README.rst +++ b/mail_outbound_static/README.rst @@ -26,8 +26,29 @@ Mail Outbound Static |badge1| |badge2| |badge3| |badge4| |badge5| This module brings Odoo outbound emails in to strict compliance with RFC-2822 -by allowing for a statically configured From header, with the sender's e-mail -being appended into the proper Sender header instead. +by allowing for a dynamically configured From header, with the sender's e-mail +being appended into the proper Sender header instead. To accomplish this we: + +* Add a domain whitelist field in the mail server model. This one represent an + allowed Domains list separated by commas. If there is not given SMTP server + it will let us to search the proper mail server to be used to sent themessages + where the message 'From' email domain match with the domain whitelist. If + there is not mail sever that match then will use the default mail server to + sent the message. + +* Add a Email From field that will let us to email from a specific address taking + into account this conditions: + + 1) If the sender domain match with the domain whitelist then the original + message's 'From' will remain as it is and will not be changed because the + mail server is able to sent in the name of the sender domain. + + 2) If the original message's 'From' does not match with the domain whitelist + then the email From is replaced with the Email From field value. + +* Add compatibility to define the smtp information in Odoo config file. Both + smtp_from and smtp_whitelist_domain values will be used if there is not mail + server configured in the system. **Table of contents** @@ -39,11 +60,13 @@ Usage * Navigate to an Outbound Email Server * Set the `Email From` option to an email address +* Set the `Domain Whitelist` option with the domain whitelist Known issues / Roadmap ====================== -* Allow for domain-based whitelist that will not be manipulated +* Add validation of smtp_from field to ensure that is a valid email address +* Add validation of domain_whitelist field to ensure that they are valid domains Bug Tracker =========== @@ -63,6 +86,7 @@ Authors * brain-tec AG * LasLabs +* Adhoc SA Contributors ~~~~~~~~~~~~ @@ -70,6 +94,8 @@ Contributors * Frédéric Garbely * Dave Lasley * Lorenzo Battistini +* Katherine Zaoral +* Juan José Scarafía Maintainers ~~~~~~~~~~~ diff --git a/mail_outbound_static/__manifest__.py b/mail_outbound_static/__manifest__.py index 3b8c5939c..313419903 100644 --- a/mail_outbound_static/__manifest__.py +++ b/mail_outbound_static/__manifest__.py @@ -7,7 +7,7 @@ "version": "13.0.1.0.1", "category": "Discuss", "website": "https://github.com/OCA/social", - "author": "brain-tec AG, LasLabs, Odoo Community Association (OCA)", + "author": "brain-tec AG, LasLabs, Adhoc SA, Odoo Community Association (OCA)", "license": "LGPL-3", "application": False, "installable": True, diff --git a/mail_outbound_static/i18n/es.po b/mail_outbound_static/i18n/es.po new file mode 100644 index 000000000..1ca5962f1 --- /dev/null +++ b/mail_outbound_static/i18n/es.po @@ -0,0 +1,63 @@ +# Translation of Odoo Server. +# This file contains the translation of the following modules: +# * mail_outbound_static +# +msgid "" +msgstr "" +"Project-Id-Version: Odoo Server 13.0\n" +"Report-Msgid-Bugs-To: \n" +"POT-Creation-Date: 2020-09-03 12:53+0000\n" +"PO-Revision-Date: 2020-09-03 12:53+0000\n" +"Last-Translator: \n" +"Language-Team: \n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=UTF-8\n" +"Content-Transfer-Encoding: \n" +"Plural-Forms: \n" + +#. module: mail_outbound_static +#: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__domain_whitelist +msgid "" +"Allowed Domains list separated by commas. If there is not given SMTP server " +"it will let us to search the proper mail server to be used to sent the " +"messages where the message 'From' email domain match with the domain " +"whitelist." +msgstr "" +"Lista de dominios permitidos separados por comas. Si no se ha seleccionado un servidor SMTP " +"nos permitirá seleccionar el servidor de mail apropiado para enviar los " +"mensajes donde el dominio del email del 'De' coincida con la lista blanca " +"de dominios." + +#. module: mail_outbound_static +#: model:ir.model.fields,field_description:mail_outbound_static.field_ir_mail_server__domain_whitelist +msgid "Domain Whitelist" +msgstr "Lista blanca de dominios" + +#. module: mail_outbound_static +#: model:ir.model.fields,field_description:mail_outbound_static.field_ir_mail_server__smtp_from +msgid "Email From" +msgstr "Email De" + +#. module: mail_outbound_static +#: model:ir.model,name:mail_outbound_static.model_ir_mail_server +msgid "Mail Server" +msgstr "Servidor de correo" + +#. module: mail_outbound_static +#: code:addons/mail_outbound_static/models/ir_mail_server.py:0 +#, python-format +msgid "Please define a list of domains separate by comma" +msgstr "Por favor defina una lista de dominios separados por coma" + +#. module: mail_outbound_static +#: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__smtp_from +msgid "" +"Set this in order to email from a specific address. If the original " +"message's 'From' does not match with the domain whitelist then it is " +"replaced with this value. If does match with the domain whitelist then the " +"original message's 'From' will not change" +msgstr "" +"Definalo para usar un dirección de correo 'De' especifica. Si el 'De' del mensaje " +"original no coincide con la lista blanca de dominios entonces este será " +"remplazado con este valor. Si coincide con la lista blanca de dominios entonces " +"el 'De' del mensajee original no cambiará" diff --git a/mail_outbound_static/i18n/mail_outbound_static.pot b/mail_outbound_static/i18n/mail_outbound_static.pot index 1f4732e45..608ec104a 100644 --- a/mail_outbound_static/i18n/mail_outbound_static.pot +++ b/mail_outbound_static/i18n/mail_outbound_static.pot @@ -6,6 +6,8 @@ msgid "" msgstr "" "Project-Id-Version: Odoo Server 13.0\n" "Report-Msgid-Bugs-To: \n" +"POT-Creation-Date: 2020-09-03 12:53+0000\n" +"PO-Revision-Date: 2020-09-03 12:53+0000\n" "Last-Translator: \n" "Language-Team: \n" "MIME-Version: 1.0\n" @@ -13,6 +15,20 @@ msgstr "" "Content-Transfer-Encoding: \n" "Plural-Forms: \n" +#. module: mail_outbound_static +#: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__domain_whitelist +msgid "" +"Allowed Domains list separated by commas. If there is not given SMTP server " +"it will let us to search the proper mail server to be used to sent the " +"messages where the message 'From' email domain match with the domain " +"whitelist." +msgstr "" + +#. module: mail_outbound_static +#: model:ir.model.fields,field_description:mail_outbound_static.field_ir_mail_server__domain_whitelist +msgid "Domain Whitelist" +msgstr "" + #. module: mail_outbound_static #: model:ir.model.fields,field_description:mail_outbound_static.field_ir_mail_server__smtp_from msgid "Email From" @@ -24,6 +40,16 @@ msgid "Mail Server" msgstr "" #. module: mail_outbound_static -#: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__smtp_from -msgid "Set this in order to email from a specific address." +#: code:addons/mail_outbound_static/models/ir_mail_server.py:0 +#, python-format +msgid "Please define a list of domains separate by comma" +msgstr "" + +#. module: mail_outbound_static +#: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__smtp_from +msgid "" +"Set this in order to email from a specific address. If the original " +"message's 'From' does not match with the domain whitelist then it is " +"replaced with this value. If does match with the domain whitelist then the " +"original message's 'From' will not change" msgstr "" diff --git a/mail_outbound_static/models/ir_mail_server.py b/mail_outbound_static/models/ir_mail_server.py index c0a3cda88..f983f8a3f 100644 --- a/mail_outbound_static/models/ir_mail_server.py +++ b/mail_outbound_static/models/ir_mail_server.py @@ -1,7 +1,10 @@ # Copyright 2017 LasLabs Inc. # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). -from odoo import api, fields, models +from email.utils import formataddr, parseaddr + +from odoo import _, api, fields, models, tools +from odoo.exceptions import ValidationError class IrMailServer(models.Model): @@ -9,28 +12,72 @@ class IrMailServer(models.Model): _inherit = "ir.mail_server" smtp_from = fields.Char( - string="Email From", help="Set this in order to email from a specific address." + string="Email From", + help="Set this in order to email from a specific address." + " If the original message's 'From' does not match with the domain" + " whitelist then it is replaced with this value. If does match with the" + " domain whitelist then the original message's 'From' will not change", ) + domain_whitelist = fields.Char( + help="Allowed Domains list separated by commas. If there is not given" + " SMTP server it will let us to search the proper mail server to be" + " used to sent the messages where the message 'From' email domain" + " match with the domain whitelist." + ) + + @api.constrains("domain_whitelist") + def check_valid_domain_whitelist(self): + if self.domain_whitelist: + values = False + try: + values = list(self.domain_whitelist.split(",")) + except Exception: + pass + + if not isinstance(values, list): + raise ValidationError( + _("Please define a list of domains separate by comma") + ) + + @api.model + def _get_domain_whitelist(self, domain_whitelist_string): + res = domain_whitelist_string.split(",") if domain_whitelist_string else [] + res = [item.strip() for item in res] + return res @api.model def send_email( self, message, mail_server_id=None, smtp_server=None, *args, **kwargs ): + # Get email_from and name_from + if message["From"].count("<") > 1: + split_from = message["From"].rsplit(" <", 1) + name_from = split_from[0] + email_from = split_from[-1].replace(">", "") + else: + name_from, email_from = parseaddr(message["From"]) + + email_domain = email_from.split("@")[1] # Replicate logic from core to get mail server - mail_server = None + # Get proper mail server to use + if not smtp_server and not mail_server_id: + mail_server_id = self._get_mail_sever(email_domain) + + # If not mail sever defined use smtp_from defined in odoo config if mail_server_id: mail_server = self.sudo().browse(mail_server_id) - elif not smtp_server: - mail_server = self.sudo().search([], order="sequence", limit=1) + domain_whitelist = mail_server.domain_whitelist + smtp_from = mail_server.smtp_from + else: + domain_whitelist = tools.config.get("smtp_domain_whitelist") + smtp_from = tools.config.get("smtp_from") - if mail_server and mail_server.smtp_from: - split_from = message["From"].rsplit(" <", 1) - if len(split_from) > 1: - email_from = "{} <{}>".format(split_from[0], mail_server.smtp_from) - else: - email_from = mail_server.smtp_from + domain_whitelist = self._get_domain_whitelist(domain_whitelist) + # Replace the From only if needed + if smtp_from and (not domain_whitelist or email_domain not in domain_whitelist): + email_from = formataddr((name_from, smtp_from)) message.replace_header("From", email_from) bounce_alias = ( self.env["ir.config_parameter"].sudo().get_param("mail.bounce.alias") @@ -46,3 +93,32 @@ class IrMailServer(models.Model): return super(IrMailServer, self).send_email( message, mail_server_id, smtp_server, *args, **kwargs ) + + @tools.ormcache("email_domain") + def _get_mail_sever(self, email_domain): + """ return the mail server id that match with the domain_whitelist + If not match then return the default mail server id available one """ + mail_server_id = None + for item in self.sudo().search( + [("domain_whitelist", "!=", False)], order="sequence" + ): + domain_whitelist = self._get_domain_whitelist(item.domain_whitelist) + if email_domain in domain_whitelist: + mail_server_id = item.id + break + if not mail_server_id: + mail_server_id = self.sudo().search([], order="sequence", limit=1).id + return mail_server_id + + @api.model + def create(self, values): + self.clear_caches() + return super().create(values) + + def write(self, values): + self.clear_caches() + return super().write(values) + + def unlink(self): + self.clear_caches() + return super().unlink() diff --git a/mail_outbound_static/readme/CONTRIBUTORS.rst b/mail_outbound_static/readme/CONTRIBUTORS.rst index 8003b64b1..1376f822b 100644 --- a/mail_outbound_static/readme/CONTRIBUTORS.rst +++ b/mail_outbound_static/readme/CONTRIBUTORS.rst @@ -1,3 +1,5 @@ * Frédéric Garbely * Dave Lasley * Lorenzo Battistini +* Katherine Zaoral +* Juan José Scarafía diff --git a/mail_outbound_static/readme/DESCRIPTION.rst b/mail_outbound_static/readme/DESCRIPTION.rst index b49deb9c3..f8e91fdbf 100644 --- a/mail_outbound_static/readme/DESCRIPTION.rst +++ b/mail_outbound_static/readme/DESCRIPTION.rst @@ -1,3 +1,24 @@ This module brings Odoo outbound emails in to strict compliance with RFC-2822 -by allowing for a statically configured From header, with the sender's e-mail -being appended into the proper Sender header instead. +by allowing for a dynamically configured From header, with the sender's e-mail +being appended into the proper Sender header instead. To accomplish this we: + +* Add a domain whitelist field in the mail server model. This one represent an + allowed Domains list separated by commas. If there is not given SMTP server + it will let us to search the proper mail server to be used to sent themessages + where the message 'From' email domain match with the domain whitelist. If + there is not mail sever that match then will use the default mail server to + sent the message. + +* Add a Email From field that will let us to email from a specific address taking + into account this conditions: + + 1) If the sender domain match with the domain whitelist then the original + message's 'From' will remain as it is and will not be changed because the + mail server is able to sent in the name of the sender domain. + + 2) If the original message's 'From' does not match with the domain whitelist + then the email From is replaced with the Email From field value. + +* Add compatibility to define the smtp information in Odoo config file. Both + smtp_from and smtp_whitelist_domain values will be used if there is not mail + server configured in the system. diff --git a/mail_outbound_static/readme/ROADMAP.rst b/mail_outbound_static/readme/ROADMAP.rst index 9cc754794..48f758aba 100644 --- a/mail_outbound_static/readme/ROADMAP.rst +++ b/mail_outbound_static/readme/ROADMAP.rst @@ -1 +1,2 @@ -* Allow for domain-based whitelist that will not be manipulated +* Add validation of smtp_from field to ensure that is a valid email address +* Add validation of domain_whitelist field to ensure that they are valid domains diff --git a/mail_outbound_static/readme/USAGE.rst b/mail_outbound_static/readme/USAGE.rst index e0283cf97..195f5a939 100644 --- a/mail_outbound_static/readme/USAGE.rst +++ b/mail_outbound_static/readme/USAGE.rst @@ -1,2 +1,3 @@ * Navigate to an Outbound Email Server * Set the `Email From` option to an email address +* Set the `Domain Whitelist` option with the domain whitelist diff --git a/mail_outbound_static/static/description/index.html b/mail_outbound_static/static/description/index.html index a3b689e27..d89cab775 100644 --- a/mail_outbound_static/static/description/index.html +++ b/mail_outbound_static/static/description/index.html @@ -369,8 +369,28 @@ ul.auto-toc { !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->

Beta License: LGPL-3 OCA/social Translate me on Weblate Try me on Runbot

This module brings Odoo outbound emails in to strict compliance with RFC-2822 -by allowing for a statically configured From header, with the sender’s e-mail -being appended into the proper Sender header instead.

+by allowing for a dynamically configured From header, with the sender’s e-mail +being appended into the proper Sender header instead. To accomplish this we:

+
    +
  • Add a domain whitelist field in the mail server model. This one represent an +allowed Domains list separated by commas. If there is not given SMTP server +it will let us to search the proper mail server to be used to sent themessages +where the message ‘From’ email domain match with the domain whitelist. If +there is not mail sever that match then will use the default mail server to +sent the message.
    • +
  • Add a Email From field that will let us to email from a specific address taking +into account this conditions:
      +
    1. If the sender domain match with the domain whitelist then the original +message’s ‘From’ will remain as it is and will not be changed because the +mail server is able to sent in the name of the sender domain.
      2. +
    2. If the original message’s ‘From’ does not match with the domain whitelist +then the email From is replaced with the Email From field value.
      3. +
    +
    • +
  • Add compatibility to define the smtp information in Odoo config file. Both +smtp_from and smtp_whitelist_domain values will be used if there is not mail +server configured in the system.
    • +

Table of contents

    @@ -390,12 +410,14 @@ being appended into the proper Sender header instead.

    • Navigate to an Outbound Email Server
    • Set the Email From option to an email address
      • +
    • Set the Domain Whitelist option with the domain whitelist

Known issues / Roadmap

    -
  • Allow for domain-based whitelist that will not be manipulated
    • +
  • Add validation of smtp_from field to ensure that is a valid email address
    • +
  • Add validation of domain_whitelist field to ensure that they are valid domains
@@ -413,6 +435,7 @@ If you spotted it first, help us smashing it by providing a detailed and welcome
  • brain-tec AG
  • LasLabs
    • +
  • Adhoc SA
@@ -421,6 +444,8 @@ If you spotted it first, help us smashing it by providing a detailed and welcome
  • Frédéric Garbely <frederic.garbely@braintec-group.com>
  • Dave Lasley <dave@laslabs.com>
  • Lorenzo Battistini <https://github.com/eLBati>
    • +
  • Katherine Zaoral <kz@adhoc.com.ar>
    • +
  • Juan José Scarafía <jjs@adhoc.com.ar>
    • diff --git a/mail_outbound_static/tests/test_ir_mail_server.py b/mail_outbound_static/tests/test_ir_mail_server.py index ce8ec76e1..cd452bffc 100644 --- a/mail_outbound_static/tests/test_ir_mail_server.py +++ b/mail_outbound_static/tests/test_ir_mail_server.py @@ -1,14 +1,18 @@ # Copyright 2017 LasLabs Inc. # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). +import logging import os import threading from email import message_from_string from mock import MagicMock +import odoo.tools as tools from odoo.tests.common import TransactionCase +_logger = logging.getLogger(__name__) + class TestIrMailServer(TransactionCase): def setUp(self): @@ -17,6 +21,7 @@ class TestIrMailServer(TransactionCase): self.email_from_another = "another@example.com" self.Model = self.env["ir.mail_server"] self.parameter_model = self.env["ir.config_parameter"] + self._delete_mail_servers() self.Model.create( { "name": "localhost", @@ -30,6 +35,44 @@ class TestIrMailServer(TransactionCase): with open(message_file, "r") as fh: self.message = message_from_string(fh.read()) + def _init_mail_server_domain_whilelist_based(self): + self._delete_mail_servers() + self.mail_server_domainone = self.Model.create( + { + "name": "sandbox domainone", + "smtp_host": "localhost", + "smtp_from": "notifications@domainone.com", + "domain_whitelist": "domainone.com", + } + ) + self.mail_server_domaintwo = self.Model.create( + { + "name": "sandbox domaintwo", + "smtp_host": "localhost", + "smtp_from": "hola@domaintwo.com", + "domain_whitelist": "domaintwo.com", + } + ) + self.mail_server_domainthree = self.Model.create( + { + "name": "sandbox domainthree", + "smtp_host": "localhost", + "smtp_from": "notifications@domainthree.com", + "domain_whitelist": "domainthree.com,domainmulti.com", + } + ) + + def _skip_test(self, reason): + _logger.warn(reason) + self.skipTest(reason) + + def _delete_mail_servers(self): + """ Delete all available mail servers """ + all_mail_servers = self.Model.search([]) + if all_mail_servers: + all_mail_servers.unlink() + self.assertFalse(self.Model.search([])) + def _send_mail(self, message=None, mail_server_id=None, smtp_server=None): if message is None: message = self.message @@ -71,7 +114,175 @@ class TestIrMailServer(TransactionCase): # Also check passing mail_server_id mail_server_id = self.Model.sudo().search([], order="sequence", limit=1)[0].id message = self._send_mail(mail_server_id=mail_server_id) - self.assertEqual(message["From"], "{} <{}>".format(user, self.email_from)) + self.assertEqual(message["From"], '"{}" <{}>'.format(user, self.email_from)) self.assertEqual( - message["Return-Path"], "{} <{}>".format(user, self.email_from) + message["Return-Path"], '"{}" <{}>'.format(user, self.email_from) + ) + + def test_1_from_outgoing_server_domainone(self): + self._init_mail_server_domain_whilelist_based() + domain = "domainone.com" + email_from = "Mitchell Admin " % domain + expected_mail_server = self.mail_server_domainone + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual(message["From"], email_from) + + used_mail_server = self.Model._get_mail_sever(domain) + used_mail_server = self.Model.browse(used_mail_server) + self.assertEqual( + used_mail_server, + expected_mail_server, + "It using %s but we expect to use %s" + % (used_mail_server.name, expected_mail_server.name), + ) + + def test_2_from_outgoing_server_domaintwo(self): + self._init_mail_server_domain_whilelist_based() + domain = "domaintwo.com" + email_from = "Mitchell Admin " % domain + expected_mail_server = self.mail_server_domaintwo + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual(message["From"], email_from) + + used_mail_server = self.Model._get_mail_sever(domain) + used_mail_server = self.Model.browse(used_mail_server) + self.assertEqual( + used_mail_server, + expected_mail_server, + "It using %s but we expect to use %s" + % (used_mail_server.name, expected_mail_server.name), + ) + + def test_3_from_outgoing_server_another(self): + self._init_mail_server_domain_whilelist_based() + domain = "example.com" + email_from = "Mitchell Admin " % domain + expected_mail_server = self.mail_server_domainone + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual( + message["From"], "Mitchell Admin <%s>" % expected_mail_server.smtp_from + ) + + used_mail_server = self.Model._get_mail_sever(domain) + used_mail_server = self.Model.browse(used_mail_server) + self.assertEqual( + used_mail_server, + expected_mail_server, + "It using %s but we expect to use %s" + % (used_mail_server.name, expected_mail_server.name), + ) + + def test_4_from_outgoing_server_none_use_config(self): + self._init_mail_server_domain_whilelist_based() + domain = "example.com" + email_from = "Mitchell Admin " % domain + + self._delete_mail_servers() + + # Find config values + config_smtp_from = tools.config.get("smtp_from") + config_smtp_domain_whitelist = tools.config.get("smtp_domain_whitelist") + if not config_smtp_from or not config_smtp_domain_whitelist: + self._skip_test( + "Cannot test transactions because there is not either smtp_from" + " or smtp_domain_whitelist." + ) + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual(message["From"], "Mitchell Admin <%s>" % config_smtp_from) + + used_mail_server = self.Model._get_mail_sever("example.com") + used_mail_server = self.Model.browse(used_mail_server) + self.assertFalse( + used_mail_server, "using this mail server %s" % (used_mail_server.name) + ) + + def test_5_from_outgoing_server_none_same_domain(self): + self._init_mail_server_domain_whilelist_based() + + # Find config values + config_smtp_from = tools.config.get("smtp_from") + config_smtp_domain_whitelist = domain = tools.config.get( + "smtp_domain_whitelist" + ) + if not config_smtp_from or not config_smtp_domain_whitelist: + self._skip_test( + "Cannot test transactions because there is not either smtp_from" + " or smtp_domain_whitelist." + ) + + email_from = "Mitchell Admin " % domain + + self._delete_mail_servers() + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual(message["From"], email_from) + + used_mail_server = self.Model._get_mail_sever(domain) + used_mail_server = self.Model.browse(used_mail_server) + self.assertFalse(used_mail_server) + + def test_6_from_outgoing_server_no_name_from(self): + self._init_mail_server_domain_whilelist_based() + domain = "example.com" + email_from = "test@%s" % domain + expected_mail_server = self.mail_server_domainone + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual(message["From"], expected_mail_server.smtp_from) + + used_mail_server = self.Model._get_mail_sever(domain) + used_mail_server = self.Model.browse(used_mail_server) + self.assertEqual( + used_mail_server, + expected_mail_server, + "It using %s but we expect to use %s" + % (used_mail_server.name, expected_mail_server.name), + ) + + def test_7_from_outgoing_server_multidomain_1(self): + self._init_mail_server_domain_whilelist_based() + domain = "domainthree.com" + email_from = "Mitchell Admin " % domain + expected_mail_server = self.mail_server_domainthree + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual(message["From"], email_from) + + used_mail_server = self.Model._get_mail_sever(domain) + used_mail_server = self.Model.browse(used_mail_server) + self.assertEqual( + used_mail_server, + expected_mail_server, + "It using %s but we expect to use %s" + % (used_mail_server.name, expected_mail_server.name), + ) + + def test_8_from_outgoing_server_multidomain_3(self): + self._init_mail_server_domain_whilelist_based() + domain = "domainmulti.com" + email_from = "test@%s" % domain + expected_mail_server = self.mail_server_domainthree + + self.message.replace_header("From", email_from) + message = self._send_mail() + self.assertEqual(message["From"], email_from) + + used_mail_server = self.Model._get_mail_sever(domain) + used_mail_server = self.Model.browse(used_mail_server) + self.assertEqual( + used_mail_server, + expected_mail_server, + "It using %s but we expect to use %s" + % (used_mail_server.name, expected_mail_server.name), ) diff --git a/mail_outbound_static/views/ir_mail_server_view.xml b/mail_outbound_static/views/ir_mail_server_view.xml index b74d4aa8f..586e316fe 100644 --- a/mail_outbound_static/views/ir_mail_server_view.xml +++ b/mail_outbound_static/views/ir_mail_server_view.xml @@ -13,6 +13,7 @@ + From 542a0b90c0577fe99a3d3216cd90d5146db90981 Mon Sep 17 00:00:00 2001 From: Katherine Zaoral Date: Fri, 4 Sep 2020 17:45:56 -0300 Subject: [PATCH 2/2] new changes to squash after review: - fix misspellings errors in documentation - add unit test to avoid github checkers red status in codecov tests. - add validation of email from and domain whitelist fields. - update documentation remove roadmap for already updated functionalities. - update translations --- mail_outbound_static/README.rst | 14 ++--- mail_outbound_static/i18n/es.po | 30 ++++++---- .../i18n/mail_outbound_static.pot | 10 +++- mail_outbound_static/models/ir_mail_server.py | 40 ++++++++++--- mail_outbound_static/readme/DESCRIPTION.rst | 8 +-- mail_outbound_static/readme/ROADMAP.rst | 2 - .../static/description/index.html | 36 +++++------- .../tests/test_ir_mail_server.py | 57 ++++++++++++++++--- .../views/ir_mail_server_view.xml | 2 +- 9 files changed, 132 insertions(+), 67 deletions(-) diff --git a/mail_outbound_static/README.rst b/mail_outbound_static/README.rst index bda2333b3..4723d73d9 100644 --- a/mail_outbound_static/README.rst +++ b/mail_outbound_static/README.rst @@ -31,17 +31,17 @@ being appended into the proper Sender header instead. To accomplish this we: * Add a domain whitelist field in the mail server model. This one represent an allowed Domains list separated by commas. If there is not given SMTP server - it will let us to search the proper mail server to be used to sent themessages + it will let us to search the proper mail server to be used to send the messages where the message 'From' email domain match with the domain whitelist. If - there is not mail sever that match then will use the default mail server to - sent the message. + there is not mail server that matches then will use the default mail server to + send the message. * Add a Email From field that will let us to email from a specific address taking into account this conditions: 1) If the sender domain match with the domain whitelist then the original message's 'From' will remain as it is and will not be changed because the - mail server is able to sent in the name of the sender domain. + mail server is able to send in the name of the sender domain. 2) If the original message's 'From' does not match with the domain whitelist then the email From is replaced with the Email From field value. @@ -62,12 +62,6 @@ Usage * Set the `Email From` option to an email address * Set the `Domain Whitelist` option with the domain whitelist -Known issues / Roadmap -====================== - -* Add validation of smtp_from field to ensure that is a valid email address -* Add validation of domain_whitelist field to ensure that they are valid domains - Bug Tracker =========== diff --git a/mail_outbound_static/i18n/es.po b/mail_outbound_static/i18n/es.po index 1ca5962f1..8e0d95675 100644 --- a/mail_outbound_static/i18n/es.po +++ b/mail_outbound_static/i18n/es.po @@ -15,6 +15,16 @@ msgstr "" "Content-Transfer-Encoding: \n" "Plural-Forms: \n" +#. module: mail_outbound_static +#: code:addons/mail_outbound_static/models/ir_mail_server.py:0 +#, python-format +msgid "" +"%s is not a valid domain. Please define a list of valid domains separated by" +" comma" +msgstr "" +"%s no es un dominio válido. Por favor defina una lista de dominios validos separados por" +" comas" + #. module: mail_outbound_static #: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__domain_whitelist msgid "" @@ -23,10 +33,10 @@ msgid "" "messages where the message 'From' email domain match with the domain " "whitelist." msgstr "" -"Lista de dominios permitidos separados por comas. Si no se ha seleccionado un servidor SMTP " -"nos permitirá seleccionar el servidor de mail apropiado para enviar los " -"mensajes donde el dominio del email del 'De' coincida con la lista blanca " -"de dominios." +"Lista de dominios permitidos separados por comas. Si no se ha seleccionado " +"un servidor SMTP nos permitirá seleccionar el servidor de mail apropiado " +"para enviar los mensajes donde el dominio del email del 'De' coincida con la" +" lista blanca de dominios." #. module: mail_outbound_static #: model:ir.model.fields,field_description:mail_outbound_static.field_ir_mail_server__domain_whitelist @@ -46,8 +56,8 @@ msgstr "Servidor de correo" #. module: mail_outbound_static #: code:addons/mail_outbound_static/models/ir_mail_server.py:0 #, python-format -msgid "Please define a list of domains separate by comma" -msgstr "Por favor defina una lista de dominios separados por coma" +msgid "Not a valid Email From" +msgstr "No es un Email De válido" #. module: mail_outbound_static #: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__smtp_from @@ -57,7 +67,7 @@ msgid "" "replaced with this value. If does match with the domain whitelist then the " "original message's 'From' will not change" msgstr "" -"Definalo para usar un dirección de correo 'De' especifica. Si el 'De' del mensaje " -"original no coincide con la lista blanca de dominios entonces este será " -"remplazado con este valor. Si coincide con la lista blanca de dominios entonces " -"el 'De' del mensajee original no cambiará" +"Definalo para usar un dirección de correo 'De' especifica. Si el 'De' del " +"mensaje original no coincide con la lista blanca de dominios entonces este " +"será remplazado con este valor. Si coincide con la lista blanca de dominios " +"entonces el 'De' del mensajee original no cambiará" diff --git a/mail_outbound_static/i18n/mail_outbound_static.pot b/mail_outbound_static/i18n/mail_outbound_static.pot index 608ec104a..4c715d2c6 100644 --- a/mail_outbound_static/i18n/mail_outbound_static.pot +++ b/mail_outbound_static/i18n/mail_outbound_static.pot @@ -15,6 +15,14 @@ msgstr "" "Content-Transfer-Encoding: \n" "Plural-Forms: \n" +#. module: mail_outbound_static +#: code:addons/mail_outbound_static/models/ir_mail_server.py:0 +#, python-format +msgid "" +"%s is not a valid domain. Please define a list of valid domains separated by" +" comma" +msgstr "" + #. module: mail_outbound_static #: model:ir.model.fields,help:mail_outbound_static.field_ir_mail_server__domain_whitelist msgid "" @@ -42,7 +50,7 @@ msgstr "" #. module: mail_outbound_static #: code:addons/mail_outbound_static/models/ir_mail_server.py:0 #, python-format -msgid "Please define a list of domains separate by comma" +msgid "Not a valid Email From" msgstr "" #. module: mail_outbound_static diff --git a/mail_outbound_static/models/ir_mail_server.py b/mail_outbound_static/models/ir_mail_server.py index f983f8a3f..0311cf5dc 100644 --- a/mail_outbound_static/models/ir_mail_server.py +++ b/mail_outbound_static/models/ir_mail_server.py @@ -1,6 +1,7 @@ # Copyright 2017 LasLabs Inc. # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). +import re from email.utils import formataddr, parseaddr from odoo import _, api, fields, models, tools @@ -28,16 +29,37 @@ class IrMailServer(models.Model): @api.constrains("domain_whitelist") def check_valid_domain_whitelist(self): if self.domain_whitelist: - values = False - try: - values = list(self.domain_whitelist.split(",")) - except Exception: - pass + domains = list(self.domain_whitelist.split(",")) + for domain in domains: + if not self._is_valid_domain(domain): + raise ValidationError( + _( + "%s is not a valid domain. Please define a list of" + " valid domains separated by comma" + ) + % (domain) + ) - if not isinstance(values, list): - raise ValidationError( - _("Please define a list of domains separate by comma") - ) + @api.constrains("smtp_from") + def check_valid_smtp_from(self): + if self.smtp_from: + match = re.match( + r"^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\." + r"[a-z]{2,4})$", + self.smtp_from, + ) + if match is None: + raise ValidationError(_("Not a valid Email From")) + + def _is_valid_domain(self, domain_name): + domain_regex = ( + r"(([\da-zA-Z])([_\w-]{,62})\.){,127}(([\da-zA-Z])" + r"[_\w-]{,61})?([\da-zA-Z]\.((xn\-\-[a-zA-Z\d]+)|([a-zA-Z\d]{2,})))" + ) + domain_regex = "{}$".format(domain_regex) + valid_domain_name_regex = re.compile(domain_regex, re.IGNORECASE) + domain_name = domain_name.lower().strip() + return True if re.match(valid_domain_name_regex, domain_name) else False @api.model def _get_domain_whitelist(self, domain_whitelist_string): diff --git a/mail_outbound_static/readme/DESCRIPTION.rst b/mail_outbound_static/readme/DESCRIPTION.rst index f8e91fdbf..b4ffa0ac6 100644 --- a/mail_outbound_static/readme/DESCRIPTION.rst +++ b/mail_outbound_static/readme/DESCRIPTION.rst @@ -4,17 +4,17 @@ being appended into the proper Sender header instead. To accomplish this we: * Add a domain whitelist field in the mail server model. This one represent an allowed Domains list separated by commas. If there is not given SMTP server - it will let us to search the proper mail server to be used to sent themessages + it will let us to search the proper mail server to be used to send the messages where the message 'From' email domain match with the domain whitelist. If - there is not mail sever that match then will use the default mail server to - sent the message. + there is not mail server that matches then will use the default mail server to + send the message. * Add a Email From field that will let us to email from a specific address taking into account this conditions: 1) If the sender domain match with the domain whitelist then the original message's 'From' will remain as it is and will not be changed because the - mail server is able to sent in the name of the sender domain. + mail server is able to send in the name of the sender domain. 2) If the original message's 'From' does not match with the domain whitelist then the email From is replaced with the Email From field value. diff --git a/mail_outbound_static/readme/ROADMAP.rst b/mail_outbound_static/readme/ROADMAP.rst index 48f758aba..e69de29bb 100644 --- a/mail_outbound_static/readme/ROADMAP.rst +++ b/mail_outbound_static/readme/ROADMAP.rst @@ -1,2 +0,0 @@ -* Add validation of smtp_from field to ensure that is a valid email address -* Add validation of domain_whitelist field to ensure that they are valid domains diff --git a/mail_outbound_static/static/description/index.html b/mail_outbound_static/static/description/index.html index d89cab775..7d57d21ea 100644 --- a/mail_outbound_static/static/description/index.html +++ b/mail_outbound_static/static/description/index.html @@ -374,15 +374,15 @@ being appended into the proper Sender header instead. To accomplish this we:

    • Add a domain whitelist field in the mail server model. This one represent an allowed Domains list separated by commas. If there is not given SMTP server -it will let us to search the proper mail server to be used to sent themessages +it will let us to search the proper mail server to be used to send the messages where the message ‘From’ email domain match with the domain whitelist. If -there is not mail sever that match then will use the default mail server to -sent the message.
      • +there is not mail server that matches then will use the default mail server to +send the message.
    • Add a Email From field that will let us to email from a specific address taking into account this conditions:
      1. If the sender domain match with the domain whitelist then the original message’s ‘From’ will remain as it is and will not be changed because the -mail server is able to sent in the name of the sender domain.
        2. +mail server is able to send in the name of the sender domain.
      2. If the original message’s ‘From’ does not match with the domain whitelist then the email From is replaced with the Email From field value.
      @@ -395,12 +395,11 @@ server configured in the system.
      • -
      -

      Known issues / Roadmap

      -
        -
      • Add validation of smtp_from field to ensure that is a valid email address
        • -
      • Add validation of domain_whitelist field to ensure that they are valid domains
        • -
      -
      -

      Bug Tracker

      +

      Bug Tracker

      Bugs are tracked on GitHub Issues. In case of trouble, please check there if your issue has already been reported. If you spotted it first, help us smashing it by providing a detailed and welcomed @@ -429,9 +421,9 @@ If you spotted it first, help us smashing it by providing a detailed and welcome

      Do not contact contributors directly about support or help with technical issues.

      -

      Credits

      +

      Credits

      -

      Authors

      +

      Authors

      • brain-tec AG
      • LasLabs
        • @@ -439,7 +431,7 @@ If you spotted it first, help us smashing it by providing a detailed and welcome
      -

      Contributors

      +

      Contributors

      -

      Maintainers

      +

      Maintainers

      This module is maintained by the OCA.

      Odoo Community Association

      OCA, or the Odoo Community Association, is a nonprofit organization whose diff --git a/mail_outbound_static/tests/test_ir_mail_server.py b/mail_outbound_static/tests/test_ir_mail_server.py index cd452bffc..435ab9a39 100644 --- a/mail_outbound_static/tests/test_ir_mail_server.py +++ b/mail_outbound_static/tests/test_ir_mail_server.py @@ -9,6 +9,7 @@ from email import message_from_string from mock import MagicMock import odoo.tools as tools +from odoo.exceptions import ValidationError from odoo.tests.common import TransactionCase _logger = logging.getLogger(__name__) @@ -119,7 +120,7 @@ class TestIrMailServer(TransactionCase): message["Return-Path"], '"{}" <{}>'.format(user, self.email_from) ) - def test_1_from_outgoing_server_domainone(self): + def test_01_from_outgoing_server_domainone(self): self._init_mail_server_domain_whilelist_based() domain = "domainone.com" email_from = "Mitchell Admin " % domain @@ -138,7 +139,7 @@ class TestIrMailServer(TransactionCase): % (used_mail_server.name, expected_mail_server.name), ) - def test_2_from_outgoing_server_domaintwo(self): + def test_02_from_outgoing_server_domaintwo(self): self._init_mail_server_domain_whilelist_based() domain = "domaintwo.com" email_from = "Mitchell Admin " % domain @@ -157,7 +158,7 @@ class TestIrMailServer(TransactionCase): % (used_mail_server.name, expected_mail_server.name), ) - def test_3_from_outgoing_server_another(self): + def test_03_from_outgoing_server_another(self): self._init_mail_server_domain_whilelist_based() domain = "example.com" email_from = "Mitchell Admin " % domain @@ -178,7 +179,7 @@ class TestIrMailServer(TransactionCase): % (used_mail_server.name, expected_mail_server.name), ) - def test_4_from_outgoing_server_none_use_config(self): + def test_04_from_outgoing_server_none_use_config(self): self._init_mail_server_domain_whilelist_based() domain = "example.com" email_from = "Mitchell Admin " % domain @@ -204,7 +205,7 @@ class TestIrMailServer(TransactionCase): used_mail_server, "using this mail server %s" % (used_mail_server.name) ) - def test_5_from_outgoing_server_none_same_domain(self): + def test_05_from_outgoing_server_none_same_domain(self): self._init_mail_server_domain_whilelist_based() # Find config values @@ -230,7 +231,7 @@ class TestIrMailServer(TransactionCase): used_mail_server = self.Model.browse(used_mail_server) self.assertFalse(used_mail_server) - def test_6_from_outgoing_server_no_name_from(self): + def test_06_from_outgoing_server_no_name_from(self): self._init_mail_server_domain_whilelist_based() domain = "example.com" email_from = "test@%s" % domain @@ -249,7 +250,7 @@ class TestIrMailServer(TransactionCase): % (used_mail_server.name, expected_mail_server.name), ) - def test_7_from_outgoing_server_multidomain_1(self): + def test_07_from_outgoing_server_multidomain_1(self): self._init_mail_server_domain_whilelist_based() domain = "domainthree.com" email_from = "Mitchell Admin " % domain @@ -268,7 +269,7 @@ class TestIrMailServer(TransactionCase): % (used_mail_server.name, expected_mail_server.name), ) - def test_8_from_outgoing_server_multidomain_3(self): + def test_08_from_outgoing_server_multidomain_3(self): self._init_mail_server_domain_whilelist_based() domain = "domainmulti.com" email_from = "test@%s" % domain @@ -286,3 +287,43 @@ class TestIrMailServer(TransactionCase): "It using %s but we expect to use %s" % (used_mail_server.name, expected_mail_server.name), ) + + def test_09_not_valid_domain_whitelist(self): + self._init_mail_server_domain_whilelist_based() + mail_server = self.mail_server_domainone + mail_server.domain_whitelist = "example.com" + error_msg = ( + "%s is not a valid domain. Please define a list of valid" + " domains separated by comma" + ) + + with self.assertRaisesRegex(ValidationError, error_msg % "asdasd"): + mail_server.domain_whitelist = "asdasd" + + with self.assertRaisesRegex(ValidationError, error_msg % "asdasd"): + mail_server.domain_whitelist = "example.com, asdasd" + + with self.assertRaisesRegex(ValidationError, error_msg % "invalid"): + mail_server.domain_whitelist = "example.com; invalid" + + with self.assertRaisesRegex(ValidationError, error_msg % ";"): + mail_server.domain_whitelist = ";" + + with self.assertRaisesRegex(ValidationError, error_msg % "."): + mail_server.domain_whitelist = "hola.com,." + + def test_10_not_valid_smtp_from(self): + self._init_mail_server_domain_whilelist_based() + mail_server = self.mail_server_domainone + error_msg = "Not a valid Email From" + + with self.assertRaisesRegex(ValidationError, error_msg): + mail_server.smtp_from = "asdasd" + + with self.assertRaisesRegex(ValidationError, error_msg): + mail_server.smtp_from = "example.com" + + with self.assertRaisesRegex(ValidationError, error_msg): + mail_server.smtp_from = "." + + mail_server.smtp_from = "notifications@test.com" diff --git a/mail_outbound_static/views/ir_mail_server_view.xml b/mail_outbound_static/views/ir_mail_server_view.xml index 586e316fe..70a9112ce 100644 --- a/mail_outbound_static/views/ir_mail_server_view.xml +++ b/mail_outbound_static/views/ir_mail_server_view.xml @@ -14,7 +14,7 @@ - +