Keycloak auth manager: enforce team‑scoped authorization (AIP‑67)

[stegololz]

Description

This PR updates the Keycloak auth manager to enforce multi‑team authorization as outlined in AIP‑67. When core.multi_team is enabled, team context (e.g., team_name from DAGs, connections, assets) is now used to build team‑scoped permission checks so users can only list and access resources for their teams. The authorization flow also supports users in multiple teams by evaluating team‑scoped permissions consistently for list and non‑list requests, while preserving backward compatibility for non‑multi‑team deployments.

This is the auth‑manager part of the split. The CLI provisioning changes are in the companion PR: 61256

related issue: #60885

Open Questions:

• Scope alignment: the first review raised questions about which resources should be team‑scoped and whether that should align strictly with resource_details.py. I’m still validating the intended model and would welcome guidance on the correct scope.

Was generative AI tooling used to co-author this PR?

• Yes (Codex)

[stegololz]

Hello,

Latest commit fixes many comments from the review done on branch #61256 before the split.

• Team scoped authorization is now aligned with model available on resource_details.py

• About the “weird LIST if clause”: I first removed it completely on my local environment to get back the 403 errors and understand them.

The 403 actually comes from the API auth flow. e.g. for dags, when the UI calls GET /api/v2/dags/~/dagRuns, flow goes like this:

API goes through requires_access_dag function

    dag_id = dag_id if dag_id != "~" else None
    team_name = DagModel.get_team_name(dag_id) if dag_id else None

It normalizes ~ to None, so team_name=None, and then calls is_authorized_dag , that goes into _is_authorized.

    if resource_id:
        context_attributes[RESOURCE_ID_ATTRIBUTE_NAME] = resource_id
    elif method == "GET":
        method = "LIST"

Since there’s no resource_id, _is_authorized converts GET -> LIST and checks Dag#LIST.

With my first multi‑team implementation, only team‑scoped resources existed (e.g. Dag:team), so a global Dag#LIST check was denied -> 403.

First implementation did not solve this cleanly, so I refactored: for teamless LIST, I now check a global LIST permission on the global resource (e.g. Dag#LIST, no :team).
This keeps team_name required for non‑LIST while letting list endpoints proceed.

There is a theoretical caveat as it assumes every list endpoint applies permission filters later. I verified the code locally and it seems true for dags/dagRuns/pools/connections/variables/tags/dashboard stats, but I'd like your validation on this point to know if my assumption is correct.

I’ll keep the PR as draft for now because I still have unresolved 403 errors around assets.

[dabla]

Nice work @stegololz , multi-team support in Keycloak is coming soon :-)

[stegololz]

I’ll keep the PR as draft for now because I still have unresolved 403 errors around assets.

Solved. Non team scoped resources ( Asset, AssetAlias, Configuration) also have LIST calls. I created relevant permissions in Keycloak. CLI updated to make it work: c6eb21e

[bugraoz93]

Thanks for the PR @stegololz! Looks good!
I cannot see c6eb21e in the PR that you mentioned includes CLI changes. Are you planning to create another PR for CLI? I would suggest to please include into this PR so there would be a clear connectionwith the current implementation, as I see +7 -2 only, which is not high.
Could you please also check the tests? It fails for permission in Python 3.10 and in some others
https://github.com/apache/airflow/actions/runs/21682494606/job/62540136484?pr=61351
I would still like to get @o-nikolas's and @vincbeck's take on the multi-team approach, when you have time

[vincbeck]

Thanks for the PR @stegololz! Looks good! I cannot see c6eb21e in the PR that you mentioned includes CLI changes. Are you planning to create another PR for CLI? I would suggest to please include into this PR so there would be a clear connectionwith the current implementation, as I see +7 -2 only, which is not high. Could you please also check the tests? It fails for permission in Python 3.10 and in some others https://github.com/apache/airflow/actions/runs/21682494606/job/62540136484?pr=61351 I would still like to get @o-nikolas's and @vincbeck's take on the multi-team approach, when you have time

Hey Bugra, thanks for the review. We agreed with @stegololz to split the PR in 2 PRs, one for CLI and one for the auth manager implementation. It makes it easier to review it :) You can find the PR related to CLI here: #61256 but overall you can track the overall effort in this issue: #60885

[stegololz]

Thanks for the PR @stegololz! Looks good! I cannot see c6eb21e in the PR that you mentioned includes CLI changes. Are you planning to create another PR for CLI? I would suggest to please include into this PR so there would be a clear connectionwith the current implementation, as I see +7 -2 only, which is not high. Could you please also check the tests? It fails for permission in Python 3.10 and in some others https://github.com/apache/airflow/actions/runs/21682494606/job/62540136484?pr=61351 I would still like to get @o-nikolas's and @vincbeck's take on the multi-team approach, when you have time

Thanks for the review! Vincent already pointed you to the other other and the issue.
About the tests, last commit fix them locally, and the last rebase should be enough for the static checks.

[bugraoz93]

Hey Bugra, thanks for the review. We agreed with @stegololz to split the PR in 2 PRs, one for CLI and one for the auth manager implementation. It makes it easier to review it :) You can find the PR related to CLI here: #61256 but overall you can track the overall effort in this issue: #60885

Thanks for the details Vincent!t Thats why I assumed I may missing some context and specifically tagged both :)
Sorry @stegololz for confusion :)

[vincbeck]

We're close! :)

[stegololz]

Thanks for your review @vincbeck !

The tests evolved alongside the iterations on keycloak_auth_manager.py throughout this PR. After the last round of changes, the test file clearly needed a refactor to keep the intent of each test case explicit. That refactor is now done. The following tests were added for multi-team support:

• test_team_name_ignored_when_multi_team_disabled — when multi_team is off, team_name in details has no effect on the permission string (stays global, e.g. Dag#GET)
• test_with_team_name_uses_team_scoped_permission — when multi_team is on and team_name is set, the permission is scoped (e.g. Dag:team-a#GET)
• test_without_team_name_uses_global_permission — when multi_team is on but no team_name, the permission stays global
• test_list_without_team_name_uses_global_permission — LIST without team_name produces a global permission (e.g. Dag#LIST)
• test_list_with_team_name_uses_team_scoped_permission — LIST with team_name produces a scoped permission (e.g. Dag:team-a#LIST)
• test_list_with_mismatched_team_delegates_to_keycloak — when the user's team doesn't match the requested team, the request is still forwarded to Keycloak (not short-circuited) to allow cross-team policies
• test_filter_authorized_dag_ids_team_mismatch / test_filter_authorized_dag_ids_team_match — filter_authorized_dag_ids delegates per dag and returns the correct subset
• test_filter_authorized_pools_no_team_returns_empty — filter_authorized_pools delegates to is_authorized_pool correctly

The refactor should cover your last review!

[vincbeck]

Thanks for your review @vincbeck !

The tests evolved alongside the iterations on keycloak_auth_manager.py throughout this PR. After the last round of changes, the test file clearly needed a refactor to keep the intent of each test case explicit. That refactor is now done. The following tests were added for multi-team support:

• test_team_name_ignored_when_multi_team_disabled — when multi_team is off, team_name in details has no effect on the permission string (stays global, e.g. Dag#GET)
• test_with_team_name_uses_team_scoped_permission — when multi_team is on and team_name is set, the permission is scoped (e.g. Dag:team-a#GET)
• test_without_team_name_uses_global_permission — when multi_team is on but no team_name, the permission stays global
• test_list_without_team_name_uses_global_permission — LIST without team_name produces a global permission (e.g. Dag#LIST)
• test_list_with_team_name_uses_team_scoped_permission — LIST with team_name produces a scoped permission (e.g. Dag:team-a#LIST)
• test_list_with_mismatched_team_delegates_to_keycloak — when the user's team doesn't match the requested team, the request is still forwarded to Keycloak (not short-circuited) to allow cross-team policies
• test_filter_authorized_dag_ids_team_mismatch / test_filter_authorized_dag_ids_team_match — filter_authorized_dag_ids delegates per dag and returns the correct subset
• test_filter_authorized_pools_no_team_returns_empty — filter_authorized_pools delegates to is_authorized_pool correctly

The refactor should cover your last review!

Overall I like the tests, thank you! I added some comments, more style related which would help having a code easier to read

[vincbeck]

Thanks @stegololz for the hard work!! Looks really nice :) Will merge once the CI is green

[boring-cyborg]

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.