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

Fix Flask Login user setting for Flask 2.2 and Flask-Login 0.6.2 #25318

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jul 26, 2022

The Google openid auth backend of ours had hard-coded way of
seting the current user which was not compatible with Flask 2.2

With Flask-login 0.6.2 the user is stored in g module of flask, where
before, it was stored in _request_ctx_stack. Unforatunately the
google_openid rather than using _update_request_context_with_user
set the user directly in context. In Flask-login 0.6.2 this stopped
working.

This change switches to use the _update_request_context_with_user
method rather than directly storing user in context which works before
and after the Flask-Login 0.6.2 change.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

The Google openid auth backend of ours had hard-coded way of
seting the current user which was not compatible with Flask 2.2

With Flask-login 0.6.2 the user is stored in g module of flask, where
before, it was stored in _request_ctx_stack. Unforatunately the
google_openid rather than using _update_request_context_with_user
set the user directly in context. In Flask-login 0.6.2 this stopped
working.

This change switches to use the _update_request_context_with_user
method rather than directly storing user in context which works before
and after the Flask-Login 0.6.2 change.
@potiuk
Copy link
Member Author

potiuk commented Jul 26, 2022

@mik-laj - question - i see that you were the original author of the openid integration here. Is this something to be used also beyond the scope of the experimental API ? Is this something that can be used in the new REST API as well (I am trying to understand the extent of the problem and see if this is something we should warn the users against).

@mik-laj
Copy link
Member

mik-laj commented Jul 26, 2022

I am looking at it now, but I need to refresh the Breeze first.

@mik-laj
Copy link
Member

mik-laj commented Jul 26, 2022

Screenshot 2022-07-26 at 23 38 42

Tested and it works with V1 API.

@potiuk potiuk merged commit 8bc1471 into apache:main Jul 26, 2022
@potiuk potiuk deleted the fix-setting-user-by-google-openid branch July 26, 2022 22:01
@mik-laj
Copy link
Member

mik-laj commented Jul 30, 2022

To use this backend with v1 API, i executed the following commands:

export AIRFLOW__LOGGING__LOGGING_LEVEL="DEBUG"
export AIRFLOW__API__AUTH_BACKENDS="airflow.providers.google.common.auth_backend.google_openid"
export AIRFLOW__API__GOOGLE_OAUTH2_AUDIENCE="project-id-random-value.apps.googleusercontent.com"
airflow webserver
gcloud auth activate-service-account --key-file /files/data-archive-303923-85ecb6d96736.json
airflow users create --email test-airflow-openid@data-archive-303923.iam.gserviceaccount.com -f 1 -l 2 -r Admin -u u
ENDPOINT_URL="http://localhost:8080/"
export AIRFLOW__API__GOOGLE_OAUTH2_AUDIENCE=project-id-random-value.apps.googleusercontent.com
AUDIENCE=$AIRFLOW__API__GOOGLE_OAUTH2_AUDIENCE
ID_TOKEN="$(gcloud auth print-identity-token "--audiences=${AUDIENCE}")"
curl -X GET  \
    "${ENDPOINT_URL}/api/v1/pools" \
    -H 'Content-Type: application/json' \
    -H 'Cache-Control: no-cache' \
    -H "Authorization: Bearer ${ID_TOKEN}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants