Skip to content

fix issue where whitelist always passes for email validation#46

Closed
johannestaas wants to merge 3 commits intopython-validators:masterfrom
johannestaas:master
Closed

fix issue where whitelist always passes for email validation#46
johannestaas wants to merge 3 commits intopython-validators:masterfrom
johannestaas:master

Conversation

@johannestaas
Copy link
Copy Markdown

So, it looks like the logic was off for the email validation and whitelist keyword. It'd always pass the whitelist if the domain was fine. check the top level if condition in the original code and you see that if it's not in whitelist and the domain_regex passes, it just skips over to return True.

if domain_part not in whitelist and not domain_regex.match(domain_part):

Testing patch:

In [2]: email('test@riskiq.net')
Out[2]: True

In [3]: email('test@riskiq.net', whitelist=['riskiq.net'])
Out[3]: True

In [4]: email('test@riskiq.net', whitelist=['riskiq.com'])
Out[4]: ValidationFailure(func=email, args={'value': 'test@riskiq.net', 'whitelist': ['riskiq.com']})

@johannestaas
Copy link
Copy Markdown
Author

Oh... I see what you were doing... I misunderstood what "whitelist" was supposed to be. It's a whitelist so domains can pass that don't match the regex like "localhost".

In that case, it might be nice to have another keyword argument that fails it if the email doesn't have a certain domain, like email('test@example.org', whitelist=['example.org'], only_whitelist=True)

I could edit this to match that logic if that's alright. My use case might be for passivetotal.org, where if someone wants to join the organization "passivetotal", it can check their email against their domain to validate they belong there. Org admins could automatically whitelist emails with domains from their org.

@johannestaas
Copy link
Copy Markdown
Author

johannestaas commented Jan 20, 2017

Alright, I implemented only_whitelist boolean that checks if the domain part is in the whitelist and only passes if so. Otherwise, it follows your old behavior. I added some unit tests for it as well, and everything seems to pass again. With email validation, it seems pretty common to want to check if an email is valid format and if it is from a specific set of domains, so this might be nice to have in here.

In [1]: from validators.email import email

In [2]: email('johan@example.org')
Out[2]: True

In [3]: email('johan@example', whitelist=['example'])
Out[3]: True

In [4]: email('johan@example.org', whitelist=['example.org'], only_whitelist=True)
Out[4]: True

In [5]: email('johan@example.org', whitelist=['example.net'], only_whitelist=True)
Out[5]: ValidationFailure(func=email, args={'value': 'johan@example.org', 'only_whitelist': True, 'whitelist': ['example.net']})

@nandgator nandgator added enhancement Issue/PR: A new feature outdated Issue/PR: Open for more than 3 months labels Mar 3, 2023
@nandgator
Copy link
Copy Markdown
Collaborator

nandgator commented Mar 14, 2023

ref #241 | Whitelisting will be considered later, the test cases (hopefully all) in this PR passed.

@nandgator nandgator closed this Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Issue/PR: A new feature outdated Issue/PR: Open for more than 3 months

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants