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

Flask-JWT-Extended >= 4.0 breaks server #19

Closed
jotelha opened this issue May 11, 2021 · 21 comments
Closed

Flask-JWT-Extended >= 4.0 breaks server #19

jotelha opened this issue May 11, 2021 · 21 comments
Assignees
Labels
bug Something isn't working

Comments

@jotelha
Copy link
Member

jotelha commented May 11, 2021

When revisiting a testing workflow that worked half a year ago and stopped working recently (livMatS/dserver-dependency-graph-plugin@7f16849), I realized that Flask-JWT-Extended >= 4.0 introduces many breaking changes (https://flask-jwt-extended.readthedocs.io/en/stable/v4_upgrade_guide/) and leads to most of the server's tests to fail with

AssertionError: View function mapping is overwriting an existing endpoint function: dataset.wrapper

(more details at livMatS/dserver-dependency-graph-plugin#2 (comment))

To avoid looking into the issue in detail, a Flask-JWT-Extended<4.0 in the requirements should solve this for now.

Best, Johannes

@tjelvar-olsson
Copy link
Contributor

Hi Johannes,

Thank you for reporting this. I've added a temporary fix in 4c53504

@tjelvar-olsson
Copy link
Contributor

Fix in release: https://pypi.org/project/dtool-lookup-server/0.17.1/

Please let me know if you run into any problems. I'll look into upgrading to the latest version of the flask-jwt-extended API over the next couple of weeks.

@tjelvar-olsson tjelvar-olsson self-assigned this May 12, 2021
@tjelvar-olsson tjelvar-olsson added the bug Something isn't working label May 12, 2021
@pastewka
Copy link
Contributor

This seems to work, but it collides with token_generator_ldap that by default currently installs Flask-JWT-Extended 4.2. In particular, they changed the default for JWT_IDENTITY_CLAIM from identity to sub. This given a Missing claim: identity error. The fix is to either downgrad Flask-JWT-Extended for token_generator_ldap.

@tjelvar-olsson
Copy link
Contributor

I think I have made the dtool-lookup-server code compatible with jwt-extended 4. Please have a look at:
https://github.com/jic-dtool/dtool-lookup-server/tree/jwt-extended-4
and let me know what you think.

@pastewka
Copy link
Contributor

pastewka commented May 15, 2021

I get the following error. It goes away if I downgrade Flask-JWT-Extended. Not sure how this is related.

dtool_lookup_server_1   | Traceback (most recent call last):
dtool_lookup_server_1   |   File "/usr/local/bin/flask", line 8, in <module>
dtool_lookup_server_1   |     sys.exit(main())
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 967, in main
dtool_lookup_server_1   |     cli.main(args=sys.argv[1:], prog_name="python -m flask" if as_module else None)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 586, in main
dtool_lookup_server_1   |     return super(FlaskGroup, self).main(*args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 782, in main
dtool_lookup_server_1   |     rv = self.invoke(ctx)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
dtool_lookup_server_1   |     return _process_result(sub_ctx.command.invoke(sub_ctx))
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
dtool_lookup_server_1   |     return _process_result(sub_ctx.command.invoke(sub_ctx))
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
dtool_lookup_server_1   |     return ctx.invoke(self.callback, **ctx.params)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/core.py", line 610, in invoke
dtool_lookup_server_1   |     return callback(*args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/click/decorators.py", line 21, in new_func
dtool_lookup_server_1   |     return f(get_current_context(), *args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 425, in decorator
dtool_lookup_server_1   |     with __ctx.ensure_object(ScriptInfo).load_app().app_context():
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 388, in load_app
dtool_lookup_server_1   |     app = locate_app(self, import_name, name)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 257, in locate_app
dtool_lookup_server_1   |     return find_best_app(script_info, module)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 83, in find_best_app
dtool_lookup_server_1   |     app = call_factory(script_info, app_factory)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/cli.py", line 119, in call_factory
dtool_lookup_server_1   |     return app_factory()
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/dtool_lookup_server/__init__.py", line 83, in create_app
dtool_lookup_server_1   |     app.register_blueprint(bp)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 98, in wrapper_func
dtool_lookup_server_1   |     return f(self, *args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1168, in register_blueprint
dtool_lookup_server_1   |     blueprint.register(self, options, first_registration)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/blueprints.py", line 256, in register
dtool_lookup_server_1   |     deferred(state)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/blueprints.py", line 294, in <lambda>
dtool_lookup_server_1   |     self.record(lambda s: s.add_url_rule(rule, endpoint, view_func, **options))
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/blueprints.py", line 81, in add_url_rule
dtool_lookup_server_1   |     self.app.add_url_rule(
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 98, in wrapper_func
dtool_lookup_server_1   |     return f(self, *args, **kwargs)
dtool_lookup_server_1   |   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1282, in add_url_rule
dtool_lookup_server_1   |     raise AssertionError(
dtool_lookup_server_1   | AssertionError: View function mapping is overwriting an existing endpoint function: mongo.wrapper

@tjelvar-olsson
Copy link
Contributor

Hi Lars,
This looks like the error from before. Did you use the code from the branch jwt-extended-4?
Best,
Tjelvar

@pastewka
Copy link
Contributor

I just double checked. It is installing JWT-Extended 4.2 and I am using pip install git+https://github.com/jic-dtool/dtool-lookup-server.git@jwt-extended-4 to install the development branch, still the same error. I am using the following packages (all through pip):

Jinja2-2.11.3 Mako-1.1.4 MarkupSafe-2.0.0 PyJWT-2.1.0 SQLAlchemy-1.4.15 Werkzeug-1.0.1 alembic-1.6.2 boto3-1.17.73 botocore-1.20.73 cffi-1.14.5 click-7.1.2 click-plugins-1.1.1 cryptography-3.4.7 dtool-cli-0.7.0 dtool-ecs-0.5.0 dtool-irods-0.10.1 dtool-lookup-server-0.17.1 dtool-lookup-server-dependency-graph-plugin-0.1.4 dtool-lookup-server-direct-mongo-plugin-0.1.3 dtool-lookup-server-elastic-search-plugin-0.1.dev11 dtool-s3-0.10.0 dtool-smb-0.1.0 dtoolcore-3.18.0 flask-1.1.4 flask-cors-3.0.10 flask-jwt-extended-4.2.1 flask-migrate-3.0.0 flask-pymongo-2.3.0 flask-sqlalchemy-2.5.1 greenlet-1.1.0 itsdangerous-1.1.0 jmespath-0.10.0 pyasn1-0.4.8 pycparser-2.20 pymongo-3.11.4 pysmb-1.2.6 python-dateutil-2.8.1 python-editor-1.0.4 pyyaml-5.4.1 s3transfer-0.4.2 six-1.16.0 urllib3-1.26.4

@pastewka
Copy link
Contributor

Sorry, I am seeing now that it is reporting 0.17.1 of the lookup server. Not sure what's going on. Will double check.

@pastewka
Copy link
Contributor

I just double checked, it is reporting 0.17.1 because the version information is hard-coded but is actually installing the latest. So this problem persists on my configuration.

@tjelvar-olsson
Copy link
Contributor

I think this may be to do with the behaviour of pip. You already have 0.17.1 installed so it does nothing.

I'll make a new release once I have fixed: #20

@pastewka
Copy link
Contributor

Okay, sounds good

@tjelvar-olsson
Copy link
Contributor

The latest release should be flask-jwt-extended 4.0 compatible: https://pypi.org/project/dtool-lookup-server/0.17.2/

@pastewka
Copy link
Contributor

The problem persists on with 0.17.2. Let me try to debug this on my end.

@jotelha
Copy link
Member Author

jotelha commented May 18, 2021

The tests work on a fresh 0.17.2 lookup server installation (following session has a mongo server running and starts in a clean venv within the current git repository root),

python -m venv ~/venv/20210518-test-python-3.8
source ~/venv/20210518-test-python-3.8/bin/activate
pip install dtool-lookup-server
pip install pytest-cov
pip freeze | tee after_pytest_cov_install.txt
pytest # OK

but fail again after installing any plugin (even if just running the plugin tests),

pip install dtool-lookup-server-direct-mongo-plugin
pip freeze | tee after_direct_mongo_plugin_intall.txt
pytest # fails as with 0.17.0

thus we will have to apply Tjelvar's server side modifications to each plugin as well.

Environment at after_pytest_cov_install.txt is

alembic==1.6.2
attrs==21.2.0
boto3==1.17.75
botocore==1.20.75
cffi==1.14.5
click==7.1.2
click-plugins==1.1.1
coverage==5.5
cryptography==3.4.7
dtool-cli==0.7.0
dtool-ecs==0.5.0
dtool-irods==0.10.1
dtool-lookup-server==0.17.2
dtool-s3==0.10.0
dtoolcore==3.18.0
Flask==1.1.4
Flask-Cors==3.0.10
Flask-JWT-Extended==4.2.1
Flask-Migrate==3.0.0
Flask-PyMongo==2.3.0
Flask-SQLAlchemy==2.5.1
greenlet==1.1.0
iniconfig==1.1.1
itsdangerous==1.1.0
Jinja2==2.11.3
jmespath==0.10.0
Mako==1.1.4
MarkupSafe==2.0.1
packaging==20.9
pluggy==0.13.1
py==1.10.0
pycparser==2.20
PyJWT==2.1.0
pymongo==3.11.4
pyparsing==2.4.7
pytest==6.2.4
pytest-cov==2.12.0
python-dateutil==2.8.1
python-editor==1.0.4
PyYAML==5.4.1
s3transfer==0.4.2
six==1.16.0
SQLAlchemy==1.4.15
toml==0.10.2
urllib3==1.26.4
Werkzeug==1.0.1

and no other modification to the environment is introduced by the plugin installation,

$ diff after_pytest_cov_install.txt after_direct_mongo_plugin_intall.txt 
13a14
> dtool-lookup-server-direct-mongo-plugin==0.1.3

@jotelha
Copy link
Member Author

jotelha commented May 18, 2021

@tjelvar-olsson looking at all the changes you have applied, will replacements like

@@ -22,7 +22,7 @@ bp = Blueprint("base_uri", __name__, url_prefix="/admin/base_uri")
 
 
 @bp.route("/register", methods=["POST"])
-@jwt_required
+@jwt_required()
 def register():
     """Register a base URI.

at all blueprint routes within the plugins resolve those AssertionError: View function mapping is overwriting an existing endpoint function: mongo.wrapper errors?

@jotelha
Copy link
Member Author

jotelha commented May 18, 2021

To answer that question myself: yes, livMatS/dserver-direct-mongo-plugin@f7ff5d4

@pastewka
Copy link
Contributor

@jotelha - this would also affect the graph plugin correct? Can you update both and make new releases?

@jotelha
Copy link
Member Author

jotelha commented May 19, 2021

@jotelha - this would also affect the graph plugin correct? Can you update both and make new releases?

@pastewka already done last night

@pastewka
Copy link
Contributor

Okay I can confirm that things now work properly with Flask-JWT-Extended>=4.0. Thanks @tjelvar-olsson @jotelha

@tjelvar-olsson
Copy link
Contributor

@tjelvar-olsson
Copy link
Contributor

Closing this issue now

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

No branches or pull requests

3 participants