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

Allow email validation errors to be returned even when GENERIC_RESPON… #961

Merged
merged 1 commit into from
Mar 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@
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:

Check warning on line 176 in flask_security/forms.py

View check run for this annotation

Codecov / codecov/patch

flask_security/forms.py#L176

Added line #L176 was not covered by tests
# 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]

Check warning on line 179 in flask_security/forms.py

View check run for this annotation

Codecov / codecov/patch

flask_security/forms.py#L179

Added line #L179 was not covered by tests
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="mike@lp.com",
password="awesome sunset",
)
client = app.test_client()
response = client.post("/register", data=data)
assert b"No mikes allowed" in response.data
Loading