Skip to content

Commit

Permalink
Fix #806 - Login shouldn't attempt email delivery validation. (#812)
Browse files Browse the repository at this point in the history
Enpoints that need to actually send email - such as registration, forgot, confirmation continue to use the email_validator that by default checks for proper syntax as well as deliverability. /login no longer does - it just checks for syntax.

- Removed a reference to Flask-Mongoengine in docs.
- Document API AsaList()
- remove pydantic dependency since webauthn has fixed it on their end.
- bump package dependency requirements for extras.

closes #806
  • Loading branch information
jwag956 authored Jul 6, 2023
1 parent 03ca4d1 commit 08a2dbd
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Fixes
- (:pr:`809`) Fix MongoDB support by eliminating dependency on flask-mongoengine.
Improve MongoDB quickstart.
- (:issue:`801`) Fix Quickstart for SQLAlchemy with scoped session.
- (:issue:`806`) Login no longer, by default, check for email deliverability.

Backwards Compatibility Concerns
+++++++++++++++++++++++++++++++++
Expand Down
2 changes: 2 additions & 0 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ Utils

.. autofunction:: flask_security.tf_send_security_token

.. autoclass:: flask_security.AsaList

.. autoclass:: flask_security.SmsSenderBaseClass
:members: send_sms

Expand Down
9 changes: 7 additions & 2 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,16 @@ These configuration keys are used globally across all features.

.. py:data:: SECURITY_EMAIL_VALIDATOR_ARGS
Email address are validated using the `email_validator`_ package. Its methods
have some configurable options - these can be set here and will be passed in.
Email address are validated and normalized via the ``mail_util_cls`` which
defaults to :class:`.MailUtil`. That uses the `email_validator`_ package whose methods
have configurable options - these can be set here and will be passed in.
For example setting this to: ``{"check_deliverability": False}`` is useful
when unit testing if the emails are fake.

``mail_util_cls`` has 2 methods - ``normalize`` and ``validate``. Both
ensure the passed value is a valid email address, and returns a normalized
version. ``validate`` additionally, by default, verifies that the email
address can likely actually receive an email.

Default: ``None``, meaning use the defaults from email_validator package.

Expand Down
4 changes: 2 additions & 2 deletions docs/installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ Your application will also need a database backend:
* Sqlite is supported out of the box.
* For PostgreSQL install `psycopg2`_.
* For MySQL install `pymysql`_.
* For MongoDB install `Flask-Mongoengine`_.
* For MongoDB install `Mongoengine`_.

For additional details on configuring your database engine connector - refer to `sqlalchemy_engine`_

.. _psycopg2: https://pypi.org/project/psycopg2/
.. _pymysql: https://pypi.org/project/PyMySQL/
.. _Flask-Mongoengine: https://pypi.org/project/flask-mongoengine/
.. _Mongoengine: https://pypi.org/project/mongoengine/
.. _sqlalchemy_engine: https://docs.sqlalchemy.org/en/14/core/engines.html
13 changes: 8 additions & 5 deletions docs/models.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,24 @@ Additional Functionality
------------------------

Depending on the application's configuration, additional fields may need to be
added to your `User` model.
added to your database models. Note some fields are specified as 'list of string'
the ORM you are using is responsible for translating the list of string to a suitable
DB data type. For standard SQL-like databases, Flask-Security provides a utility
method :class:`.AsaList`.

Confirmable
^^^^^^^^^^^

If you enable account confirmation by setting your application's
`SECURITY_CONFIRMABLE` configuration value to `True`, your `User` model will
:py:data:`SECURITY_CONFIRMABLE` configuration value to `True`, your `User` model will
require the following additional field:

* ``confirmed_at`` (datetime)

Trackable
^^^^^^^^^

If you enable user tracking by setting your application's `SECURITY_TRACKABLE`
If you enable user tracking by setting your application's :py:data:`SECURITY_TRACKABLE`
configuration value to `True`, your `User` model will require the following
additional fields:

Expand All @@ -76,14 +79,14 @@ additional fields:
Two_Factor
^^^^^^^^^^

If you enable two-factor by setting your application's `SECURITY_TWO_FACTOR`
If you enable two-factor by setting your application's :py:data:`SECURITY_TWO_FACTOR`
configuration value to `True`, your `User` model will require the following
additional fields:

* ``tf_totp_secret`` (string, 255 bytes, nullable)
* ``tf_primary_method`` (string)

If you include 'sms' in `SECURITY_TWO_FACTOR_ENABLED_METHODS`, your `User` model
If you include 'sms' in :py:data:`SECURITY_TWO_FACTOR_ENABLED_METHODS`, your `User` model
will require the following additional field:

* ``tf_phone_number`` (string, 128 bytes, nullable)
Expand Down
18 changes: 16 additions & 2 deletions flask_security/datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,14 @@ def delete(self, model):
import sqlalchemy.types as types

class AsaList(types.TypeDecorator):
# SQL-like DBs don't have a List type - so do that here by converting to a comma
# separate string.
"""
SQL-like DBs don't have a List type - so do that here by converting to a comma
separate string.
For SQLAlchemy-based datastores, this can be used as::
Column(MutableList.as_mutable(AsaList()), nullable=True)
"""

impl = types.UnicodeText

def process_bind_param(self, value, dialect):
Expand All @@ -58,6 +64,14 @@ def process_result_value(self, value, dialect):
except ImportError: # pragma: no cover

class AsaList: # type: ignore
"""
SQL-like DBs don't have a List type - so do that here by converting to a comma
separate string.
For SQLAlchemy-based datastores, this can be used as::
Column(MutableList.as_mutable(AsaList()), nullable=True)
"""

pass


Expand Down
29 changes: 22 additions & 7 deletions flask_security/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
:copyright: (c) 2012 by Matt Wright.
:copyright: (c) 2017 by CERN.
:copyright: (c) 2019-2022 by J. Christopher Wagner (jwag).
:copyright: (c) 2019-2023 by J. Christopher Wagner (jwag).
:license: MIT, see LICENSE for more details.
"""

Expand Down Expand Up @@ -134,14 +134,24 @@ class Length(ValidatorMixin, validators.Length):
class EmailValidation:
"""Simple interface to email_validator.
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.
"""

def __init__(self, *args, **kwargs):
self.verify = kwargs.get("verify", False)

def __call__(self, form, field):
if field.data is None: # pragma: no cover
raise ValidationError(get_message("EMAIL_NOT_PROVIDED")[0])

try:
field.data = _security._mail_util.validate(field.data)
if self.verify:
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]
# we stop further validators if email isn't valid.
Expand All @@ -152,7 +162,6 @@ def __call__(self, form, field):


email_required = Required(message="EMAIL_NOT_PROVIDED")
email_validator = EmailValidation()
password_required = Required(message="PASSWORD_NOT_PROVIDED")


Expand Down Expand Up @@ -288,15 +297,15 @@ class UserEmailFormMixin:
email = EmailField(
get_form_field_label("email"),
render_kw={"autocomplete": "email"},
validators=[email_required, email_validator, valid_user_email],
validators=[email_required, EmailValidation(verify=True), valid_user_email],
)


class UniqueEmailFormMixin:
email = EmailField(
get_form_field_label("email"),
render_kw={"autocomplete": "email"},
validators=[email_required, email_validator, unique_user_email],
validators=[email_required, EmailValidation(verify=True), unique_user_email],
)


Expand Down Expand Up @@ -447,9 +456,15 @@ def validate(self, **kwargs: t.Any) -> bool:
return True


class PasswordlessLoginForm(Form, UserEmailFormMixin):
class PasswordlessLoginForm(Form):
"""The passwordless login form"""

email = EmailField(
get_form_field_label("email"),
render_kw={"autocomplete": "email"},
validators=[email_required, EmailValidation(verify=False), valid_user_email],
)

submit = SubmitField(get_form_field_label("send_login_link"))

def __init__(self, *args: t.Any, **kwargs: t.Any):
Expand All @@ -474,7 +489,7 @@ class LoginForm(Form, PasswordFormMixin, NextFormMixin):
email = EmailField(
get_form_field_label("email"),
render_kw={"autocomplete": "email"},
validators=[Optional(), email_validator],
validators=[Optional(), EmailValidation(verify=False)],
)

# username is added dynamically based on USERNAME_ENABLED.
Expand Down
23 changes: 19 additions & 4 deletions flask_security/mail_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
Utility class providing methods for validating, normalizing and sending emails.
:copyright: (c) 2020-2022 by J. Christopher Wagner (jwag).
:copyright: (c) 2020-2023 by J. Christopher Wagner (jwag).
:license: MIT, see LICENSE for more details.
While this default implementation uses Flask-Mailman - we want to make sure that
Expand Down Expand Up @@ -111,21 +111,36 @@ def send_mail(

def normalize(self, email: str) -> str:
"""
Given an input email - return a normalized version.
Given an input email - return a normalized version or raises ValueError
if field value isn't syntactically valid.
This is called for 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.
Will throw email_validator.EmailNotValidError if email isn't even valid.
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 = config_value("EMAIL_VALIDATOR_ARGS") or {
"check_deliverability": False
}
valid = email_validator.validate_email(email, **validator_args)
return valid.email

def validate(self, email: str) -> str:
"""
Validate the given email.
If valid, the normalized version is returned.
This is used by forms/views that require an email that likely can have an
actual email sent to it.
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.
ValueError is thrown if not valid.
"""
Expand Down
1 change: 0 additions & 1 deletion requirements/tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,5 @@ requests
sqlalchemy
sqlalchemy-utils
webauthn
pydantic<2.0
werkzeug
zxcvbn
7 changes: 1 addition & 6 deletions requirements_low/tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,11 @@ mongoengine==0.27.0
mongomock==4.1.2
pony==0.7.16;python_version<'3.11'
phonenumberslite==8.13.11
# issue with pyopenssl requiring very new cryptography
pyopenssl==23.1.1
qrcode==7.4.2
# authlib requires requests
requests
sqlalchemy==2.0.12
sqlalchemy-utils==0.41.1
# These 2 come from webauthn requirements
cryptography==40.0.2
webauthn==1.8.0;python_version>='3.8'
pydantic<2.0
webauthn==1.9.0
werkzeug==2.3.3
zxcvbn==4.4.28
10 changes: 5 additions & 5 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ babel=
babel>=2.12.1
flask_babel>=3.1.0
fsqla=
flask_sqlalchemy>=3.0.2
sqlalchemy>=1.4.35
sqlalchemy-utils>=0.38.3
flask_sqlalchemy>=3.0.3
sqlalchemy>=2.0.12
sqlalchemy-utils>=0.41.1
common=
bcrypt>=4.0.1
flask_mailman>=0.3.0
Expand All @@ -14,7 +14,7 @@ mfa=
cryptography>=40.0.2
qrcode>=7.4.2
phonenumberslite>=8.13.11
webauthn>=1.8.0
webauthn>=1.9.0

[aliases]
test=pytest
Expand All @@ -28,7 +28,7 @@ domain = flask_security

[extract_messages]
project= Flask-Security
version=5.2.0
version=5.3.0
msgid_bugs_address = jwag956@github.com
mapping-file = babel.ini
output-file = flask_security/translations/flask_security.pot
Expand Down
29 changes: 27 additions & 2 deletions tests/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
ChangePasswordForm,
ConfirmRegisterForm,
EmailField,
EmailValidation,
ForgotPasswordForm,
LoginForm,
PasswordField,
Expand All @@ -52,7 +53,6 @@
SendConfirmationForm,
StringField,
email_required,
email_validator,
valid_user_email,
)
from flask_security import auth_required, roles_required
Expand Down Expand Up @@ -124,7 +124,7 @@ class MyRegisterForm(RegisterForm):
class MyForgotPasswordForm(ForgotPasswordForm):
email = EmailField(
"My Forgot Email Address Field",
validators=[email_required, email_validator, valid_user_email],
validators=[email_required, EmailValidation(verify=True), valid_user_email],
)

class MyResetPasswordForm(ResetPasswordForm):
Expand Down Expand Up @@ -1419,3 +1419,28 @@ def test_multi_app(app, sqlalchemy_datastore):

assert hasattr(security2.forms["register_form"].cls, "username")
assert "username" in security2.user_identity_attributes[1].keys()


@pytest.mark.registerable()
def test_login_email_whatever(app, client, get_message):
# login, by default, shouldn't verify email address is deliverable..
# register etc can/should do that.
app.config["SECURITY_EMAIL_VALIDATOR_ARGS"] = {"check_deliverability": True}

# register should fail since non-deliverable TLD
data = dict(
email="dude@me.mytld",
password="awesome sunset",
)
response = client.post("/register", json=data)
assert response.status_code == 400
assert response.json["response"]["errors"][0].encode("utf-8") == get_message(
"INVALID_EMAIL_ADDRESS"
)

# login should work since we are just checking for identity
response = client.post(
"/login", data=dict(email="matt@lp.com", password="password")
)
assert response.status_code == 302
assert "/" in response.location

0 comments on commit 08a2dbd

Please sign in to comment.