Skip to content
Jump to bottom

Automated Test: feature-clients-authz-implementation #313

Closed
wants to merge 1 commit into from
+866 −138
Closed

Automated Test: feature-clients-authz-implementation #313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 20 additions & 2 deletions server-spi-private/src/main/java/org/keycloak/authorization/AdminPermissionsSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,27 @@
import org.keycloak.representations.idm.authorization.ScopeRepresentation;

public class AdminPermissionsSchema extends AuthorizationSchema {

public static final String USERS_RESOURCE_TYPE = "Users";
public static final String CLIENTS_RESOURCE_TYPE = "Clients";

//scopes
public static final String MANAGE = "manage";
public static final String VIEW = "view";
public static final String IMPERSONATE = "impersonate";
public static final String MAP_ROLES = "map-roles";
public static final String MANAGE_GROUP_MEMBERSHIP = "manage-group-membership";
public static final String CONFIGURE = "configure";
public static final String MAP_ROLES_CLIENT_SCOPE = "map-roles-client-scope";
public static final String MAP_ROLES_COMPOSITE = "map-roles-composite";

public static final ResourceType USERS = new ResourceType(USERS_RESOURCE_TYPE, Set.of(MANAGE, VIEW, IMPERSONATE, MAP_ROLES, MANAGE_GROUP_MEMBERSHIP));
public static final ResourceType CLIENTS = new ResourceType(CLIENTS_RESOURCE_TYPE, Set.of(CONFIGURE, MANAGE, MAP_ROLES, MAP_ROLES_CLIENT_SCOPE, MAP_ROLES_COMPOSITE, VIEW));

public static final AdminPermissionsSchema SCHEMA = new AdminPermissionsSchema();

private AdminPermissionsSchema() {
super(Map.of(USERS_RESOURCE_TYPE, USERS));
super(Map.of(USERS_RESOURCE_TYPE, USERS, CLIENTS_RESOURCE_TYPE, CLIENTS));
}

public Resource getOrCreateResource(KeycloakSession session, ResourceServer resourceServer, String policyType, String resourceType, String id) {
Expand All @@ -85,6 +90,8 @@ public Resource getOrCreateResource(KeycloakSession session, ResourceServer reso

if (USERS.getType().equals(resourceType)) {
name = resolveUser(session, id);
} else if (CLIENTS.getType().equals(resourceType)) {
name = resolveClient(session, id);
}

if (name == null) {
Expand Down Expand Up @@ -165,6 +172,17 @@ private String resolveUser(KeycloakSession session, String id) {
return user == null ? null : user.getId();
}

private String resolveClient(KeycloakSession session, String id) {
RealmModel realm = session.getContext().getRealm();
ClientModel client = session.clients().getClientById(realm, id);

if (client == null) {
client = session.clients().getClientByClientId(realm, id);
}

return client == null ? null : client.getId();
}

private StoreFactory getStoreFactory(KeycloakSession session) {
AuthorizationProvider authzProvider = session.getProvider(AuthorizationProvider.class);
return authzProvider.getStoreFactory();
Expand Down
32 changes: 17 additions & 15 deletions ...ces/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,25 @@ public static void registerListener(ProviderEventManager manager) {
manager.register(new ProviderEventListener() {
@Override
public void onEvent(ProviderEvent event) {
if (event instanceof RoleContainerModel.RoleRemovedEvent) {
RoleContainerModel.RoleRemovedEvent cast = (RoleContainerModel.RoleRemovedEvent)event;
RoleModel role = cast.getRole();
RealmModel realm;
if (role.getContainer() instanceof ClientModel) {
realm = ((ClientModel)role.getContainer()).getRealm();
if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) {
if (event instanceof RoleContainerModel.RoleRemovedEvent) {
RoleContainerModel.RoleRemovedEvent cast = (RoleContainerModel.RoleRemovedEvent) event;
RoleModel role = cast.getRole();
RealmModel realm;
if (role.getContainer() instanceof ClientModel) {
realm = ((ClientModel) role.getContainer()).getRealm();

} else {
realm = (RealmModel)role.getContainer();
} else {
realm = (RealmModel) role.getContainer();
}
management(cast.getKeycloakSession(), realm).roles().setPermissionsEnabled(role, false);
} else if (event instanceof ClientModel.ClientRemovedEvent) {
ClientModel.ClientRemovedEvent cast = (ClientModel.ClientRemovedEvent) event;
management(cast.getKeycloakSession(), cast.getClient().getRealm()).clients().setPermissionsEnabled(cast.getClient(), false);
} else if (event instanceof GroupModel.GroupRemovedEvent) {
GroupModel.GroupRemovedEvent cast = (GroupModel.GroupRemovedEvent) event;
management(cast.getKeycloakSession(), cast.getRealm()).groups().setPermissionsEnabled(cast.getGroup(), false);
}
management(cast.getKeycloakSession(), realm).roles().setPermissionsEnabled(role, false);
} else if (event instanceof ClientModel.ClientRemovedEvent) {
ClientModel.ClientRemovedEvent cast = (ClientModel.ClientRemovedEvent)event;
management(cast.getKeycloakSession(), cast.getClient().getRealm()).clients().setPermissionsEnabled(cast.getClient(), false);
} else if (event instanceof GroupModel.GroupRemovedEvent) {
GroupModel.GroupRemovedEvent cast = (GroupModel.GroupRemovedEvent)event;
management(cast.getKeycloakSession(), cast.getRealm()).groups().setPermissionsEnabled(cast.getGroup(), false);
}
}
});
Expand Down
108 changes: 108 additions & 0 deletions ...ain/java/org/keycloak/services/resources/admin/permissions/ClientPermissionEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.keycloak.services.resources.admin.permissions;

import org.keycloak.models.AdminRoles;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;

Expand All @@ -31,54 +32,161 @@ public interface ClientPermissionEvaluator {

void setPermissionsEnabled(ClientModel client, boolean enable);

/**
* Throws ForbiddenException if {@link #canListClientScopes()} returns {@code false}.
*/
void requireListClientScopes();

/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canManage();

/**
* Throws ForbiddenException if {@link #canManage()} returns {@code false}.
*/
void requireManage();

/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canManageClientScopes();

/**
* Throws ForbiddenException if {@link #canManageClientScopes()} returns {@code false}.
*/
void requireManageClientScopes();

/**
* Returns {@code true} if the caller has at least one of the {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} or {@link org.keycloak.models.AdminRoles#VIEW_CLIENTS} roles.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW}.
*/
boolean canView();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
* <p/>
* Or if the caller has at least one of the {@link AdminRoles#QUERY_CLIENTS} or {@link AdminRoles#QUERY_USERS} roles.
*/
boolean canList();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
*/
boolean canViewClientScopes();

/**
* Throws ForbiddenException if {@link #canList()} returns {@code false}.
*/
void requireList();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
* <p/>
* Or if the caller has {@link AdminRoles#QUERY_CLIENTS} role.
*/
boolean canListClientScopes();

/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
*/
void requireView();
Comment on lines +95 to 98
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

JavaDoc incorrectly states "Returns true" for a void method.

The JavaDoc for requireView() says "Returns {@code true} if..." but this method has a void return type. Based on the pattern of other requireXxx methods in this interface (e.g., lines 47-50, 59-62), this should say "Throws ForbiddenException if {@link #canView()} returns {@code false}."

📝 Proposed fix 
-    /**
-     * Returns {`@code` true} if {`@link` `#canView`()} returns {`@code` true}.
-     */
+    /**
+     * Throws ForbiddenException if {`@link` `#canView`()} returns {`@code` false}.
+     */
     void requireView();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Returns {@code true} if {@link #canView()} returns {@code true}.
*/
void requireView();
/**
* Throws ForbiddenException if {`@link` `#canView`()} returns {`@code` false}.
*/
void requireView();
🤖 Prompt for AI Agents 
In
`@services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionEvaluator.java`
around lines 95 - 98, Update the JavaDoc for the interface method
ClientPermissionEvaluator.requireView() to stop saying "Returns {`@code` true}"
and instead document that the method throws a ForbiddenException if {`@link`
`#canView`()} returns {`@code` false}; mirror the wording used by other requireXxx
methods (e.g., requireManage(), requireConfigure()) so the comment correctly
reflects the void signature and the thrown exception behavior.


/**
* Returns {@code true} if {@link #canViewClientScopes()} returns {@code true}.
*/
void requireViewClientScopes();
Comment on lines +100 to 103
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

JavaDoc incorrectly states "Returns true" for a void method.

Similar to requireView(), the JavaDoc for requireViewClientScopes() incorrectly describes the return behavior for a void method.

📝 Proposed fix 
-    /**
-     * Returns {`@code` true} if {`@link` `#canViewClientScopes`()} returns {`@code` true}.
-     */
+    /**
+     * Throws ForbiddenException if {`@link` `#canViewClientScopes`()} returns {`@code` false}.
+     */
     void requireViewClientScopes();
🤖 Prompt for AI Agents 
In
`@services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionEvaluator.java`
around lines 100 - 103, The JavaDoc for requireViewClientScopes() erroneously
says "Returns {`@code` true}" though the method is void; update its Javadoc to
match the semantics used in requireView() (e.g., state that the method enforces
the permission check and will throw an exception if the caller lacks permission
or simply requires view permissions) and remove any "Returns" phrasing so the
comment accurately describes the behavior of requireViewClientScopes().


/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* Or if the caller has a permission to {@link AdminPermissionManagement#MANAGE_SCOPE} the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE} all clients.
*/
boolean canManage(ClientModel client);

/**
* Returns {@code true} if {@link #canManage(ClientModel)} returns {@code true}.
* <p/>
* Or if the caller has a permission to {@link ClientPermissionManagement#CONFIGURE_SCOPE} the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#CONFIGURE} all clients.
*/
boolean canConfigure(ClientModel client);

/**
* Throws ForbiddenException if {@link #canConfigure(ClientModel)} returns {@code false}.
*/
void requireConfigure(ClientModel client);

/**
* Throws ForbiddenException if {@link #canManage(ClientModel)} returns {@code false}.
*/
void requireManage(ClientModel client);

/**
* Returns {@code true} if {@link #canView()} or {@link #canConfigure(ClientModel)} returns {@code true}.
* <p/>
* Or if the caller has a permission to {@link AdminPermissionManagement#VIEW_SCOPE} the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW} all clients.
*/
boolean canView(ClientModel client);

/**
* Throws ForbiddenException if {@link #canView(ClientModel)} returns {@code false}.
*/
void requireView(ClientModel client);

/**
* Returns {@code true} if the caller has {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} role.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canManage(ClientScopeModel clientScope);

/**
* Throws ForbiddenException if {@link #canManage(ClientScopeModel)} returns {@code false}.
*/
void requireManage(ClientScopeModel clientScope);

/**
* Returns {@code true} if the caller has at least one of the {@link org.keycloak.models.AdminRoles#VIEW_CLIENTS} or {@link org.keycloak.models.AdminRoles#MANAGE_CLIENTS} roles.
* <p/>
* For V2 only: Also if it has permission to {@link org.keycloak.authorization.AdminPermissionsSchema#VIEW} or {@link org.keycloak.authorization.AdminPermissionsSchema#MANAGE}.
*/
boolean canView(ClientScopeModel clientScope);

/**
* Throws ForbiddenException if {@link #canView(ClientScopeModel)} returns {@code false}.
*/
void requireView(ClientScopeModel clientScope);

/**
* Returns {@code true} if the caller has a permission to {@link ClientPermissionManagement#MAP_ROLES_SCOPE} for the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MAP_ROLES} for all clients.
*/
boolean canMapRoles(ClientModel client);

/**
* Returns {@code true} if the caller has a permission to {@link ClientPermissionManagement#MAP_ROLES_COMPOSITE_SCOPE} for the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MAP_ROLES_COMPOSITE} for all clients.
*/
boolean canMapCompositeRoles(ClientModel client);

/**
* Returns {@code true} if the caller has a permission to {@link ClientPermissionManagement#MAP_ROLES_CLIENT_SCOPE} for the client.
* <p/>
* For V2 only: Also if the caller has a permission to {@link org.keycloak.authorization.AdminPermissionsSchema#MAP_ROLES_CLIENT_SCOPE} for all clients.
*/
boolean canMapClientScopeRoles(ClientModel client);

Map<String, Boolean> getAccess(ClientModel client);
Expand Down