From 55f6347bbf1df4f0a8c161b1dbc935ab29124ace Mon Sep 17 00:00:00 2001 From: Florian VANDENBULCKE Date: Fri, 5 Oct 2018 10:00:48 +0200 Subject: [PATCH 1/6] feat(useDnsServer): add field in admin view, use in resolver if defined. unit tests --- Dockerfile.dev | 5 +++-- modoboa/admin/app_settings.py | 9 +++++++++ modoboa/admin/lib.py | 5 +++++ modoboa/admin/tests/test_mx.py | 11 ++++++++++- modoboa/core/tests/test_views.py | 3 ++- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/Dockerfile.dev b/Dockerfile.dev index 2621f68d3..33f195eb8 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -2,13 +2,14 @@ FROM python:3.6-alpine MAINTAINER Antoine Nguyen -RUN apk add --update python3-dev libffi-dev gcc musl-dev libxml2-dev libxslt-dev \ +RUN apk add --update openssl python3-dev libffi-dev gcc musl-dev libxml2-dev libxslt-dev \ libressl-dev jpeg-dev \ && rm -rf /var/cache/apk/* WORKDIR /tmp COPY requirements.txt /tmp -RUN pip install -r requirements.txt +COPY test-requirements.txt /tmp +RUN pip install -r requirements.txt -r test-requirements.txt RUN mkdir /code WORKDIR /code diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index cd1afa3e7..899ca7161 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -10,6 +10,7 @@ from django import forms from django.conf import settings from django.utils.encoding import force_text from django.utils.translation import ugettext as _, ugettext_lazy +from django.forms.fields import GenericIPAddressField from modoboa.lib.form_utils import SeparatorField, YesNoField from modoboa.lib.sysutils import exec_cmd @@ -78,6 +79,14 @@ class AdminParametersForm(param_forms.AdminParametersForm): ) ) + use_specific_dns_server = GenericIPAddressField( + label=ugettext_lazy("Use specific DNS server"), + required=False, + help_text=ugettext_lazy( + "Give specific DNS server instead of local server configuration" + ) + ) + mbsep = SeparatorField(label=ugettext_lazy("Mailboxes")) handle_mailboxes = YesNoField( diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py index 2b1819bed..84c7b39bb 100644 --- a/modoboa/admin/lib.py +++ b/modoboa/admin/lib.py @@ -175,6 +175,11 @@ def get_domain_mx_list(domain): """Return a list of MX IP address for domain.""" result = [] logger = logging.getLogger("modoboa.admin") + + use_specific_dns_server = param_tools.get_global_parameter("use_specific_dns_server") + if use_specific_dns_server : + dns.resolver.default_resolver.nameservers = [use_specific_dns_server] + try: dns_answers = dns.resolver.query(domain, "MX") except dns.resolver.NXDOMAIN as e: diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index fea42ee53..fe8bd7d27 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -14,6 +14,7 @@ from django.utils.translation import ugettext as _ from modoboa.core import factories as core_factories from modoboa.lib.tests import ModoTestCase +from modoboa.parameters import tools as param_tools from . import utils from .. import factories, models from ..lib import get_domain_mx_list @@ -107,7 +108,15 @@ class MXTestCase(ModoTestCase): management.call_command( "modo", "check_mx", "--domain", "toto.com") - + + @mock.patch("socket.getaddrinfo") + @mock.patch.object(dns.resolver.Resolver, "query") + def test_get_domain_mx_list_with_dsn_server(self, mock_query, mock_getaddrinfo): + mock_query.side_effect = utils.mock_dns_query_result + mock_getaddrinfo.side_effect = utils.mock_ip_query_result + self.set_global_parameter("use_specific_dns_server","123.45.67.89") + get_domain_mx_list("does-not-exist.example.com") + @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "query") def test_get_domain_mx_list_logging(self, mock_query, mock_getaddrinfo): diff --git a/modoboa/core/tests/test_views.py b/modoboa/core/tests/test_views.py index 221445348..84f4a5cd6 100644 --- a/modoboa/core/tests/test_views.py +++ b/modoboa/core/tests/test_views.py @@ -67,7 +67,8 @@ SETTINGS_SAMPLE = { "core-random_password_length": "8", "admin-create_alias_on_mbox_rename": False, "admin-dkim_keys_storage_dir": "", - "admin-dkim_default_key_length": 1024 + "admin-dkim_default_key_length": 1024, + "admin-use_specific_dns_server": "193.43.55.67" } From 5d104556952b41abf7017a7a7242ed905f8b37aa Mon Sep 17 00:00:00 2001 From: Florian VANDENBULCKE Date: Fri, 5 Oct 2018 10:36:44 +0200 Subject: [PATCH 2/6] feat(useDnsServer): correctif error pipeline - indentation,lines too long --- modoboa/admin/lib.py | 7 +++---- modoboa/admin/tests/test_mx.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py index 84c7b39bb..ef5c94dc6 100644 --- a/modoboa/admin/lib.py +++ b/modoboa/admin/lib.py @@ -175,10 +175,9 @@ def get_domain_mx_list(domain): """Return a list of MX IP address for domain.""" result = [] logger = logging.getLogger("modoboa.admin") - - use_specific_dns_server = param_tools.get_global_parameter("use_specific_dns_server") - if use_specific_dns_server : - dns.resolver.default_resolver.nameservers = [use_specific_dns_server] + dns_server = param_tools.get_global_parameter("use_specific_dns_server") + if dns_server: + dns.resolver.default_resolver.nameservers = [dns_server] try: dns_answers = dns.resolver.query(domain, "MX") diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index fe8bd7d27..70cbca379 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -14,7 +14,6 @@ from django.utils.translation import ugettext as _ from modoboa.core import factories as core_factories from modoboa.lib.tests import ModoTestCase -from modoboa.parameters import tools as param_tools from . import utils from .. import factories, models from ..lib import get_domain_mx_list @@ -108,15 +107,16 @@ class MXTestCase(ModoTestCase): management.call_command( "modo", "check_mx", "--domain", "toto.com") - + @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "query") - def test_get_domain_mx_list_with_dsn_server(self, mock_query, mock_getaddrinfo): - mock_query.side_effect = utils.mock_dns_query_result - mock_getaddrinfo.side_effect = utils.mock_ip_query_result - self.set_global_parameter("use_specific_dns_server","123.45.67.89") - get_domain_mx_list("does-not-exist.example.com") - + def test_get_domain_mx_list_dsn_server(self, mock_query, mock_getaddrinfo): + """Test to get mx list from specific DNS server.""" + mock_query.side_effect = utils.mock_dns_query_result + mock_getaddrinfo.side_effect = utils.mock_ip_query_result + self.set_global_parameter("use_specific_dns_server", "123.45.67.89") + get_domain_mx_list("does-not-exist.example.com") + @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "query") def test_get_domain_mx_list_logging(self, mock_query, mock_getaddrinfo): From a35eb426046f577bfe0da22cc7ebb94ff498114c Mon Sep 17 00:00:00 2001 From: Florian VANDENBULCKE Date: Fri, 5 Oct 2018 11:18:20 +0200 Subject: [PATCH 3/6] feat(useDnsServer): correctif error pipeline - unit test name too long --- modoboa/admin/tests/test_mx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index 70cbca379..3eb56bc23 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -110,7 +110,7 @@ class MXTestCase(ModoTestCase): @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "query") - def test_get_domain_mx_list_dsn_server(self, mock_query, mock_getaddrinfo): + def test_get_mx_list_dsn_server(self, mock_query, mock_getaddrinfo): """Test to get mx list from specific DNS server.""" mock_query.side_effect = utils.mock_dns_query_result mock_getaddrinfo.side_effect = utils.mock_ip_query_result From 35c031fe3b528eead73578d6fc2adbb1a556840b Mon Sep 17 00:00:00 2001 From: Florian VANDENBULCKE Date: Fri, 5 Oct 2018 12:13:25 +0200 Subject: [PATCH 4/6] fix(useDnsServer): change help message on form field --- modoboa/admin/app_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index 899ca7161..ebe6032f3 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -83,7 +83,7 @@ class AdminParametersForm(param_forms.AdminParametersForm): label=ugettext_lazy("Use specific DNS server"), required=False, help_text=ugettext_lazy( - "Give specific DNS server instead of local server configuration" + "Use a specific DNS server instead of local server configuration" ) ) From cbbe8405223fc05d91f1d6ee16d7683cb0ed71cf Mon Sep 17 00:00:00 2001 From: Florian VANDENBULCKE Date: Sun, 14 Oct 2018 18:34:44 +0200 Subject: [PATCH 5/6] feat(dns-resolver): application to get ip address + unit tests --- modoboa/admin/lib.py | 37 +++++++++---------- modoboa/admin/tests/test_mx.py | 16 ++++---- modoboa/admin/tests/utils.py | 67 ++++++++++++++++++++++++++++++---- 3 files changed, 85 insertions(+), 35 deletions(-) diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py index ef5c94dc6..9991d6110 100644 --- a/modoboa/admin/lib.py +++ b/modoboa/admin/lib.py @@ -196,28 +196,25 @@ def get_domain_mx_list(domain): try: mx_domain = dns_answer.exchange.to_unicode( omit_final_dot=True, idna_codec=IDNA_2008_UTS_46) - ip_answers = socket.getaddrinfo( - mx_domain, 25, 0, socket.SOCK_STREAM) - except socket.gaierror as e: - logger.warning( - _("Unable to lookup ip addresses for %(domain)s; " - "%(error)s") - % {"domain": mx_domain, "error": str(e)}, - exc_info=e) + ip_answers = dns.resolver.query(mx_domain) + except dns.resolver.NXDOMAIN as e: + logger.error(_("No DNS records found without MX for %s") % domain, exc_info=e) else: for ip_answer in ip_answers: - try: - mx_ip = ipaddress.ip_address( - smart_text(ip_answer[4][0])) - except ValueError as e: - logger.warning( - _("Invalid IP address format for %(domain)s; " - "%(addr)s") - % {"domain": mx_domain, - "addr": smart_text(ip_answer[4][0])}, - exc_info=e) - else: - result.append((mx_domain, mx_ip)) + for answer_set in ip_answer.response.answer: + for address in answer_set: + try: + mx_ip = ipaddress.ip_address(smart_text(address)) + except ValueError as e: + logger.warning( + _("Invalid IP address format for %(domain)s; " + "%(addr)s") + % {"domain": mx_domain, + "addr": smart_text(address)}, + exc_info=e) + else: + result.append((mx_domain, mx_ip)) + return result diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index 3eb56bc23..239aa7c43 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -117,11 +117,13 @@ class MXTestCase(ModoTestCase): self.set_global_parameter("use_specific_dns_server", "123.45.67.89") get_domain_mx_list("does-not-exist.example.com") - @mock.patch("socket.getaddrinfo") + #@mock.patch("socket.getaddrinfo") + @mock.patch("ipaddress.ip_address") @mock.patch.object(dns.resolver.Resolver, "query") - def test_get_domain_mx_list_logging(self, mock_query, mock_getaddrinfo): + def test_get_domain_mx_list_logging(self, mock_query, mock_ip_address): mock_query.side_effect = utils.mock_dns_query_result - mock_getaddrinfo.side_effect = utils.mock_ip_query_result + #mock_getaddrinfo.side_effect = utils.mock_ip_query_result + mock_ip_address.side_effect = utils.mock_ip_address_result with LogCapture("modoboa.admin") as log: get_domain_mx_list("does-not-exist.example.com") get_domain_mx_list("no-mx.example.com") @@ -139,12 +141,12 @@ class MXTestCase(ModoTestCase): ("modoboa.admin", "WARNING", _("DNS resolution timeout, unable to query %s at the moment") % "timeout.example.com"), - ("modoboa.admin", "WARNING", - _("Unable to lookup ip addresses for %(domain)s; %(error)s") - % {"domain": "does-not-exist.example.com", "error": ""}), + ("modoboa.admin", "ERROR", + _("No DNS records found without MX for %(domain)s") + % {"domain": "no-lookup.example.com"}), ("modoboa.admin", "WARNING", _("Invalid IP address format for %(domain)s; %(addr)s") - % {"domain": "bad-response.example.com", "addr": "BAD"}), + % {"domain": "bad-response.example.com", "addr": "000.0.0.0"}), ) diff --git a/modoboa/admin/tests/utils.py b/modoboa/admin/tests/utils.py index 797fc6dcb..3e9161d2b 100644 --- a/modoboa/admin/tests/utils.py +++ b/modoboa/admin/tests/utils.py @@ -3,6 +3,7 @@ """Test utilities.""" from __future__ import unicode_literals +from dns.rrset import RRset import socket @@ -10,12 +11,38 @@ from dns.name import Name from dns.rdtypes.ANY.MX import MX from dns.resolver import NXDOMAIN, NoAnswer, NoNameservers, Timeout + + +class RRsetResponse(object): + + def __init__(self): + self.answer = [["192.0.2.1"]] + +class RRset(object): + + def __init__(self): + self.response = RRsetResponse() + +class RRsetResponseInvalid(object): + + def __init__(self): + self.answer = [["000.0.0.0"]] + +class RRsetInvalid(object): + + def __init__(self): + self.response = RRsetResponseInvalid() + + + _MX_RECORD_1 = MX("IN", "MX", 10, Name("mx.example.com".split("."))) _MX_RECORD_2 = MX("IN", "MX", 10, Name("mx2.example.com".split("."))) _BAD_MX_RECORD = MX("IN", "MX", 10, Name("bad-response.example.com".split("."))) _DNE_MX_RECORD = MX("IN", "MX", 10, Name( "does-not-exist.example.com".split("."))) _MX_RECORDS = [_MX_RECORD_1] +_IP_SET_RECORDS = [RRset()] + _IPV4_RECORD_1 = (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("192.0.2.1", 25)) _IPV4_RECORD_2 = (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("192.0.2.2", 25)) @@ -33,6 +60,12 @@ _POSSIBLE_DNS_RESULTS = { "no-lookup.example.com": [_DNE_MX_RECORD], } +_POSSIBLE_DNS_RESULTS_NO_MX = { + "does-not-exist.example.com": NXDOMAIN(), + "mx2.example.com": NXDOMAIN(), + "no-lookup.example.com": NXDOMAIN(), + "bad-response.example.com": [RRsetInvalid()], +} _POSSIBLE_IP_RESULTS = { "test3.com": [_IPV4_RECORD_2], "mx2.example.com": [_IPV4_RECORD_2], @@ -40,18 +73,36 @@ _POSSIBLE_IP_RESULTS = { "does-not-exist.example.com": socket.gaierror(), "192.0.2.254": socket.gaierror(), "2001:db8:254": socket.gaierror(), + "000.0.0.0": ValueError(), } def mock_dns_query_result(qname, *args, **kwargs): - if qname in _POSSIBLE_DNS_RESULTS: - if isinstance(_POSSIBLE_DNS_RESULTS[qname], Exception): - raise _POSSIBLE_DNS_RESULTS[qname] + if "MX" in args: + if qname in _POSSIBLE_DNS_RESULTS: + return get_mock_dns_query_response(_POSSIBLE_DNS_RESULTS, qname) + return _MX_RECORDS + elif qname in _POSSIBLE_DNS_RESULTS_NO_MX: + return get_mock_dns_query_response(_POSSIBLE_DNS_RESULTS_NO_MX, qname) + else: + return _IP_SET_RECORDS + + +def get_mock_dns_query_response(responses, attributeName): + if isinstance(responses[attributeName], Exception): + raise responses[attributeName] + else: + return responses[attributeName] + +def mock_ip_address_result(host, *args, **kwargs): + if host in _POSSIBLE_IP_RESULTS: + if isinstance(_POSSIBLE_IP_RESULTS[host], Exception): + raise _POSSIBLE_IP_RESULTS[host] else: - return _POSSIBLE_DNS_RESULTS[qname] - - return _MX_RECORDS - + return _POSSIBLE_IP_RESULTS[host] + + return _IP_RECORDS + def mock_ip_query_result(host, port, *args, **kwargs): if host in _POSSIBLE_IP_RESULTS: @@ -59,5 +110,5 @@ def mock_ip_query_result(host, port, *args, **kwargs): raise _POSSIBLE_IP_RESULTS[host] else: return _POSSIBLE_IP_RESULTS[host] - + return _IP_RECORDS From 6c2fbb169aadd86f57749c6e6ff6fd4030fa1e03 Mon Sep 17 00:00:00 2001 From: Florian VANDENBULCKE Date: Sun, 14 Oct 2018 18:55:02 +0200 Subject: [PATCH 6/6] feat(dns-resolver): correction lint --- modoboa/admin/lib.py | 13 +++++++------ modoboa/admin/tests/test_mx.py | 11 +++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py index 9991d6110..a8aabd51d 100644 --- a/modoboa/admin/lib.py +++ b/modoboa/admin/lib.py @@ -198,23 +198,24 @@ def get_domain_mx_list(domain): omit_final_dot=True, idna_codec=IDNA_2008_UTS_46) ip_answers = dns.resolver.query(mx_domain) except dns.resolver.NXDOMAIN as e: - logger.error(_("No DNS records found without MX for %s") % domain, exc_info=e) + logger.error(_("No DNS records found without MX for %s") + % domain, exc_info=e) else: for ip_answer in ip_answers: for answer_set in ip_answer.response.answer: for address in answer_set: try: - mx_ip = ipaddress.ip_address(smart_text(address)) + address_smart = smart_text(address) + mx_ip = ipaddress.ip_address(address_smart) except ValueError as e: logger.warning( - _("Invalid IP address format for %(domain)s; " - "%(addr)s") + _("Invalid IP address format for " + "%(domain)s; %(addr)s") % {"domain": mx_domain, "addr": smart_text(address)}, exc_info=e) else: - result.append((mx_domain, mx_ip)) - + result.append((mx_domain, mx_ip)) return result diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index 239aa7c43..b7bdc962f 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -117,12 +117,11 @@ class MXTestCase(ModoTestCase): self.set_global_parameter("use_specific_dns_server", "123.45.67.89") get_domain_mx_list("does-not-exist.example.com") - #@mock.patch("socket.getaddrinfo") @mock.patch("ipaddress.ip_address") @mock.patch.object(dns.resolver.Resolver, "query") def test_get_domain_mx_list_logging(self, mock_query, mock_ip_address): + """Test to get error loggins from specific DNS server.""" mock_query.side_effect = utils.mock_dns_query_result - #mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_ip_address.side_effect = utils.mock_ip_address_result with LogCapture("modoboa.admin") as log: get_domain_mx_list("does-not-exist.example.com") @@ -142,11 +141,11 @@ class MXTestCase(ModoTestCase): _("DNS resolution timeout, unable to query %s at the moment") % "timeout.example.com"), ("modoboa.admin", "ERROR", - _("No DNS records found without MX for %(domain)s") - % {"domain": "no-lookup.example.com"}), + _("No DNS records found without MX for %(domain)s") + % {"domain": "no-lookup.example.com"}), ("modoboa.admin", "WARNING", - _("Invalid IP address format for %(domain)s; %(addr)s") - % {"domain": "bad-response.example.com", "addr": "000.0.0.0"}), + _("Invalid IP address format for %(domain)s; %(addr)s") + % {"domain": "bad-response.example.com", "addr": "000.0.0.0"}), )