8000 refactor: adding a roles service for v2 by shawkins · Pull Request #55049 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

refactor: adding a roles service for v2 #55049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shawkins wants to merge 2 commits into
base: main
Choose a base branch
from
+567 −399
Open

refactor: adding a roles service for v2 #55049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fcd977
refactor: adding a roles service for v2
shawkins Nov 11, 2026
f62f7de
Fix condition for retrieving role composites stream
shawkins Nov 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions rest/admin-v2/services/src/main/java/org/keycloak/services/RolesService.java
View file
Open in desktop

This file was deleted.

23 changes: 10 additions & 13 deletions .../services/src/main/java/org/keycloak/services/client/scim/ClientResourceTypeProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.keycloak.representations.admin.v2.validation.PatchClient;
import org.keycloak.representations.admin.v2.validation.PutClient;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.scim.filter.ScimFilterException;
import org.keycloak.scim.model.filter.ScimAttributeJpaExpressionResolver;
import org.keycloak.scim.model.filter.ScimJPAPredicateEvaluator;
Expand All @@ -57,7 +56,6 @@
import org.keycloak.scim.resource.spi.BaseResourceTypeProvider;
import org.keycloak.scim.resource.spi.SearchOptions;
import org.keycloak.scim.resource.spi.SortField;
import org.keycloak.services.RolesService;
import org.keycloak.services.ServiceException;
import org.keycloak.services.client.ClientSortField;
import org.keycloak.services.client.SimpleClientModel;
Expand All @@ -75,9 +73,9 @@
import org.keycloak.services.managers.RealmManager;
import org.keycloak.services.resources.admin.AdminEventBuilder;
import org.keycloak.services.resources.admin.AdminRoot;
import org.keycloak.services.resources.admin.RoleContainerResource;
import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator;
import org.keycloak.services.resources.admin.fgap.AdminPermissions;
import org.keycloak.services.roles.RoleService;
import org.keycloak.validation.ValidationUtil;
import org.keycloak.validation.jakarta.HibernateValidatorProvider;
import org.keycloak.validation.jakarta.JakartaValidatorProvider;
Expand All @@ -101,7 +99,7 @@ public class ClientResourceTypeProvider extends BaseResourceTypeProvider<ClientM
private final AdminEventBuilder adminEventBuilder;
private final RealmModel realm;
private final JakartaValidatorProvider validator;
private final RolesService rolesService;
private final RoleService roleService;

public ClientResourceTypeProvider(KeycloakSession session) {
super(session, SCHEMAS.values().stream());
Expand All @@ -110,7 +108,7 @@ public ClientResourceTypeProvider(KeycloakSession session) {
this.permissions = AdminPermissions.evaluator(session, realm, authInfo);
this.adminEventBuilder = new AdminEventV2Builder(realm, permissions.adminAuth(), session, session.getContext().getConnection()).resource(ResourceType.CLIENT);
this.validator = new HibernateValidatorProvider(new ValidationContext(session, realm));
this.rolesService = new RolesService(session, realm, permissions, adminEventBuilder);
this.roleService = new RoleService(session, realm, permissions);
}

public Map<String, BaseClientModelSchema<? extends BaseClientRepresentation>> getSchemaMap() {
Expand Down Expand Up @@ -234,8 +232,7 @@ private BaseClientRepresentation onCreateOrUpdate(BaseClientRepresentation clien
}

// Setup roles
var clientRoles = rolesService.resource(model);
handleRoles(clientRoles, client.getRoles());
handleRoles(model, client.getRoles());

// OIDC specific
if (client instanceof OIDCClientRepresentation oidcClient) {
Expand Down Expand Up @@ -473,25 +470,25 @@ private String generateClientSecretIfNeeded(BaseClientRepresentation client, Cli
/**
* Declaratively manage client roles - ensures the client has exactly the roles specified in 'rolesFromRep'
* <p>
* Reuses API v1 logic
* Role events are intentionally not fired here, the client event carries the change.
*/
protected void handleRoles(RoleContainerResource clientRoles, Set<String> rolesFromRep) {
protected void handleRoles(ClientModel client, Set<String> rolesFromRep) {
Set<String> desiredRoleNames = Optional.ofNullable(rolesFromRep)
.orElse(Collections.emptySet());

Set<String> currentRoleNames = clientRoles.getRoles(null, null, null, false)
.map(RoleRepresentation::getName)
Set<String> currentRoleNames = roleService.list(client, null, null, null)
.map(RoleModel::getName)
.collect(Collectors.toSet());

// Add missing roles (in desiredRoleNames but not in currentRoleNames)
desiredRoleNames.stream()
.filter(roleName -> !currentRoleNames.contains(roleName))
.forEach(roleName -> clientRoles.doCreateRole(new RoleRepresentation(roleName, "", false)));
.forEach(roleName -> roleService.create(client, roleName, "", null));

// Remove extra roles (in currentRoleNames but not in desiredRoleNames)
currentRoleNames.stream()
.filter(role -> !desiredRoleNames.contains(role))
.forEach(clientRoles::deleteRole);
.forEach(roleName -> roleService.delete(client, roleName));
}

/**
Expand Down
36 changes: 36 additions & 0 deletions rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/AdminEventV2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@

import java.io.ByteArrayInputStream;
import java.util.List;
import java.util.Set;

import org.keycloak.authentication.authenticators.client.ClientIdAndSecretAuthenticator;
import org.keycloak.common.Profile;
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.representations.admin.v2.OIDCClientRepresentation;
import org.keycloak.representations.idm.AdminEventRepresentation;
import org.keycloak.representations.idm.RealmEventsConfigRepresentation;
Expand Down Expand Up @@ -209,6 +211,40 @@ public void deleteClientFiresV2Event() throws Exception {
}
}

@Test
public void updateClientRolesFiresOnlyClientEvent() throws Exception {
createTestClient();
try {
testRealm.admin().clearAdminEvents();
OIDCClientRepresentation rep = getTestClientRepresentation();
rep.setRoles(Set.of("role-a", "role-b"));

try (var response = getClientApi(TEST_CLIENT_ID).createOrUpdateClient(rep)) {
assertEquals(200, response.getStatus());
}

List<AdminEventRepresentation> events = testRealm.admin().getAdminEvents();
assertThat("Only the client event should be fired, not per-role events", events.size(), is(1));
AdminEventRepresentation event = events.get(0);
assertThat(event.getResourceType(), is(ResourceType.CLIENT.toString()));
assertThat(event.getOperationType(), is(OperationType.UPDATE.toString()));
assertThat(event.getDetails().get(API_VERSION_DETAIL_KEY), is(API_VERSION_V2));

testRealm.admin().clearAdminEvents();
rep.setRoles(Set.of("role-a"));

try (var response = getClientApi(TEST_CLIENT_ID).createOrUpdateClient(rep)) {
assertEquals(200, response.getStatus());
}

events = testRealm.admin().getAdminEvents();
assertThat("Only the client event should be fired when removing a role", events.size(), is(1));
assertThat(events.get(0).getResourceType(), is(ResourceType.CLIENT.toString()));
} finally {
deleteTestClient();
}
}

private void assertUpdateEventFired(String newDescription){
// Verify v2 UPDATE event was fired
List<AdminEventRepresentation> events = testRealm.admin().getAdminEvents();
Expand Down
0 ...g/keycloak/services/ServiceException.java → ...g/keycloak/services/ServiceException.java
View file
Open in desktop
File renamed without changes.
82 changes: 22 additions & 60 deletions services/src/main/java/org/keycloak/services/resources/admin/RoleByIdResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,20 @@
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;

import org.keycloak.common.Profile;
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.ManagementPermissionReference;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.services.ErrorResponse;
import org.keycloak.services.resources.KeycloakOpenAPI;
import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator;
import org.keycloak.services.resources.admin.fgap.AdminPermissionManagement;
import org.keycloak.services.resources.admin.fgap.AdminPermissions;
import org.keycloak.services.roles.RoleService;
import org.keycloak.utils.ProfileHelper;

import org.eclipse.microprofile.openapi.annotations.Operation;
Expand Down Expand Up @@ -76,13 +73,15 @@ public class RoleByIdResource extends RoleResource {
private final AdminEventBuilder adminEvent;

private final KeycloakSession session;
private final RoleService roleService;

public RoleByIdResource(KeycloakSession session, AdminPermissionEvaluator auth, AdminEventBuilder adminEvent) {
super(session.getContext().getRealm());
this.session = session;
this.realm = session.getContext().getRealm();
this.auth = auth;
this.adminEvent = adminEvent;
this.roleService = new RoleService(session, realm, auth);
}

/**
Expand All @@ -102,9 +101,7 @@ public RoleByIdResource(KeycloakSession session, AdminPermissionEvaluator auth,
@APIResponse(responseCode = "403", description = "Forbidden")
})
public RoleRepresentation getRole(final @Parameter(description = "id of role") @PathParam("role-id") String id) {
RoleModel roleModel = getRoleModel(id);
auth.roles().requireView(roleModel);
return getRole(roleModel);
return getRole(call(() -> roleService.get(getRoleModel(id))));
}

protected RoleModel getRoleModel(String id) {
Expand Down Expand Up @@ -133,22 +130,12 @@ protected RoleModel getRoleModel(String id) {
public void deleteRole(final @Parameter(description = "id of role") @PathParam("role-id") String id) {
if (realm.getDefaultRole() == null) {
logger.warnf("Default role for realm with id '%s' doesn't exist.", realm.getId());
} else if (realm.getDefaultRole().getId().equals(id)) {
throw ErrorResponse.error(realm.getDefaultRole().getName() + " is default role of the realm and cannot be removed.",
Response.Status.BAD_REQUEST);
}

RoleModel role = getRoleModel(id);
auth.roles().requireManage(role);
deleteRole(role);
RoleModel role = call(() -> roleService.delete(getRoleModel(id)));

if (role.isClientRole()) {
adminEvent.resource(ResourceType.CLIENT_ROLE);
} else {
adminEvent.resource(ResourceType.REALM_ROLE);
}

adminEvent.operation(OperationType.DELETE).resourcePath(session.getContext().getUri()).success();
adminEvent.resource(resourceType(role))
.operation(OperationType.DELETE).resourcePath(session.getContext().getUri()).success();
}

/**
Expand All @@ -168,17 +155,10 @@ public void deleteRole(final @Parameter(description = "id of role") @PathParam("
@APIResponse(responseCode = "403", description = "Forbidden")
})
public void updateRole(final @Parameter(description = "id of role") @PathParam("role-id") String id, final RoleRepresentation rep) {
RoleModel role = getRoleModel(id);
auth.roles().requireManage(role);
updateRole(rep, role, realm, session);
RoleModel role = call(() -> roleService.update(getRoleModel(id), rep.getName(), rep.getDescription(), rep.getAttributes()));

if (role.isClientRole()) {
adminEvent.resource(ResourceType.CLIENT_ROLE);
} else {
adminEvent.resource(ResourceType.REALM_ROLE);
}

adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(rep).success();
adminEvent.resource(resourceType(role))
.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(rep).success();
}

/**
Expand All @@ -198,9 +178,10 @@ public void updateRole(final @Parameter(description = "id of role") @PathParam("
})
public void addComposites(final @PathParam("role-id") String id, List<RoleRepresentation> roles) {
// any role by ID
RoleModel role = getRoleModel(id);
auth.roles().requireManage(role);
addComposites(auth, adminEvent, session.getContext().getUri(), roles, role);
RoleModel role = call(() -> roleService.addComposites(getRoleModel(id), ids(roles)));

adminEvent.resource(resourceType(role))
.operation(OperationType.CREATE).resourcePath(session.getContext().getUri()).representation(roles).success();
}

/**
Expand Down Expand Up @@ -228,20 +209,8 @@ public Stream<RoleRepresentation> getRoleComposites(final @PathParam("role-id")
) {

logger.debugf("*** getRoleComposites: '%s'", id);
RoleModel role = getRoleModel(id);
auth.roles().requireView(role);

Stream<RoleModel> composites = role.getCompositesStream(search, null, null)
.filter(r -> auth.roles().canView(r));

if (first != null && first > 0) {
composites = composites.skip(first);
}
if (max != null && max >= 0) {
composites = composites.limit(max);
}

return composites.map(ModelToRepresentation::toBriefRepresentation);
return call(() -> roleService.getComposites(getRoleModel(id), search, first, max))
.map(ModelToRepresentation::toBriefRepresentation);
}

/**
Expand All @@ -261,9 +230,7 @@ public Stream<RoleRepresentation> getRoleComposites(final @PathParam("role-id")
@APIResponse(responseCode = "403", description = "Forbidden")
})
public Stream<RoleRepresentation> getRealmRoleComposites(final @PathParam("role-id") String id) {
RoleModel role = getRoleModel(id);
auth.roles().requireView(role);
return getRealmRoleComposites(auth, role);
return call(() -> roleService.getRealmComposites(getRoleModel(id))).map(ModelToRepresentation::toBriefRepresentation);
}

/**
Expand All @@ -287,13 +254,7 @@ public Stream<RoleRepresentation> getRealmRoleComposites(final @PathParam("role-
public Stream<RoleRepresentation> getClientRoleComposites(final @PathParam("role-id") String id,
final @PathParam("clientUuid") String clientUuid) {

RoleModel role = getRoleModel(id);
auth.roles().requireView(role);
ClientModel clientModel = realm.getClientById(clientUuid);
if (clientModel == null) {
throw new NotFoundException("Could not find client");
}
return getClientRoleComposites(auth, clientModel, role);
return call(() -> roleService.getClientComposites(getRoleModel(id), clientUuid)).map(ModelToRepresentation::toBriefRepresentation);
}

/**
Expand All @@ -313,9 +274,10 @@ public Stream<RoleRepresentation> getClientRoleComposites(final @PathParam("role
})
public void deleteComposites(final @Parameter(description = "Role id") @PathParam("role-id") String id,
@Parameter(description = "A set of roles to be removed") List<RoleRepresentation> roles) {
RoleModel role = getRoleModel(id);
auth.roles().requireManage(role);
deleteComposites(auth, adminEvent, session.getContext().getUri(), roles, role);
RoleModel role = call(() -> roleService.removeComposites(getRoleModel(id), ids(roles)));

adminEvent.resource(resourceType(role))
.operation(OperationType.DELETE).resourcePath(session.getContext().getUri()).representation(roles).success();
}

/**
Expand Down
Loading
Loading
0