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 committed May 7, 2024
1 parent 0e9b42a commit b6b4acc
Show file tree
Hide file tree
Showing 10 changed files with 286 additions and 20 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.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -73,10 +74,14 @@ public OrganizationModel create(String name, Set<String> domains) {
throw new ModelValidationException("Name can not be null");
}

GroupModel group = createOrganizationGroup(name);
OrganizationEntity entity = new OrganizationEntity();
if (getAllStream(name, true, null, null).findAny().isPresent()) {
throw new ModelDuplicateException("A organization with the same name already exists.");
}

entity.setId(KeycloakModelUtils.generateId());
String id = KeycloakModelUtils.generateId();
GroupModel group = createOrganizationGroup(name, id);
OrganizationEntity entity = new OrganizationEntity();
entity.setId(id);
entity.setGroupId(group.getId());
entity.setRealmId(realm.getId());
entity.setName(name);
Expand Down Expand Up @@ -327,21 +332,18 @@ private OrganizationEntity getEntity(String id, boolean failIfNotFound) {
return entity;
}

private GroupModel createOrganizationGroup(String name) {
private GroupModel createOrganizationGroup(String name, String orgId) {
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, orgId), 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 groupId = KeycloakModelUtils.generateId();
GroupModel group = groupProvider.createGroup(realm, groupId, groupId);
group.setSingleAttribute(ORGANIZATION_ATTRIBUTE, orgId);

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 @@ -61,6 +61,7 @@
import java.util.stream.Stream;
import org.keycloak.utils.GroupUtils;

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

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

checkForOrgRelatedGroupRep(session, 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 @@ -194,6 +197,8 @@ public Response addChild(GroupRepresentation rep) {
throw ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
}

checkForOrgRelatedGroupRep(session, rep);

try {
Response.ResponseBuilder builder = Response.status(204);
GroupModel child = null;
Expand Down Expand Up @@ -367,6 +372,7 @@ 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);

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

}

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

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


/**
Expand Down Expand Up @@ -125,6 +127,9 @@ public GroupResource getGroupById(@PathParam("group-id") String id) {
if (group == null) {
throw new NotFoundException("Could not find group by id");
}

checkForOrgRelatedGroupModel(session, group);

return new GroupResource(realm, group, session, this.auth, adminEvent);
}

Expand Down Expand Up @@ -176,6 +181,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 @@ -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
74 changes: 74 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,74 @@
/*
* 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 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 @@ -30,6 +30,8 @@
import org.keycloak.admin.client.resource.OrganizationResource;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.admin.client.resource.UsersResource;
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 @@ -242,4 +244,16 @@ protected UserRepresentation getUserRepresentation(String userEmail) {
Assert.assertEquals(1, reps.size());
return reps.get(0);
}

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 @@ -289,11 +289,11 @@ 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")));
}

@Test
Expand Down

0 comments on commit b6b4acc

Please sign in to comment.