Skip to content

Commit

Permalink
Allow email validation errors to be returned even when GENERIC_RESPON…
Browse files Browse the repository at this point in the history
…SES is set to True. (#961)

We need to be careful not to expose whether an account already exists - but syntax or other errors should be returned.
This enables custom implementations of MailUtil to enforce their own conventions and return useful results to callers.

closes #942
  • Loading branch information
jwag956 authored Mar 23, 2024
1 parent fe1628a commit 7481d43
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ repos:
- id: pyupgrade
args: [--py38-plus]
- repo: https://github.com/psf/black
rev: 24.2.0
rev: 24.3.0
hooks:
- id: black
- repo: https://github.com/pycqa/flake8
Expand Down
3 changes: 2 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ Here you can see the full list of changes between each Flask-Security release.
Version 5.4.3
-------------

Released March xx, 2024
Released March 23, 2024

Fixes
+++++
- (:issue:`950`) Regression - some templates no longer getting correct config (thanks pete7863).
- (:issue:`954`) CSRF not properly ignored for application forms using SECURITY_CSRF_PROTECT_MECHANISMS.
- (:pr:`957`) Improve jp translations (e-goto)
- (:issue:`959`) Regression - datetime_factory should still be an attribute (thanks TimotheeJeannin)
- (:issue:`942`) GENERIC_RESPONSES hide email validation/syntax errors.

Version 5.4.2
-------------
Expand Down
2 changes: 2 additions & 0 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ Security() instantiation.
:members:
:special-members: __init__

.. autoclass:: flask_security.EmailValidateException

.. autoclass:: flask_security.PasswordUtil
:members:
:special-members: __init__
Expand Down
2 changes: 1 addition & 1 deletion flask_security/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
VerifyForm,
unique_identity_attribute,
)
from .mail_util import MailUtil
from .mail_util import MailUtil, EmailValidateException
from .oauth_glue import OAuthGlue
from .oauth_provider import FsOAuthProvider
from .password_util import PasswordUtil
Expand Down
2 changes: 1 addition & 1 deletion flask_security/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ def unauth_csrf(
.. versionadded:: 3.3.0
.. versionchanged:: 5.4.2
.. versionchanged:: 5.4.3
The fall_through parameter is now ignored.
Add code to properly handle JSON errors.
"""
Expand Down
13 changes: 9 additions & 4 deletions flask_security/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

from .babel import is_lazy_string, make_lazy_string
from .confirmable import requires_confirmation
from .mail_util import EmailValidateException
from .proxies import _security
from .utils import (
_,
Expand Down Expand Up @@ -150,8 +151,8 @@ class EmailValidation:
N.B. Side-effect - if valid email, the field.data is set to the normalized value.
The 'verify' keyword informs the validator to perform checks to be more sure
that the email can actually receive an email. Set to False - validation is done
to normalize and use for identity purposes.
that the email can actually receive an email (as well as normalize).
Set to False - just normalize (for use with identity purposes).
"""

def __init__(self, *args, **kwargs):
Expand All @@ -166,12 +167,16 @@ def __call__(self, form, field):
field.data = _security._mail_util.validate(field.data)
else:
field.data = _security._mail_util.normalize(field.data)
except ValueError:
msg = get_message("INVALID_EMAIL_ADDRESS")[0]
except EmailValidateException as e:
# we stop further validators if email isn't valid.
# TODO: email_validator provides some really nice error messages - however
# they aren't localized. And there isn't an easy way to add multiple
# errors at once.
raise StopValidation(e.msg)
except ValueError:
# Backwards compat - mail_util no longer raises this - but app subclasses
# might (and we're making this change in 5.4.3).
msg = get_message("INVALID_EMAIL_ADDRESS")[0]
raise StopValidation(msg)


Expand Down
37 changes: 25 additions & 12 deletions flask_security/mail_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,22 @@
import email_validator
from flask import current_app

from .utils import config_value
from .utils import config_value, get_message

if t.TYPE_CHECKING: # pragma: no cover
import flask


class EmailValidateException(ValueError):
"""This is raised for any email validation errors.
This can be used by custom MailUtil implementations to provide
custom error messages.
"""

def __init__(self, message: str) -> None:
self.msg = message


class MailUtil:
"""
Utility class providing methods for validating, normalizing and sending emails.
Expand Down Expand Up @@ -114,24 +124,24 @@ def send_mail(

def normalize(self, email: str) -> str:
"""
Given an input email - return a normalized version or raises ValueError
if field value isn't syntactically valid.
Given an input email - return a normalized version or
raise EmailValidateException if field value isn't syntactically valid.
This is called for forms that use email as an identity to be looked up.
This is called by forms that use email as an identity to be looked up.
Must be called in app context and uses :py:data:`SECURITY_EMAIL_VALIDATOR_ARGS`
config variable to pass any relevant arguments to
email_validator.validate_email() method.
This defaults to NOT checking for deliverability (i.e. DNS checks).
Will throw email_validator.EmailNotValidError (ValueError)
if email isn't syntactically valid.
"""
validator_args = config_value("EMAIL_VALIDATOR_ARGS") or {}
validator_args["check_deliverability"] = False
valid = email_validator.validate_email(email, **validator_args)
return valid.normalized
try:
valid = email_validator.validate_email(email, **validator_args)
return valid.normalized
except ValueError:
raise EmailValidateException(get_message("INVALID_EMAIL_ADDRESS")[0])

def validate(self, email: str) -> str:
"""
Expand All @@ -144,9 +154,12 @@ def validate(self, email: str) -> str:
config variable to pass any relevant arguments to
email_validator.validate_email() method.
ValueError is thrown if not valid.
EmailValidationException is thrown on invalid email.
"""

validator_args = config_value("EMAIL_VALIDATOR_ARGS") or {}
valid = email_validator.validate_email(email, **validator_args)
return valid.normalized
try:
valid = email_validator.validate_email(email, **validator_args)
return valid.normalized
except ValueError:
raise EmailValidateException(get_message("INVALID_EMAIL_ADDRESS")[0])
12 changes: 9 additions & 3 deletions flask_security/registerable.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ def register_existing(form: ConfirmRegisterForm) -> bool:
issuing requests. One way to mitigate that is to use signals and add specific
application code.
Returning False means to return normal error messages.
Returns True if the only 'error' is an existing email/user. In this case we
simulate a normal registration and email the existing account to inform.
"""

if not (
Expand All @@ -100,12 +104,14 @@ def register_existing(form: ConfirmRegisterForm) -> bool:
return False

# There are 2 classes of error - an existing email/username and non-compliant
# username/password. We want to give the user feedback on a non-compliant
# username/password - but not give away whether the email/username is already taken.
# email/username/password. We want to give the user feedback on a non-compliant
# input - but not give away whether the email/username is already taken.
# Since in this case we have an 'existing' entry - we simply Null out those
# errors.
# This also means for JSON there is no way to tell if things worked or not.
fields_to_squash: dict[str, dict[str, str]] = dict(email=dict())
fields_to_squash: dict[str, dict[str, str]] = dict()
if form.existing_email_user:
fields_to_squash["email"] = dict()
if hasattr(form, "username") and form.existing_username_user:
fields_to_squash["username"] = dict()
form_errors_munge(form, fields_to_squash)
Expand Down
25 changes: 25 additions & 0 deletions tests/test_registerable.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
authenticate,
check_xlation,
get_form_input,
init_app_with_options,
json_authenticate,
logout,
)
Expand Down Expand Up @@ -931,3 +932,27 @@ class MyRegisterForm(NewPasswordFormMixinEx, ConfirmRegisterForm):
)
assert response.status_code == 400
assert "Really - don't start" in response.json["response"]["errors"][0]


@pytest.mark.confirmable()
@pytest.mark.settings(return_generic_responses=True)
def test_my_mail_util(app, sqlalchemy_datastore):
# Test that with generic_responses - we still can get syntax/validation errors.
from flask_security import MailUtil, EmailValidateException

class MyMailUtil(MailUtil):
def validate(self, email):
if email.startswith("mike"):
raise EmailValidateException("No mikes allowed")

init_app_with_options(
app, sqlalchemy_datastore, **{"security_args": {"mail_util_cls": MyMailUtil}}
)

data = dict(
email="[email protected]",
password="awesome sunset",
)
client = app.test_client()
response = client.post("/register", data=data)
assert b"No mikes allowed" in response.data

0 comments on commit 7481d43

Please sign in to comment.