diff --git a/app/config.py b/app/config.py index 1b4434333..6425091d5 100644 --- a/app/config.py +++ b/app/config.py @@ -409,6 +409,8 @@ def facebook_enabled(): ALERT_MAILBOX_IS_ALIAS = "mailbox_is_alias" AlERT_WRONG_MX_RECORD_CUSTOM_DOMAIN = "custom_domain_mx_record_issue" +ALERT_WRONG_DKIM_RECORD_CUSTOM_DOMAIN = "custom_domain_dkim_record_issue" +ALERT_WRONG_DMARC_RECORD_CUSTOM_DOMAIN = "custom_domain_dmarc_record_issue" # alert when a new alias is about to be created on a disabled directory ALERT_DIRECTORY_DISABLED_ALIAS_CREATION = "alert_directory_disabled_alias_creation" diff --git a/app/models.py b/app/models.py index 3d1de8c60..5ecf5a8de 100644 --- a/app/models.py +++ b/app/models.py @@ -2618,13 +2618,24 @@ class CustomDomain(Base, ModelMixin): sa.Boolean, nullable=False, default=False, server_default="0" ) - # incremented when a check is failed on the domain + # incremented when the MX check is failed on the domain # alert when the number exceeds a threshold # used in check_custom_domain() nb_failed_checks = sa.Column( sa.Integer, default=0, server_default="0", nullable=False ) + dkim_nb_failed_checks = sa.Column( + sa.Integer, default=0, server_default="0", nullable=False + ) + dkim_nb_failed_checks_updated_at = sa.Column(ArrowType, default=None, nullable=True) + dmarc_nb_failed_checks = sa.Column( + sa.Integer, default=0, server_default="0", nullable=False + ) + dmarc_nb_failed_checks_updated_at = sa.Column( + ArrowType, default=None, nullable=True + ) + # only domain has the ownership verified can go the next DNS step # MX verified domains before this change don't have to do the TXT check # and therefore have ownership_verified=True diff --git a/migrations/versions/2026_090412_b7c1a9d3e2f4_add_dkim_dmarc_debounce_columns.py b/migrations/versions/2026_090412_b7c1a9d3e2f4_add_dkim_dmarc_debounce_columns.py new file mode 100644 index 000000000..b902a249e --- /dev/null +++ b/migrations/versions/2026_090412_b7c1a9d3e2f4_add_dkim_dmarc_debounce_columns.py @@ -0,0 +1,55 @@ +"""Add independent DKIM/DMARC debounce columns to custom_domain + +Revision ID: b7c1a9d3e2f4 +Revises: 4a9f8c2e1b3d +Create Date: 2026-09-04 12:00:00.000000 + +""" +import sqlalchemy_utils +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "b7c1a9d3e2f4" +down_revision = "4a9f8c2e1b3d" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "custom_domain", + sa.Column( + "dkim_nb_failed_checks", sa.Integer(), server_default="0", nullable=False + ), + ) + op.add_column( + "custom_domain", + sa.Column( + "dkim_nb_failed_checks_updated_at", + sqlalchemy_utils.types.arrow.ArrowType(), + nullable=True, + ), + ) + op.add_column( + "custom_domain", + sa.Column( + "dmarc_nb_failed_checks", sa.Integer(), server_default="0", nullable=False + ), + ) + op.add_column( + "custom_domain", + sa.Column( + "dmarc_nb_failed_checks_updated_at", + sqlalchemy_utils.types.arrow.ArrowType(), + nullable=True, + ), + ) + + +def downgrade(): + op.drop_column("custom_domain", "dmarc_nb_failed_checks_updated_at") + op.drop_column("custom_domain", "dmarc_nb_failed_checks") + op.drop_column("custom_domain", "dkim_nb_failed_checks_updated_at") + op.drop_column("custom_domain", "dkim_nb_failed_checks") diff --git a/tasks/check_custom_domains.py b/tasks/check_custom_domains.py index 4d86cbcd0..afcde989a 100644 --- a/tasks/check_custom_domains.py +++ b/tasks/check_custom_domains.py @@ -1,3 +1,5 @@ +from dataclasses import dataclass + import arrow from sqlalchemy.orm.exc import ObjectDeletedError @@ -6,8 +8,38 @@ from app.db import Session from app.dns_utils import get_mx_domains from app.email_utils import send_email_with_rate_control, render +from app.errors import ProtonPartnerNotSetUp from app.log import LOG from app.models import CustomDomain, Alias +from app.proton.proton_partner import get_proton_partner + + +@dataclass +class RecordAlertConfig: + record_name: str + alert_type: str + template: str + subject_infix: str + + +MX_ALERT = RecordAlertConfig( + record_name="MX", + alert_type=config.AlERT_WRONG_MX_RECORD_CUSTOM_DOMAIN, + template="transactional/custom-domain-dns-issue.txt.jinja2", + subject_infix="", +) +DKIM_ALERT = RecordAlertConfig( + record_name="DKIM", + alert_type=config.ALERT_WRONG_DKIM_RECORD_CUSTOM_DOMAIN, + template="transactional/custom-domain-dkim-issue.txt.jinja2", + subject_infix="DKIM ", +) +DMARC_ALERT = RecordAlertConfig( + record_name="DMARC", + alert_type=config.ALERT_WRONG_DMARC_RECORD_CUSTOM_DOMAIN, + template="transactional/custom-domain-dmarc-issue.txt.jinja2", + subject_infix="DMARC ", +) def check_all_custom_domains(): @@ -54,9 +86,45 @@ def check_all_custom_domains(): Session.close() +def _send_alert( + custom_domain: CustomDomain, + user, + domain_dns_url: str, + provider: str, + cfg: RecordAlertConfig, +): + LOG.w( + "Alert domain %s check fails %s about %s", cfg.record_name, user, custom_domain + ) + send_email_with_rate_control( + user, + cfg.alert_type, + user.email, + f"Please update {custom_domain.domain} {cfg.subject_infix}DNS on {provider}", + render( + cfg.template, + user=user, + custom_domain=custom_domain, + domain_dns_url=domain_dns_url, + ), + max_nb_alert=1, + nb_day=30, + retries=3, + ) + + def check_single_custom_domain(custom_domain: CustomDomain): + if custom_domain.is_sl_subdomain: + return if custom_domain.user.disabled: return + user = custom_domain.user + # snapshot before validate_dkim_records()/validate_dmarc_records() below can commit + # and bump these, which would otherwise throw off the once-a-day throttles + mx_last_updated_at = custom_domain.updated_at + dkim_last_updated_at = custom_domain.dkim_nb_failed_checks_updated_at + dmarc_last_updated_at = custom_domain.dmarc_nb_failed_checks_updated_at + mx_domains = get_mx_domains(custom_domain.domain) validator = CustomDomainValidation( dkim_domain=config.EMAIL_DOMAIN, @@ -64,50 +132,81 @@ def check_single_custom_domain(custom_domain: CustomDomain): partner_domains_validation_prefixes=config.PARTNER_CUSTOM_DOMAIN_VALIDATION_PREFIXES, ) expected_custom_domains = validator.get_expected_mx_records(custom_domain) - if not is_mx_equivalent(mx_domains, expected_custom_domains): - user = custom_domain.user + mx_ok = is_mx_equivalent(mx_domains, expected_custom_domains) + + dkim_errors = validator.validate_dkim_records(custom_domain) + dkim_ok = len(dkim_errors) == 0 + dmarc_ok = validator.validate_dmarc_records(custom_domain).success + + domain_dns_url = f"{config.URL}/dashboard/domains/{custom_domain.id}/dns" + try: + is_proton_domain = custom_domain.partner_id == get_proton_partner().id + except ProtonPartnerNotSetUp: + is_proton_domain = False + provider = "Proton" if is_proton_domain else "SimpleLogin" + now = arrow.now() + + if mx_ok: + custom_domain.nb_failed_checks = 0 + else: LOG.w( - f"The MX record is not correctly set for domain {custom_domain} of user {user}. Got {mx_domains}. Retried {custom_domain.nb_failed_checks} days", + f"MX check failed for domain {custom_domain} of user {user}. " + f"Retried {custom_domain.nb_failed_checks} days", ) - - if ( - not custom_domain.updated_at - or custom_domain.updated_at <= arrow.now().shift(days=-1) - ): - # Only update it once a day + if not mx_last_updated_at or mx_last_updated_at <= now.shift(days=-1): custom_domain.nb_failed_checks += 1 - # send alert if fail for MAX_DOMAIN_CHECKS consecutive days if custom_domain.nb_failed_checks > config.MAX_DOMAIN_CHECKS: - domain_dns_url = f"{config.URL}/dashboard/domains/{custom_domain.id}/dns" - LOG.w("Alert domain MX check fails %s about %s", user, custom_domain) - send_email_with_rate_control( - user, - config.AlERT_WRONG_MX_RECORD_CUSTOM_DOMAIN, - user.email, - f"Please update {custom_domain.domain} DNS on SimpleLogin", - render( - "transactional/custom-domain-dns-issue.txt.jinja2", - user=user, - custom_domain=custom_domain, - domain_dns_url=domain_dns_url, - ), - max_nb_alert=1, - nb_day=30, - retries=3, - ) + _send_alert(custom_domain, user, domain_dns_url, provider, MX_ALERT) LOG.w( "De-verifying domain %s after %d failed MX checks", custom_domain, custom_domain.nb_failed_checks, ) - # reset domain custom_domain.verified = False - custom_domain.dkim_verified = False - custom_domain.dmarc_verified = False custom_domain.spf_verified = False custom_domain.nb_failed_checks = 0 + + if dkim_ok: + custom_domain.dkim_nb_failed_checks = 0 else: - # reset checks - custom_domain.nb_failed_checks = 0 + LOG.w( + f"DKIM check failed for domain {custom_domain} of user {user}. " + f"Retried {custom_domain.dkim_nb_failed_checks} days", + ) + if not dkim_last_updated_at or dkim_last_updated_at <= now.shift(days=-1): + custom_domain.dkim_nb_failed_checks += 1 + custom_domain.dkim_nb_failed_checks_updated_at = now + + if custom_domain.dkim_nb_failed_checks > config.MAX_DOMAIN_CHECKS: + _send_alert(custom_domain, user, domain_dns_url, provider, DKIM_ALERT) + LOG.w( + "Un-verifying DKIM for domain %s after %d failed checks", + custom_domain, + custom_domain.dkim_nb_failed_checks, + ) + custom_domain.dkim_verified = False + custom_domain.dkim_nb_failed_checks = 0 + + if dmarc_ok: + custom_domain.dmarc_nb_failed_checks = 0 + else: + LOG.w( + f"DMARC check failed for domain {custom_domain} of user {user}. " + f"Retried {custom_domain.dmarc_nb_failed_checks} days", + ) + if not dmarc_last_updated_at or dmarc_last_updated_at <= now.shift(days=-1): + custom_domain.dmarc_nb_failed_checks += 1 + custom_domain.dmarc_nb_failed_checks_updated_at = now + + if custom_domain.dmarc_nb_failed_checks > config.MAX_DOMAIN_CHECKS: + _send_alert(custom_domain, user, domain_dns_url, provider, DMARC_ALERT) + LOG.w( + "Un-verifying DMARC for domain %s after %d failed checks", + custom_domain, + custom_domain.dmarc_nb_failed_checks, + ) + custom_domain.dmarc_verified = False + custom_domain.dmarc_nb_failed_checks = 0 + Session.commit() diff --git a/templates/emails/transactional/custom-domain-dkim-issue.txt.jinja2 b/templates/emails/transactional/custom-domain-dkim-issue.txt.jinja2 new file mode 100644 index 000000000..f0f345574 --- /dev/null +++ b/templates/emails/transactional/custom-domain-dkim-issue.txt.jinja2 @@ -0,0 +1,9 @@ +{% extends "base.txt.jinja2" %} + +{% block content %} +We have detected that the DKIM records for your domain {{ custom_domain.domain }} are no longer correctly set up. + +To fix this, please update the DKIM CNAME records and re-run the DNS check at {{ domain_dns_url }}. + +Until this is fixed, emails sent from {{ custom_domain.domain }} may not be signed correctly and could be marked as spam. +{% endblock %} diff --git a/templates/emails/transactional/custom-domain-dmarc-issue.txt.jinja2 b/templates/emails/transactional/custom-domain-dmarc-issue.txt.jinja2 new file mode 100644 index 000000000..425276eed --- /dev/null +++ b/templates/emails/transactional/custom-domain-dmarc-issue.txt.jinja2 @@ -0,0 +1,7 @@ +{% extends "base.txt.jinja2" %} + +{% block content %} +We have detected that the DMARC record for your domain {{ custom_domain.domain }} is no longer correctly set up. + +To fix this, please update the DMARC TXT record and re-run the DNS check at {{ domain_dns_url }}. +{% endblock %} diff --git a/tests/tasks/test_check_custom_domain.py b/tests/tasks/test_check_custom_domain.py index d2eb0ad68..688aff1b8 100644 --- a/tests/tasks/test_check_custom_domain.py +++ b/tests/tasks/test_check_custom_domain.py @@ -1,13 +1,15 @@ import arrow from unittest.mock import patch +from app.custom_domain_validation import DomainValidationResult from app.db import Session from app.models import CustomDomain +from app.proton.proton_partner import get_proton_partner from tasks.check_custom_domains import ( check_all_custom_domains, check_single_custom_domain, ) -from tests.utils import create_new_user, random_string +from tests.utils import create_partner_linked_user, create_new_user, random_string def test_check_single_custom_domain_increments_failed_checks(flask_client): @@ -68,12 +70,182 @@ def test_check_single_custom_domain_resets_on_success(flask_client): ) with patch("tasks.check_custom_domains.get_mx_domains", return_value=[]), patch( "tasks.check_custom_domains.is_mx_equivalent", return_value=True + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dkim_records", + return_value=[], + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dmarc_records", + return_value=DomainValidationResult(success=True, errors=[]), ): check_single_custom_domain(custom_domain) assert custom_domain.nb_failed_checks == 0 assert custom_domain.verified is True +def test_check_single_custom_domain_dkim_failure_is_independent_of_mx(flask_client): + user = create_new_user() + custom_domain = CustomDomain.create( + user_id=user.id, + domain=random_string(), + verified=True, + dkim_verified=True, + dmarc_verified=True, + nb_failed_checks=0, + dkim_nb_failed_checks=0, + commit=True, + ) + custom_domain.dkim_nb_failed_checks_updated_at = arrow.now().shift(days=-2) + Session.commit() + + with patch("tasks.check_custom_domains.get_mx_domains", return_value=[]), patch( + "tasks.check_custom_domains.is_mx_equivalent", return_value=True + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dkim_records", + return_value=["bad dkim"], + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dmarc_records", + return_value=DomainValidationResult(success=True, errors=[]), + ), patch("tasks.check_custom_domains.send_email_with_rate_control"): + check_single_custom_domain(custom_domain) + + assert custom_domain.dkim_nb_failed_checks == 1 + assert custom_domain.nb_failed_checks == 0 + assert custom_domain.verified is True + assert custom_domain.dkim_verified is True + assert custom_domain.dmarc_verified is True + + +def test_check_single_custom_domain_dkim_deactivates_only_dkim_after_threshold( + flask_client, +): + user = create_new_user() + custom_domain = CustomDomain.create( + user_id=user.id, + domain=random_string(), + verified=True, + dkim_verified=True, + dmarc_verified=True, + spf_verified=True, + nb_failed_checks=0, + dkim_nb_failed_checks=4, + commit=True, + ) + custom_domain.dkim_nb_failed_checks_updated_at = arrow.now().shift(days=-2) + Session.commit() + + with patch("tasks.check_custom_domains.get_mx_domains", return_value=[]), patch( + "tasks.check_custom_domains.is_mx_equivalent", return_value=True + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dkim_records", + return_value=["bad dkim"], + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dmarc_records", + return_value=DomainValidationResult(success=True, errors=[]), + ), patch("tasks.check_custom_domains.send_email_with_rate_control"): + check_single_custom_domain(custom_domain) + + assert custom_domain.dkim_nb_failed_checks == 0 + assert custom_domain.dkim_verified is False + assert custom_domain.verified is True + assert custom_domain.spf_verified is True + assert custom_domain.dmarc_verified is True + assert custom_domain.nb_failed_checks == 0 + + +def test_check_single_custom_domain_dkim_throttled_within_a_day(flask_client): + user = create_new_user() + custom_domain = CustomDomain.create( + user_id=user.id, + domain=random_string(), + verified=True, + dkim_verified=True, + dmarc_verified=True, + nb_failed_checks=0, + dkim_nb_failed_checks=1, + commit=True, + ) + custom_domain.dkim_nb_failed_checks_updated_at = arrow.now() + Session.commit() + + with patch("tasks.check_custom_domains.get_mx_domains", return_value=[]), patch( + "tasks.check_custom_domains.is_mx_equivalent", return_value=True + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dkim_records", + return_value=["bad dkim"], + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dmarc_records", + return_value=DomainValidationResult(success=True, errors=[]), + ), patch("tasks.check_custom_domains.send_email_with_rate_control"): + check_single_custom_domain(custom_domain) + + assert custom_domain.dkim_nb_failed_checks == 1 + + +def test_check_single_custom_domain_provider_follows_domain_partner_not_user( + flask_client, +): + user, _ = create_partner_linked_user() + custom_domain = CustomDomain.create( + user_id=user.id, + domain=random_string(), + verified=True, + partner_id=None, + nb_failed_checks=4, + commit=True, + ) + custom_domain.updated_at = arrow.now().shift(days=-2) + Session.commit() + + with patch("tasks.check_custom_domains.get_mx_domains", return_value=[]), patch( + "tasks.check_custom_domains.is_mx_equivalent", return_value=False + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dkim_records", + return_value=[], + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dmarc_records", + return_value=DomainValidationResult(success=True, errors=[]), + ), patch( + "tasks.check_custom_domains.send_email_with_rate_control" + ) as send_email_mock: + check_single_custom_domain(custom_domain) + + subject = send_email_mock.call_args.args[3] + assert "SimpleLogin" in subject + assert "Proton" not in subject + + +def test_check_single_custom_domain_provider_uses_proton_when_domain_partner_is_proton( + flask_client, +): + user = create_new_user() + custom_domain = CustomDomain.create( + user_id=user.id, + domain=random_string(), + verified=True, + partner_id=get_proton_partner().id, + nb_failed_checks=4, + commit=True, + ) + custom_domain.updated_at = arrow.now().shift(days=-2) + Session.commit() + + with patch("tasks.check_custom_domains.get_mx_domains", return_value=[]), patch( + "tasks.check_custom_domains.is_mx_equivalent", return_value=False + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dkim_records", + return_value=[], + ), patch( + "app.custom_domain_validation.CustomDomainValidation.validate_dmarc_records", + return_value=DomainValidationResult(success=True, errors=[]), + ), patch( + "tasks.check_custom_domains.send_email_with_rate_control" + ) as send_email_mock: + check_single_custom_domain(custom_domain) + + subject = send_email_mock.call_args.args[3] + assert "Proton" in subject + + def test_check_custom_domain_deletes_old_domains(): user = create_new_user() now = arrow.utcnow()