Do not return an error when moving a group to the current parent

Closes https://github.com/keycloak/keycloak/issues/21242

services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java

@@ -24,6 +24,7 @@ import org.keycloak.events.admin.ResourceType;
 import org.keycloak.models.Constants;
 import org.keycloak.models.GroupModel;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ModelDuplicateException;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.models.utils.ModelToRepresentation;
@@ -31,7 +32,6 @@ import org.keycloak.representations.idm.GroupRepresentation;
 import org.keycloak.representations.idm.ManagementPermissionReference;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.services.ErrorResponse;
-import org.keycloak.services.Urls;
 import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
 import org.keycloak.services.resources.admin.permissions.AdminPermissionManagement;
 import org.keycloak.services.resources.admin.permissions.AdminPermissions;
@@ -156,39 +156,41 @@ public class GroupResource {
         if (ObjectUtil.isBlank(groupName)) {
             throw ErrorResponse.error("Group name is missing", Response.Status.BAD_REQUEST);
         }
-        boolean childExists = group.getSubGroupsStream().anyMatch(s -> Objects.equals(s.getName(), groupName));
+
-        if (childExists) {
+        try {
+            Response.ResponseBuilder builder = Response.status(204);
+            GroupModel child = null;
+            if (rep.getId() != null) {
+                child = realm.getGroupById(rep.getId());
+                if (child == null) {
+                    throw new NotFoundException("Could not find child by id");
+                }
+                if (!Objects.equals(child.getParentId(), group.getId())) {
+                    realm.moveGroup(child, group);
+                }
+                adminEvent.operation(OperationType.UPDATE);
+            } else {
+                child = realm.createGroup(groupName, group);
+                updateGroup(rep, child, realm, session);
+                URI uri = session.getContext().getUri().getBaseUriBuilder()
+                        .path(AdminRoot.class)
+                        .path(AdminRoot.class, "getRealmsAdmin")
+                        .path(RealmsAdminResource.class, "getRealmAdmin")
+                        .path(RealmAdminResource.class, "getGroups")
+                        .path(GroupsResource.class, "getGroupById")
+                        .build(realm.getName(), child.getId());
+                builder.status(201).location(uri);
+                rep.setId(child.getId());
+                adminEvent.operation(OperationType.CREATE);
+
+            }
+            adminEvent.resourcePath(session.getContext().getUri()).representation(rep).success();
+
+            GroupRepresentation childRep = ModelToRepresentation.toGroupHierarchy(child, true);
+            return builder.type(MediaType.APPLICATION_JSON_TYPE).entity(childRep).build();
+        } catch (ModelDuplicateException e) {
             throw ErrorResponse.exists("Sibling group named '" + groupName + "' already exists.");
         }
-
-        Response.ResponseBuilder builder = Response.status(204);
-        GroupModel child = null;
-        if (rep.getId() != null) {
-            child = realm.getGroupById(rep.getId());
-            if (child == null) {
-                throw new NotFoundException("Could not find child by id");
-            }
-            realm.moveGroup(child, group);
-            adminEvent.operation(OperationType.UPDATE);
-        } else {
-            child = realm.createGroup(groupName, group);
-            updateGroup(rep, child, realm, session);
-            URI uri = session.getContext().getUri().getBaseUriBuilder()
-                    .path(AdminRoot.class)
-                    .path(AdminRoot.class, "getRealmsAdmin")
-                    .path(RealmsAdminResource.class, "getRealmAdmin")
-                    .path(RealmAdminResource.class, "getGroups")
-                    .path(GroupsResource.class, "getGroupById")
-                    .build(realm.getName(), child.getId());
-            builder.status(201).location(uri);
-            rep.setId(child.getId());
-            adminEvent.operation(OperationType.CREATE);
-
-        }
-        adminEvent.resourcePath(session.getContext().getUri()).representation(rep).success();
-
-        GroupRepresentation childRep = ModelToRepresentation.toGroupHierarchy(child, true);
-        return builder.type(MediaType.APPLICATION_JSON_TYPE).entity(childRep).build();
     }
 
     public static void updateGroup(GroupRepresentation rep, GroupModel model, RealmModel realm, KeycloakSession session) {

services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java

@@ -157,7 +157,9 @@ public class GroupsResource {
                 if (child == null) {
                     throw new NotFoundException("Could not find child by id");
                 }
-                realm.moveGroup(child, null);
+                if (child.getParentId() != null) {
+                    realm.moveGroup(child, null);
+                }
                 adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri());
             } else {
                 child = realm.createGroup(groupName);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java

@@ -183,10 +183,16 @@ public class GroupTest extends AbstractGroupTest {
         assertSameNameNotAllowed(response,"Top level group named 'top' already exists.");
         response.close();
 
+        // allow moving the group to top level (nothing is done)
+        response = realm.groups().add(topGroup);
+        assertEquals(Response.Status.NO_CONTENT, response.getStatusInfo());
+        response.close();
+
         GroupRepresentation level2Group = new GroupRepresentation();
         level2Group.setName("level2");
         response = realm.groups().group(topGroup.getId()).subGroup(level2Group);
         assertEquals(201, response.getStatus()); // created status
+        level2Group.setId(ApiUtil.getCreatedId(response));
         response.close();
 
         GroupRepresentation anotherlevel2Group = new GroupRepresentation();
@@ -194,6 +200,11 @@ public class GroupTest extends AbstractGroupTest {
         response = realm.groups().group(topGroup.getId()).subGroup(anotherlevel2Group);
         assertSameNameNotAllowed(response,"Sibling group named 'level2' already exists.");
         response.close();
+
+        // allow moving the group to the same parent (nothing is done)
+        response = realm.groups().group(topGroup.getId()).subGroup(level2Group);
+        assertEquals(Response.Status.NO_CONTENT, response.getStatusInfo());
+        response.close();
     }
 
     @Test