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

update Authlib requirement to allow version 1.0.1 #1861

Closed
thesuperzapper opened this issue Jun 17, 2022 · 12 comments
Closed

update Authlib requirement to allow version 1.0.1 #1861

thesuperzapper opened this issue Jun 17, 2022 · 12 comments

Comments

@thesuperzapper
Copy link
Contributor

thesuperzapper commented Jun 17, 2022

Currently, we are restricting Authlib to <1.0.0 in our requirements, authlib has now released version 1.0.1 with lots of fixes and security patches, we should update our pin to allow 1.0.1.

Lots of people have been inadvertently testing using 1.0.1 for us! This is because airflow forgot to pin authlib to the same versions as Flask-AppBuilder, so the latest authlib is always installed with airflow.

@potiuk you may want to look at that!


The main issues I see from people who have already tried to use 1.0.1, are related to jwks_uri not being set (rather than any actual issue with Flask-AppBuilder).

The error text is: Error authorizing OAuth access token: Missing "jwks_uri" in metadata, see related issues:


Currently, all of our OAUTH security examples don't show setting jwks_uri which is now required when using id_token auth.
We need to update our examples to set jwks_uri so they continue to work with authlib 1.0.1.

Example for Google with jwks_url:

OAUTH_PROVIDERS = [
    {
        "name": "google",
        "icon": "fa-google",
        "token_key": "access_token",
        "remote_app": {
            "client_id": "GOOGLE_KEY",
            "client_secret": "GOOGLE_SECRET",
            "api_base_url": "https://www.googleapis.com/oauth2/v2/",
            "client_kwargs": {"scope": "email profile"},
            "access_token_url": "https://accounts.google.com/o/oauth2/token",
            "authorize_url": "https://accounts.google.com/o/oauth2/auth",
            "jwks_uri": "https://www.googleapis.com/oauth2/v3/certs",
        },
    },
]

We may also want to show setting server_metadata_url to an OpenID /.well-known/openid-configuration URL, (which replaces the need to set authorize_url, access_token_url, and jwks_uri so is a bit easier for people).

Example for Google with server_metadata_url:

OAUTH_PROVIDERS = [
    {
        "name": "google",
        "icon": "fa-google",
        "token_key": "access_token",
        "remote_app": {
            "client_id": "GOOGLE_KEY",
            "client_secret": "GOOGLE_SECRET",
            "api_base_url": "https://www.googleapis.com/oauth2/v2/",
            "server_metadata_url": "https://accounts.google.com/.well-known/openid-configuration",
            "client_kwargs": {"scope": "email profile"},
        },
    },
]

Example for Okta with sever_metadata_url:

OAUTH_PROVIDERS = [
  {
      "name": "okta",
      "icon": "fa-circle-o",
      "token_key": "access_token",
      "remote_app": {
          "client_id": "OKTA_KEY",
          "client_secret": "OKTA_SECRET",
          "api_base_url": "https://OKTA_DOMAIN.okta.com/oauth2/v1/",
          "server_metadata_url": "https://OKTA_DOMAIN.okta.com/.well-known/openid-configuration",
          "client_kwargs": {"scope": "openid profile email groups"},
      },
  },
]
@thesuperzapper
Copy link
Contributor Author

@dpgaspar thoughts on this?

@potiuk
Copy link
Contributor

potiuk commented Jun 17, 2022

@potiuk you may want to look at that!

Ah. Cool yhanks for that. I will update it. Actually the setup.py entry is wrong in a different way. We should use "flask-app-builder[oauth]" as dependency there so that we do not have to manually sync dependencies when FAB updates it.

BTW. We have just started to migrate to FAB 4.0 @thesuperzapper - you might want to join our crew of people who will help with testing it (there are many breaking changes in the underlying libraries). I've added you so to that you are aware and possibly help us with testing Airfflow apache/airflow#22397 (comment)

In the meantime - indeed upgrading of the authlib in FAB 4.1.2 might be a good idea indeed (and once we use oauth extra, airflow deps will update automatically)

@dpgaspar
Copy link
Owner

I'll open a PR to bump and test authlib

potiuk added a commit to potiuk/airflow that referenced this issue Jun 17, 2022
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861
@potiuk
Copy link
Contributor

potiuk commented Jun 17, 2022

Also @thesuperzapper we have a way to fix that for you users too (and ours). I am going to update constraint and re-generate docker images with authlib=1.0.0

@thesuperzapper
Copy link
Contributor Author

thesuperzapper commented Jun 17, 2022

@potiuk actually the version would need to be the last 0.X version (which is 0.15.5) for this jwks_uri issue not to be a problem.

But I think we may as well keep 1.0.1 as the authlib version in the docker containers (it's only installed in the 2.2.0 containers and later anyway). There are large amounts of fixes in the 1.0.X branch.

@potiuk
Copy link
Contributor

potiuk commented Jun 17, 2022

Right....

@potiuk
Copy link
Contributor

potiuk commented Jun 17, 2022

I will fix it though anyway.

@potiuk
Copy link
Contributor

potiuk commented Jun 17, 2022

I prefer to get it "updated to latest authlib" in upcoming 2.3.3 (I hope we will upgrade to FAB 4 by then rather than break oauth for 2.2.5 - 2.3.2 users :)

@potiuk
Copy link
Contributor

potiuk commented Jun 17, 2022

BTW. 2.2.4 and below had 0.15.5

@potiuk
Copy link
Contributor

potiuk commented Jun 17, 2022

@thesuperzapper - FYI: all constraints and images of Airlfow 2.2.5, 2.3.0, 2.3.1. 2.3.2 are refreshed now.

potiuk added a commit to apache/airflow that referenced this issue Jun 19, 2022
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861
potiuk added a commit to potiuk/airflow that referenced this issue Jun 29, 2022
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

(cherry picked from commit 5674491)
@dpgaspar
Copy link
Owner

the authlib bump (now is <2) is merge into master.

Thank you once more @thesuperzapper

leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Oct 4, 2022
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

(cherry picked from commit 5674491dc8e8ed1685cdb4c04922cb72ad8ba9b4)

GitOrigin-RevId: 2a7c1f842407f815d8abe0d2239e64f317307439
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Dec 7, 2022
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

(cherry picked from commit 5674491dc8e8ed1685cdb4c04922cb72ad8ba9b4)

GitOrigin-RevId: 2a7c1f842407f815d8abe0d2239e64f317307439
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jan 30, 2023
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

GitOrigin-RevId: 5674491dc8e8ed1685cdb4c04922cb72ad8ba9b4
@Narender-007
Copy link

Narender-007 commented Aug 1, 2023

when i am using azure authentication in flask app builder getting this error :

Error returning OAuth user info: %s 'upn'
2023-08-01 12:04:40,989:ERROR:flask_appbuilder.security.views:Error returning OAuth user info: 'upn'

i have got jwt token credentials are verified but getting UPN key error

how can i resolve it

kosteev pushed a commit to kosteev/composer-airflow-test-copybara that referenced this issue Sep 12, 2024
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

(cherry picked from commit 5674491dc8e8ed1685cdb4c04922cb72ad8ba9b4)

GitOrigin-RevId: 2a7c1f842407f815d8abe0d2239e64f317307439
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 18, 2024
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

GitOrigin-RevId: 5674491dc8e8ed1685cdb4c04922cb72ad8ba9b4
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Nov 7, 2024
The dependency we have for flask-appbuilder oauth authentication
(for github/google authentication) should follow the limits
that flask-appbuilder current version has. We added authlib there
but apparently FAB currently limits authlib to <= 1.0 - we should
follow fab rather than have our own dependency here.

This has been pointed out in
dpgaspar/Flask-AppBuilder#1861

GitOrigin-RevId: 5674491dc8e8ed1685cdb4c04922cb72ad8ba9b4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants