From 64abd5522ec84dee6090988e9bf30a553b4bde1e Mon Sep 17 00:00:00 2001 From: waterWang Date: Sat, 8 Aug 2026 03:57:30 +0800 Subject: [PATCH] fix(keycloak): bind ReadOnly permission to explicit resources to prevent multi-team isolation bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Problem:** The scope-based permission is created with (unbound), which matches *every* resource carrying GET/LIST/MENU scopes in Keycloak — including per-team resources like . This shadows the correctly built aggregate permissions and grants read access on role alone, bypassing team isolation. **Fix (4 changes):** 1. **Bind to explicit global resources** — both in the (non-teams and teams) and in , so the permission covers only non-team resources (Dag, Connection, Pool, Variable, Team, Asset, AssetAlias, Configuration) instead of matching everything. 2. **Expand to all team-scoped resources** — previously only covered and . Now covers all (Connection, Dag, Pool, Team, Variable), consistent with the permission. 3. **Expand global permission to include global resources** — under , the global Admin permission now covers both team and global resources, so Admin users can interact with shared (teamless) resources as documented. 4. **Set explicitly in for both and permissions** — replaces with the full list of known resources, preventing unbound-scope leakage. Fixes: #71277 --- .../keycloak/auth_manager/cli/commands.py | 29 ++++++++++++------- .../auth_manager/cli/test_commands.py | 29 +++++++++++++++---- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/cli/commands.py b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/cli/commands.py index 6ead332155b17..c23724b1ad5dd 100644 --- a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/cli/commands.py +++ b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/cli/commands.py @@ -267,7 +267,7 @@ def _attach_default_role_permissions( permission_name="ReadOnly", policy_name=_role_policy_name(role_name), scope_names=["GET", "MENU", "LIST"], - resource_names=[], + resource_names=list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), decision_strategy="AFFIRMATIVE", _dry_run=_dry_run, ) @@ -300,7 +300,7 @@ def _attach_default_role_permissions( permission_name="Admin", policy_name=_role_policy_name(role_name), scope_names=_get_extended_resource_methods() + ["LIST"], - resource_names=[], + resource_names=list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), _dry_run=_dry_run, ) @@ -426,6 +426,7 @@ def _get_permissions_to_create( "name": "ReadOnly", "type": "scope-based", "scope_names": ["GET", "MENU", "LIST"], + "resources": list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), }, { "name": "Admin", @@ -463,10 +464,7 @@ def _get_permissions_to_create( "name": f"ReadOnly-{team}", "type": "scope-based", "scope_names": ["GET", "LIST"], - "resources": [ - f"{KeycloakResource.DAG.value}:{team}", - f"{KeycloakResource.TEAM.value}:{team}", - ], + "resources": [f"{resource}:{team}" for resource in TEAM_SCOPED_RESOURCE_NAMES], }, { "name": f"User-{team}", @@ -492,9 +490,11 @@ def _get_permissions_to_create( "name": "Admin", "type": "scope-based", "scope_names": _get_extended_resource_methods() + ["LIST"], - "resources": [ - f"{resource}:{team}" for team in teams for resource in TEAM_SCOPED_RESOURCE_NAMES - ], + "resources": ( + [f"{resource}:{team}" for team in teams for resource in TEAM_SCOPED_RESOURCE_NAMES] + + list(TEAM_SCOPED_RESOURCE_NAMES) + + list(GLOBAL_SCOPED_RESOURCE_NAMES) + ), } ) perm_configs.append( @@ -505,6 +505,14 @@ def _get_permissions_to_create( "resources": list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), } ) + perm_configs.append( + { + "name": "ReadOnly", + "type": "scope-based", + "scope_names": ["GET", "MENU", "LIST"], + "resources": list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), + } + ) perm_configs.append( { "name": "ViewAccess", @@ -780,8 +788,7 @@ def _attach_team_permissions( f"{KeycloakResource.DAG.value}:{team}", ] team_readable_resources = [ - f"{KeycloakResource.DAG.value}:{team}", - f"{KeycloakResource.TEAM.value}:{team}", + f"{resource}:{team}" for resource in sorted(TEAM_SCOPED_RESOURCE_NAMES) ] team_scoped_resources = [f"{resource}:{team}" for resource in sorted(TEAM_SCOPED_RESOURCE_NAMES)] diff --git a/providers/keycloak/tests/unit/keycloak/auth_manager/cli/test_commands.py b/providers/keycloak/tests/unit/keycloak/auth_manager/cli/test_commands.py index 62c61f95f7249..865f11a35cec4 100644 --- a/providers/keycloak/tests/unit/keycloak/auth_manager/cli/test_commands.py +++ b/providers/keycloak/tests/unit/keycloak/auth_manager/cli/test_commands.py @@ -27,6 +27,7 @@ SUPER_ADMIN_ROLE_NAME, TEAM_ROLE_NAMES, TEAM_SCOPED_RESOURCE_NAMES, + GLOBAL_SCOPED_RESOURCE_NAMES, _get_extended_resource_methods, _get_resource_methods, add_user_to_team_command, @@ -274,6 +275,7 @@ def test_create_permissions( "logic": "POSITIVE", "decisionStrategy": "UNANIMOUS", "scopes": ["1", "2", "3"], # GET, MENU, LIST + "resources": ["r1", "r2", "r3"], # Dag, Asset, Connection (matched from TEAM_SCOPED + GLOBAL_SCOPED) }, ), call( @@ -412,7 +414,18 @@ def test_create_permissions_with_teams(self, mock_get_client): "logic": "POSITIVE", "decisionStrategy": "UNANIMOUS", "scopes": ["1", "3"], - "resources": ["r1"], + "resources": ["r1", "r2", "r3", "r4"], # Dag:team-a, Connection:team-a, Pool:team-a, Variable:team-a + }, + ) + client.create_client_authz_scope_permission.assert_any_call( + client_id="test-id", + payload={ + "name": "ReadOnly", + "type": "scope", + "logic": "POSITIVE", + "decisionStrategy": "UNANIMOUS", + "scopes": ["1", "2", "3"], + "resources": ["r6", "r7", "r8", "r9", "r10", "r11", "r12"], # non-team resources }, ) client.create_client_authz_resource_based_permission.assert_any_call( @@ -468,7 +481,7 @@ def test_create_permissions_attaches_default_role_policies( permission_name="ReadOnly", policy_name=f"Allow-{role_name}", scope_names=["GET", "MENU", "LIST"], - resource_names=[], + resource_names=list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), decision_strategy="AFFIRMATIVE", _dry_run=False, ) @@ -478,7 +491,7 @@ def test_create_permissions_attaches_default_role_policies( permission_name="Admin", policy_name="Allow-Admin", scope_names=_get_extended_resource_methods() + ["LIST"], - resource_names=[], + resource_names=list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), _dry_run=False, ) mock_attach_scope_policy.assert_any_call( @@ -487,7 +500,7 @@ def test_create_permissions_attaches_default_role_policies( permission_name="Admin", policy_name="Allow-SuperAdmin", scope_names=_get_extended_resource_methods() + ["LIST"], - resource_names=[], + resource_names=list(TEAM_SCOPED_RESOURCE_NAMES) + list(GLOBAL_SCOPED_RESOURCE_NAMES), _dry_run=False, ) mock_attach_resource_policy.assert_any_call( @@ -573,7 +586,13 @@ def test_create_team_command( permission_name="ReadOnly-team-a", policy_name="Allow-Viewer-team-a", scope_names=["GET", "LIST"], - resource_names=["Dag:team-a", "Team:team-a"], + resource_names=[ + "Connection:team-a", + "Dag:team-a", + "Pool:team-a", + "Team:team-a", + "Variable:team-a", + ], _dry_run=False, ) mock_attach_policy.assert_any_call(