diff --git a/modoboa/admin/api/v2/serializers.py b/modoboa/admin/api/v2/serializers.py index 532093049..618732532 100644 --- a/modoboa/admin/api/v2/serializers.py +++ b/modoboa/admin/api/v2/serializers.py @@ -223,6 +223,7 @@ class AdminGlobalParametersSerializer(serializers.Serializer): enable_dkim_checks = serializers.BooleanField(default=True) enable_dmarc_checks = serializers.BooleanField(default=True) enable_autoconfig_checks = serializers.BooleanField(default=True) + enable_dns_notifications = serializers.BooleanField(default=True) custom_dns_server = serializers.IPAddressField(allow_blank=True, allow_null=True) enable_dnsbl_checks = serializers.BooleanField(default=True) dkim_keys_storage_dir = serializers.CharField(allow_blank=True, allow_null=True) diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py index 1ddbfad1c..fd40589c9 100644 --- a/modoboa/admin/app_settings.py +++ b/modoboa/admin/app_settings.py @@ -110,6 +110,18 @@ GLOBAL_PARAMETERS_STRUCT = collections.OrderedDict( ), }, ), + ( + "enable_dns_notifications", + { + "label": gettext_lazy( + "Notify domain admins about DNS issues" + ), + "help_text": gettext_lazy( + "Send a notification by email to domain administrators " + "when DNS issues are encountered" + ), + }, + ), ( "custom_dns_server", { diff --git a/modoboa/admin/management/commands/subcommands/_mx.py b/modoboa/admin/dns_checker.py similarity index 64% rename from modoboa/admin/management/commands/subcommands/_mx.py rename to modoboa/admin/dns_checker.py index ff1d9e890..038cd0a57 100644 --- a/modoboa/admin/management/commands/subcommands/_mx.py +++ b/modoboa/admin/dns_checker.py @@ -1,16 +1,10 @@ -"""Management command to check defined domains.""" - import ipaddress - -import gevent -from gevent import socket +import socket from django.conf import settings from django.core.mail import EmailMessage -from django.core.management.base import BaseCommand from django.template.loader import render_to_string from django.utils import timezone -from django.utils.encoding import smart_str from django.utils.functional import cached_property from django.utils.translation import gettext as _ @@ -19,10 +13,7 @@ from modoboa.dnstools import models as dns_models from modoboa.parameters import tools as param_tools -class CheckMXRecords(BaseCommand): - """Command class.""" - - help = "Check defined domains." # NOQA:A003 +class DNSChecker: @cached_property def providers(self): @@ -36,52 +27,24 @@ class CheckMXRecords(BaseCommand): """Return sender address for notifications.""" return param_tools.get_global_parameter("sender_address", app="core") + @cached_property + def config(self) -> dict: + return dict(param_tools.get_global_parameters("admin")) + @cached_property def valid_mxs(self): """Return valid MXs set in admin.""" - valid_mxs = param_tools.get_global_parameter("valid_mxs") + valid_mxs = self.config["valid_mxs"] return [ - ipaddress.ip_network(smart_str(v.strip())) - for v in valid_mxs.split() - if v.strip() + ipaddress.ip_network(str(v.strip())) for v in valid_mxs.split() if v.strip() ] - 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( - "--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( - "--domain", - type=str, - action="append", - default=[], - help="Domain name or id to update.", - ) - 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 query_dnsbl(self, mx_list, provider): """Check given IP against given DNSBL provider.""" results = {} for ip, mxs in mx_list.items(): try: - ip = ipaddress.ip_address(smart_str(ip)) + ip = ipaddress.ip_address(str(ip)) except ValueError: continue else: @@ -99,9 +62,9 @@ class CheckMXRecords(BaseCommand): result = False for mx in mxs: results[mx] = result - return provider, results + return results - def store_dnsbl_result(self, domain, provider, results, **options): + def store_dnsbl_result(self, domain, provider, results): """Store DNSBL provider results for domain. Return a list of alerts. @@ -142,23 +105,24 @@ class CheckMXRecords(BaseCommand): models.DNSBLResult.objects.bulk_create(to_create) return alerts - def send_alert_notifications(self, domain, alerts, subject, tpl, **options): + def send_alert_notifications( + self, domain: models.Domain, alerts: list, subject: str, tpl: str + ) -> None: """Send email notifications about given alerts.""" - emails = list(options["email"]) - if not options["skip_admin_emails"]: - emails.extend( - domain.admins.exclude(mailbox__isnull=True).values_list( - "email", flat=True - ) - ) + if not self.config["enable_dns_notifications"]: + return + emails = domain.admins.exclude(mailbox__isnull=True).values_list( + "email", flat=True + ) if not len(emails): return content = render_to_string(tpl, {"domain": domain, "alerts": alerts}) msg = EmailMessage(subject, content.strip(), self.sender, emails) msg.send() - def check_valid_mx(self, domain, mx_list, **options): - """Check that domain's MX record exist. + def check_valid_mx(self, domain: models.Domain, mx_list: list) -> None: + """ + Check that domain's MX record exist. If `valid_mx` is provided, retrieved MX records must be contained in it. @@ -212,26 +176,25 @@ class CheckMXRecords(BaseCommand): return subject = _("[modoboa] MX issue(s) for domain {}").format(domain.name) tpl = "admin/notifications/domain_invalid_mx.html" - self.send_alert_notifications(domain, alerts, subject, tpl, **options) + self.send_alert_notifications(domain, alerts, subject, tpl) + + def run(self, domain: models.Domain, ttl: int = 7200): + # Remove deprecated records first + domain.dnsblresult_set.exclude(provider__in=self.providers).delete() - def check_domain(self, domain, timeout=3, ttl=7200, **options): - """Check specified domain.""" mx_list = list(models.MXRecord.objects.get_or_create_for_domain(domain, ttl)) - if param_tools.get_global_parameter("enable_mx_checks"): - self.check_valid_mx(domain, mx_list, **options) + if self.config["enable_mx_checks"]: + self.check_valid_mx(domain, mx_list) - if param_tools.get_global_parameter("enable_spf_checks"): + if self.config["enable_spf_checks"]: dns_models.DNSRecord.objects.get_or_create_for_domain(domain, "spf", ttl) - condition = ( - param_tools.get_global_parameter("enable_dkim_checks") - and domain.dkim_public_key - ) + condition = self.config["enable_dkim_checks"] and domain.dkim_public_key if condition: dns_models.DNSRecord.objects.get_or_create_for_domain(domain, "dkim", ttl) - if param_tools.get_global_parameter("enable_dmarc_checks"): + if self.config["enable_dmarc_checks"]: dns_models.DNSRecord.objects.get_or_create_for_domain(domain, "dmarc", ttl) - if param_tools.get_global_parameter("enable_autoconfig_checks"): + if self.config["enable_autoconfig_checks"]: dns_models.DNSRecord.objects.get_or_create_for_domain( domain, "autoconfig", ttl ) @@ -239,11 +202,8 @@ class CheckMXRecords(BaseCommand): domain, "autodiscover", ttl ) - condition = ( - not param_tools.get_global_parameter("enable_dnsbl_checks") - or options["no_dnsbl"] is True - ) - if condition or not mx_list: + condition = not self.config["enable_dnsbl_checks"] or not mx_list + if condition: return mx_by_ip = {} @@ -253,44 +213,14 @@ class CheckMXRecords(BaseCommand): elif mx not in mx_by_ip[mx.address]: mx_by_ip[mx.address].append(mx) - jobs = [ - gevent.spawn(self.query_dnsbl, mx_by_ip, provider) - for provider in self.providers - ] - gevent.joinall(jobs, timeout) alerts = [] - for job in jobs: - if not job.successful(): - continue - provider, results = job.value - alerts += self.store_dnsbl_result(domain, provider, results, **options) + for provider in self.providers: + results = self.query_dnsbl(mx_by_ip, provider) + alerts += self.store_dnsbl_result(domain, provider, results) + if not alerts: return + subject = _("[modoboa] DNSBL issue(s) for domain {}").format(domain.name) tpl = "admin/notifications/domain_in_dnsbl.html" - self.send_alert_notifications(domain, alerts, subject, tpl, **options) - - def handle(self, *args, **options): - """Command entry point.""" - # Remove deprecated records first - models.DNSBLResult.objects.exclude(provider__in=self.providers).delete() - - if options["domain"]: - domains = [] - for domain in options["domain"]: - try: - if domain.isdigit(): - domains.append(models.Domain.objects.get(pk=domain)) - else: - domains.append(models.Domain.objects.get(name=domain)) - except models.Domain.DoesNotExist: - pass - else: - domains = models.Domain.objects.filter(enabled=True, enable_dns_checks=True) - - options.pop("domain") - - for domain in domains: - if domain.uses_a_reserved_tld: - continue - self.check_domain(domain, **options) + self.send_alert_notifications(domain, alerts, subject, tpl) diff --git a/modoboa/admin/jobs.py b/modoboa/admin/jobs.py index 836940dab..4080a0fb3 100644 --- a/modoboa/admin/jobs.py +++ b/modoboa/admin/jobs.py @@ -4,8 +4,14 @@ import logging import os import shutil +from django.db.models import F +from django.utils import timezone + +import django_rq + +from modoboa.admin import models from modoboa.admin.app_settings import load_admin_settings -from modoboa.admin.models.mailbox import MailboxOperation +from modoboa.admin.dns_checker import DNSChecker from modoboa.lib.sysutils import exec_cmd from modoboa.parameters import tools as param_tools @@ -56,8 +62,25 @@ def handle_mailbox_operations(): load_admin_settings() if not param_tools.get_global_parameter("handle_mailboxes"): return - for ope in MailboxOperation.objects.all(): + for ope in models.MailboxOperation.objects.all(): if ope.type == "rename": rename_mailbox(ope) elif ope.type == "delete": delete_mailbox(ope) + + +def launch_domain_dns_checks(domain_id: int): + domain = models.Domain.objects.get(id=domain_id) + DNSChecker().run(domain) + + +def handle_dns_checks(): + """Launch DNS checks for every possible domain.""" + minute = timezone.now().minute + queue = django_rq.get_queue("modoboa") + for domain in models.Domain.objects.annotate(slot=F("id") % 60).filter( + enable_dns_checks=True, slot=minute + ): + if domain.uses_a_reserved_tld: + continue + queue.enqueue(launch_domain_dns_checks, domain.id) diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index 790d4f006..6b1963555 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -3,12 +3,17 @@ from unittest import mock import dns.resolver +from rq import SimpleWorker from testfixtures import LogCapture -from django.core import mail, management +from django.core import mail from django.test import override_settings from django.utils.translation import gettext as _ +import django_rq + +from modoboa.admin import jobs +from modoboa.admin.dns_checker import DNSChecker from modoboa.core import models as core_models, factories as core_factories from modoboa.lib.tests import ModoTestCase from . import utils @@ -44,26 +49,26 @@ class MXTestCase(ModoTestCase): cls.localconfig.save() models.MXRecord.objects.all().delete() - @mock.patch("gevent.socket.gethostbyname") + @mock.patch("socket.gethostbyname") @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "resolve") - def test_management_command( - self, mock_query, mock_getaddrinfo, mock_g_gethostbyname - ): + def test_dns_checker(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname): """Check that command works fine.""" mock_query.side_effect = utils.mock_dns_query_result mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_g_gethostbyname.return_value = "1.2.3.4" + self.set_global_parameter("enable_dnsbl_checks", False) + self.assertEqual(models.MXRecord.objects.count(), 0) with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--no-dnsbl", "--ttl=0") + DNSChecker().run(self.domain, 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 with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--no-dnsbl", "--ttl=7200") + DNSChecker().run(self.domain) qs = models.MXRecord.objects.filter(domain=self.domain) self.assertNotEqual(id_, qs[0].id) id_ = qs[0].id @@ -71,40 +76,11 @@ class MXTestCase(ModoTestCase): # assume that mxrecords ids are the same. means that we taking care of # ttl with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--no-dnsbl") + DNSChecker().run(self.domain) qs = models.MXRecord.objects.filter(domain=self.domain) self.assertEqual(id_, qs[0].id) - @mock.patch("gevent.socket.gethostbyname") - @mock.patch("socket.getaddrinfo") - @mock.patch.object(dns.resolver.Resolver, "resolve") - def test_single_domain_update( - self, mock_query, mock_getaddrinfo, mock_g_gethostbyname - ): - """Update only one domain.""" - mock_query.side_effect = utils.mock_dns_query_result - mock_getaddrinfo.side_effect = utils.mock_ip_query_result - mock_g_gethostbyname.return_value = "1.2.3.4" - with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--domain", self.domain.name) - self.assertTrue(models.MXRecord.objects.filter(domain=self.domain).exists()) - self.assertFalse( - models.MXRecord.objects.filter(domain=self.bad_domain).exists() - ) - - with LogCapture("modoboa.dns"): - management.call_command( - "modo", "check_mx", "--domain", str(self.bad_domain.pk) - ) - self.assertFalse( - models.MXRecord.objects.filter(domain=self.bad_domain).exists() - ) - self.assertEqual(len(mail.outbox), 1) - - with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--domain", "toto.com") - - @mock.patch("gevent.socket.gethostbyname") + @mock.patch("socket.gethostbyname") @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "resolve") def test_invalid_mx(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname): @@ -112,6 +88,8 @@ class MXTestCase(ModoTestCase): mock_query.side_effect = utils.mock_dns_query_result mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_g_gethostbyname.return_value = "1.2.3.4" + self.set_global_parameter("enable_dnsbl_checks", False) + domain = factories.DomainFactory(name="invalid-mx.com") # Add domain admin with mailbox mb = factories.MailboxFactory( @@ -122,14 +100,10 @@ class MXTestCase(ModoTestCase): ) domain.add_admin(mb.user) with LogCapture("modoboa.dns"): - management.call_command( - "modo", "check_mx", "--domain", domain.name, "--no-dnsbl" - ) + DNSChecker().run(domain) self.assertEqual(domain.alarms.count(), 1) with LogCapture("modoboa.dns"): - management.call_command( - "modo", "check_mx", "--domain", domain.name, "--no-dnsbl" - ) + DNSChecker().run(domain) self.assertEqual(domain.alarms.count(), 1) self.assertEqual(len(mail.outbox), 1) @@ -226,6 +200,15 @@ class DNSBLTestCase(ModoTestCase): """Create some data.""" super().setUpTestData() cls.domain = factories.DomainFactory(name="modoboa.org") + # Add domain admin with mailbox + mb = factories.MailboxFactory( + address="admin", + domain=cls.domain, + user__username="admin@modoboa.org", + user__groups=("DomainAdmins",), + ) + cls.domain.add_admin(mb.user) + factories.DomainFactory(name="does-not-exist.example.com") cls.domain2 = factories.DomainFactory( name="test.localhost" @@ -235,19 +218,21 @@ class DNSBLTestCase(ModoTestCase): ) models.DNSBLResult.objects.all().delete() - @mock.patch("gevent.socket.gethostbyname") + @mock.patch("socket.gethostbyname") @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "resolve") - def test_management_command( - self, mock_query, mock_getaddrinfo, mock_g_gethostbyname - ): + def test_job(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname): """Check that command works fine.""" mock_query.side_effect = utils.mock_dns_query_result mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_g_gethostbyname.return_value = "1.2.3.4" self.assertEqual(models.DNSBLResult.objects.count(), 0) + + queue = django_rq.get_queue("modoboa") + worker = SimpleWorker([queue], connection=queue.connection) with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx") + jobs.handle_dns_checks() + worker.work(burst=True) self.assertTrue(models.DNSBLResult.objects.filter(domain=self.domain).exists()) self.assertFalse( models.DNSBLResult.objects.filter(domain=self.domain3).exists() @@ -255,7 +240,7 @@ class DNSBLTestCase(ModoTestCase): self.assertFalse(self.domain.uses_a_reserved_tld) self.assertTrue(self.domain2.uses_a_reserved_tld) - @mock.patch("gevent.socket.gethostbyname") + @mock.patch("socket.gethostbyname") @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "resolve") def test_notifications(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname): @@ -264,10 +249,10 @@ class DNSBLTestCase(ModoTestCase): mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_g_gethostbyname.return_value = "127.0.0.4" with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--email", "user@example.test") - self.assertEqual(len(mail.outbox), 2) + DNSChecker().run(self.domain) + self.assertEqual(len(mail.outbox), 1) - @mock.patch("gevent.socket.gethostbyname") + @mock.patch("socket.gethostbyname") @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "resolve") def test_notifications_wrong_dnsbl_response( @@ -278,10 +263,10 @@ class DNSBLTestCase(ModoTestCase): mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_g_gethostbyname.return_value = "127.255.255.254" # <--Spamhaus response when querying from an open resolver with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--email", "user@example.test") + DNSChecker().run(self.domain) self.assertEqual(len(mail.outbox), 1) - @mock.patch("gevent.socket.gethostbyname") + @mock.patch("socket.gethostbyname") @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "resolve") def test_management_command_no_dnsbl( @@ -291,9 +276,10 @@ class DNSBLTestCase(ModoTestCase): mock_query.side_effect = utils.mock_dns_query_result mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_g_gethostbyname.return_value = "1.2.3.4" + self.set_global_parameter("enable_dnsbl_checks", False) self.assertEqual(models.DNSBLResult.objects.count(), 0) with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--no-dnsbl") + DNSChecker().run(self.domain) self.assertFalse(models.DNSBLResult.objects.filter(domain=self.domain).exists()) @@ -306,7 +292,7 @@ class DNSChecksTestCase(ModoTestCase): super().setUpTestData() cls.domain = factories.DomainFactory(name="dns-checks.com") - @mock.patch("gevent.socket.gethostbyname") + @mock.patch("socket.gethostbyname") @mock.patch("socket.getaddrinfo") @mock.patch.object(dns.resolver.Resolver, "resolve") def test_management_command( @@ -316,13 +302,14 @@ class DNSChecksTestCase(ModoTestCase): mock_query.side_effect = utils.mock_dns_query_result mock_getaddrinfo.side_effect = utils.mock_ip_query_result mock_g_gethostbyname.return_value = "1.2.3.4" + self.set_global_parameter("enable_dnsbl_checks", False) self.domain.enable_dkim = True self.domain.dkim_public_key = "XXXXX" self.domain.save(update_fields=["enable_dkim", "dkim_public_key"]) with LogCapture("modoboa.dns"): - management.call_command("modo", "check_mx", "--no-dnsbl", "--ttl=0") + DNSChecker().run(self.domain, ttl=0) self.assertIsNot(self.domain.spf_record, None) self.assertIsNot(self.domain.dkim_record, None) diff --git a/pyproject.toml b/pyproject.toml index 42ef78220..6dd12054f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,7 +48,6 @@ dependencies = [ "dnspython==2.8.0", "feedparser==6.0.12", "fido2==2.0.0", - "gevent==25.9.1", "progressbar33==2.4", "python-dateutil", "cryptography", diff --git a/test_project/test_project/cron_config.py b/test_project/test_project/cron_config.py index dd88463ee..0e3ee92e2 100644 --- a/test_project/test_project/cron_config.py +++ b/test_project/test_project/cron_config.py @@ -23,6 +23,7 @@ register( ) register(admin_jobs.handle_mailbox_operations, queue_name="dovecot", cron="* * * * *") +register(admin_jobs.handle_dns_checks, queue_name="modoboa", cron="* * * * *") register(maillog_jobs.logparser, queue_name="modoboa", cron="*/5 * * * *") register(maillog_jobs.update_statistics, queue_name="modoboa", cron="0 * * * *") @@ -30,5 +31,5 @@ register(maillog_jobs.update_statistics, queue_name="modoboa", cron="0 * * * *") register(calendars_jobs.generate_rights, queue_name="modoboa", cron="*/2 * * * *") if "modoboa.amavis" in settings.MODOBOA_APPS: - register(amavis_jobs.qcleanup, queue="modoboa", cron="0 0 * * *") - register(amavis_jobs.amnotify, queue="modoboa", cron="0 12 * * *") + register(amavis_jobs.qcleanup, queue_name="modoboa", cron="0 0 * * *") + register(amavis_jobs.amnotify, queue_name="modoboa", cron="0 12 * * *")