spiral-project / ihatemoney

A simple shared budget manager web application
https://ihatemoney.org
Other
1.18k stars 267 forks source link

Generate Password Hash isn't working anymore #1241

Closed hofermo closed 5 months ago

hofermo commented 12 months ago

When using the latest Docker image, version 6.1.1, it is not possible to create a password, as the application context isn't created.

This could be the breaking change, but I am not too familiar with the project. https://github.com/spiral-project/ihatemoney/commit/857ca2d5b01c7722016a8bc1fd89324acbfcab55#diff-91addaa276f43da0d54547b88d095bed802e84332616f73e08e218679213842fR455

Here the console output when trying to run docker run -it --rm --entrypoint ihatemoney ihatemoney/ihatemoney generate_password_hash, like it is stated in the documentation:

/src/ihatemoney/monkeypath_continuum.py:16: UserWarning: SQLAlchemy-continuum version changed. Please check monkeypatching usefulness.
  warnings.warn(
Password: <entered a password here and pressed Enter>
Traceback (most recent call last):
  File "/usr/local/bin/ihatemoney", line 33, in <module>
    sys.exit(load_entry_point('ihatemoney', 'console_scripts', 'ihatemoney')())
  File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.10/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/src/ihatemoney/manage.py", line 39, in password_hash
    print(generate_password_hash(password))
  File "/src/ihatemoney/utils.py", line 455, in generate_password_hash
    if current_app.config.get("PASSWORD_HASH_METHOD"):
  File "/usr/local/lib/python3.10/site-packages/werkzeug/local.py", line 316, in __get__
    obj = instance._get_current_object()
  File "/usr/local/lib/python3.10/site-packages/werkzeug/local.py", line 513, in _get_current_object
    raise RuntimeError(unbound_message) from None
RuntimeError: Working outside of application context.

This typically means that you attempted to use functionality that needed
the current application. To solve this, set up an application context
with app.app_context(). See the documentation for more information.

~The quick fix for now is to just use version 6.1.0 where the hashing still works.~

zorun commented 12 months ago

Thanks for the report, I believe your analysis is correct.

@azmeuk any clue about how to fix this issue? Do we need to create a context when working from the command line? We should definitely add a test that calls the command line to detect this kind of regression.

azmeuk commented 12 months ago

Reproducible in local with ./ihatemoney/manage.py generate_password_hash. I just submitted a patch that fixes this, indeed a context application was needed.

Strangely, there is already a test for this command, and it was not failing. I am not sure why though.

https://github.com/spiral-project/ihatemoney/blob/e360ee7dfbd7123b8912ab0779ee70ea1237e8f1/ihatemoney/tests/main_test.py#L93-L97

azmeuk commented 12 months ago

Ah :thinking: That might be pytest-flask inducing an application context for the whole testsuite, and thus provoking a false negative (or is it false positive? I am not sure which one this should be called :shrug:).

azmeuk commented 11 months ago

I did not understand why I could not reproduce the bug with ./ihatemoney/manage.py generate_password_hash, but now the issue is solved in master.

My understanding that using @cli.command instead of @click.command probably injected the needed application context in generate_password_hash.

https://github.com/spiral-project/ihatemoney/commit/eb7338c76c6fcbd11ca6ec88679217e7b4b2e997#diff-395aa9b2d6ecd7f89f2770b3ecef6053a905ad7e387df33214ce435714888150

I will try to reproduce a MRE to check that there is a problem with pytest-flask, and open a bug report there if needed.

zorun commented 5 months ago

Closing, this was fixed with c2fc33a8ca44fc6e867d84b77d1ca7c695879929 in the stable-6.1 branch (released as 6.1.2), and the bug no longer triggers in master due to the @click/@cli change.