diff --git a/flask_security/core.py b/flask_security/core.py index 5739b4f6..2efd9ce1 100644 --- a/flask_security/core.py +++ b/flask_security/core.py @@ -56,7 +56,7 @@ VerifyForm, build_register_username_field, build_register_form, - login_username_field, + build_login_form, ) from .json import setup_json from .mail_util import MailUtil @@ -1562,6 +1562,12 @@ def init_app( ): # pragma: no cover raise ValueError("User model must contain fs_uniquifier as of 4.0.0") + if not cv("PASSWORD_REQUIRED", app=app) and not cv("UNIFIED_SIGNIN", app=app): + raise ValueError( + "SECURITY_PASSWORD_REQUIRED can only be set to False if" + " the SECURITY_UNIFIED_SIGNIN feature is enabled" + ) + # Check for pre-4.0 SECURITY_USER_IDENTITY_ATTRIBUTES format for uia in cv("USER_IDENTITY_ATTRIBUTES", app=app): # pragma: no cover if not isinstance(uia, dict): @@ -1655,9 +1661,10 @@ def init_app( fcls = self.forms["confirm_register_form"].cls if fcls and issubclass(fcls, RegisterFormMixin): fcls.username = build_register_username_field(app) - fcls = self.forms["login_form"].cls - if fcls and issubclass(fcls, LoginForm): - fcls.username = login_username_field + + fcls = self.forms["login_form"].cls + if fcls and issubclass(fcls, LoginForm): + build_login_form(app=app, fcls=fcls) # new unified RegisterForm fcls = self.forms["register_form"].cls diff --git a/flask_security/forms.py b/flask_security/forms.py index 24d36946..cf7b3768 100644 --- a/flask_security/forms.py +++ b/flask_security/forms.py @@ -6,7 +6,7 @@ :copyright: (c) 2012 by Matt Wright. :copyright: (c) 2017 by CERN. - :copyright: (c) 2019-2024 by J. Christopher Wagner (jwag). + :copyright: (c) 2019-2025 by J. Christopher Wagner (jwag). :license: MIT, see LICENSE for more details. """ @@ -113,7 +113,7 @@ class ValidatorMixin: def __init__(self, *args, **kwargs): # If the message is available from config[MSG_xx] then it can be xlated. - # Otherwise it will be used as is. + # Otherwise, it will be used as is. if "message" in kwargs: self._original_message = kwargs["message"] del kwargs["message"] @@ -193,7 +193,7 @@ def _local_xlate(text): def get_form_field_label(key): """This is called during import since form fields are declared as part of - class. Thus can't call 'localize_callback' until we need to actually + class. Thus, can't call 'localize_callback' until we need to actually translate/render form. """ return make_lazy_string(_local_xlate, _default_field_labels.get(key, "")) @@ -354,32 +354,6 @@ class PasswordConfirmFormMixin: ) -def build_password_field(is_confirm=False, autocomplete="new-password", app=None): - # Based on configuration return PasswordField with appropriate validators - # Note that length and breached validators are done as part of form. - validators = list() - render_kw = {"autocomplete": autocomplete} - if cv("PASSWORD_REQUIRED", app) or not cv("UNIFIED_SIGNIN", app): - validators.append(password_required) - - if is_confirm: - validators.append( - EqualToLocalize("password", message="RETYPE_PASSWORD_MISMATCH") - ) - return PasswordField( - label=get_form_field_label("retype_password"), - render_kw=render_kw, - validators=validators, - ) - - # normal password field - return PasswordField( - label=get_form_field_label("password"), - render_kw=render_kw, - validators=validators, - ) - - class NextFormMixin: next = HiddenField() @@ -418,13 +392,6 @@ def build_register_username_field(app): ) -login_username_field = StringField( - get_form_field_label("username"), - render_kw={"autocomplete": "username"}, - validators=[username_validator], -) - - class RegisterFormMixin: submit = SubmitField(get_form_field_label("register")) @@ -526,7 +493,7 @@ def validate(self, **kwargs: t.Any) -> bool: class LoginForm(Form, PasswordFormMixin, NextFormMixin): - """The default login form + """The default login form. The following fields are defined: * email @@ -534,18 +501,25 @@ class LoginForm(Form, PasswordFormMixin, NextFormMixin): * password * remember (checkbox) * next - - If a subclass wants to handle identity, it can set self.ifield to the - form field that it validated. That will cause the validation logic here around - identity to be skipped. The subclass must also set self.user to the found User. + * submit + + The following attributes might be useful for subclasses: + * ifield - If a subclass wants to handle identity, it can set self.ifield to the + form field that it validated. That will cause the validation logic here + around identity to be skipped. + The subclass must also set self.user to the found User. + * user - as stated above - if subclass does identity check it must set this + field. """ - # email field - we don't use valid_user_email since for login - # with username feature it is potentially optional. + # email field - we don't use valid_user_email since for login we must check + # user_identity_attributes to ensure 'email' is listed. + # If USERNAME_ENABLE is set - this field will be replaced to be Optional() + # see build_login_form() email = EmailField( get_form_field_label("email"), render_kw={"autocomplete": "email"}, - validators=[Optional(), EmailValidation(verify=False)], + validators=[email_required, EmailValidation(verify=False)], ) # username is added dynamically based on USERNAME_ENABLED. @@ -582,7 +556,9 @@ def validate(self, **kwargs: t.Any) -> bool: # responsible to deal with USER_IDENTITY_ATTRIBUTES if it cares. if not self.ifield: uia_email = get_identity_attribute("email") - if uia_email and self.email.data: + # pedantic checks - if app subclasses form, removes email but forgets to + # remove "email" from identity_attributes + if uia_email and hasattr(self, "email") and self.email and self.email.data: self.ifield = self.email self.user = _datastore.find_user( case_insensitive=uia_email.get("case_insensitive", False), @@ -635,6 +611,30 @@ def validate(self, **kwargs: t.Any) -> bool: return True +def build_login_form(app, fcls): + # Based on app configuration, add optional/configurable fields to the login form + # Allow app to override the field using mixins. + # Note this is only called (from init_app()) if form is subclassed from ours. + # We really don't want to touch the form unless there is a specific option + # requested - so that subclasses can change/do what they want (including deleting + # email for example). + if cv("USERNAME_ENABLE", app): + fcls.username = StringField( + get_form_field_label("username"), + render_kw={"autocomplete": "username"}, + validators=[username_validator], + ) + if hasattr(fcls, "email") and fcls.email: + # Make field Optional() + # let subclass easily get rid of this + # Note that WTForms 'del' operator actually sets this to None + fcls.email = EmailField( + get_form_field_label("email"), + render_kw={"autocomplete": "email"}, + validators=[Optional(), EmailValidation(verify=False)], + ) + + class VerifyForm(Form, PasswordFormMixin): """The verify authentication form""" @@ -685,7 +685,7 @@ def validate(self, **kwargs: t.Any) -> bool: # whether a password is required is a config variable (PASSWORD_REQUIRED). # For unified signin there are many other ways to authenticate - if cv("PASSWORD_REQUIRED") or not cv("UNIFIED_SIGNIN"): + if cv("PASSWORD_REQUIRED"): if not self.password.data or not self.password.data.strip(): self.password.errors.append(get_message("PASSWORD_NOT_PROVIDED")[0]) failed = True @@ -739,19 +739,24 @@ def __init__(self, *args, **kwargs): self.next.data = request.args.get("next", "") -class RegisterFormV2(Form, UniqueEmailFormMixin, NextFormMixin): +class RegisterFormV2( + Form, + UniqueEmailFormMixin, + NextFormMixin, + NewPasswordFormMixin, + PasswordConfirmFormMixin, +): """This form is used for registering. The following fields are defined: * email (required) - * password (required if :py:data:`SECURITY_PASSWORD_REQUIRED` is ``True`` - OR :py:data:`SECURITY_UNIFIED_SIGNIN` is ``False``) + * password (required if :py:data:`SECURITY_PASSWORD_REQUIRED` is ``True``) * password_confirm (based on :py:data:`SECURITY_PASSWORD_CONFIRM_REQUIRED`) * username (based on :py:data:`SECURITY_USERNAME_ENABLE`) * next Since there are many configuration options that alter which fields and - which validators (e.g. Required), some fields are added dynamically at + which validators (e.g. Required), some fields are added or changed at Security init_app() time by calling the internal method build_register_form(). We want to support OWASP best-practice around mitigating user enumeration. @@ -765,8 +770,6 @@ class RegisterFormV2(Form, UniqueEmailFormMixin, NextFormMixin): submit = SubmitField(get_form_field_label("register")) username: t.ClassVar[Field] - password: t.ClassVar[Field] - password_confirm: t.ClassVar[Field] def to_dict(self, only_user): """ @@ -826,11 +829,23 @@ def __init__(self, *args, **kwargs): def build_register_form(app, fcls): # Based on app configuration, add optional/configurable fields to the register form # Allow app to override the field using mixins - if not (hasattr(fcls, "password") and fcls.password): - fcls.password = build_password_field(app=app) - if cv("PASSWORD_CONFIRM_REQUIRED", app=app): - if not (hasattr(fcls, "password_confirm") and fcls.password_confirm): - fcls.password_confirm = build_password_field(app=app, is_confirm=True) + # Note that this occurs AFTER app might have sub-classed the form + if not cv("PASSWORD_REQUIRED", app): + # mark password field as Optional + fcls.password = PasswordField( + label=get_form_field_label("password"), + render_kw={"autocomplete": "new-password"}, + validators=[Optional()], + ) + fcls.password_confirm = PasswordField( + get_form_field_label("retype_password"), + render_kw={"autocomplete": "new-password"}, + validators=[ + EqualToLocalize("password", message="RETYPE_PASSWORD_MISMATCH"), + ], + ) + if not cv("PASSWORD_CONFIRM_REQUIRED", app=app): + fcls.password_confirm = None if cv("USERNAME_ENABLE", app): fcls.username = build_register_username_field(app=app) diff --git a/flask_security/templates/security/login_user.html b/flask_security/templates/security/login_user.html index 2d254921..42a4dbac 100644 --- a/flask_security/templates/security/login_user.html +++ b/flask_security/templates/security/login_user.html @@ -7,9 +7,10 @@

{{ _fsdomain('Login') }}

{{ login_user_form.hidden_tag() }} {{ render_form_errors(login_user_form) }} - {% if "email" in identity_attributes %}{{ render_field_with_errors(login_user_form.email) }}{% endif %} + {% if login_user_form.email and "email" in identity_attributes %} + {{ render_field_with_errors(login_user_form.email) }}{% endif %} {% if login_user_form.username and "username" in identity_attributes %} - {% if "email" in identity_attributes %}

{{ _fsdomain("or") }}

{% endif %} + {% if login_user_form.email and "email" in identity_attributes %}

{{ _fsdomain("or") }}

{% endif %} {{ render_field_with_errors(login_user_form.username) }} {% endif %}
{{ render_field_with_errors(login_user_form.password) }}
diff --git a/tests/conftest.py b/tests/conftest.py index 406a763d..3f8d1107 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -340,10 +340,13 @@ def revert_forms(): del app.security.forms[form_name].cls.username from flask_security import RegisterFormV2 + from flask_security.forms import PasswordConfirmFormMixin, NewPasswordFormMixin - for attr in ["username", "password", "password_confirm"]: + for attr in ["username"]: if hasattr(RegisterFormV2, attr): delattr(RegisterFormV2, attr) + RegisterFormV2.password_confirm = PasswordConfirmFormMixin.password_confirm + RegisterFormV2.password = NewPasswordFormMixin.password request.addfinalizer(revert_forms) yield app diff --git a/tests/test_basic.py b/tests/test_basic.py index a386bb00..ce6217ac 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -228,7 +228,7 @@ def test_authenticate_case_insensitive_email(app, client): def test_authenticate_with_invalid_input(client, get_message): response = client.post( "/login", - json=dict(password="password"), + json=dict(password="password", email="mememe@test.com"), headers={"Content-Type": "application/json"}, ) assert get_message("USER_DOES_NOT_EXIST") in response.data diff --git a/tests/test_misc.py b/tests/test_misc.py index 1045081f..16ba920b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -34,6 +34,7 @@ logout, populate_data, reset_fresh, + get_form_input, ) from tests.test_webauthn import HackWebauthnUtil, reg_2_keys @@ -375,7 +376,6 @@ class MyRegisterForm(RegisterFormV2): security.init_app(app) client = app.test_client() - response = client.get("/login") assert b"My Login Email Address Field" in response.data @@ -1559,3 +1559,43 @@ def test_secret_key_fallbacks(app, verify_secret_key, verify_fallbacks, should_p else: with pytest.raises(BadTimeSignature): serializer.loads(token) + + +@pytest.mark.settings(username_enable=True) +def test_custom_login_form(app, sqlalchemy_datastore, get_message): + # Test custom login form that deletes email and uses username only + # Also test that if app leave 'email' in as a user identity attribute we + # will ignore it + class MyLoginForm(LoginForm): + email = None + + app.security = Security( + app, + datastore=sqlalchemy_datastore, + login_form=MyLoginForm, + ) + + populate_data(app) + client = app.test_client() + + response = client.get("/login", follow_redirects=False) + assert not get_form_input(response, "email") + + response = client.post( + "/login", json=dict(email="jill@lp.com", password="password") + ) + assert response.status_code == 400 + assert ( + get_message("USER_DOES_NOT_EXIST") + == response.json["response"]["field_errors"][""][0].encode() + ) + + response = client.post("/login", json=dict(username="jill", password="password")) + assert response.status_code == 200 + + +@pytest.mark.settings(password_required=False) +def test_password_required_setting(app, sqlalchemy_datastore): + with pytest.raises(ValueError) as vex: + Security(app=app, datastore=sqlalchemy_datastore) + assert "SECURITY_PASSWORD_REQUIRED can only be" in str(vex.value) diff --git a/tests/test_registerable.py b/tests/test_registerable.py index f50a5417..4d6127ca 100644 --- a/tests/test_registerable.py +++ b/tests/test_registerable.py @@ -1125,3 +1125,33 @@ class MyRegisterForm(RegisterFormV2): response.json["response"]["errors"][0] == "Field must be at least 8 characters long." ) + + +@pytest.mark.unified_signin() +@pytest.mark.confirmable() +@pytest.mark.settings(password_required=False, use_register_v2=True) +def test_allow_null_password_v2(client, get_message): + # Test password not required with new RegisterFormV2 + response = client.get("/register") + data = dict(email="trp@lp.com") + pw = get_form_input(response, "password") + assert "required" not in pw + pwc = get_form_input(response, "password_confirm") + assert "required" not in pwc + + response = client.post("/register", data=data, follow_redirects=True) + assert get_message("CONFIRM_REGISTRATION", email="trp@lp.com") in response.data + logout(client) + + # should be able to register with password and password_confirm + data = dict(email="trp2@lp.com", password="battery staple") + response = client.post("/register", data=data, follow_redirects=True) + assert get_message("RETYPE_PASSWORD_MISMATCH") in response.data + + data = dict( + email="trp2@lp.com", + password="battery staple", + password_confirm="battery staple", + ) + response = client.post("/register", data=data, follow_redirects=True) + assert get_message("CONFIRM_REGISTRATION", email="trp2@lp.com") in response.data