This is an automated email from the ASF dual-hosted git repository.
vincbeck pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 6f2820c7993 Fix Keycloak multi-team permission repair (#71614)
6f2820c7993 is described below
commit 6f2820c79939e47e0ef255fa116728448b3af9d2
Author: Vincent Hsiao <[email protected]>
AuthorDate: Fri Aug 14 22:41:04 2026 +0800
Fix Keycloak multi-team permission repair (#71614)
---
providers/keycloak/docs/changelog.rst | 8 +--
.../keycloak/auth_manager/cli/commands.py | 41 ++++++++++++-
.../keycloak/auth_manager/cli/test_commands.py | 71 ++++++++++++++++++++++
3 files changed, 115 insertions(+), 5 deletions(-)
diff --git a/providers/keycloak/docs/changelog.rst
b/providers/keycloak/docs/changelog.rst
index 64f75516305..76b5a6bf2c1 100644
--- a/providers/keycloak/docs/changelog.rst
+++ b/providers/keycloak/docs/changelog.rst
@@ -27,10 +27,10 @@ Changelog
.. note::
Upgrading the provider does not update existing Keycloak permissions. For
each existing team,
- run ``airflow keycloak-auth-manager create-team <team>`` again to update
the team ReadOnly
- permission. For non-team installations, run
- ``airflow keycloak-auth-manager create-permissions`` again without
``--teams`` to update the
- global Admin permission.
+ run ``airflow keycloak-auth-manager create-team <team>`` again to update
the team-specific
+ permissions and repair the global ``ReadOnly`` and ``Admin`` permissions.
For non-team
+ installations, run ``airflow keycloak-auth-manager create-permissions``
again without
+ ``--teams`` to update the global ``ReadOnly`` and ``Admin`` permissions.
Manually added policies attached to these permissions will also be
evaluated under the
``AFFIRMATIVE`` strategy after the update.
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 5992e64e425..4daeef9b008 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
@@ -306,6 +306,40 @@ def _attach_default_role_permissions(
)
+def _update_read_only_permission_resources(
+ client: KeycloakAdmin, client_uuid: str, *, _dry_run: bool = False
+) -> None:
+ if _dry_run:
+ print("Would update permission 'ReadOnly' with unscoped readable
resources.")
+ return
+
+ permissions = client.get_client_authz_permissions(client_uuid)
+ match = next((perm for perm in permissions if perm.get("name") ==
"ReadOnly"), None)
+ if not match:
+ return
+
+ permission_id = match["id"]
+ scopes = client.get_client_authz_scopes(client_uuid)
+ resources = client.get_client_authz_resources(client_uuid)
+ scope_ids = [s["id"] for s in scopes if s["name"] in ["GET", "MENU",
"LIST"]]
+ resource_names = TEAM_SCOPED_RESOURCE_NAMES | GLOBAL_SCOPED_RESOURCE_NAMES
+ resource_ids = [r["_id"] for r in resources if r["name"] in resource_names]
+ policy_ids = _get_permission_policy_ids(client, client_uuid, permission_id)
+ payload = {
+ "id": permission_id,
+ "name": "ReadOnly",
+ "type": "scope",
+ "logic": "POSITIVE",
+ "decisionStrategy": "AFFIRMATIVE",
+ "scopes": scope_ids,
+ "resources": resource_ids,
+ "policies": policy_ids,
+ }
+ client.update_client_authz_scope_permission(
+ payload=payload, client_id=client_uuid, scope_id=permission_id
+ )
+
+
def _preview_scopes(*args, **kwargs):
"""Preview scopes that would be created."""
scopes = _get_scopes_to_create()
@@ -702,7 +736,11 @@ def _update_admin_permission_resources(
or r["name"] in GLOBAL_SCOPED_RESOURCE_NAMES
]
- policy_ids = _get_permission_policy_ids(client, client_uuid, permission_id)
+ existing_policy_ids = _get_permission_policy_ids(client, client_uuid,
permission_id)
+ super_admin_policy_id = _get_policy_id(client, client_uuid,
_role_policy_name(SUPER_ADMIN_ROLE_NAME))
+ policy_ids = [policy_id for policy_id in existing_policy_ids if policy_id
== super_admin_policy_id]
+ if super_admin_policy_id and super_admin_policy_id not in policy_ids:
+ policy_ids.append(super_admin_policy_id)
payload = {
"id": permission_id,
"name": "Admin",
@@ -736,6 +774,7 @@ def create_team_command(args):
_attach_team_permissions(client, client_uuid, team, _dry_run=args.dry_run)
_attach_team_menu_permissions(client, client_uuid, team,
_dry_run=args.dry_run)
_attach_superadmin_permissions(client, client_uuid, team,
_dry_run=args.dry_run)
+ _update_read_only_permission_resources(client, client_uuid,
_dry_run=args.dry_run)
_update_admin_permission_resources(client, client_uuid,
_dry_run=args.dry_run)
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 f4b9c714fca..bb31ebbbc99 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
@@ -29,6 +29,8 @@ from airflow.providers.keycloak.auth_manager.cli.commands
import (
TEAM_SCOPED_RESOURCE_NAMES,
_get_extended_resource_methods,
_get_resource_methods,
+ _update_admin_permission_resources,
+ _update_read_only_permission_resources,
add_user_to_team_command,
create_all_command,
create_permissions_command,
@@ -438,6 +440,72 @@ class TestCommands:
skip_exists=True,
)
+
@patch("airflow.providers.keycloak.auth_manager.cli.commands._get_permission_policy_ids")
+ def test_update_read_only_permission_resources_excludes_team_resources(
+ self, mock_get_permission_policy_ids
+ ):
+ client = Mock()
+ client.get_client_authz_permissions.return_value = [{"id":
"readonly-id", "name": "ReadOnly"}]
+ client.get_client_authz_scopes.return_value = [
+ {"id": "scope-get", "name": "GET"},
+ {"id": "scope-list", "name": "LIST"},
+ {"id": "scope-menu", "name": "MENU"},
+ {"id": "scope-put", "name": "PUT"},
+ ]
+ client.get_client_authz_resources.return_value = [
+ {"_id": "dag-global", "name": "Dag"},
+ {"_id": "dag-team-a", "name": "Dag:team-a"},
+ {"_id": "variable-global", "name": "Variable"},
+ {"_id": "variable-team-a", "name": "Variable:team-a"},
+ {"_id": "asset-global", "name": "Asset"},
+ ]
+ mock_get_permission_policy_ids.return_value = ["viewer-policy",
"admin-policy"]
+
+ _update_read_only_permission_resources(client, "test-id")
+
+ client.update_client_authz_scope_permission.assert_called_once_with(
+ client_id="test-id",
+ scope_id="readonly-id",
+ payload={
+ "id": "readonly-id",
+ "name": "ReadOnly",
+ "type": "scope",
+ "logic": "POSITIVE",
+ "decisionStrategy": "AFFIRMATIVE",
+ "scopes": ["scope-get", "scope-list", "scope-menu"],
+ "resources": ["dag-global", "variable-global", "asset-global"],
+ "policies": ["viewer-policy", "admin-policy"],
+ },
+ )
+
+
@patch("airflow.providers.keycloak.auth_manager.cli.commands._get_policy_id")
+
@patch("airflow.providers.keycloak.auth_manager.cli.commands._get_permission_policy_ids")
+ def test_update_admin_permission_resources_removes_admin_role_policy(
+ self, mock_get_permission_policy_ids, mock_get_policy_id
+ ):
+ client = Mock()
+ client.get_client_authz_permissions.return_value = [{"id": "admin-id",
"name": "Admin"}]
+ client.get_client_authz_scopes.return_value = [
+ {"id": "scope-get", "name": "GET"},
+ {"id": "scope-list", "name": "LIST"},
+ {"id": "scope-put", "name": "PUT"},
+ ]
+ client.get_client_authz_resources.return_value = [
+ {"_id": "dag-global", "name": "Dag"},
+ {"_id": "dag-team-a", "name": "Dag:team-a"},
+ {"_id": "dag-team-b", "name": "Dag:team-b"},
+ {"_id": "asset-global", "name": "Asset"},
+ ]
+ mock_get_permission_policy_ids.return_value = ["admin-policy",
"superadmin-policy"]
+ mock_get_policy_id.return_value = "superadmin-policy"
+
+ _update_admin_permission_resources(client, "test-id")
+
+ client.update_client_authz_scope_permission.assert_called_once()
+ payload =
client.update_client_authz_scope_permission.call_args.kwargs["payload"]
+ assert payload["policies"] == ["superadmin-policy"]
+ assert payload["resources"] == ["dag-team-a", "dag-team-b",
"asset-global"]
+
@patch("airflow.providers.keycloak.auth_manager.cli.commands._attach_policy_to_resource_permission")
@patch("airflow.providers.keycloak.auth_manager.cli.commands._attach_policy_to_scope_permission")
@patch("airflow.providers.keycloak.auth_manager.cli.commands._ensure_role_policy")
@@ -512,6 +580,7 @@ class TestCommands:
)
@patch("airflow.providers.keycloak.auth_manager.cli.commands._update_admin_permission_resources")
+
@patch("airflow.providers.keycloak.auth_manager.cli.commands._update_read_only_permission_resources")
@patch("airflow.providers.keycloak.auth_manager.cli.commands._ensure_scope_permission")
@patch("airflow.providers.keycloak.auth_manager.cli.commands._attach_policy_to_resource_permission")
@patch("airflow.providers.keycloak.auth_manager.cli.commands._attach_policy_to_scope_permission")
@@ -532,6 +601,7 @@ class TestCommands:
mock_attach_policy,
mock_attach_resource_policy,
mock_ensure_scope_permission,
+ mock_update_read_only_permission_resources,
mock_update_admin_permission_resources,
):
client = Mock()
@@ -568,6 +638,7 @@ class TestCommands:
mock_create_permissions.assert_called_once_with(
client, "test-id", teams=["team-a"], include_global_admin=False,
_dry_run=False
)
+
mock_update_read_only_permission_resources.assert_called_once_with(client,
"test-id", _dry_run=False)
mock_update_admin_permission_resources.assert_called_once_with(client,
"test-id", _dry_run=False)
mock_ensure_group_policy.assert_called_once_with(client, "test-id",
"team-a", _dry_run=False)
assert mock_ensure_aggregate_policy.call_count == 4
