abhishekmauryaKsolves commented on PR #70647: URL: https://github.com/apache/airflow/pull/70647#issuecomment-5129370877
Digging into this further to understand the actual conflict between the two suggestions: I confirmed that Keycloak resources are registered generically per-type, not per-instance — see resources.py's KeycloakResource enum and commands.py:350 (standard_resources = [(resource.value, scopes) for resource in KeycloakResource]). There's a single "Connection" resource in Keycloak, not one resource per connection ID. Per-instance authorization for connections/pools/variables/dags works entirely through the resource_id claim, pushed via claim_token in _get_payload (see _is_authorized, line ~422). The problem: Keycloak's batch UMA-ticket endpoint (_get_batch_payload) takes a single permission array and does not support a claim_token at all — there's one shared claim context for the whole request, not one per permission item. That means there is no way to batch N different resource_id values into a single Keycloak call while still discriminating between them. This is different from filter_authorized_menu_items, where each menu category (Assets, Connections, etc.) actually is its own registered Keycloak resource — no resource_id claim is needed there, since the resource name itself carries the identity, so batching works cleanly. So @vincbeck is right that the current batch refactor silently drops fine-grained access — and @stephen-bracken's suggestion, while a great optimization for resources that are individually registered in Keycloak (like menu items), doesn't hold for connections/pools/variables/dags, which rely on the resource_id claim mechanism instead. Proposal: revert batch_is_authorized_connection, batch_is_authorized_dag, batch_is_authorized_pool, batch_is_authorized_variable, filter_authorized_connections, filter_authorized_pools, and filter_authorized_variables back to the original N-parallel-calls-via-is_authorized_* approach (with ThreadPoolExecutor + single_flight caching, as before) — this is the only way to preserve per-instance checks given how these resources are registered. I'll keep _is_batch_authorized reserved for cases where the resource itself is individually registered in Keycloak. Let me know if this matches your understanding, or if there's a way to register per-instance resources in Keycloak that I'm missing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
