-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Implement multiple API auth backends #21472
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
Conversation
UPDATING.md
Outdated
|
|
||
| ### `auth_backends` replaces `auth_backend` configuration setting | ||
|
|
||
| Previously, only one backend was used to authorize use of the experimental REST API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Previously, only one backend was used to authorize use of the experimental REST API. | |
| Previously, only one backend was used to authorize use of the REST API. |
As (confusingly) the auth_backend is used by both new and old APIs.
Hmmm I wonder if this would un-intentionally make the old API available again? The Default auth backend of deny_all effectively made the old API not usable I think)
airflow/api/client/__init__.py
Outdated
| auth_backends = api.load_auth() | ||
| session = None | ||
| session_factory = getattr(auth_backend, 'create_client_session', None) | ||
| session_factory = getattr(auth_backends, 'create_client_session', None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would need to loop over the list and check for these attributes in some form or other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I pushed the branch after a search/replace but not necessarily in a shippable state. :)
I've fixed this somewhat in 3f3126f, and whilst it now satisfies the tests it is not doing the right thing. There's an assumption of one auth in the returned api_client which means more work than merely making the tests pass by returning the first backend.
d97e888 to
ead413a
Compare
|
In the card description @uranusjr wrote: "The backends are queried one by one, and the first valid identity returned by anyone is used (and 403 if none of the backends recognise the request)." but in the existing tests there was a difference between deny_all->403 and auth_failed->401, so I have retained that. |
Following the change in Airflow apache/airflow#21472, we update this as well
Following the change in Airflow apache/airflow#21472, we update this as well
As part of AIP-42, the
auth_backendsetting is expanded toauth_backends, and on an API request each is tried one after the other until one succeeds. A new auth backend ofsessionis added that will validate against the signed-in user in the case where requests are made via JavaScript from the UI.