Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: check MX records against prohibited domains #16596

Merged
merged 6 commits into from
Sep 12, 2024

Conversation

miketheman
Copy link
Member

Using the email-validator package, which is already part of wtforms's dependencies, check if the domain is actually deliverable prior to accepting the address.

Deliverability check produces the MX records in the response.
Use them to check against the Prohibited Domains lists.

Resolves: #16136

@miketheman miketheman requested a review from a team as a code owner August 29, 2024 16:15
@miketheman miketheman added security Security-related issues and pull requests malware-detection Issues related to automated malware detection. labels Aug 29, 2024
Copy link
Member

@ewdurbin ewdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried that this is going to have unintended side effects in combination with #16586.

For instance:

$ dig microsoft.com MX +short
10 microsoft-com.mail.protection.outlook.com.

And we have outlook.comon our prohibited domains list, which I think means we will bar @microsoft.com and other domains with outlook hosted mail from registering?

@ewdurbin
Copy link
Member

Yeah, it seems applying:

diff --git a/tests/unit/manage/test_forms.py b/tests/unit/manage/test_forms.py
index 6250fbb6e..22bfd934d 100644
--- a/tests/unit/manage/test_forms.py
+++ b/tests/unit/manage/test_forms.py
@@ -247,7 +247,7 @@ class TestAddEmailForm:
 
         Simulate that `mail.net` handles emails sent to `wutang.net`.
         """
-        mock_deliverability_info = {"mx": [(10, "mail.net")]}
+        mock_deliverability_info = {"mx": [(10, "microsoft-com.mail.protection.outlook.com.")]}
 
         def mock_function(*args, **kwargs):
             return mock_deliverability_info
@@ -257,12 +257,12 @@ class TestAddEmailForm:
             mock_function,
         )
 
-        prohibited_mx_domain = ProhibitedEmailDomain(domain="mail.net")
+        prohibited_mx_domain = ProhibitedEmailDomain(domain="outlook.com")
         db_request.db.add(prohibited_mx_domain)
 
         form = forms.AddEmailForm(
             request=db_request,
-            formdata=MultiDict({"email": "foo@wutang.net"}),
+            formdata=MultiDict({"email": "foo@microsoft.com"}),
             user_service=pretend.stub(find_userid_by_email=lambda _: None),
             user_id=pretend.stub(),
         )

still passes test (aka blocks the address).

@miketheman
Copy link
Member Author

Thanks for poking at it!
I think this is a use case that needs some more thought, so I'll convert to Draft for now.

@miketheman miketheman marked this pull request as draft August 29, 2024 22:18
@miketheman miketheman force-pushed the miketheman/16136-check-mx-domains branch from 8859b6f to 3afbd37 Compare September 9, 2024 19:31
We get `email-validator` from `wtforms`, but use it explicitly in our
own code, so declare it a direct dependency in case `wtforms` ever drops
it.

Refs: pypi#15631

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman miketheman force-pushed the miketheman/16136-check-mx-domains branch from 3afbd37 to 14daa50 Compare September 11, 2024 22:49
Using the `email-validator` package, which is already part of
`wtforms`'s dependencies, check if the domain is actually deliverable
prior to accepting the address.

Deliverability check produces the MX records in the response.
Use them to check against the Prohibited Domains lists.

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman miketheman force-pushed the miketheman/16136-check-mx-domains branch from 14daa50 to 777b356 Compare September 11, 2024 23:24
@miketheman miketheman marked this pull request as ready for review September 11, 2024 23:30
@miketheman miketheman requested review from ewdurbin and a team September 11, 2024 23:30
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman miketheman enabled auto-merge (squash) September 12, 2024 19:02
@miketheman miketheman merged commit 3c8a4b8 into pypi:main Sep 12, 2024
18 checks passed
@miketheman miketheman deleted the miketheman/16136-check-mx-domains branch September 12, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
malware-detection Issues related to automated malware detection. security Security-related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check Email MX domains against prohibited email list
2 participants