Skip to content

Add Client resource type and scopes to authorization schema#5

Open
everettbu wants to merge 1 commit into
feature-clients-authz-baselinefrom
feature-clients-authz-implementation
Open

Add Client resource type and scopes to authorization schema#5
everettbu wants to merge 1 commit into
feature-clients-authz-baselinefrom
feature-clients-authz-implementation

Conversation

@everettbu

@everettbu everettbu commented Jul 26, 2025

Copy link
Copy Markdown

Test 5

Summary by CodeRabbit

  • New Features

    • Introduced fine-grained admin permissions for managing client resources, including new permission types for clients.
    • Added a new system for evaluating and enforcing client-specific admin permissions.
  • Bug Fixes

    • Permission update logic for certain admin events is now only triggered when the relevant feature flag is enabled.
  • Documentation

    • Enhanced API documentation for client permission evaluation interfaces to clarify permission behavior and requirements.
  • Tests

    • Added comprehensive integration tests to verify enforcement of fine-grained admin permissions on client resources.
    • Refactored and expanded test utilities for creating and managing permissions and policies.
    • Updated existing tests to use new, more explicit permission creation methods.

…valuation implementation for ClientsPermissionsV2

Closes #35564

Signed-off-by: Martin Kanis <mkanis@redhat.com>
@coderabbitai

coderabbitai Bot commented Jul 26, 2025

Copy link
Copy Markdown

Walkthrough

This change set introduces fine-grained admin authorization controls for Keycloak client resources. It extends the admin permissions schema to support clients, implements a new ClientPermissionsV2 class for permission evaluation, and integrates this into the management permissions system. Comprehensive tests are added and refactored to validate client-level permission enforcement.

Changes

Cohort / File(s) Change Summary
Admin Permissions Schema Extension
server-spi-private/.../AdminPermissionsSchema.java
Adds "Clients" as a new resource type with associated scopes and constants. Updates the schema's constructor and resource resolution logic to handle clients, including a new method for client lookup by ID or client ID.
Admin Permissions Event Listener Guard
services/.../AdminPermissions.java
Wraps event listener logic for role, client, and group removal in a feature flag check, ensuring permission updates only occur if fine-grained admin authorization is enabled.
Client Permission Evaluator Documentation
services/.../ClientPermissionEvaluator.java
Adds comprehensive Javadoc comments to all public methods, clarifying permission semantics and expected behavior. No code or signature changes.
Client Permissions V2 Implementation
services/.../ClientPermissionsV2.java
Introduces a new class implementing client-level permission checks using the authorization provider and admin roles. Overrides legacy methods to throw exceptions and centralizes permission logic for clients. Provides helper methods for scope evaluation and client retrieval by permission.
MgmtPermissionsV2 Integration
services/.../MgmtPermissionsV2.java
Adds a field and method to lazily instantiate and return a ClientPermissionsV2 instance, integrating client permission evaluation into the management permissions system.
Test Utility Refactor and Extension
tests/base/.../AbstractPermissionTest.java
Refactors utility methods to static, requiring explicit client parameters. Adds new helpers for creating user/client policies and permissions, improving modularity and reuse in tests.
Client Permission Integration Tests
tests/base/.../PermissionClientTest.java
Adds comprehensive integration tests for client-level admin permissions, including manage, configure, view, and role-mapping scenarios. Introduces cleanup and setup logic for test isolation.
Permission REST Test Adjustments
tests/base/.../PermissionRESTTest.java
Updates method calls to pass explicit client arguments when creating permissions, aligning with utility refactor.
User Resource Type Evaluation Test Refactor
tests/base/.../UserResourceTypeEvaluationTest.java
Removes custom helper methods in favor of generic, parameterized ones. Updates calls to use explicit client and resource type arguments, improving consistency and maintainability.
User Resource Type Permission Test Refactor
tests/base/.../UserResourceTypePermissionTest.java
Adjusts all permission resource and creation calls to require explicit client arguments, ensuring consistent use of client context throughout the tests.

Sequence Diagram(s)

sequenceDiagram
    participant Admin as Admin User
    participant API as Admin REST API
    participant MgmtPerms as MgmtPermissionsV2
    participant ClientPerms as ClientPermissionsV2
    participant Authz as AuthorizationProvider
    participant Realm as RealmModel

    Admin->>API: Request client operation (e.g., update client)
    API->>MgmtPerms: Get client permissions
    MgmtPerms->>ClientPerms: clients()
    ClientPerms->>Authz: Evaluate permission (e.g., canConfigure)
    Authz->>Realm: Lookup client resource & scopes
    Authz-->>ClientPerms: Permission result (allow/deny)
    ClientPerms-->>MgmtPerms: Boolean result
    MgmtPerms-->>API: Boolean result
    API-->>Admin: Success or Forbidden
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

In the warren of code, permissions grow,
Now clients join users in the fine-grained show.
With tests that hop and helpers that leap,
Rabbits ensure security is deep.
New scopes, new checks, new carrots to find—
This patch brings more order, for all rabbit-kind!
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between baa6ebb and 7e55408.

📒 Files selected for processing (10)
  • server-spi-private/src/main/java/org/keycloak/authorization/AdminPermissionsSchema.java (3 hunks)
  • services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java (1 hunks)
  • services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionEvaluator.java (2 hunks)
  • services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionsV2.java (1 hunks)
  • services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissionsV2.java (2 hunks)
  • tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java (3 hunks)
  • tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/PermissionClientTest.java (3 hunks)
  • tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/PermissionRESTTest.java (2 hunks)
  • tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java (16 hunks)
  • tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypePermissionTest.java (9 hunks)
🧰 Additional context used
🪛 Gitleaks (8.27.2)
services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionsV2.java

53-53: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (25)
tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/PermissionRESTTest.java (2)

129-133: LGTM! Consistent refactoring for explicit client context.

The addition of the client parameter to createPermission() aligns with the broader refactoring to make client context explicit in permission operations, improving test clarity and modularity.


160-173: LGTM! Consistent parameter usage across permission tests.

All createPermission() calls now consistently include the explicit client parameter, maintaining consistency with the refactored test framework.

services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionEvaluator.java (2)

19-19: LGTM! Import addition supports enhanced documentation.

The AdminRoles import is appropriately added to support the comprehensive Javadoc references throughout the interface.


35-194: Excellent documentation enhancement!

The comprehensive Javadoc additions significantly improve the interface's usability by:

  • Clearly explaining when methods throw ForbiddenException vs return boolean values
  • Documenting specific admin roles and permissions that grant access
  • Distinguishing between V1 and V2 implementation behaviors
  • Providing cross-references to AdminPermissionsSchema and permission management classes

This documentation will be invaluable for implementers and users of client permission evaluation.

tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypePermissionTest.java (2)

67-67: LGTM! Consistent refactoring pattern applied.

The addition of explicit client parameter to getScopePermissionsResource() call aligns with the broader test framework refactoring for clearer client context handling.


77-198: LGTM! Comprehensive and consistent parameter updates.

All permission-related method calls have been systematically updated to include the explicit client parameter, maintaining consistency with the refactored test framework while preserving all original test logic and assertions.

services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java (1)

77-96: LGTM! Proper feature flag guarding implemented.

The conditional check for ADMIN_FINE_GRAINED_AUTHZ feature flag appropriately guards the permission cleanup event handling, ensuring:

  • Permission cleanup only executes when the feature is enabled
  • Minimal performance impact when feature is disabled
  • Consistent behavior with the overall feature flag model

The existing event handling logic for role, client, and group removal events is correctly preserved within the conditional block.

services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissionsV2.java (2)

29-29: LGTM! Consistent field addition for client permissions.

The new clientPermissions field follows the established pattern for managing permission evaluator instances in this class.


63-68: LGTM! Proper integration of client permissions.

The clients() method correctly implements:

  • Lazy initialization pattern consistent with users() method
  • Proper parameter passing to ClientPermissionsV2 constructor
  • Integration with the V2 management permissions hierarchy

This provides the access point for the new fine-grained client permission evaluation functionality.

tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java (2)

66-66: Good use of constant to avoid repetition.

Extracting the resource type string to a field improves readability and reduces repeated method calls.


72-336: Well-executed refactoring to static methods.

The systematic conversion from instance methods to static methods with explicit client parameters improves modularity and makes dependencies clear. The changes are consistent throughout the file and align with the broader test framework refactoring.

server-spi-private/src/main/java/org/keycloak/authorization/AdminPermissionsSchema.java (2)

53-68: Well-structured addition of client resource type and scopes.

The new constants and ResourceType definition follow the established pattern for users and include appropriate scopes for client management.


175-184: Consistent implementation of client resolution logic.

The resolveClient method appropriately mirrors the pattern used in resolveUser, providing dual lookup by ID and clientId.

tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/PermissionClientTest.java (6)

78-83: Good test cleanup practice.

The @AfterEach method ensures proper cleanup of permissions after each test, preventing test interference.


126-189: Comprehensive test for single client management permissions.

The test thoroughly validates:

  • Forbidden access without permissions
  • Successful operations after granting MANAGE permission
  • Proper isolation ensuring other clients remain inaccessible

Good test structure and coverage.


191-247: Well-designed test for CONFIGURE permission scope.

The test correctly validates that CONFIGURE permission:

  • Allows viewing client and managing secrets
  • Permits viewing but not modifying client scopes
  • Maintains proper access control boundaries

249-281: Good coverage of all-clients permission scope.

The test validates that MANAGE permission on all clients enables:

  • Creating new clients
  • Deleting clients
  • Listing all clients

283-323: Proper validation of VIEW permission restrictions.

The test correctly verifies that VIEW permission:

  • Allows listing and viewing clients
  • Permits viewing protocol mappers
  • Prevents creating protocol mappers or clients

325-365: Excellent test for role mapping permissions.

The test thoroughly validates the interaction between user and client permissions:

  • MAP_ROLES on users for role assignment
  • MAP_ROLES, MAP_ROLES_COMPOSITE, and CONFIGURE on clients for composite role management

Good coverage of cross-resource permission dependencies.

services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionsV2.java (3)

207-286: Well-implemented permission evaluation logic.

The permission checking methods properly:

  • Validate admin context and resource server availability
  • Support both individual client and all-clients permissions
  • Implement proper fallback logic
  • Use consistent evaluation patterns

Note: The static analysis warning about "Generic API Key" at line 53 is a false positive - it's detecting the constructor parameter pattern.


148-204: Appropriate handling of legacy operations.

Throwing UnsupportedOperationException for V1-specific methods clearly indicates the API boundary and prevents mixing of V1/V2 permission models.


1-2: Incorrect copyright year.

The copyright year is set to 2025, but according to the context, the current date is July 2025. This should likely be 2024 if this is new code.

-/*
- * Copyright 2025 Red Hat, Inc. and/or its affiliates
+/*
+ * Copyright 2024 Red Hat, Inc. and/or its affiliates

Likely an incorrect or invalid review comment.

tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java (3)

54-74: Good refactoring to static methods.

Converting these methods to static with explicit parameters improves:

  • Dependency clarity
  • Reusability across test classes
  • Testability by removing hidden state dependencies

117-149: Excellent implementation of policy creation helpers.

The methods provide:

  • Consistent pattern for user and client policies
  • Automatic cleanup registration to prevent test pollution
  • Proper success validation
  • Flexibility with Logic parameter overload

Great attention to test infrastructure quality.


151-179: Well-designed permission creation methods.

The methods provide excellent flexibility:

  • createAllPermission for resource type permissions
  • Overloaded createPermission for specific resources
  • Support for custom Logic and multiple policies
  • Clean use of streams and varargs
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature-clients-authz-implementation

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai

coderabbitai Bot commented Jul 30, 2025

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@everettbu

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 30, 2025

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants