From 28c99abb46797be94c83b9fdadd4dc337205b95c Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Tue, 13 Sep 2016 16:24:04 +0200 Subject: [PATCH 1/6] introduce mx record --- modoboa/admin/lib.py | 44 +++++++++++++++- modoboa/admin/management/commands/modo.py | 2 + .../management/commands/subcommands/_dnsbl.py | 51 +++++-------------- .../management/commands/subcommands/_mx.py | 19 +++++++ modoboa/admin/migrations/0007_mxrecord.py | 27 ++++++++++ .../migrations/0008_auto_20160913_1414.py | 21 ++++++++ modoboa/admin/models/__init__.py | 3 +- modoboa/admin/models/domain.py | 18 ++++++- 8 files changed, 145 insertions(+), 40 deletions(-) create mode 100644 modoboa/admin/management/commands/subcommands/_mx.py create mode 100644 modoboa/admin/migrations/0007_mxrecord.py create mode 100644 modoboa/admin/migrations/0008_auto_20160913_1414.py diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py index 932a1504e..93785064d 100644 --- a/modoboa/admin/lib.py +++ b/modoboa/admin/lib.py @@ -2,9 +2,15 @@ """Internal library for admin.""" +import socket +import ipaddress +from datetime import datetime +from datetime import timedelta from functools import wraps from itertools import chain +import dns.resolver + from django.contrib.contenttypes.models import ContentType from django.db.models import Q from django.utils.translation import ugettext as _ @@ -14,7 +20,7 @@ from modoboa.core import signals as core_signals from modoboa.lib import events from modoboa.lib.exceptions import PermDeniedException -from .models import Domain, DomainAlias, Alias +from .models import Domain, DomainAlias, Alias, MXRecord def needs_mailbox(): @@ -100,6 +106,42 @@ def get_domains(user, domfilter=None, searchquery=None, **extrafilters): return domains +def get_mx_records_for_domain(domain, ttl=7200): + try: + answers = dns.resolver.query(domain.name, "MX") + except dns.resolver.NoAnswer: + return + now = datetime.now() + delta = timedelta(seconds=ttl) + records = MXRecord.objects.filter(domain=domain, + updated__gte=now - delta) + if records.count(): + print('skip update') + for record in records: + yield record + raise StopIteration() + MXRecord.objects.filter(domain=domain).delete() + for answer in answers: + address = None + try: + ipaddress.ip_address(str(answer.exchange)) + except ValueError: + try: + address = socket.gethostbyname(str(answer.exchange)) + except socket.gaierror: + pass + else: + address = str(answer.exchange) + finally: + if address is not None: + record = MXRecord.objects.create( + domain=domain, + name=str(answer.exchange).strip('.'), + address=address, + updated=now) + yield record + + def check_if_domain_exists(name, extra_checks=None): """Check if a domain already exists. diff --git a/modoboa/admin/management/commands/modo.py b/modoboa/admin/management/commands/modo.py index b5aba2f6d..1edebd35e 100644 --- a/modoboa/admin/management/commands/modo.py +++ b/modoboa/admin/management/commands/modo.py @@ -3,6 +3,7 @@ from subcommand.base import SubcommandCommand from .subcommands._dnsbl import CheckDNSBLCommand +from .subcommands._mx import CheckMXRecords from .subcommands._export import ExportCommand from .subcommands._import import ImportCommand @@ -18,5 +19,6 @@ class Command(SubcommandCommand): subcommands = { "export": ExportCommand, "import": ImportCommand, + "check_mx": CheckMXRecords, "check_dnsbl": CheckDNSBLCommand, } diff --git a/modoboa/admin/management/commands/subcommands/_dnsbl.py b/modoboa/admin/management/commands/subcommands/_dnsbl.py index ff0a5d24c..7b9f09612 100644 --- a/modoboa/admin/management/commands/subcommands/_dnsbl.py +++ b/modoboa/admin/management/commands/subcommands/_dnsbl.py @@ -1,8 +1,5 @@ """Management command to check defined domains against DNSBL providers.""" -import ipaddress - -import dns.resolver import gevent from gevent import socket @@ -14,6 +11,7 @@ from django.utils.translation import ugettext as _ from modoboa.admin import constants from modoboa.admin import models +from modoboa.admin.lib import get_mx_records_for_domain from modoboa.lib import parameters from modoboa.lib import email_utils @@ -41,37 +39,37 @@ class CheckDNSBLCommand(BaseCommand): "--timeout", type=int, default=3, help="Timeout used for queries.") - def query(self, ip_list, provider): + def query(self, mx_list, provider): """Check given IP against given provider.""" results = {} - for ip in ip_list: - reverse = ".".join(reversed(ip.split("."))) + for mx in mx_list: + reverse = ".".join(reversed(mx.address.split("."))) pattern = "{}.{}".format(reverse, provider) try: - results[ip] = socket.gethostbyname(pattern) + results[mx] = socket.gethostbyname(pattern) except socket.gaierror: - results[ip] = False + results[mx] = False return provider, results def store_domain_result(self, domain, provider, results): """Store provider results for domain.""" alerts = {} to_create = [] - for ip in results.keys(): - result = "" if not results[ip] else results[ip] + for mx in results.keys(): + result = "" if not results[mx] else results[mx] dnsbl_result = models.DNSBLResult.objects.filter( - domain=domain, provider=provider, mx=ip).first() + domain=domain, provider=provider, mx=mx).first() if dnsbl_result is None: to_create.append( models.DNSBLResult( - domain=domain, provider=provider, mx=ip, + domain=domain, provider=provider, mx=mx, status=result) ) else: if not dnsbl_result.status and result: if domain not in alerts: alerts[domain] = [] - alerts[domain].append((provider, ip)) + alerts[domain].append((provider, mx)) dnsbl_result.status = result dnsbl_result.save() models.DNSBLResult.objects.bulk_create(to_create) @@ -90,34 +88,13 @@ class CheckDNSBLCommand(BaseCommand): self.sender, admin.email, domainsubject=subject, content=content) if not status: - print msg + print(msg) def check_domain(self, domain, timeout): """Check specified domain.""" - resolver = dns.resolver.Resolver() - try: - answers = resolver.query(domain.name, "MX") - except dns.resolver.NoAnswer: - return - ip_list = [] - for answer in answers: - address = None - try: - ipaddress.ip_address(str(answer.exchange)) - except ValueError: - try: - address = socket.gethostbyname(str(answer.exchange)) - except socket.gaierror: - pass - else: - address = str(answer.exchange) - finally: - if address is not None: - ip_list.append(address) - if len(ip_list) == 0: - return + mx_list = get_mx_records_for_domain(domain) jobs = [ - gevent.spawn(self.query, ip_list, provider) + gevent.spawn(self.query, mx_list, provider) for provider in self.providers] gevent.joinall(jobs, timeout) for job in jobs: diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py new file mode 100644 index 000000000..32ee6395f --- /dev/null +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +"""Management command to check defined domains against MX records.""" + +from django.core.management.base import BaseCommand + +from modoboa.admin import models +from modoboa.admin.lib import get_mx_records_for_domain + + +class CheckMXRecords(BaseCommand): + """Command class.""" + + help = "Check defined domains against MX records." + + def handle(self, *args, **options): + """Command entry point.""" + for domain in models.Domain.objects.all(): + for record in get_mx_records_for_domain(domain): + print(record) diff --git a/modoboa/admin/migrations/0007_mxrecord.py b/modoboa/admin/migrations/0007_mxrecord.py new file mode 100644 index 000000000..759b5b203 --- /dev/null +++ b/modoboa/admin/migrations/0007_mxrecord.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.9 on 2016-09-13 09:16 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('admin', '0006_auto_20160910_1030'), + ] + + operations = [ + migrations.CreateModel( + name='MXRecord', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=254)), + ('address', models.GenericIPAddressField()), + ('managed', models.BooleanField(default=False)), + ('updated', models.DateTimeField()), + ('domain', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.Domain')), + ], + ), + ] diff --git a/modoboa/admin/migrations/0008_auto_20160913_1414.py b/modoboa/admin/migrations/0008_auto_20160913_1414.py new file mode 100644 index 000000000..53e6d69bb --- /dev/null +++ b/modoboa/admin/migrations/0008_auto_20160913_1414.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.9 on 2016-09-13 14:14 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('admin', '0007_mxrecord'), + ] + + operations = [ + migrations.AlterField( + model_name='dnsblresult', + name='mx', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.MXRecord'), + ), + ] diff --git a/modoboa/admin/models/__init__.py b/modoboa/admin/models/__init__.py index 29205eec3..07a1569ea 100644 --- a/modoboa/admin/models/__init__.py +++ b/modoboa/admin/models/__init__.py @@ -1,7 +1,7 @@ """Admin models.""" from .base import AdminObject -from .domain import Domain, DNSBLResult +from .domain import Domain, MXRecord, DNSBLResult from .domain_alias import DomainAlias from .mailbox import Mailbox, Quota, MailboxOperation, SenderAddress from .alias import Alias, AliasRecipient @@ -15,6 +15,7 @@ __all__ = [ "DomainAlias", "Mailbox", "MailboxOperation", + "MXRecord", "Quota", "SenderAddress", ] diff --git a/modoboa/admin/models/domain.py b/modoboa/admin/models/domain.py index d51a95d8d..fd6d39f3a 100644 --- a/modoboa/admin/models/domain.py +++ b/modoboa/admin/models/domain.py @@ -102,6 +102,10 @@ class Domain(AdminObject): def aliases(self): return self.domainalias_set + @property + def mx_records(self): + self.mxrecord_set + @cached_property def dnsbl_status_color(self): """Shortcut to DNSBL results.""" @@ -220,6 +224,18 @@ class Domain(AdminObject): reversion.register(Domain) +class MXRecord(models.Model): + + domain = models.ForeignKey(Domain) + name = models.CharField(max_length=254) + address = models.GenericIPAddressField() + managed = models.BooleanField(default=False) + updated = models.DateTimeField() + + def __unicode__(self): + return u'{0.name} ({0.address}) for {0.domain} '.format(self) + + class DNSBLQuerySet(models.QuerySet): """Custom manager for DNSBLResultManager.""" @@ -233,7 +249,7 @@ class DNSBLResult(models.Model): domain = models.ForeignKey(Domain) provider = models.CharField(max_length=254, db_index=True) - mx = models.GenericIPAddressField() + mx = models.ForeignKey(MXRecord) status = models.CharField(max_length=45, blank=True, db_index=True) objects = models.Manager.from_queryset(DNSBLQuerySet)() From dd9b63eead6277a13c2b2894c694a0ac97554526 Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Tue, 13 Sep 2016 17:21:26 +0200 Subject: [PATCH 2/6] merge mx/dnsbl commands --- modoboa/admin/lib.py | 44 +---- modoboa/admin/management/commands/modo.py | 2 - .../management/commands/subcommands/_dnsbl.py | 114 ------------- .../management/commands/subcommands/_mx.py | 160 +++++++++++++++++- .../admin/tests/{test_dnsbl.py => test_mx.py} | 13 +- 5 files changed, 166 insertions(+), 167 deletions(-) delete mode 100644 modoboa/admin/management/commands/subcommands/_dnsbl.py rename modoboa/admin/tests/{test_dnsbl.py => test_mx.py} (63%) diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py index 93785064d..932a1504e 100644 --- a/modoboa/admin/lib.py +++ b/modoboa/admin/lib.py @@ -2,15 +2,9 @@ """Internal library for admin.""" -import socket -import ipaddress -from datetime import datetime -from datetime import timedelta from functools import wraps from itertools import chain -import dns.resolver - from django.contrib.contenttypes.models import ContentType from django.db.models import Q from django.utils.translation import ugettext as _ @@ -20,7 +14,7 @@ from modoboa.core import signals as core_signals from modoboa.lib import events from modoboa.lib.exceptions import PermDeniedException -from .models import Domain, DomainAlias, Alias, MXRecord +from .models import Domain, DomainAlias, Alias def needs_mailbox(): @@ -106,42 +100,6 @@ def get_domains(user, domfilter=None, searchquery=None, **extrafilters): return domains -def get_mx_records_for_domain(domain, ttl=7200): - try: - answers = dns.resolver.query(domain.name, "MX") - except dns.resolver.NoAnswer: - return - now = datetime.now() - delta = timedelta(seconds=ttl) - records = MXRecord.objects.filter(domain=domain, - updated__gte=now - delta) - if records.count(): - print('skip update') - for record in records: - yield record - raise StopIteration() - MXRecord.objects.filter(domain=domain).delete() - for answer in answers: - address = None - try: - ipaddress.ip_address(str(answer.exchange)) - except ValueError: - try: - address = socket.gethostbyname(str(answer.exchange)) - except socket.gaierror: - pass - else: - address = str(answer.exchange) - finally: - if address is not None: - record = MXRecord.objects.create( - domain=domain, - name=str(answer.exchange).strip('.'), - address=address, - updated=now) - yield record - - def check_if_domain_exists(name, extra_checks=None): """Check if a domain already exists. diff --git a/modoboa/admin/management/commands/modo.py b/modoboa/admin/management/commands/modo.py index 1edebd35e..e5b7d3b40 100644 --- a/modoboa/admin/management/commands/modo.py +++ b/modoboa/admin/management/commands/modo.py @@ -2,7 +2,6 @@ from subcommand.base import SubcommandCommand -from .subcommands._dnsbl import CheckDNSBLCommand from .subcommands._mx import CheckMXRecords from .subcommands._export import ExportCommand from .subcommands._import import ImportCommand @@ -20,5 +19,4 @@ class Command(SubcommandCommand): "export": ExportCommand, "import": ImportCommand, "check_mx": CheckMXRecords, - "check_dnsbl": CheckDNSBLCommand, } diff --git a/modoboa/admin/management/commands/subcommands/_dnsbl.py b/modoboa/admin/management/commands/subcommands/_dnsbl.py deleted file mode 100644 index 7b9f09612..000000000 --- a/modoboa/admin/management/commands/subcommands/_dnsbl.py +++ /dev/null @@ -1,114 +0,0 @@ -"""Management command to check defined domains against DNSBL providers.""" - -import gevent -from gevent import socket - -from django.conf import settings -from django.core.management.base import BaseCommand -from django.template.loader import render_to_string -from django.utils.functional import cached_property -from django.utils.translation import ugettext as _ - -from modoboa.admin import constants -from modoboa.admin import models -from modoboa.admin.lib import get_mx_records_for_domain -from modoboa.lib import parameters -from modoboa.lib import email_utils - - -class CheckDNSBLCommand(BaseCommand): - """Command class.""" - - help = "Check defined domains against DNSBL providers." - - @cached_property - def providers(self): - """Return a list of DNSBL providers.""" - if not hasattr(settings, "DNSBL_PROVIDERS"): - return constants.DNSBL_PROVIDERS - return settings.DNSBL_PROVIDERS - - @cached_property - def sender(self): - """Return sender address for notifications.""" - return parameters.get_admin("SENDER_ADDRESS", app="core") - - def add_arguments(self, parser): - """Add extra arguments to command.""" - parser.add_argument( - "--timeout", type=int, default=3, - help="Timeout used for queries.") - - def query(self, mx_list, provider): - """Check given IP against given provider.""" - results = {} - for mx in mx_list: - reverse = ".".join(reversed(mx.address.split("."))) - pattern = "{}.{}".format(reverse, provider) - try: - results[mx] = socket.gethostbyname(pattern) - except socket.gaierror: - results[mx] = False - return provider, results - - def store_domain_result(self, domain, provider, results): - """Store provider results for domain.""" - alerts = {} - to_create = [] - for mx in results.keys(): - result = "" if not results[mx] else results[mx] - dnsbl_result = models.DNSBLResult.objects.filter( - domain=domain, provider=provider, mx=mx).first() - if dnsbl_result is None: - to_create.append( - models.DNSBLResult( - domain=domain, provider=provider, mx=mx, - status=result) - ) - else: - if not dnsbl_result.status and result: - if domain not in alerts: - alerts[domain] = [] - alerts[domain].append((provider, mx)) - dnsbl_result.status = result - dnsbl_result.save() - models.DNSBLResult.objects.bulk_create(to_create) - if not alerts: - return - content = render_to_string( - "admin/notifications/domain_in_dnsbl.html", { - "domain": domain, "alerts": alerts - }) - subject = _("[modoboa] DNSBL issue(s) for domain {}").format( - domain.name) - for admin in domain.admins: - if not admin.email: - continue - status, msg = email_utils.sendmail_simple( - self.sender, admin.email, - domainsubject=subject, content=content) - if not status: - print(msg) - - def check_domain(self, domain, timeout): - """Check specified domain.""" - mx_list = get_mx_records_for_domain(domain) - jobs = [ - gevent.spawn(self.query, mx_list, provider) - for provider in self.providers] - gevent.joinall(jobs, timeout) - for job in jobs: - if not job.successful(): - continue - provider, results = job.value - self.store_domain_result(domain, provider, results) - - def handle(self, *args, **options): - """Command entry point.""" - if parameters.get_admin("ENABLE_DNSBL_CHECKS") == "no": - return - # Remove deprecated records first - models.DNSBLResult.objects.exclude( - provider__in=self.providers).delete() - for domain in models.Domain.objects.filter(enabled=True): - self.check_domain(domain, options.get("timeout")) diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py index 32ee6395f..dc0d491d6 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -1,19 +1,165 @@ -# -*- coding: utf-8 -*- -"""Management command to check defined domains against MX records.""" +"""Management command to check defined domains.""" +import ipaddress +from datetime import datetime +from datetime import timedelta + +import dns.resolver + +import gevent +from gevent import socket + +from django.conf import settings from django.core.management.base import BaseCommand +from django.template.loader import render_to_string +from django.utils.functional import cached_property +from django.utils.translation import ugettext as _ +from modoboa.admin import constants from modoboa.admin import models -from modoboa.admin.lib import get_mx_records_for_domain +from modoboa.lib import parameters +from modoboa.lib import email_utils class CheckMXRecords(BaseCommand): """Command class.""" - help = "Check defined domains against MX records." + help = "Check defined domains." + + @cached_property + def providers(self): + """Return a list of DNSBL providers.""" + if not hasattr(settings, "DNSBL_PROVIDERS"): + return constants.DNSBL_PROVIDERS + return settings.DNSBL_PROVIDERS + + @cached_property + def sender(self): + """Return sender address for notifications.""" + return parameters.get_admin("SENDER_ADDRESS", app="core") + + def add_arguments(self, parser): + """Add extra arguments to command.""" + parser.add_argument( + "--no-dnsbl", action='store_true', default=False, + help="Skip DNSBL queries.") + parser.add_argument( + "--timeout", type=int, default=3, + help="Timeout used for queries.") + parser.add_argument( + "--ttl", type=int, default=7200, + help="TTL for dns query.") + + def get_mx_records_for_domain(self, domain, ttl=7200): + try: + answers = dns.resolver.query(domain.name, "MX") + except dns.resolver.NoAnswer: + return + now = datetime.now() + delta = timedelta(seconds=ttl) + records = models.MXRecord.objects.filter(domain=domain, + updated__gte=now - delta) + if records.count(): + for record in records: + yield record + raise StopIteration() + models.MXRecord.objects.filter(domain=domain).delete() + for answer in answers: + address = None + try: + ipaddress.ip_address(str(answer.exchange)) + except ValueError: + try: + address = socket.gethostbyname(str(answer.exchange)) + except socket.gaierror: + pass + else: + address = str(answer.exchange) + finally: + if address is not None: + record = models.MXRecord.objects.create( + domain=domain, + name=str(answer.exchange).strip('.'), + address=address, + updated=now) + yield record + + def query_dnsbl(self, mx_list, provider): + """Check given IP against given provider.""" + results = {} + for mx in mx_list: + reverse = ".".join(reversed(mx.address.split("."))) + pattern = "{}.{}".format(reverse, provider) + try: + results[mx] = socket.gethostbyname(pattern) + except socket.gaierror: + results[mx] = False + return provider, results + + def store_domain_result(self, domain, provider, results): + """Store provider results for domain.""" + alerts = {} + to_create = [] + for mx in results.keys(): + result = "" if not results[mx] else results[mx] + dnsbl_result = models.DNSBLResult.objects.filter( + domain=domain, provider=provider, mx=mx).first() + if dnsbl_result is None: + to_create.append( + models.DNSBLResult( + domain=domain, provider=provider, mx=mx, + status=result) + ) + else: + if not dnsbl_result.status and result: + if domain not in alerts: + alerts[domain] = [] + alerts[domain].append((provider, mx)) + dnsbl_result.status = result + dnsbl_result.save() + models.DNSBLResult.objects.bulk_create(to_create) + if not alerts: + return + content = render_to_string( + "admin/notifications/domain_in_dnsbl.html", { + "domain": domain, "alerts": alerts + }) + subject = _("[modoboa] DNSBL issue(s) for domain {}").format( + domain.name) + for admin in domain.admins: + if not admin.email: + continue + status, msg = email_utils.sendmail_simple( + self.sender, admin.email, + domainsubject=subject, content=content) + if not status: + print(msg) + + def check_domain(self, domain, timeout=3, ttl=7200, **options): + """Check specified domain.""" + + mx_list = self.get_mx_records_for_domain(domain, ttl=ttl) + + if parameters.get_admin("ENABLE_DNSBL_CHECKS") == "no": + return + elif options['no_dnsbl'] is True: + return + else: + jobs = [ + gevent.spawn(self.query_dnsbl, mx_list, provider) + for provider in self.providers] + gevent.joinall(jobs, timeout) + for job in jobs: + if not job.successful(): + continue + provider, results = job.value + self.store_domain_result(domain, provider, results) def handle(self, *args, **options): """Command entry point.""" - for domain in models.Domain.objects.all(): - for record in get_mx_records_for_domain(domain): - print(record) + # Remove deprecated records first + models.DNSBLResult.objects.exclude( + provider__in=self.providers).delete() + + for domain in models.Domain.objects.filter(enabled=True): + self.check_domain(domain, **options) diff --git a/modoboa/admin/tests/test_dnsbl.py b/modoboa/admin/tests/test_mx.py similarity index 63% rename from modoboa/admin/tests/test_dnsbl.py rename to modoboa/admin/tests/test_mx.py index 57a5e9772..413d3ced0 100644 --- a/modoboa/admin/tests/test_dnsbl.py +++ b/modoboa/admin/tests/test_mx.py @@ -24,9 +24,20 @@ class DNSBLTestCase(ModoTestCase): def test_management_command(self): """Check that command works fine.""" self.assertEqual(models.DNSBLResult.objects.count(), 0) - management.call_command("modo", "check_dnsbl") + management.call_command("modo", "check_mx") self.assertTrue( models.DNSBLResult.objects.filter(domain=self.domain).exists()) response = self.client.get( reverse("admin:dnsbl_domain_detail", args=[self.domain.pk])) self.assertEqual(response.status_code, 200) + + @override_settings(DNSBL_PROVIDERS=["zen.spamhaus.org"]) + def test_management_command_no_dnsbl(self): + """Check that command works fine without dnsbl.""" + self.assertEqual(models.DNSBLResult.objects.count(), 0) + management.call_command("modo", "check_mx", "--no-dnsbl") + self.assertFalse( + models.DNSBLResult.objects.filter(domain=self.domain).exists()) + response = self.client.get( + reverse("admin:dnsbl_domain_detail", args=[self.domain.pk])) + self.assertEqual(response.status_code, 200) From 3e5095a97cc08f2312cc48d9278b3c75999f3ea6 Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Tue, 13 Sep 2016 17:29:39 +0200 Subject: [PATCH 3/6] rename methodes --- modoboa/admin/management/commands/subcommands/_mx.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py index dc0d491d6..5a52e824f 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -96,7 +96,7 @@ class CheckMXRecords(BaseCommand): results[mx] = False return provider, results - def store_domain_result(self, domain, provider, results): + def store_dnsbl_result(self, domain, provider, results): """Store provider results for domain.""" alerts = {} to_create = [] @@ -153,7 +153,7 @@ class CheckMXRecords(BaseCommand): if not job.successful(): continue provider, results = job.value - self.store_domain_result(domain, provider, results) + self.store_dnsbl_result(domain, provider, results) def handle(self, *args, **options): """Command entry point.""" From 8b914246cfe37a81a58c317256c0426cb415b294 Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Tue, 13 Sep 2016 18:11:50 +0200 Subject: [PATCH 4/6] cleaner api / notification --- .../management/commands/subcommands/_mx.py | 40 ++++++++++++++++++- .../notifications/domain_invalid_mx.html | 9 +++++ 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 modoboa/admin/templates/admin/notifications/domain_invalid_mx.html diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py index 5a52e824f..0ff974be4 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -40,6 +40,9 @@ class CheckMXRecords(BaseCommand): def add_arguments(self, parser): """Add extra arguments to command.""" + parser.add_argument( + "--valid-mx", type=str, action='append', default=[], + help="Valid MX ips.") parser.add_argument( "--no-dnsbl", action='store_true', default=False, help="Skip DNSBL queries.") @@ -131,14 +134,47 @@ class CheckMXRecords(BaseCommand): continue status, msg = email_utils.sendmail_simple( self.sender, admin.email, - domainsubject=subject, content=content) + subject=subject, content=content) + if not status: + print(msg) + + def check_valid_mx(self, domain, mx_list, valid_mx=None, **options): + alerts = [] + check = False + mx_names = [mx.name for mx in mx_list] + mxs = mx_names + [mx.address for mx in mx_list] + if valid_mx: + check = True in [mx in mxs for mx in valid_mx] + if not mxs: + alerts.append(_('Domain {} as no MX record').format(domain)) + elif valid_mx and check is False: + args = (domain, ', '.join(mx_names)) + alerts.append( + _('Domain {0} got an invalid MX record. Got {1}').format(*args) + ) + if not alerts: + return + content = render_to_string( + "admin/notifications/domain_invalid_mx.html", { + "domain": domain, "valid_mx": valid_mx, "alerts": alerts + }) + subject = _("[modoboa] MX issue(s) for domain {}").format( + domain.name) + for admin in domain.admins: + if not admin.email: + continue + status, msg = email_utils.sendmail_simple( + self.sender, admin.email, + subject=subject, content=content) if not status: print(msg) def check_domain(self, domain, timeout=3, ttl=7200, **options): """Check specified domain.""" - mx_list = self.get_mx_records_for_domain(domain, ttl=ttl) + mx_list = list(self.get_mx_records_for_domain(domain, ttl=ttl)) + + self.check_valid_mx(domain, mx_list, **options) if parameters.get_admin("ENABLE_DNSBL_CHECKS") == "no": return diff --git a/modoboa/admin/templates/admin/notifications/domain_invalid_mx.html b/modoboa/admin/templates/admin/notifications/domain_invalid_mx.html new file mode 100644 index 000000000..34abbf230 --- /dev/null +++ b/modoboa/admin/templates/admin/notifications/domain_invalid_mx.html @@ -0,0 +1,9 @@ +{% load i18n %} + +{% blocktrans trimmed with domain=domain.name %} + Modoboa detected that domain {{ domain }} has no valid MX record: +{% endblocktrans %} + +{% for alert in alerts %} + - {{ alert }} +{% endfor %} From 05fcb531092397005dff1bfff69f571c57d9751b Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Wed, 14 Sep 2016 11:11:12 +0200 Subject: [PATCH 5/6] reset admin's migrations steps to avoid complex migration stuff; add moar options; improve coding style --- modoboa/admin/app_settings.py | 8 ++ .../management/commands/subcommands/_mx.py | 109 +++++++++++------- .../migrations/0004_auto_20160608_1423.py | 30 ----- .../migrations/0004_auto_20160914_0907.py | 67 +++++++++++ .../migrations/0005_auto_20160719_0903.py | 25 ---- .../migrations/0006_auto_20160910_1030.py | 28 ----- modoboa/admin/migrations/0007_mxrecord.py | 27 ----- .../migrations/0008_auto_20160913_1414.py | 21 ---- 8 files changed, 142 insertions(+), 173 deletions(-) delete mode 100644 modoboa/admin/migrations/0004_auto_20160608_1423.py create mode 100644 modoboa/admin/migrations/0004_auto_20160914_0907.py delete mode 100644 modoboa/admin/migrations/0005_auto_20160719_0903.py delete mode 100644 modoboa/admin/migrations/0006_auto_20160910_1030.py delete mode 100644 modoboa/admin/migrations/0007_mxrecord.py delete mode 100644 modoboa/admin/migrations/0008_auto_20160913_1414.py diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index efe660930..f551790e3 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -53,6 +53,14 @@ class AdminParametersForm(parameters.AdminParametersForm): dom_sep = SeparatorField(label=ugettext_lazy("Domains")) + enable_mx_checks = YesNoField( + label=ugettext_lazy("Enable MX checks"), + initial="yes", + help_text=ugettext_lazy( + "Check that every domain has a valid MX record" + ) + ) + enable_dnsbl_checks = YesNoField( label=ugettext_lazy("Enable DNSBL checks"), initial="yes", diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py index 0ff974be4..04dc42223 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -1,8 +1,8 @@ """Management command to check defined domains.""" -import ipaddress from datetime import datetime from datetime import timedelta +import ipaddress import dns.resolver @@ -42,10 +42,18 @@ class CheckMXRecords(BaseCommand): """Add extra arguments to command.""" parser.add_argument( "--valid-mx", type=str, action='append', default=[], - help="Valid MX ips.") + help="Valid MX ip(s) or subnet(s) used to check that domain's MX " + "are your own.") parser.add_argument( "--no-dnsbl", action='store_true', default=False, help="Skip DNSBL queries.") + parser.add_argument( + "--email", type=str, action='append', default=[], + help="One or more email to notify") + parser.add_argument( + "--skip-admin-emails", action='store_true', + default=False, + help="Skip domain's admins email notification.") parser.add_argument( "--timeout", type=int, default=3, help="Timeout used for queries.") @@ -54,6 +62,8 @@ class CheckMXRecords(BaseCommand): help="TTL for dns query.") def get_mx_records_for_domain(self, domain, ttl=7200): + """Return one or more `models.MXRecord` for `domain`. DNS queries are + not performed while `ttl` (in seconds) is still valid""" try: answers = dns.resolver.query(domain.name, "MX") except dns.resolver.NoAnswer: @@ -88,7 +98,7 @@ class CheckMXRecords(BaseCommand): yield record def query_dnsbl(self, mx_list, provider): - """Check given IP against given provider.""" + """Check given IP against given DNSBL provider.""" results = {} for mx in mx_list: reverse = ".".join(reversed(mx.address.split("."))) @@ -99,8 +109,8 @@ class CheckMXRecords(BaseCommand): results[mx] = False return provider, results - def store_dnsbl_result(self, domain, provider, results): - """Store provider results for domain.""" + def store_dnsbl_result(self, domain, provider, results, **options): + """Store DNSBL provider results for domain.""" alerts = {} to_create = [] for mx in results.keys(): @@ -123,64 +133,74 @@ class CheckMXRecords(BaseCommand): models.DNSBLResult.objects.bulk_create(to_create) if not alerts: return - content = render_to_string( - "admin/notifications/domain_in_dnsbl.html", { - "domain": domain, "alerts": alerts - }) - subject = _("[modoboa] DNSBL issue(s) for domain {}").format( - domain.name) - for admin in domain.admins: - if not admin.email: - continue - status, msg = email_utils.sendmail_simple( - self.sender, admin.email, - subject=subject, content=content) - if not status: - print(msg) + emails = options['email'] + if not options['skip_admin_emails']: + emails.extend([a.email for a in domain.admins if a.email]) + if emails: + content = render_to_string( + "admin/notifications/domain_in_dnsbl.html", { + "domain": domain, "alerts": alerts + }) + subject = _("[modoboa] DNSBL issue(s) for domain {}").format( + domain.name) + for email in emails: + status, msg = email_utils.sendmail_simple( + self.sender, email, + subject=subject, content=content) + if not status: + print(msg) def check_valid_mx(self, domain, mx_list, valid_mx=None, **options): + """Check that domain's MX record exist and is contains in `valid_mx` if + the option is provided""" alerts = [] check = False - mx_names = [mx.name for mx in mx_list] - mxs = mx_names + [mx.address for mx in mx_list] + mxs = [ipaddress.ip_address(u"%s" % mx.address) for mx in mx_list] if valid_mx: - check = True in [mx in mxs for mx in valid_mx] + for subnet in valid_mx: + check = True in [mx in subnet for mx in mxs] + if check is True: + break if not mxs: - alerts.append(_('Domain {} as no MX record').format(domain)) + alerts.append(_("Domain {} as no MX record").format(domain)) elif valid_mx and check is False: - args = (domain, ', '.join(mx_names)) + mx_names = ["{0.name} ({0.address})".format(mx) for mx in mx_list] + args = (domain, ", ".join(mx_names)) alerts.append( - _('Domain {0} got an invalid MX record. Got {1}').format(*args) + _("Domain {0} got an invalid MX record. Got {1}").format(*args) ) if not alerts: return - content = render_to_string( - "admin/notifications/domain_invalid_mx.html", { - "domain": domain, "valid_mx": valid_mx, "alerts": alerts - }) - subject = _("[modoboa] MX issue(s) for domain {}").format( - domain.name) - for admin in domain.admins: - if not admin.email: - continue - status, msg = email_utils.sendmail_simple( - self.sender, admin.email, - subject=subject, content=content) - if not status: - print(msg) + emails = options['email'] + if not options['skip_admin_emails']: + emails.extend([a.email for a in domain.admins if a.email]) + if emails: + content = render_to_string( + "admin/notifications/domain_invalid_mx.html", { + "domain": domain, "valid_mx": valid_mx, "alerts": alerts + }) + subject = _("[modoboa] MX issue(s) for domain {}").format( + domain.name) + for email in emails: + status, msg = email_utils.sendmail_simple( + self.sender, email, + subject=subject, content=content) + if not status: + print(msg) def check_domain(self, domain, timeout=3, ttl=7200, **options): """Check specified domain.""" mx_list = list(self.get_mx_records_for_domain(domain, ttl=ttl)) - self.check_valid_mx(domain, mx_list, **options) + if parameters.get_admin("ENABLE_MX_CHECKS") != "no": + self.check_valid_mx(domain, mx_list, **options) if parameters.get_admin("ENABLE_DNSBL_CHECKS") == "no": return elif options['no_dnsbl'] is True: return - else: + elif mx_list: jobs = [ gevent.spawn(self.query_dnsbl, mx_list, provider) for provider in self.providers] @@ -189,10 +209,15 @@ class CheckMXRecords(BaseCommand): if not job.successful(): continue provider, results = job.value - self.store_dnsbl_result(domain, provider, results) + self.store_dnsbl_result(domain, provider, results, **options) def handle(self, *args, **options): """Command entry point.""" + # Check that user provide valid network addresses + valid_mx = options['valid_mx'] + options['valid_mx'] = [ipaddress.ip_network(u"%s" % v) + for v in valid_mx] + # Remove deprecated records first models.DNSBLResult.objects.exclude( provider__in=self.providers).delete() diff --git a/modoboa/admin/migrations/0004_auto_20160608_1423.py b/modoboa/admin/migrations/0004_auto_20160608_1423.py deleted file mode 100644 index 177e0a78c..000000000 --- a/modoboa/admin/migrations/0004_auto_20160608_1423.py +++ /dev/null @@ -1,30 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.9.5 on 2016-06-08 12:23 -from __future__ import unicode_literals - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('admin', '0003_auto_20151118_1215'), - ] - - operations = [ - migrations.CreateModel( - name='DNSBLResult', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('provider', models.CharField(max_length=254, db_index=True)), - ('mx', models.GenericIPAddressField()), - ('status', models.CharField(blank=True, max_length=45, db_index=True)), - ('domain', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.Domain')), - ], - ), - migrations.AlterUniqueTogether( - name='dnsblresult', - unique_together=set([('domain', 'provider', 'mx')]), - ), - ] diff --git a/modoboa/admin/migrations/0004_auto_20160914_0907.py b/modoboa/admin/migrations/0004_auto_20160914_0907.py new file mode 100644 index 000000000..32aa5e148 --- /dev/null +++ b/modoboa/admin/migrations/0004_auto_20160914_0907.py @@ -0,0 +1,67 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.9 on 2016-09-14 09:07 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('admin', '0003_auto_20151118_1215'), + ] + + operations = [ + migrations.CreateModel( + name='DNSBLResult', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('provider', models.CharField(db_index=True, max_length=254)), + ('status', models.CharField(blank=True, db_index=True, max_length=45)), + ('domain', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.Domain')), + ], + ), + migrations.CreateModel( + name='MXRecord', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=254)), + ('address', models.GenericIPAddressField()), + ('managed', models.BooleanField(default=False)), + ('updated', models.DateTimeField()), + ('domain', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.Domain')), + ], + ), + migrations.CreateModel( + name='SenderAddress', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('address', models.EmailField(max_length=254)), + ('mailbox', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.Mailbox')), + ], + ), + migrations.AddField( + model_name='alias', + name='description', + field=models.TextField(blank=True), + ), + migrations.AddField( + model_name='alias', + name='expire_at', + field=models.DateTimeField(blank=True, null=True), + ), + migrations.AddField( + model_name='dnsblresult', + name='mx', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.MXRecord'), + ), + migrations.AlterUniqueTogether( + name='senderaddress', + unique_together=set([('address', 'mailbox')]), + ), + migrations.AlterUniqueTogether( + name='dnsblresult', + unique_together=set([('domain', 'provider', 'mx')]), + ), + ] diff --git a/modoboa/admin/migrations/0005_auto_20160719_0903.py b/modoboa/admin/migrations/0005_auto_20160719_0903.py deleted file mode 100644 index 343ac82b1..000000000 --- a/modoboa/admin/migrations/0005_auto_20160719_0903.py +++ /dev/null @@ -1,25 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.9.5 on 2016-07-19 07:03 -from __future__ import unicode_literals - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('admin', '0004_auto_20160608_1423'), - ] - - operations = [ - migrations.AddField( - model_name='alias', - name='description', - field=models.TextField(blank=True), - ), - migrations.AddField( - model_name='alias', - name='expire_at', - field=models.DateTimeField(blank=True, null=True), - ), - ] diff --git a/modoboa/admin/migrations/0006_auto_20160910_1030.py b/modoboa/admin/migrations/0006_auto_20160910_1030.py deleted file mode 100644 index 75c5bd8f5..000000000 --- a/modoboa/admin/migrations/0006_auto_20160910_1030.py +++ /dev/null @@ -1,28 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.9.5 on 2016-09-10 08:30 -from __future__ import unicode_literals - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('admin', '0005_auto_20160719_0903'), - ] - - operations = [ - migrations.CreateModel( - name='SenderAddress', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('address', models.EmailField(max_length=254)), - ('mailbox', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.Mailbox')), - ], - ), - migrations.AlterUniqueTogether( - name='senderaddress', - unique_together=set([('address', 'mailbox')]), - ), - ] diff --git a/modoboa/admin/migrations/0007_mxrecord.py b/modoboa/admin/migrations/0007_mxrecord.py deleted file mode 100644 index 759b5b203..000000000 --- a/modoboa/admin/migrations/0007_mxrecord.py +++ /dev/null @@ -1,27 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.9.9 on 2016-09-13 09:16 -from __future__ import unicode_literals - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('admin', '0006_auto_20160910_1030'), - ] - - operations = [ - migrations.CreateModel( - name='MXRecord', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('name', models.CharField(max_length=254)), - ('address', models.GenericIPAddressField()), - ('managed', models.BooleanField(default=False)), - ('updated', models.DateTimeField()), - ('domain', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.Domain')), - ], - ), - ] diff --git a/modoboa/admin/migrations/0008_auto_20160913_1414.py b/modoboa/admin/migrations/0008_auto_20160913_1414.py deleted file mode 100644 index 53e6d69bb..000000000 --- a/modoboa/admin/migrations/0008_auto_20160913_1414.py +++ /dev/null @@ -1,21 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.9.9 on 2016-09-13 14:14 -from __future__ import unicode_literals - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('admin', '0007_mxrecord'), - ] - - operations = [ - migrations.AlterField( - model_name='dnsblresult', - name='mx', - field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='admin.MXRecord'), - ), - ] From ea3b9277baab14898bb51db286e207141618842d Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Wed, 14 Sep 2016 13:51:04 +0200 Subject: [PATCH 6/6] improve coverage --- .../management/commands/subcommands/_mx.py | 20 ++++++----- modoboa/admin/tests/test_mx.py | 35 +++++++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/management/commands/subcommands/_mx.py index 04dc42223..0b9a459af 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/management/commands/subcommands/_mx.py @@ -64,19 +64,23 @@ class CheckMXRecords(BaseCommand): def get_mx_records_for_domain(self, domain, ttl=7200): """Return one or more `models.MXRecord` for `domain`. DNS queries are not performed while `ttl` (in seconds) is still valid""" - try: - answers = dns.resolver.query(domain.name, "MX") - except dns.resolver.NoAnswer: - return now = datetime.now() - delta = timedelta(seconds=ttl) records = models.MXRecord.objects.filter(domain=domain, - updated__gte=now - delta) + updated__gt=now) if records.count(): for record in records: yield record raise StopIteration() - models.MXRecord.objects.filter(domain=domain).delete() + else: + models.MXRecord.objects.filter(domain=domain).delete() + + try: + answers = dns.resolver.query(domain.name, "MX") + except dns.resolver.NoAnswer: + raise StopIteration() + + delta = timedelta(seconds=ttl) + for answer in answers: address = None try: @@ -94,7 +98,7 @@ class CheckMXRecords(BaseCommand): domain=domain, name=str(answer.exchange).strip('.'), address=address, - updated=now) + updated=now + delta) yield record def query_dnsbl(self, mx_list, provider): diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index 413d3ced0..7fccef4a5 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -10,6 +10,41 @@ from .. import factories from .. import models +class MXTestCase(ModoTestCase): + """TestCase for DNSBL related features.""" + + @classmethod + def setUpTestData(cls): + """Create some data.""" + super(MXTestCase, cls).setUpTestData() + cls.domain = factories.DomainFactory(name="modoboa.org") + factories.DomainFactory(name="pouet.com") # should not exist + + def test_management_command(self): + """Check that command works fine.""" + self.assertEqual(models.MXRecord.objects.count(), 0) + management.call_command("modo", "check_mx", "--no-dnsbl", + "--valid-mx=127.0.0.1", "--ttl=0") + self.assertTrue( + models.MXRecord.objects.filter(domain=self.domain).exists()) + + # we passed a ttl to 0. this will reset the cache this time + qs = models.MXRecord.objects.filter(domain=self.domain) + id = qs[0].id + management.call_command("modo", "check_mx", "--no-dnsbl", + "--valid-mx=127.0.0.1", "--ttl=7200") + qs = models.MXRecord.objects.filter(domain=self.domain) + self.assertNotEqual(id, qs[0].id) + id = qs[0].id + + # assume that mxrecords ids are the same. means that we taking care of + # ttl + management.call_command("modo", "check_mx", "--no-dnsbl", + "--valid-mx=127.0.0.1") + qs = models.MXRecord.objects.filter(domain=self.domain) + self.assertEqual(id, qs[0].id) + + class DNSBLTestCase(ModoTestCase): """TestCase for DNSBL related features."""