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

replace deprecated (Timed)JSONWebSignatureSerializer #386

Merged
merged 8 commits into from
Oct 26, 2022
Merged

replace deprecated (Timed)JSONWebSignatureSerializer #386

merged 8 commits into from
Oct 26, 2022

Conversation

metzm
Copy link
Contributor

@metzm metzm commented Oct 14, 2022

Fixes #345

This PR proposes to use jwt instead of itsdangerous to 1) no longer use a deprecated component, 2) solve python lib conflicts with the actinia-stac plugin.
(Timed)JSONWebSignatureSerializer uses by default HS512 as hash algorithm, therefore this PR also uses this algorithm with jwt, in the hope that existing API keys are still recognized as valid keys.

@metzm metzm added the bug Something isn't working label Oct 14, 2022
@metzm metzm requested a review from mmacata October 14, 2022 16:38
@metzm
Copy link
Contributor Author

metzm commented Oct 14, 2022

Test failure:
/src/actinia_core/tests/test_login.py:307: AssertionError: 400 != 200 : HTML status code is wrong 400

Copy link
Member

@mmacata mmacata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested locally and get the following error when I try to call http://127.0.0.1:8088/api/v3/api_key:

{
"time": "2022-10-25T12:10:38.513745Z",
"level": "ERROR",
"component": "werkzeug",
"module": "_internal",
"message": "Error on request:\nTraceback (most recent call last):\n File "/src/actinia_core/src/actinia_core/core/common/user.py", line 672, in verify_auth_token\n data = jwt.decode(\nAttributeError: module 'jwt' has no attribute 'decode'\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n File "/usr/lib/python3.9/site-packages/werkzeug/serving.py", line 323, in run_wsgi\n execute(self.server.app)\n File "/usr/lib/python3.9/site-packages/werkzeug/serving.py", line 312, in execute\n application_iter = app(environ, start_response)\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 2464, in call\n return self.wsgi_app(environ, start_response)\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 2450, in wsgi_app\n response = self.handle_exception(e)\n File "/usr/lib/python3.9/site-packages/flask_restful/init.py", line 271, in error_router\n return original_handler(e)\n File "/usr/lib/python3.9/site-packages/flask_cors/extension.py", line 165, in wrapped_function\n return cors_after_request(app.make_response(f(*args, **kwargs)))\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 1867, in handle_exception\n reraise(exc_type, exc_value, tb)\n File "/usr/lib/python3.9/site-packages/flask/_compat.py", line 38, in reraise\n raise value.with_traceback(tb)\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 2447, in wsgi_app\n response = self.full_dispatch_request()\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 1952, in full_dispatch_request\n rv = self.handle_user_exception(e)\n File "/usr/lib/python3.9/site-packages/flask_restful/init.py", line 271, in error_router\n return original_handler(e)\n File "/usr/lib/python3.9/site-packages/flask_cors/extension.py", line 165, in wrapped_function\n return cors_after_request(app.make_response(f(*args, **kwargs)))\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 1821, in handle_user_exception\n reraise(exc_type, exc_value, tb)\n File "/usr/lib/python3.9/site-packages/flask/_compat.py", line 38, in reraise\n raise value.with_traceback(tb)\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 1950, in full_dispatch_request\n rv = self.dispatch_request()\n File "/usr/lib/python3.9/site-packages/flask/app.py", line 1936, in dispatch_request\n return self.view_functionsrule.endpoint\n File "/usr/lib/python3.9/site-packages/flask_restful/init.py", line 467, in wrapper\n resp = resource(*args, **kwargs)\n File "/usr/lib/python3.9/site-packages/flask_httpauth.py", line 159, in decorated\n user = self.authenticate(auth, password)\n File "/usr/lib/python3.9/site-packages/flask_httpauth.py", line 240, in authenticate\n return self.ensure_sync(self.verify_password_callback)(\n File "/src/actinia_core/src/actinia_core/rest/base/user_auth.py", line 64, in verify_password\n user = ActiniaUser.verify_auth_token(username_or_token)\n File "/src/actinia_core/src/actinia_core/core/common/user.py", line 679, in verify_auth_token\n except jwt.exceptions.DecodeError:\nAttributeError: module 'jwt.exceptions' has no attribute 'DecodeError'",
"pathname": "/usr/lib/python3.9/site-packages/werkzeug/_internal.py",
"lineno": 113,
"processName": "MainProcess",
"threadName": "Thread-11"
}

@metzm
Copy link
Contributor Author

metzm commented Oct 25, 2022

The AttributeError: module 'jwt' has no attribute 'decode' is strange because of
https://pyjwt.readthedocs.io/en/stable/api.html#jwt.decode
Same for AttributeError: module 'jwt.exceptions' has no attribute 'DecodeError'
Which version of PyJWT are you using?

This is even more strange because all PR tests are passed.

@mmacata
Copy link
Member

mmacata commented Oct 26, 2022

Sorry, I installed the wrong module 🙈
With the correct one, everything works fine, so sorry for the noise.

Copy link
Member

@mmacata mmacata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool, that we can keep this funtionality.
@anikaweinmann does this interfere in any way with the keycloak implementation?
And I am not sure if it is backwards-compatible. Therefore it might make sense to make a major version update? What do you think?

@anikaweinmann
Copy link
Member

anikaweinmann commented Oct 26, 2022

Really cool, that we can keep this funtionality. @anikaweinmann does this interfere in any way with the keycloak implementation? And I am not sure if it is backwards-compatible. Therefore it might make sense to make a major version update? What do you think?

I moved maybe some parts in a base class, but does not change things in the functions.
I am also not sure if it is backwards-compatible.

@metzm metzm merged commit 7be5abb into main Oct 26, 2022
@metzm metzm deleted the issue_345 branch October 26, 2022 15:45
@neteler neteler added this to the next_patch milestone Nov 8, 2022
@mmacata mmacata mentioned this pull request Nov 8, 2022
@mmacata mmacata modified the milestones: next_patch, 4.4.0 Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecated usage of python package in user.py
4 participants