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

SECURITY_CONFIRMABLE and Confirm Password #1043

Closed
antos07 opened this issue Dec 2, 2024 · 4 comments · Fixed by #1050
Closed

SECURITY_CONFIRMABLE and Confirm Password #1043

antos07 opened this issue Dec 2, 2024 · 4 comments · Fixed by #1050
Assignees
Milestone

Comments

@antos07
Copy link

antos07 commented Dec 2, 2024

I suggest reconsidering #804. The solution proposed there (setting confirm_register_form=flask_security.RegisterForm) actually breaks the CLI, as flask users create relies on confirm_register_form. flask_security.RegisterForm meanwhile relies on the request context, so it's impossible to use it from CLI. And I believe it also won't pass validation anyway but I haven't checked it

@jwag956
Copy link
Collaborator

jwag956 commented Dec 3, 2024

I'm not sure it's broken in quite the way you are describing.. did you try:
create [email protected] password_confirm:'battery staple' --password 'battery staple'

@antos07
Copy link
Author

antos07 commented Dec 3, 2024

$ flask users create [email protected] password_confirm:'battery staple' --password 'battery staple'
Traceback (most recent call last):
  File ".../bin/flask", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File ".../lib/python3.12/site-packages/flask/cli.py", line 1129, in main
    cli.main()
  File ".../lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/click/decorators.py", line 33, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/flask/cli.py", line 400, in decorator
    return ctx.invoke(f, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/flask_security/cli.py", line 61, in wrapper
    fn(*args, **kwargs)
  File ".../lib/python3.12/site-packages/flask_security/cli.py", line 130, in users_create
    form = build_form("confirm_register_form", meta={"csrf": False}, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/flask_security/forms.py", line 895, in build_form
    return _security.forms[form_name].instantiator(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/flask_security/core.py", line 651, in _default_form_instantiator
    return cls(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/wtforms/form.py", line 209, in __call__
    return type.__call__(cls, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/flask_security/forms.py", line 701, in __init__
    self.next.data = request.args.get("next", "")
                     ^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/werkzeug/local.py", line 318, in __get__
    obj = instance._get_current_object()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../lib/python3.12/site-packages/werkzeug/local.py", line 519, in _get_current_object
    raise RuntimeError(unbound_message) from None
RuntimeError: Working outside of request context.

This typically means that you attempted to use functionality that needed
an active HTTP request. Consult the documentation on testing for
information about how to avoid this problem.

The error occurs in RegisterForm.__init__(), because it has the following lines that require request context:

 if not self.next.data:
    self.next.data = request.args.get("next", "")

Modifying the command to pass the next parameter won't help either because NextFormMixin.validate_next() also requires request context.

@jwag956
Copy link
Collaborator

jwag956 commented Dec 3, 2024

Ahh yes - interesting - that actually shows that in addition to the extra confirm_password field - the other difference between the 2 register paths is the 'next' parameter.

I assume that ultimately what you are looking for is a register form with CONFIRMABLE and requiring the user to type in the password twice??

For the CLI - I assume there is no reason to require 'password_confirm' (which is the unintended side-effect of the above form change).

@antos07
Copy link
Author

antos07 commented Dec 3, 2024

I assume that ultimately what you are looking for is a register form with CONFIRMABLE and requiring the user to type in the password twice??

That's right.

For the CLI - I assume there is no reason to require 'password_confirm' (which is the unintended side-effect of the above form change).

Yes, it's already handled by the CLI (if no --password argument has been provided).

@jwag956 jwag956 self-assigned this Dec 3, 2024
@jwag956 jwag956 added this to the 5.6 milestone Dec 19, 2024
jwag956 added a commit that referenced this issue Dec 31, 2024
This deprecates RegisterForm and ConfirmRegisterForm and introduces RegisterFormV2.

Currently, the old forms are used so no backwards compat issue. The new form can be used by setting SECURITY_REGISTER_V2 to True.

Add the SECURITY_PASSWORD_CONFIRM_REQUIRED option that controls whether the RegisterFormV2 add that field.

Deprecate RegisterForm, ConfirmRegisterForm and confirm_register_form options.

closes #1043
jwag956 added a commit that referenced this issue Dec 31, 2024
This deprecates RegisterForm and ConfirmRegisterForm and introduces RegisterFormV2.

Currently, the old forms are used so no backwards compat issue. The new form can be used by setting SECURITY_REGISTER_V2 to True.

Add the SECURITY_PASSWORD_CONFIRM_REQUIRED option that controls whether the RegisterFormV2 add that field.

Deprecate RegisterForm, ConfirmRegisterForm and confirm_register_form options.

closes #1043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants