Skip to content

Add REST API auth flow for the new UI#15042

Closed
ephraimbuddy wants to merge 53 commits into
apache:mainfrom
astronomer:api-auth-flow
Closed

Add REST API auth flow for the new UI#15042
ephraimbuddy wants to merge 53 commits into
apache:mainfrom
astronomer:api-auth-flow

Conversation

@ephraimbuddy

@ephraimbuddy ephraimbuddy commented Mar 26, 2021

Copy link
Copy Markdown
Contributor

This PR is to provide authentication to the new UI. The implementation uses the existing Flask Appbuilder authentication method configurations.
First, the react app checks the configuration on the server by sending a GET request to api/v1/auth-info. This endpoint returns information about the configured auth method. With the information, the react app can then present a login form to the user if necessary.

Below I explain the flow for OAuth

OAuth
This authentication uses two endpoints, one is used to get the authorization URL while the other is used for verification and sign-in. The react app makes a GET request to api/v1/auth-oauth/{provider} with the provider name and redirect url e.g api/v1/auth-oauth/google?redirect_url=http:localhost:5000, the endpoint responds with authorization URL which the react app will redirect users to.
When the user is authenticated by the provider, the provider will redirect user to the redirect_url provided by the react app. The react app should handle the redirection, and make a GET request to server at api/v1/oauth-authorized/{provider} with the state returned from the provider. If everything is OK, user is logged in and user data is returned.

The one advantage we can get using the refresh token is to get ride of auth_backend.

OpenID is not currently implemented.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@boring-cyborg boring-cyborg Bot added area:API Airflow's REST/HTTP API area:webserver Webserver related Issues labels Mar 26, 2021
@ephraimbuddy ephraimbuddy force-pushed the api-auth-flow branch 2 times, most recently from 6b32c58 to e83eb4a Compare March 29, 2021 14:03
@ephraimbuddy ephraimbuddy marked this pull request as ready for review March 29, 2021 14:58
@kaxil kaxil requested a review from jhtimmins March 29, 2021 22:08
Comment thread airflow/api_connexion/schemas/auth_schema.py Outdated
Comment thread airflow/api_connexion/openapi/v1.yaml Outdated
Comment thread tests/api_connexion/endpoints/test_auth_endpoint.py Outdated
@github-actions

Copy link
Copy Markdown
Contributor

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions

Copy link
Copy Markdown
Contributor

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

Comment thread airflow/api_connexion/openapi/v1.yaml Outdated
@ephraimbuddy ephraimbuddy marked this pull request as draft April 6, 2021 08:47
@github-actions

github-actions Bot commented Apr 6, 2021

Copy link
Copy Markdown
Contributor

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions

github-actions Bot commented Apr 6, 2021

Copy link
Copy Markdown
Contributor

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions

github-actions Bot commented Apr 6, 2021

Copy link
Copy Markdown
Contributor

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions

github-actions Bot commented Apr 6, 2021

Copy link
Copy Markdown
Contributor

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.

@ephraimbuddy ephraimbuddy marked this pull request as ready for review April 7, 2021 06:50
@github-actions

github-actions Bot commented Aug 4, 2021

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions Bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Aug 4, 2021
AUTH_OID: "auth_oid",
AUTH_OAUTH: "auth_oauth",
AUTH_REMOTE_USER: "auth_remote_user",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think type_mapping can be a constant & be out of this function.

schema:
$ref: '#/components/schemas/JwtAuthAndUser'
'401':
$ref: '#/components/responses/Unauthenticated'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't we need more status codes like 50X which happens when the auth configs are wrongly setup & callback fails?

data = token_schema.load(body)
except ValidationError as err:
raise BadRequest(detail=str(err.messages))
token = decode_token(data['token'])

@msumit msumit Aug 4, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Couldn't understand the logic here or maybe I misinterpreted it. How is a user going to provide the token of another user?

@ephraimbuddy

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews, @msumit, I will revisit this and may have to open a new PR and address a lot of things

@github-actions github-actions Bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Aug 9, 2021
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions Bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Sep 23, 2021
@github-actions github-actions Bot closed this Sep 29, 2021
@ephraimbuddy ephraimbuddy deleted the api-auth-flow branch February 14, 2022 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:webserver Webserver related Issues stale Stale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants