inveniosoftware / invenio-oauthclient

Invenio module that provides OAuth web authorization support.
https://invenio-oauthclient.readthedocs.io
MIT License
6 stars 74 forks source link

Stop relying on Flask-OAuthlib #195

Open Sinclert opened 4 years ago

Sinclert commented 4 years ago

The package Flask-OAuthlib has been discontinued and replaced by authlib (claimed both in the GitHub repository, and in its docs).

Additionally, the usage of Flask-OAuthlib with the syntax Flask-OAuthlib >= 0.9.3, as it is currently specified inside the setup.py file, leads to inconsistent oauthlib versions being install

Inconsistency tracking

Let's track dependencies down, shall we?

Flask-OAuthlib >= 0.9.3 resolves into v0.9.5 (last version in Pypi), which declares the following (check here):

lnielsen commented 4 years ago

Replacing Flask-OAuthlib is quite big task as it's both in Invenio-OAuthClient and Invenio-OAuth2Server. Also, authlib is, like Flask-OAuthlib, a one-man show by the same person and I must admit that I'm quite concerned of spending a lot of effort migrating to authlib if he makes the same move again.

Obviously there's a problem here, just not clear what the solution is - i.e stay and fork flask-oauthlib, or switch to authlib, or …?

Sinclert commented 4 years ago

That's a good point.

Maybe for now just locking the Flask-OAuthlib version to 0.9.3 or 0.9.4 is enough. It is just the latest version (0.9.5) the one that does not play well with requests-oauthlib.

Both 0.9.3 (check here) and 0.9.4 (check here) do not bound oauthlib with the "<3.0.0" bomb.

slint commented 4 years ago

In the setup.py we also pin requests-oauthlib to <1.2.0:

https://github.com/inveniosoftware/invenio-oauthclient/blob/ca0e40e6483d2f8fe8ccda670c56bc6423bb69b0/setup.py#L72

...which should be enough to protect anyone doing pip install invenio-oauthclient.

The actual issue might be that pip messes up the installation order of the packages, which can indeed cause problems, e.g. if Flask-OAuthlib is installed first, what @Sinclert described above happens...

Unfortunately, this is not an easy issue to fix, and that's also why when deploying Python apps in production, people have requirements.txt with the exact combination of pinned dependencies that are known to work, or use tools like pipenv, poetry and dephell exist.

Also, I guess Flask-OAuthlib pins requests-oauthlib for a reason (e.g. incompatibility)

Sinclert commented 4 years ago

You are right, invenio-oauthclient specifies requests-oauthlib...<1.2.0, which would solve the problem if the order is appropriate.

However, given that anyone who uses requirements.txt is likely to specify dependencies in alphabetical order, Flask-OAuthlib will most certainly be installed before invenio-oauthclient for all requirements.txt users, creating the incompatibility.

Could you limit Flask-OAuthlib in that regard?

Something like:

    'Flask-OAuthlib>=0.9.3,<0.9.5',
alexdutton commented 4 years ago

c.f. https://github.com/inveniosoftware/invenio-oauth2server/issues/195