anzinmhd commented on PR #74371: URL: https://github.com/apache/airflow/pull/74371#issuecomment-6129616422
I've updated the implementation and PR description following the review, including explicit resource-server `AFFIRMATIVE` configuration, removal of unused generic Menu permissions, the `DefaultMenuAccess` name, prerequisite validation, and improved dry-run previews. Validation completed: - Ruff checks passed. - Keycloak CLI unit tests: 79 passed. - Fresh non-team client on Keycloak 23.0.6: 480/480 manually executed authorization checks passed across Viewer, User, Op, Admin, SuperAdmin, and no role. - Airflow Pools UI: Viewer could read pools but creation returned 403; Op could create, update, and delete a temporary pool. I'd appreciate guidance on three remaining points: 1. **Existing realms and migration:** this PR does not remove legacy/custom permissions or automatically migrate realms during a provider upgrade. Existing grants may still affect effective access. Should migration/cleanup be included here, or handled separately? Our live verification covered a fresh client, not an upgraded realm. 2. **Automated behavioral coverage:** the independent role × resource × scope matrix was run manually; the local harness is not included in this PR. Would you prefer a live Keycloak integration test in this PR to address the requested behavioral coverage? 3. **Menu visibility:** the generated defaults still allow all five roles to read resources and access menu entries. Write permissions are restricted, but stricter Admin-only menu visibility from the linked issue is not implemented. Is retaining those read/menu defaults appropriate for this PR's scope? The PR description now documents the access matrix, manual verification results, and these limitations. -- 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]
