fix: improve realm_exists method by using a lightweight endpoint - #33
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesRealm validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pas/plugins/kimug/utils.py`:
- Around line 640-642: Update the admin-access probe described near the users
migration flow to use an endpoint requiring the same create/update-user
privileges as the migration’s write path, rather than relying on /users/count;
alternatively, rename the method and adjust its contract to explicitly promise
only user-count access if that is the intended behavior.
- Around line 649-650: Update the requests.head call in the realm URL probe to
explicitly enable redirects with allow_redirects=True, while preserving the
existing timeout and non-200 status handling.
- Around line 649-650: Update realm_exists to call requests.head with redirects
enabled and handle RequestException for both the HEAD and GET requests,
returning the existing failure result instead of propagating network errors.
Replace reliance on /admin/realms/{realm}/users/count with a check that verifies
full realm-management access, avoiding false positives from
view-users/query-users permissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35192c23-da6b-4be5-a98a-ae31b77e8c86
📒 Files selected for processing (2)
CHANGES.mdsrc/pas/plugins/kimug/utils.py
| (tiny cached endpoint, no RealmRepresentation rebuild) followed by a GET on | ||
| /admin/realms/{realm}/users/count to confirm effective admin access with a | ||
| minimal integer payload. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file first, then inspect the relevant sections.
wc -l src/pas/plugins/kimug/utils.py
ast-grep outline src/pas/plugins/kimug/utils.py --view expanded
sed -n '600,700p' src/pas/plugins/kimug/utils.py
rg -n "users/count|view-users|manage-users|realm-management|can count users|count users" src/pas/plugins/kimug/utils.pyRepository: IMIO/pas.plugins.kimug
Length of output: 7131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect callers and nearby logic for realm_exists and admin operations.
rg -n "realm_exists\(" -n src/pas/plugins/kimug/utils.py src -g '*.py'
sed -n '430,520p' src/pas/plugins/kimug/utils.py
sed -n '700,740p' src/pas/plugins/kimug/utils.pyRepository: IMIO/pas.plugins.kimug
Length of output: 5541
🌐 Web query:
Keycloak users/count endpoint permissions view-users manage-users realm-management docs
💡 Result:
To access the Keycloak users/count endpoint via the Admin REST API, a user must possess sufficient permissions granted through the realm-management client [1]. Permission Requirements: The count operation on the Users resource requires permissions equivalent to querying or viewing users [2]. Specifically, the evaluator for this resource requires that the caller has at least one of the following administrative roles, which are defined within the realm-management client [2][1]: 1. view-users: Allows an administrator to view all users and groups in the realm [3][4]. 2. manage-users: Provides broader access, including all privileges granted by view-users, as well as the ability to manage user memberships, roles, and groups [3][4]. 3. query-users: A more specific role that provides permission to search and query user information [4]. Technical Implementation: The UsersResource API class handles these requests, and the underlying logic (UserPermissionEvaluator) verifies that the caller has at least one of these roles [2][5][6]. If the feature is enabled, fine-grained authorization (FGAP) may also grant access if specific permissions are configured for the user resource [3][2]. In standard configurations, possessing the view-users or manage-users role is sufficient to successfully call the count endpoint [2][5]. If a user lacks these roles, the API will return a 403 Forbidden status [7]. Ensure that the service account or user account being used to authenticate the API call is mapped to at least one of these roles under the realm-management client for the respective realm [4][1].
Citations:
- 1: https://github.com/keycloak/keycloak/blob/main/docs/documentation/server_admin/topics/admin-console-permissions/per-realm.adoc
- 2: https://www.keycloak.org/docs-api/26.5.4/javadocs/org/keycloak/services/resources/admin/fgap/UserPermissionEvaluator.html
- 3: https://github.com/keycloak/keycloak/blob/main/docs/documentation/server_admin/topics/admin-console-permissions/fine-grain-v2.adoc
- 4: https://docs.redhat.com/en/documentation/red_hat_build_of_keycloak/26.2/html/server_administration_guide/admin_permissions
- 5: https://www.keycloak.org/docs-api/26.4.6/javadocs/org/keycloak/admin/client/resource/UsersResource.html
- 6: https://access.redhat.com/webassets/avalon/d/red_hat_build_of_keycloak-26.6/javadocs/org/keycloak/services/resources/admin/UsersResource.html
- 7: https://github.com/keycloak/keycloak/blob/main/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Keycloak user access path that follows realm_exists.
sed -n '220,360p' src/pas/plugins/kimug/utils.py
sed -n '360,470p' src/pas/plugins/kimug/utils.py
sed -n '470,620p' src/pas/plugins/kimug/utils.pyRepository: IMIO/pas.plugins.kimug
Length of output: 17141
Use a check that matches the migration’s write path. /users/count can return 200 with only view/query-user privileges, but this flow later creates and updates users. Probe an endpoint that requires the same admin rights as the migration, or rename this method so it only promises user-count access.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pas/plugins/kimug/utils.py` around lines 640 - 642, Update the
admin-access probe described near the users migration flow to use an endpoint
requiring the same create/update-user privileges as the migration’s write path,
rather than relying on /users/count; alternatively, rename the method and adjust
its contract to explicitly promise only user-count access if that is the
intended behavior.
1 participant
Summary by CodeRabbit
Bug Fixes
Documentation