Skip to content

Commit

Permalink
Prevent to manage groups associated with organizations from different…
Browse files Browse the repository at this point in the history
… APIs

Closes #28734

Signed-off-by: Martin Kanis <mkanis@redhat.com>
  • Loading branch information
martin-kanis authored and pedroigor committed May 2, 2024
1 parent 3e16af8 commit c0728c7
Show file tree
Hide file tree
Showing 12 changed files with 306 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.keycloak.utils.StreamsUtil.closing;

import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -319,18 +320,15 @@ private OrganizationEntity getEntity(String id, boolean failIfNotFound) {
private GroupModel createOrganizationGroup(String name) {
throwExceptionIfObjectIsNull(name, "Name of the group");

String groupName = getCanonicalGroupName(name);
GroupModel group = groupProvider.getGroupByName(realm, null, name);

if (group != null) {
throw new ModelDuplicateException("A group with the same name already exist and it is bound to different organization");
if (groupProvider.searchGroupsByAttributes(realm, Map.of(ORGANIZATION_ATTRIBUTE, name), 0, 1).findAny().isPresent()) {
throw new ModelDuplicateException("A group with the same name (attribute) already exist and it is bound to different organization");
}

return groupProvider.createGroup(realm, groupName);
}
String orgId = KeycloakModelUtils.generateId();
GroupModel group = groupProvider.createGroup(realm, orgId, orgId);
group.setSingleAttribute(ORGANIZATION_ATTRIBUTE, name);

private String getCanonicalGroupName(String name) {
return "kc.org." + name;
return group;
}

private GroupModel getOrganizationGroup(OrganizationModel organization) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.jboss.resteasy.reactive.NoCache;

import jakarta.ws.rs.NotFoundException;
import org.keycloak.models.GroupModel;
import org.keycloak.services.resources.KeycloakOpenAPI;
import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
import org.keycloak.events.admin.OperationType;
Expand Down Expand Up @@ -59,6 +60,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.keycloak.utils.OrganizationUtils.checkForOrgRelatedGroupModel;

/**
* @resource Client Role Mappings
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
Expand Down Expand Up @@ -166,6 +169,10 @@ public Stream<RoleRepresentation> getAvailableClientRoleMappings() {
public void addClientRoleMapping(List<RoleRepresentation> roles) {
managePermission.require();

if (user instanceof GroupModel groupModel) {
checkForOrgRelatedGroupModel(session, groupModel);
}

try {
for (RoleRepresentation role : roles) {
RoleModel roleModel = client.getRole(role.getName());
Expand Down Expand Up @@ -199,6 +206,10 @@ public void addClientRoleMapping(List<RoleRepresentation> roles) {
public void deleteClientRoleMapping(List<RoleRepresentation> roles) {
managePermission.require();

if (user instanceof GroupModel groupModel) {
checkForOrgRelatedGroupModel(session, groupModel);
}

if (roles == null) {
roles = user.getClientRoleMappingsStream(client)
.peek(roleModel -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
import java.util.stream.Stream;
import org.keycloak.utils.GroupUtils;

import static org.keycloak.utils.OrganizationUtils.checkForOrgRelatedGroupModel;
import static org.keycloak.utils.OrganizationUtils.checkForOrgRelatedGroupModelAndRep;
import static org.keycloak.utils.StreamsUtil.paginatedStream;

/**
Expand Down Expand Up @@ -121,6 +123,8 @@ public Response updateGroup(GroupRepresentation rep) {
throw ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
}

checkForOrgRelatedGroupModelAndRep(session, group, rep);

if (!Objects.equals(groupName, group.getName())) {
boolean exists = siblings().filter(s -> !Objects.equals(s.getId(), group.getId()))
.anyMatch(s -> Objects.equals(s.getName(), groupName));
Expand Down Expand Up @@ -149,6 +153,8 @@ private Stream<GroupModel> siblings() {
public void deleteGroup() {
this.auth.groups().requireManage(group);

checkForOrgRelatedGroupModel(session, group);

realm.removeGroup(group);
adminEvent.operation(OperationType.DELETE).resourcePath(session.getContext().getUri()).success();
}
Expand Down Expand Up @@ -194,6 +200,8 @@ public Response addChild(GroupRepresentation rep) {
throw ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
}

checkForOrgRelatedGroupModelAndRep(session, group, rep);

try {
Response.ResponseBuilder builder = Response.status(204);
GroupModel child = null;
Expand Down Expand Up @@ -367,6 +375,9 @@ public static ManagementPermissionReference toMgmtRef(GroupModel group, AdminPer
@Operation( summary = "Return object stating whether client Authorization permissions have been initialized or not and a reference")
public ManagementPermissionReference setManagementPermissionsEnabled(ManagementPermissionReference ref) {
auth.groups().requireManage(group);

checkForOrgRelatedGroupModel(session, group);

AdminPermissionManagement permissions = AdminPermissions.management(session, realm);
permissions.groups().setPermissionsEnabled(group, ref.isEnabled());
if (ref.isEnabled()) {
Expand All @@ -375,6 +386,5 @@ public ManagementPermissionReference setManagementPermissionsEnabled(ManagementP
return new ManagementPermissionReference();
}
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.keycloak.utils.GroupUtils;
import org.keycloak.utils.SearchQueryUtils;

import static org.keycloak.utils.OrganizationUtils.checkForOrgRelatedGroupRep;


/**
Expand Down Expand Up @@ -176,6 +177,8 @@ public Response addTopLevelGroup(GroupRepresentation rep) {
throw ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
}

checkForOrgRelatedGroupRep(session, rep);

try {
if (rep.getId() != null) {
child = realm.getGroupById(rep.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.keycloak.services.resources.admin;

import static org.keycloak.util.JsonSerialization.readValue;
import static org.keycloak.utils.OrganizationUtils.checkForOrgRelatedGroupModel;

import java.io.InputStream;
import java.security.cert.X509Certificate;
Expand All @@ -30,7 +31,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import jakarta.enterprise.inject.Default;
import jakarta.ws.rs.DefaultValue;
import org.eclipse.microprofile.openapi.annotations.Operation;
import org.eclipse.microprofile.openapi.annotations.extensions.Extension;
Expand All @@ -49,7 +49,6 @@
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.PathSegment;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;
import jakarta.ws.rs.core.StreamingOutput;
Expand Down Expand Up @@ -94,7 +93,6 @@
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.models.utils.RepresentationToModel;
import org.keycloak.models.utils.StripSecretsUtils;
import org.keycloak.partialimport.ErrorResponseException;
import org.keycloak.partialimport.PartialImportResult;
import org.keycloak.partialimport.PartialImportResults;
Expand Down Expand Up @@ -1053,6 +1051,9 @@ public void addDefaultGroup(@PathParam("groupId") String groupId) {
if (group == null) {
throw new NotFoundException("Group not found");
}

checkForOrgRelatedGroupModel(session, group);

realm.addDefaultGroup(group);

adminEvent.operation(OperationType.CREATE).resource(ResourceType.GROUP).resourcePath(session.getContext().getUri()).success();
Expand All @@ -1070,6 +1071,9 @@ public void removeDefaultGroup(@PathParam("groupId") String groupId) {
if (group == null) {
throw new NotFoundException("Group not found");
}

checkForOrgRelatedGroupModel(session, group);

realm.removeDefaultGroup(group);

adminEvent.operation(OperationType.DELETE).resource(ResourceType.GROUP).resourcePath(session.getContext().getUri()).success();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.ClientModel;
import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelException;
import org.keycloak.models.ModelIllegalStateException;
Expand Down Expand Up @@ -68,6 +69,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.keycloak.utils.OrganizationUtils.checkForOrgRelatedGroupModel;

/**
* Base resource for managing users
*
Expand Down Expand Up @@ -234,6 +237,10 @@ public Stream<RoleRepresentation> getAvailableRealmRoleMappings() {
public void addRealmRoleMappings(@Parameter(description = "Roles to add") List<RoleRepresentation> roles) {
managePermission.require();

if (roleMapper instanceof GroupModel groupModel) {
checkForOrgRelatedGroupModel(session, groupModel);
}

logger.debugv("** addRealmRoleMappings: {0}", roles);

try {
Expand Down Expand Up @@ -269,6 +276,10 @@ public void addRealmRoleMappings(@Parameter(description = "Roles to add") List<R
public void deleteRealmRoleMappings(List<RoleRepresentation> roles) {
managePermission.require();

if (roleMapper instanceof GroupModel groupModel) {
checkForOrgRelatedGroupModel(session, groupModel);
}

logger.debug("deleteRealmRoleMappings");
if (roles == null) {
roles = roleMapper.getRealmRoleMappingsStream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@
import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_ID;
import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_USERNAME;
import static org.keycloak.userprofile.UserProfileContext.USER_API;
import static org.keycloak.utils.OrganizationUtils.checkForOrgRelatedGroupModel;

/**
* Base resource for managing users
Expand Down Expand Up @@ -1017,6 +1018,8 @@ public void removeMembership(@PathParam("groupId") String groupId) {
}
auth.groups().requireManageMembership(group);

checkForOrgRelatedGroupModel(session, group);

try {
if (user.isMemberOf(group)){
user.leaveGroup(group);
Expand Down Expand Up @@ -1044,6 +1047,9 @@ public void joinGroup(@PathParam("groupId") String groupId) {
throw new NotFoundException("Group not found");
}
auth.groups().requireManageMembership(group);

checkForOrgRelatedGroupModel(session, group);

if (!RoleUtils.isDirectMember(user.getGroupsStream(),group)){
user.joinGroup(group);
adminEvent.operation(OperationType.CREATE).resource(ResourceType.GROUP_MEMBERSHIP).representation(ModelToRepresentation.toRepresentation(group, true)).resourcePath(session.getContext().getUri()).success();
Expand Down
81 changes: 81 additions & 0 deletions services/src/main/java/org/keycloak/utils/OrganizationUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright 2024 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.keycloak.utils;

import jakarta.ws.rs.core.Response;
import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.OrganizationModel;
import org.keycloak.organization.OrganizationProvider;
import org.keycloak.representations.idm.GroupRepresentation;
import org.keycloak.services.ErrorResponse;

import java.util.List;
import java.util.Map;

public class OrganizationUtils {

public static void checkForOrgRelatedGroupRep(KeycloakSession session, GroupRepresentation rep) {
if (isOrgsEnabled(session)) {
checkRep(rep);
}
}

public static void checkForOrgRelatedGroupModelAndRep(KeycloakSession session, GroupModel model, GroupRepresentation rep) {
if (isOrgsEnabled(session)) {
checkModel(model);
checkRep(rep);
}
}

public static void checkForOrgRelatedGroupModel(KeycloakSession session, GroupModel model) {
if (isOrgsEnabled(session)) {
checkModel(model);
}
}

private static boolean isOrgsEnabled(KeycloakSession session) {
OrganizationProvider orgProvider = session.getProvider(OrganizationProvider.class);
return orgProvider != null && orgProvider.isEnabled();
}

private static boolean isOrganizationRelatedGroup(Object o) {
if (o instanceof GroupRepresentation rep) {
return attributeContains(rep.getAttributes());
} else if (o instanceof GroupModel model) {
return attributeContains(model.getAttributes());
}
return false;
}

private static boolean attributeContains(Map<String, List<String>> attributes) {
return attributes != null && attributes.containsKey(OrganizationModel.ORGANIZATION_ATTRIBUTE);
}

private static void checkModel(GroupModel model) {
if (isOrganizationRelatedGroup(model)) {
throw ErrorResponse.error("Cannot manage organization related group via non Organization API.", Response.Status.FORBIDDEN);
}
}

private static void checkRep(GroupRepresentation rep) {
if (isOrganizationRelatedGroup(rep)) {
throw ErrorResponse.error("Cannot use group attribute reserved for organizations.", Response.Status.FORBIDDEN);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ void resetSearchableAttributes() throws Exception {
reconnectAdminClient();
}

private static String buildSearchQuery(String firstAttrName, String firstAttrValue, String... furtherAttrKeysAndValues) {
public static String buildSearchQuery(String firstAttrName, String firstAttrValue, String... furtherAttrKeysAndValues) {
if (furtherAttrKeysAndValues.length % 2 != 0) {
throw new IllegalArgumentException("Invalid length of furtherAttrKeysAndValues. Must be even, but is: " + furtherAttrKeysAndValues.length);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import jakarta.ws.rs.core.Response.Status;
import org.jboss.arquillian.graphene.page.Page;
import org.keycloak.admin.client.resource.OrganizationResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.representations.idm.GroupRepresentation;
import org.keycloak.representations.idm.OrganizationDomainRepresentation;
import org.keycloak.representations.idm.OrganizationRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
Expand Down Expand Up @@ -178,4 +180,16 @@ protected UserRepresentation addMember(OrganizationResource organization, String
return actual;
}
}

protected GroupRepresentation createGroup(RealmResource realm, String name) {
GroupRepresentation group = new GroupRepresentation();
group.setName(name);
try (Response response = realm.groups().add(group)) {
String groupId = ApiUtil.getCreatedId(response);

// Set ID to the original rep
group.setId(groupId);
return group;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,10 @@ public void testDeleteGroupOnOrganizationRemoval() {
OrganizationResource organization = testRealm().organizations().get(createOrganization().getId());
addMember(organization);

assertTrue(testRealm().groups().groups().stream().anyMatch(group -> group.getName().startsWith("kc.org.")));
assertTrue(testRealm().groups().groups("", 0, 100, false).stream().anyMatch(group -> group.getAttributes().containsKey("kc.org")));

organization.delete().close();

assertFalse(testRealm().groups().groups().stream().anyMatch(group -> group.getName().startsWith("kc.org.")));
assertFalse(testRealm().groups().groups("", 0, 100, false).stream().anyMatch(group -> group.getAttributes().containsKey("kc.org")));
}
}

0 comments on commit c0728c7

Please sign in to comment.