Fix DAG-level RBAC by using claim_token per UMA spec (#61137)#61283
Fix DAG-level RBAC by using claim_token per UMA spec (#61137)#61283bugraoz93 merged 5 commits intoapache:mainfrom
Conversation
There was a problem hiding this comment.
lgtm
EDIT: Is a bump of the version number already needed with this PR? https://github.com/apache/airflow/blob/main/providers/keycloak/src/airflow/providers/keycloak/__init__.py#L32
providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py
Outdated
Show resolved
Hide resolved
|
I tested the changes locally and can confirm that it works as expected - thanks a lot for the quick implementation. Small nit on your example in the PR, the id of the requested dag is in key |
vincbeck
left a comment
There was a problem hiding this comment.
Thanks for fixing that issue. I added some comments, also, can you update/create test to cover that?
providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py
Outdated
Show resolved
Hide resolved
Ensure consistent JSON key ordering by using sort_keys=True in json.dumps() when generating claim_token for UMA ticket requests. This fixes test failures caused by non-deterministic dictionary ordering in Python. The claim_token is base64-encoded JSON, so consistent key ordering is required for test assertions and predictable behavior.
- Fix documentation URL anchor from #_service_authorization_pushing_claims to #_service_pushing_claims - Remove changelog entry as requested by release manager - Add comprehensive parametrized tests for _get_payload method covering claim_token functionality
3ce9b7c to
522ef10
Compare
bugraoz93
left a comment
There was a problem hiding this comment.
Still a quite bit of CI failing. Could you please check?
8c7cb34 to
d71e45e
Compare
|
🥳 |
* Fix DAG-level RBAC by using claim_token per UMA spec (apache#61137) * Fix claim_token ordering in Keycloak auth manager Ensure consistent JSON key ordering by using sort_keys=True in json.dumps() when generating claim_token for UMA ticket requests. This fixes test failures caused by non-deterministic dictionary ordering in Python. The claim_token is base64-encoded JSON, so consistent key ordering is required for test assertions and predictable behavior. * Address review comments: fix doc URL, remove changelog, add tests - Fix documentation URL anchor from #_service_authorization_pushing_claims to #_service_pushing_claims - Remove changelog entry as requested by release manager - Add comprehensive parametrized tests for _get_payload method covering claim_token functionality
Thanks! |
* Fix DAG-level RBAC by using claim_token per UMA spec (apache#61137) * Fix claim_token ordering in Keycloak auth manager Ensure consistent JSON key ordering by using sort_keys=True in json.dumps() when generating claim_token for UMA ticket requests. This fixes test failures caused by non-deterministic dictionary ordering in Python. The claim_token is base64-encoded JSON, so consistent key ordering is required for test assertions and predictable behavior. * Address review comments: fix doc URL, remove changelog, add tests - Fix documentation URL anchor from #_service_authorization_pushing_claims to #_service_pushing_claims - Remove changelog entry as requested by release manager - Add comprehensive parametrized tests for _get_payload method covering claim_token functionality
* Fix DAG-level RBAC by using claim_token per UMA spec (apache#61137) * Fix claim_token ordering in Keycloak auth manager Ensure consistent JSON key ordering by using sort_keys=True in json.dumps() when generating claim_token for UMA ticket requests. This fixes test failures caused by non-deterministic dictionary ordering in Python. The claim_token is base64-encoded JSON, so consistent key ordering is required for test assertions and predictable behavior. * Address review comments: fix doc URL, remove changelog, add tests - Fix documentation URL anchor from #_service_authorization_pushing_claims to #_service_pushing_claims - Remove changelog entry as requested by release manager - Add comprehensive parametrized tests for _get_payload method covering claim_token functionality
Fix DAG-level RBAC by using claim_token per UMA specification
closes: #61137
Description
This PR fixes a bug in the Keycloak Auth Manager where context attributes (such as
dag_id) were not being properly transmitted to Keycloak during authorization requests, preventing DAG-level RBAC policies from functioning.Problem
When attempting to configure DAG-level authorization policies in Keycloak, the
dag_idand other context attributes were not available to JavaScript-based policies. Network traces showed that instead of sending the actual attribute values, the literal stringcontext=attributeswas being transmitted in the authorization request.Root Cause:
The original implementation used a non-standard
contextparameter with a nested dictionary:This approach failed because:
requestslibrarycontextparameter for authorization requestsSolution
Implemented the correct UMA specification for pushing claims to Keycloak by using the
claim_tokenparameter:Key changes:
claim_tokenparameter instead ofcontext(per UMA spec)claim_token_formatparameterReference: Keycloak Authorization Services - Pushing Claims
Impact
After this fix:
dag_idand other context attributes are accessible in Keycloak policiesExample Keycloak policy that now works:
Testing
ruff formatandruff checkvalidationChanges Made
Modified files:
providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py- Added
import base64- Updated
_get_payload()method to useclaim_tokenparameterproviders/keycloak/docs/changelog.rst- Added bug fix entry for version 0.5.2
Was generative AI tooling used to co-author this PR?
Generated-by: GitHub Copilot