Implement multiple API auth backends #21472
Conversation
norm
requested review from
ashb,
bbovenzi,
dstandish,
ephraimbuddy,
jedcunningham,
jhtimmins,
kaxil,
mik-laj,
potiuk and
ryanahamilton
as code owners
boring-cyborg
bot
added
area:API
UPDATING.md
Outdated
| @@ -130,6 +130,13 @@ Previously, a task’s log is dynamically rendered from the `[core] log_filename | |||
|
|
|||
| A new `log_template` table is introduced to solve this problem. This table is synchronised with the aforementioned config values every time Airflow starts, and a new field `log_template_id` is added to every DAG run to point to the format used by tasks (`NULL` indicates the first ever entry for compatibility). | |||
|
|
|||
| ### `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.
| 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
| 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.
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.
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.
norm
force-pushed
the
rest-api-backends
branch
from
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.