anzinmhd opened a new pull request, #74371:
URL: https://github.com/apache/airflow/pull/74371
# Fix Keycloak default role permission hierarchy
The Keycloak auth manager's default role permissions do not currently
preserve the expected permission hierarchy between `User`, `Op`, and `Admin`.
An `Admin` user can therefore be denied access to resources granted through
the `Op` permission. For example, an `Admin` attempting to list pools can be
denied because the `Op` permission only contains the `Allow-Op` policy. With
the resource server using `UNANIMOUS`, that denied permission causes the
overall authorization decision to be denied even though the `Admin` permission
itself permits the request.
This PR updates the default role permission configuration so higher roles
are included in the permissions granted to lower roles:
- `User` permission: `Allow-User`, `Allow-Op`, and `Allow-Admin`
- `Op` permission: `Allow-Op` and `Allow-Admin`
These hierarchical resource permissions use the `AFFIRMATIVE` decision
strategy. This allows a higher role to satisfy the permission without also
having to satisfy every lower-role policy.
The resource server can therefore retain its existing `UNANIMOUS` decision
strategy while an `Admin` is correctly authorized for resources available to
`Op`.
The existing unit test for default role permission attachment has been
updated to cover both the role hierarchy and the `AFFIRMATIVE` decision
strategy.
### Verification
Reproduced the issue against the current `main` branch using a local
Keycloak environment.
Before the change, evaluating an `Admin` user for `Pool` with the `LIST`
scope resulted in:
```text
Op -> DENY
ReadOnly -> PERMIT
Admin -> PERMIT
Overall -> DENY
```
After applying the change and regenerating the Keycloak permissions:
```text
Op -> PERMIT
ReadOnly -> PERMIT
Admin -> PERMIT
Overall -> PERMIT
```
The top-level Keycloak resource-server decision strategy remained
`UNANIMOUS` during verification.
Tests:
```text
providers/keycloak/tests/unit/keycloak/auth_manager/cli/test_commands.py
16 passed
```
Related to #74357
This PR currently addresses the default role-hierarchy portion of the issue.
The separate `MENU` permission interaction described in #74357 is still
under investigation and is not addressed by the current change.
---
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull
request.
Write a good git commit message following this guide:
https://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the
documentation.
For user-facing UI changes, please attach before/after screenshots (or a
short
screen recording) so reviewers can assess the visual impact.
Feel free to ping (in general) for the review if you do not see reaction for
a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we
sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
- [x] Yes (ChatGPT)
---
* Read the **[[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information. Note: commit author/co-author name and email in commits
become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal
([[AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
* When adding dependency, check compliance with the [[ASF 3rd Party License
Policy](https://www.apache.org/legal/resolved.html#category-x)](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment:
`{pr_number}.significant.rst`, in
[[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments)](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
You can add this file in a follow-up commit after the PR is created so you
know the PR number.
--
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]
