Skip to content

Add Groups resource type and scopes to authorization schema#5

Open
ShashankFC wants to merge 1 commit into
feature-groups-authz-baselinefrom
feature-groups-authz-implementation
Open

Add Groups resource type and scopes to authorization schema#5
ShashankFC wants to merge 1 commit into
feature-groups-authz-baselinefrom
feature-groups-authz-implementation

Conversation

@ShashankFC

@ShashankFC ShashankFC commented Feb 13, 2026

Copy link
Copy Markdown

User description

Test 6

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced group-based access control with granular permissions for viewing members, managing membership, and managing groups.
  • Tests

    • Added comprehensive test coverage for group resource-type authorization and permission evaluation scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.


Replicated from ai-code-review-evaluation/keycloak-coderabbit#6


CodeAnt-AI Description

Add Group resource type and group-scoped admin permissions to fine-grained authorization

What Changed

  • Introduces a "Groups" admin resource type with scopes: manage, view, manage-membership, manage-members, view-members; these can be granted to restrict or allow group-specific admin actions.
  • Admin actions that involve groups or users now consider group-scoped permissions: listing/viewing/managing groups, viewing or managing group members, and managing membership are allowed when the corresponding group-level permission is granted.
  • User search and user count now respect group-level view permissions (admins granted VIEW_MEMBERS or MANAGE_MEMBERS for a group will see its members in searches); session attributes and backend queries were adjusted to use group IDs with view permission.
  • Behavior when using the newer admin fine-grained authz V2: group permission evaluations are performed against the new schema so "all-groups" and per-group permissions correctly enable create/update/map-role operations as the tests demonstrate.
  • Adds integration tests that cover group-level permission scenarios (view members, manage members, manage group, view groups, manage group membership) and adjusts test admin roles to include QUERY_GROUPS.

Impact

✅ Admins with group-scoped view permissions see group members in user searches
✅ Admins granted group manage permissions can create/update/groups and map roles for permitted groups
✅ Fewer unexpected Forbidden errors for delegated group management scenarios

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…ation implementation

Closes #35562

Signed-off-by: vramik <vramik@redhat.com>
@ShashankFC

Copy link
Copy Markdown
Author

@CodeAnt-AI: review

@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

CodeAnt AI is running the review.

@codeant-ai codeant-ai Bot added the size:XL This PR changes 500-999 lines, ignoring generated files label Feb 13, 2026
@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Permission semantics change
    canManage() delegates to hasPermission(null, AdminPermissionsSchema.VIEW, AdminPermissionsSchema.MANAGE).
    Checking VIEW scope together with MANAGE for a "manage" decision may loosen the intended semantics (a caller with only VIEW scope could be considered able to manage). Confirm intended behavior or restrict to MANAGE only.

  • Resource id/name mismatch
    The V2 implementation mixes resource id and resource name when looking up and iterating resources.

  • hasPermission(String groupId, ...) calls resourceStore.findByName(server, groupId) but callers sometimes pass a resource id (from resource.getId()) or a plain group id. This will produce false negatives.

  • getGroupIdsWithViewPermission() uses groupResource.getId() to call hasPermission(...) and to add to the result set; that getId() is the resource id, not the original group id or resource name. This mismatch can cause incorrect permission resolution and incorrect returned group ids.

  • Guard behaviour changed
    getGroupIdsWithViewPermission() now returns early only if root.users().canView() whereas the previous logic short-circuited on canView() || canManage() (or similar).
    The change in the early-return condition might alter which callers get an empty set vs. explicit evaluation; ensure this change is intentional and doesn't cause privilege regressions.

  • Session attribute lifecycle & stream laziness
    The code sets session.setAttribute(UserModel.GROUPS, groupIds) before returning a lazy Stream<UserRepresentation> from searchForUser() / getUsers(); the attribute is required later during stream processing. This couples representation evaluation with session state and may leak state across logic if not cleaned. Also, clearing the session attribute prematurely (or not at all) can cause inconsistent behavior. Consider avoiding storing transient query state in the session or ensure the attribute's lifecycle is correctly managed given laziness of returned streams.

  • Null-safety in permission evaluation
    The permission evaluation loop compares permission.getResourceId().equals(resource.getId()). If permission.getResourceId() can be null this will throw an NPE. Also the code assumes resource is non-null after the "all-users" fallback; validate null returns from getResourceTypeResource(...) and guard against nulls.

@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

CodeAnt AI finished running the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants