mirror of
https://github.com/suitenumerique/messages.git
synced 2026-08-17 21:25:41 +02:00
🐛(dns) raise new "DUPLICATE" error when there are 2 SPF records
This commit is contained in:
@@ -55,6 +55,7 @@ class Command(BaseCommand):
|
||||
status_emoji = {
|
||||
"correct": "🟢",
|
||||
"incorrect": "🟡",
|
||||
"duplicate": "🔴",
|
||||
"missing": "🔴",
|
||||
"error": "⚠️",
|
||||
}
|
||||
@@ -67,6 +68,8 @@ class Command(BaseCommand):
|
||||
line += f" — Value: {record['value']}"
|
||||
elif status == "incorrect":
|
||||
line += f" — Expected: {record['value']} | Found: {', '.join(record['_check'].get('found', []))}"
|
||||
elif status == "duplicate":
|
||||
line += f" — Multiple SPF records found: {', '.join(record['_check'].get('found', []))}"
|
||||
elif status == "missing":
|
||||
line += f" — Expected: {record['value']} | Error: {record['_check'].get('error', '')}"
|
||||
elif status == "error":
|
||||
|
||||
@@ -64,6 +64,13 @@ def check_single_record(
|
||||
answers = dns.resolver.resolve(query_name, record_type)
|
||||
found_values = [answer.to_text() for answer in answers]
|
||||
|
||||
# For SPF records, check for duplicates (multiple "v=spf1" TXT records
|
||||
# is invalid per RFC 7208 and causes delivery issues)
|
||||
if record_type.upper() == "TXT" and expected_value.startswith("v=spf1"):
|
||||
spf_records = [v for v in found_values if v.startswith("v=spf1")]
|
||||
if len(spf_records) > 1:
|
||||
return {"status": "duplicate", "found": found_values}
|
||||
|
||||
# Check if expected value is in found values
|
||||
if expected_value in found_values:
|
||||
return {"status": "correct", "found": found_values}
|
||||
|
||||
@@ -314,6 +314,104 @@ class TestDNSChecking:
|
||||
assert results[1]["_check"]["status"] == "incorrect"
|
||||
assert results[2]["_check"]["status"] == "missing"
|
||||
|
||||
def test_check_single_record_spf_duplicate(self, maildomain_factory):
|
||||
"""Test that duplicate SPF records are detected.
|
||||
|
||||
Per RFC 7208, a domain must not have multiple SPF records.
|
||||
Example in the wild: saint-sozy.fr has both
|
||||
"v=spf1 include:_spf.mail.suite.anct.gouv.fr -all"
|
||||
"v=spf1 include:_spf.legacy-provider.com ~all"
|
||||
"""
|
||||
maildomain = maildomain_factory(name="example.com")
|
||||
expected_record = {
|
||||
"type": "TXT",
|
||||
"target": "",
|
||||
"value": "v=spf1 include:_spf.example.com -all",
|
||||
}
|
||||
|
||||
with patch("core.services.dns.check.dns.resolver.resolve") as mock_resolve:
|
||||
# Mock two SPF TXT records (invalid per RFC 7208)
|
||||
mock_answer1 = MagicMock()
|
||||
mock_answer1.to_text.return_value = '"v=spf1 include:_spf.example.com -all"'
|
||||
mock_answer2 = MagicMock()
|
||||
mock_answer2.to_text.return_value = (
|
||||
'"v=spf1 include:_spf.legacy-provider.com ~all"'
|
||||
)
|
||||
mock_resolve.return_value = [mock_answer1, mock_answer2]
|
||||
|
||||
result = check_single_record(maildomain, expected_record)
|
||||
|
||||
assert result["status"] == "duplicate"
|
||||
assert len(result["found"]) == 2
|
||||
assert "v=spf1 include:_spf.example.com -all" in result["found"]
|
||||
assert "v=spf1 include:_spf.legacy-provider.com ~all" in result["found"]
|
||||
|
||||
def test_check_single_record_spf_duplicate_even_if_correct_present(
|
||||
self, maildomain_factory
|
||||
):
|
||||
"""Test that duplicate SPF is reported even when the correct value is present."""
|
||||
maildomain = maildomain_factory(name="example.com")
|
||||
expected_record = {
|
||||
"type": "TXT",
|
||||
"target": "",
|
||||
"value": "v=spf1 include:_spf.example.com -all",
|
||||
}
|
||||
|
||||
with patch("core.services.dns.check.dns.resolver.resolve") as mock_resolve:
|
||||
mock_correct = MagicMock()
|
||||
mock_correct.to_text.return_value = '"v=spf1 include:_spf.example.com -all"'
|
||||
mock_legacy = MagicMock()
|
||||
mock_legacy.to_text.return_value = (
|
||||
'"v=spf1 include:_spf.legacy-provider.com ~all"'
|
||||
)
|
||||
mock_resolve.return_value = [mock_correct, mock_legacy]
|
||||
|
||||
result = check_single_record(maildomain, expected_record)
|
||||
|
||||
# Should be duplicate, NOT correct
|
||||
assert result["status"] == "duplicate"
|
||||
|
||||
def test_check_single_record_spf_single_is_not_duplicate(self, maildomain_factory):
|
||||
"""Test that a single SPF record is not flagged as duplicate."""
|
||||
maildomain = maildomain_factory(name="example.com")
|
||||
expected_record = {
|
||||
"type": "TXT",
|
||||
"target": "",
|
||||
"value": "v=spf1 include:_spf.example.com -all",
|
||||
}
|
||||
|
||||
with patch("core.services.dns.check.dns.resolver.resolve") as mock_resolve:
|
||||
mock_spf = MagicMock()
|
||||
mock_spf.to_text.return_value = '"v=spf1 include:_spf.example.com -all"'
|
||||
# Also has a non-SPF TXT record
|
||||
mock_other = MagicMock()
|
||||
mock_other.to_text.return_value = '"google-site-verification=abc123"'
|
||||
mock_resolve.return_value = [mock_spf, mock_other]
|
||||
|
||||
result = check_single_record(maildomain, expected_record)
|
||||
|
||||
assert result["status"] == "correct"
|
||||
|
||||
def test_check_single_record_dmarc_not_affected_by_spf_duplicate_check(
|
||||
self, maildomain_factory
|
||||
):
|
||||
"""Test that duplicate detection only applies to SPF, not other TXT records."""
|
||||
maildomain = maildomain_factory(name="example.com")
|
||||
expected_record = {
|
||||
"type": "TXT",
|
||||
"target": "_dmarc",
|
||||
"value": "v=DMARC1; p=reject; adkim=s; aspf=s;",
|
||||
}
|
||||
|
||||
with patch("core.services.dns.check.dns.resolver.resolve") as mock_resolve:
|
||||
mock_answer = MagicMock()
|
||||
mock_answer.to_text.return_value = '"v=DMARC1; p=reject; adkim=s; aspf=s;"'
|
||||
mock_resolve.return_value = [mock_answer]
|
||||
|
||||
result = check_single_record(maildomain, expected_record)
|
||||
|
||||
assert result["status"] == "correct"
|
||||
|
||||
def test_check_single_record_with_subdomain(self, maildomain_factory):
|
||||
"""Test checking a record for a subdomain."""
|
||||
maildomain = maildomain_factory(name="example.com")
|
||||
|
||||
@@ -177,6 +177,7 @@
|
||||
"Domain": "Domain",
|
||||
"Domain admin": "Domain admin",
|
||||
"Domain not found": "Domain not found",
|
||||
"Duplicate": "Duplicate",
|
||||
"Download": "Download",
|
||||
"Download invitation": "Download invitation",
|
||||
"Download raw email": "Download raw email",
|
||||
|
||||
@@ -179,6 +179,7 @@
|
||||
"Domain": "Domaine",
|
||||
"Domain admin": "Gestion des domaines",
|
||||
"Domain not found": "Domaine introuvable",
|
||||
"Duplicate": "Dupliqué",
|
||||
"Download": "Télécharger",
|
||||
"Download invitation": "Télécharger l'invitation",
|
||||
"Download raw email": "Télécharger l'email brut",
|
||||
|
||||
@@ -150,6 +150,7 @@
|
||||
"Domain": "Domein",
|
||||
"Domain admin": "Domein beheerder",
|
||||
"Domain not found": "Domein niet gevonden",
|
||||
"Duplicate": "Dubbel",
|
||||
"Download": "Download",
|
||||
"Download raw email": "Download raw email",
|
||||
"Draft": "Concept",
|
||||
|
||||
@@ -153,6 +153,7 @@
|
||||
"Domain": "Домен",
|
||||
"Domain admin": "Администратор домена",
|
||||
"Domain not found": "Домен не найден",
|
||||
"Duplicate": "Дублируется",
|
||||
"Download": "Загрузить",
|
||||
"Download raw email": "Загрузить необработанный e-mail",
|
||||
"Draft": "Черновик",
|
||||
|
||||
@@ -153,6 +153,7 @@
|
||||
"Domain": "Домен",
|
||||
"Domain admin": "Адміністратор домену",
|
||||
"Domain not found": "Домен не знайдено",
|
||||
"Duplicate": "Дублюється",
|
||||
"Download": "Завантажити",
|
||||
"Download raw email": "Завантажити сирий e-mail",
|
||||
"Draft": "Чернетка",
|
||||
|
||||
@@ -30,6 +30,8 @@ function AdminDNSDataGrid({ domain, dnsRecords, isLoading, error }: AdminDNSData
|
||||
return "success";
|
||||
case "incorrect":
|
||||
return "warning";
|
||||
case "duplicate":
|
||||
return "danger";
|
||||
case "missing":
|
||||
return "danger";
|
||||
default:
|
||||
@@ -43,6 +45,8 @@ function AdminDNSDataGrid({ domain, dnsRecords, isLoading, error }: AdminDNSData
|
||||
return t("Correct");
|
||||
case "incorrect":
|
||||
return t("Incorrect");
|
||||
case "duplicate":
|
||||
return t("Duplicate");
|
||||
case "missing":
|
||||
return t("Missing");
|
||||
default:
|
||||
|
||||
Reference in New Issue
Block a user