You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by bd...@apache.org on 2023/02/01 22:12:08 UTC

[directory-scimple] 01/01: Add additional integration tests

This is an automated email from the ASF dual-hosted git repository.

bdemers pushed a commit to branch more-its
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git

commit b759c99a4a79d1e6817f04cd327d3a1c57702760
Author: Brian Demers <bd...@apache.org>
AuthorDate: Wed Feb 1 17:12:00 2023 -0500

    Add additional integration tests
    
    - User PATCH IT
    - Group CRUD IT and related fixes in examples
---
 .../directory/scim/compliance/tests/GroupsIT.java  | 188 +++++++++++++++++++++
 .../scim/compliance/tests/ScimpleITSupport.java    |  70 ++++++++
 .../directory/scim/compliance/tests/UsersIT.java   |  89 +++++++++-
 .../jersey/service/InMemoryGroupService.java       |   3 +-
 .../memory/service/InMemoryGroupService.java       |   5 +-
 .../spring/service/InMemoryGroupService.java       |   1 +
 .../server/it/testapp/InMemoryGroupService.java    |  18 +-
 .../scim/spring/it/app/InMemoryGroupService.java   |  18 +-
 8 files changed, 371 insertions(+), 21 deletions(-)

diff --git a/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/GroupsIT.java b/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/GroupsIT.java
index 1ddf5024..8c54336f 100644
--- a/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/GroupsIT.java
+++ b/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/GroupsIT.java
@@ -21,6 +21,7 @@ package org.apache.directory.scim.compliance.tests;
 
 import org.apache.directory.scim.compliance.junit.EmbeddedServerExtension;
 import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
@@ -44,4 +45,191 @@ public class GroupsIT extends ScimpleITSupport {
         "Resources[0].id", not(emptyString())
       );
   }
+
+  @Test
+  @Order(10)
+  @DisplayName("Create group with member")
+  public void createGroup() {
+
+    String email = randomEmail("createGroup");
+    String userId = createUser(randomName("createGroupTest"), email);
+    String groupName = randomName("group-createGroup");
+    String body = "{" +
+      "\"schemas\": [\"urn:ietf:params:scim:schemas:core:2.0:Group\"]," +
+      "\"displayName\": \"" + groupName + "\"," +
+      "\"members\": [{" +
+        "\"value\": \"" + userId + "\"," +
+        "\"display\": \"" + email + "\"" +
+      "}]}";
+
+    String id = post("/Groups", body)
+      .statusCode(201)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:Group"),
+        "id", not(emptyString()),
+        "displayName", is(groupName),
+        "members[0].value", is(userId),
+        "members[0].display", is(email)
+      )
+      .extract().jsonPath().get("id");
+
+    // retrieve the group by id
+    get("/Groups/" + id)
+      .statusCode(200)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:Group"),
+        "id", not(emptyString()),
+        "displayName", is(groupName),
+        "members[0].value", is(userId),
+        "members[0].display", is(email)
+      );
+
+    // posting same content again should return a conflict (409)
+    post("/Groups", body)
+      .statusCode(409)
+      .body(
+        "schemas", hasItem(SCHEMA_ERROR_RESPONSE),
+        "detail", not(emptyString())
+      );
+  }
+
+  @Test
+  @DisplayName("Test invalid Group by ID")
+  public void invalidUserId() {
+    String invalidId = randomName("invalidUserId");
+
+    get("/Groups/" + invalidId)
+      .statusCode(404)
+      .body(
+        "schemas", hasItem(SCHEMA_ERROR_RESPONSE),
+        "detail", not(emptyString())
+      );
+  }
+
+  @Test
+  @DisplayName("Delete Group")
+  public void deleteGroup() {
+
+    String groupName = randomName("group-deleteGroup");
+    String body = "{" +
+      "\"schemas\": [\"urn:ietf:params:scim:schemas:core:2.0:Group\"]," +
+      "\"displayName\": \"" + groupName + "\"," +
+      "\"members\": []}";
+
+    String id = post("/Groups", body)
+      .statusCode(201)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:Group"),
+        "id", not(emptyString())
+      )
+      .extract().jsonPath().get("id");
+
+    delete("/Groups/" + id)
+      .statusCode(204);
+  }
+
+  @Test
+  @DisplayName("Update Group")
+  public void updateGroup() {
+    String email = randomEmail("updateGroup");
+    String userId = createUser(randomName("updateGroupTest"), email);
+    String groupName = randomName("group-updateGroup");
+    String body = "{" +
+      "\"schemas\": [\"urn:ietf:params:scim:schemas:core:2.0:Group\"]," +
+      "\"displayName\": \"" + groupName + "\"," +
+      "\"members\": [{" +
+        "\"value\": \"" + userId + "\"," +
+        "\"display\": \"" + email + "\"" +
+      "}]}";
+
+    String updatedBody = "{" +
+      "\"schemas\": [\"urn:ietf:params:scim:schemas:core:2.0:Group\"]," +
+      "\"displayName\": \"" + groupName + "\"," +
+      "\"members\": []}";
+
+    String id = post("/Groups", body)
+      .statusCode(201)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:Group"),
+        "id", not(emptyString()),
+        "displayName", is(groupName),
+        "members[0].value", is(userId),
+        "members[0].display", is(email)
+      )
+      .extract().jsonPath().get("id");
+
+    // update Group,
+    put("/Groups/" + id, updatedBody)
+      .statusCode(200)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:Group"),
+        "members", empty()
+      );
+  }
+
+  @Test
+  @DisplayName("Update Group with PATCH")
+  public void updateGroupWithPatch() {
+    String email = randomEmail("updateGroupWithPatch");
+    String userId = createUser(randomName("updateGroupWithPatchTest"), email);
+    String groupName = randomName("group-updateGroupWithPatch");
+    String body = "{" +
+      "\"schemas\": [\"urn:ietf:params:scim:schemas:core:2.0:Group\"]," +
+      "\"displayName\": \"" + groupName + "\"," +
+      "\"members\": [{" +
+      "\"value\": \"" + userId + "\"," +
+      "\"display\": \"" + email + "\"" +
+      "}]}";
+
+    String patchBody = "{" +
+      "\"schemas\": [\"urn:ietf:params:scim:api:messages:2.0:PatchOp\"]," +
+      "\"Operations\": [{" +
+        "\"op\": \"remove\"," +
+        "\"path\": \"members[value eq \\\"" + userId + "\\\"]\"" +
+      "}]}";
+
+    String id = post("/Groups", body)
+      .statusCode(201)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:Group"),
+        "id", not(emptyString()),
+        "displayName", is(groupName),
+        "members[0].value", is(userId),
+        "members[0].display", is(email)
+      )
+      .extract().jsonPath().get("id");
+
+    // update Group,
+    patch("/Groups/" + id, patchBody)
+      .statusCode(200)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:Group"),
+        "members", empty()
+      );
+  }
+
+  String createUser(String name, String email) {
+    String body = "{" +
+      "\"schemas\":[\"urn:ietf:params:scim:schemas:core:2.0:User\"]," +
+      "\"userName\":\"" + email + "\"," +
+      "\"name\":{" +
+        "\"givenName\":\"Given-" + name + "\"," +
+        "\"familyName\":\"Family-" + name + "\"}," +
+      "\"emails\":[{" +
+        "\"primary\":true," +
+        "\"value\":\"" + email + "\"," +
+        "\"type\":\"work\"}]," +
+      "\"displayName\":\"Given-" + name + " Family-" + name + "\"," +
+      "\"active\":true" +
+      "}";
+
+    return post("/Users", body)
+      .statusCode(201)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:User"),
+        "active", is(true),
+        "id", not(emptyString())
+      )
+      .extract().jsonPath().get("id");
+  }
 }
diff --git a/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/ScimpleITSupport.java b/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/ScimpleITSupport.java
index d28b2f08..b7623610 100644
--- a/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/ScimpleITSupport.java
+++ b/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/ScimpleITSupport.java
@@ -27,6 +27,7 @@ import io.restassured.response.Response;
 import io.restassured.response.ValidatableResponse;
 import io.restassured.specification.FilterableRequestSpecification;
 import io.restassured.specification.FilterableResponseSpecification;
+import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.directory.scim.compliance.junit.EmbeddedServerExtension.ScimServerUri;
 import org.hamcrest.Matcher;
 
@@ -123,12 +124,81 @@ public class ScimpleITSupport {
     return responseSpec;
   }
 
+  protected ValidatableResponse put(String path, String body) {
+    ValidatableResponse responseSpec =
+      given()
+        .urlEncodingEnabled(false) // URL encoding is handled but the URI
+        .redirects().follow(false)
+        .accept(SCIM_MEDIA_TYPE)
+        .contentType(SCIM_MEDIA_TYPE)
+        .headers(requestHeaders)
+      .when()
+        .filter(logging(loggingEnabled))
+        .body(body)
+        .put(uri(path))
+      .then()
+        .contentType(SCIM_MEDIA_TYPE);
+
+    if (loggingEnabled) {
+      responseSpec.log().everything();
+    }
+    return responseSpec;
+  }
+
+  protected ValidatableResponse patch(String path, String body) {
+    ValidatableResponse responseSpec =
+      given()
+        .urlEncodingEnabled(false) // URL encoding is handled but the URI
+        .redirects().follow(false)
+        .accept(SCIM_MEDIA_TYPE)
+        .contentType(SCIM_MEDIA_TYPE)
+        .headers(requestHeaders)
+      .when()
+        .filter(logging(loggingEnabled))
+        .body(body)
+        .patch(uri(path))
+      .then()
+        .contentType(SCIM_MEDIA_TYPE);
+
+    if (loggingEnabled) {
+      responseSpec.log().everything();
+    }
+    return responseSpec;
+  }
+
+  protected ValidatableResponse delete(String path) {
+    ValidatableResponse responseSpec =
+      given()
+        .urlEncodingEnabled(false) // URL encoding is handled but the URI
+        .redirects().follow(false)
+        .accept(SCIM_MEDIA_TYPE)
+        .contentType(SCIM_MEDIA_TYPE)
+        .headers(requestHeaders)
+      .when()
+        .filter(logging(loggingEnabled))
+      .delete(uri(path))
+        .then();
+
+    if (loggingEnabled) {
+      responseSpec.log().everything();
+    }
+    return responseSpec;
+  }
+
   static Filter logging(boolean enabled) {
     return enabled
       ? new RequestLoggingFilter(LogDetail.ALL)
       : new NoOpFilter();
   }
 
+  static String randomName(String base) {
+    return base + RandomStringUtils.randomAlphanumeric(10);
+  }
+
+  static String randomEmail(String base) {
+    return base + "-" + RandomStringUtils.randomAlphanumeric(10) + "@example.com";
+  }
+
   static Matcher<?> isBoolean() {
     return instanceOf(Boolean.class);
   }
diff --git a/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/UsersIT.java b/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/UsersIT.java
index 34e05ee2..27bd4aea 100644
--- a/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/UsersIT.java
+++ b/scim-compliance-tests/src/main/java/org/apache/directory/scim/compliance/tests/UsersIT.java
@@ -33,8 +33,8 @@ import static org.hamcrest.Matchers.*;
 @ExtendWith(EmbeddedServerExtension.class)
 public class UsersIT extends ScimpleITSupport {
 
-  private final String givenName = "Given-" + RandomStringUtils.randomAlphanumeric(10);
-  private final String familyName = "Family-" + RandomStringUtils.randomAlphanumeric(10);
+  private final String givenName = randomName("Given-");
+  private final String familyName = randomName("Family-");
   private final String displayName = givenName + " " + familyName;
   private final String email = givenName + "." + familyName + "@example.com";
 
@@ -155,6 +155,50 @@ public class UsersIT extends ScimpleITSupport {
       );
   }
 
+  @Test
+  @DisplayName("Update User")
+  public void updateUser() {
+
+    String body = "{" +
+      "\"schemas\":[\"urn:ietf:params:scim:schemas:core:2.0:User\"]," +
+      "\"userName\":\"updateUser@example.com\"," +
+      "\"name\":{" +
+        "\"givenName\":\"Given-updateUser\"," +
+        "\"familyName\":\"Family-updateUser\"}," +
+      "\"emails\":[{" +
+        "\"primary\":true," +
+        "\"value\":\"updateUser@example.com\"," +
+        "\"type\":\"work\"}]," +
+      "\"displayName\":\"Given-updateUser Family-updateUser\"," +
+      "\"active\":true" +
+      "}";
+
+    String id = post("/Users", body)
+      .statusCode(201)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:User"),
+        "active", is(true),
+        "id", not(emptyString())
+      )
+      .extract().jsonPath().get("id");
+
+    String updatedBody = body.replaceFirst("}$",
+      ",\"phoneNumbers\": [{\"value\": \"555-555-5555\",\"type\": \"work\"}]}");
+
+    put("/Users/" + id, updatedBody)
+      .statusCode(200)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:User"),
+        "active", is(true),
+        "id", not(emptyString()),
+        "name.givenName", is("Given-updateUser"),
+        "name.familyName", is("Family-updateUser"),
+        "userName", equalToIgnoringCase("updateUser@example.com"),
+        "phoneNumbers[0].value", is("555-555-5555"),
+        "phoneNumbers[0].type", is("work")
+      );
+  }
+
   @Test
   @DisplayName("Username Case Sensitivity Check")
   public void userNameByFilter() {
@@ -176,4 +220,45 @@ public class UsersIT extends ScimpleITSupport {
         "totalResults", is(1)
       );
   }
+
+  @Test
+  @DisplayName("Deactivate user with PATCH")
+  public void deactivateWithPatch() {
+    String body = "{" +
+      "\"schemas\":[\"urn:ietf:params:scim:schemas:core:2.0:User\"]," +
+      "\"userName\":\"deactivateWithPatch@example.com\"," +
+      "\"name\":{" +
+        "\"givenName\":\"Given-deactivateWithPatch\"," +
+        "\"familyName\":\"Family-deactivateWithPatch\"}," +
+      "\"emails\":[{" +
+        "\"primary\":true," +
+        "\"value\":\"deactivateWithPatch@example.com\"," +
+        "\"type\":\"work\"}]," +
+      "\"displayName\":\"Given-deactivateWithPatch Family-deactivateWithPatch\"," +
+      "\"active\":true" +
+      "}";
+
+    String id = post("/Users", body)
+      .statusCode(201)
+      .body(
+        "schemas", contains("urn:ietf:params:scim:schemas:core:2.0:User"),
+        "active", is(true),
+        "id", not(emptyString())
+      )
+      .extract().jsonPath().get("id");
+
+    String patchBody = "{" +
+      "\"schemas\": [\"urn:ietf:params:scim:api:messages:2.0:PatchOp\"]," +
+      "\"Operations\": [{" +
+        "\"op\": \"replace\"," +
+        "\"value\": {" +
+          "\"active\": false" +
+        "}}]}";
+
+    patch("/Users/" + id, patchBody)
+      .statusCode(200)
+      .body(
+        "active", is(false)
+      );
+  }
 }
diff --git a/scim-server-examples/scim-server-jersey/src/main/java/org/apache/directory/scim/example/jersey/service/InMemoryGroupService.java b/scim-server-examples/scim-server-jersey/src/main/java/org/apache/directory/scim/example/jersey/service/InMemoryGroupService.java
index 897e3013..3dc64d28 100644
--- a/scim-server-examples/scim-server-jersey/src/main/java/org/apache/directory/scim/example/jersey/service/InMemoryGroupService.java
+++ b/scim-server-examples/scim-server-jersey/src/main/java/org/apache/directory/scim/example/jersey/service/InMemoryGroupService.java
@@ -66,6 +66,7 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
     ScimGroup group = new ScimGroup();
     group.setId(UUID.randomUUID().toString());
     group.setDisplayName("example-group");
+    group.setExternalId("example-group");
     groups.put(group.getId(), group);
   }
 
@@ -79,7 +80,7 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
     String id = UUID.randomUUID().toString();
 
     // if the external ID is not set, use the displayName instead
-    if (!StringUtils.isEmpty(resource.getExternalId())) {
+    if (StringUtils.isEmpty(resource.getExternalId())) {
       resource.setExternalId(resource.getDisplayName());
     }
 
diff --git a/scim-server-examples/scim-server-memory/src/main/java/org/apache/directory/scim/example/memory/service/InMemoryGroupService.java b/scim-server-examples/scim-server-memory/src/main/java/org/apache/directory/scim/example/memory/service/InMemoryGroupService.java
index c2a1a507..22bfe2bf 100644
--- a/scim-server-examples/scim-server-memory/src/main/java/org/apache/directory/scim/example/memory/service/InMemoryGroupService.java
+++ b/scim-server-examples/scim-server-memory/src/main/java/org/apache/directory/scim/example/memory/service/InMemoryGroupService.java
@@ -64,8 +64,9 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
   @PostConstruct
   public void init() {
     ScimGroup group = new ScimGroup();
-    group.setDisplayName("example-group");
     group.setId(UUID.randomUUID().toString());
+    group.setDisplayName("example-group");
+    group.setExternalId("example-group");
     groups.put(group.getId(), group);
   }
 
@@ -79,7 +80,7 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
     String id = UUID.randomUUID().toString();
 
     // if the external ID is not set, use the displayName instead
-    if (!StringUtils.isEmpty(resource.getExternalId())) {
+    if (StringUtils.isEmpty(resource.getExternalId())) {
       resource.setExternalId(resource.getDisplayName());
     }
 
diff --git a/scim-server-examples/scim-server-spring-boot/src/main/java/org/apache/directory/scim/example/spring/service/InMemoryGroupService.java b/scim-server-examples/scim-server-spring-boot/src/main/java/org/apache/directory/scim/example/spring/service/InMemoryGroupService.java
index 41a89668..388a599a 100644
--- a/scim-server-examples/scim-server-spring-boot/src/main/java/org/apache/directory/scim/example/spring/service/InMemoryGroupService.java
+++ b/scim-server-examples/scim-server-spring-boot/src/main/java/org/apache/directory/scim/example/spring/service/InMemoryGroupService.java
@@ -56,6 +56,7 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
     ScimGroup group = new ScimGroup();
     group.setId(UUID.randomUUID().toString());
     group.setDisplayName("example-group");
+    group.setExternalId("example-group");
     groups.put(group.getId(), group);
   }
 
diff --git a/scim-server/src/test/java/org/apache/directory/scim/server/it/testapp/InMemoryGroupService.java b/scim-server/src/test/java/org/apache/directory/scim/server/it/testapp/InMemoryGroupService.java
index f5e26764..9b7155b2 100644
--- a/scim-server/src/test/java/org/apache/directory/scim/server/it/testapp/InMemoryGroupService.java
+++ b/scim-server/src/test/java/org/apache/directory/scim/server/it/testapp/InMemoryGroupService.java
@@ -24,6 +24,7 @@ import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
 import jakarta.ws.rs.core.Response;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.directory.scim.server.exception.UnableToCreateResourceException;
 import org.apache.directory.scim.server.exception.UnableToUpdateResourceException;
 import org.apache.directory.scim.core.repository.Repository;
@@ -41,6 +42,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import java.util.stream.Collectors;
 
 @Named
@@ -61,7 +63,9 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
   @PostConstruct
   public void init() {
     ScimGroup group = new ScimGroup();
-    group.setId("example-group");
+    group.setId(UUID.randomUUID().toString());
+    group.setDisplayName("example-group");
+    group.setExternalId("example-group");
     groups.put(group.getId(), group);
   }
 
@@ -72,18 +76,16 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
 
   @Override
   public ScimGroup create(ScimGroup resource) throws UnableToCreateResourceException {
-    String resourceId = resource.getId();
-    int idCandidate = resource.hashCode();
-    String id = resourceId != null ? resourceId : Integer.toString(idCandidate);
+    String id = UUID.randomUUID().toString();
 
-    while (groups.containsKey(id)) {
-      id = Integer.toString(idCandidate);
-      ++idCandidate;
+    // if the external ID is not set, use the displayName instead
+    if (StringUtils.isEmpty(resource.getExternalId())) {
+      resource.setExternalId(resource.getDisplayName());
     }
 
     // check to make sure the group doesn't already exist
     boolean existingUserFound = groups.values().stream()
-      .anyMatch(group -> group.getExternalId().equals(resource.getExternalId()));
+      .anyMatch(group -> resource.getExternalId().equals(group.getExternalId()));
     if (existingUserFound) {
       // HTTP leaking into data layer
       throw new UnableToCreateResourceException(Response.Status.CONFLICT, "Group '" + resource.getExternalId() + "' already exists.");
diff --git a/support/spring-boot/src/test/java/org/apache/directory/scim/spring/it/app/InMemoryGroupService.java b/support/spring-boot/src/test/java/org/apache/directory/scim/spring/it/app/InMemoryGroupService.java
index 545d919a..5df9feb6 100644
--- a/support/spring-boot/src/test/java/org/apache/directory/scim/spring/it/app/InMemoryGroupService.java
+++ b/support/spring-boot/src/test/java/org/apache/directory/scim/spring/it/app/InMemoryGroupService.java
@@ -21,6 +21,7 @@ package org.apache.directory.scim.spring.it.app;
 
 import jakarta.annotation.PostConstruct;
 import jakarta.ws.rs.core.Response;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.directory.scim.core.repository.Repository;
 import org.apache.directory.scim.core.repository.UpdateRequest;
 import org.apache.directory.scim.core.schema.SchemaRegistry;
@@ -35,6 +36,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import java.util.stream.Collectors;
 
 @Service
@@ -51,7 +53,9 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
   @PostConstruct
   public void init() {
     ScimGroup group = new ScimGroup();
-    group.setId("example-group");
+    group.setId(UUID.randomUUID().toString());
+    group.setDisplayName("example-group");
+    group.setExternalId("example-group");
     groups.put(group.getId(), group);
   }
 
@@ -62,18 +66,16 @@ public class InMemoryGroupService implements Repository<ScimGroup> {
 
   @Override
   public ScimGroup create(ScimGroup resource) throws UnableToCreateResourceException {
-    String resourceId = resource.getId();
-    int idCandidate = resource.hashCode();
-    String id = resourceId != null ? resourceId : Integer.toString(idCandidate);
+    String id = UUID.randomUUID().toString();
 
-    while (groups.containsKey(id)) {
-      id = Integer.toString(idCandidate);
-      ++idCandidate;
+    // if the external ID is not set, use the displayName instead
+    if (StringUtils.isEmpty(resource.getExternalId())) {
+      resource.setExternalId(resource.getDisplayName());
     }
 
     // check to make sure the group doesn't already exist
     boolean existingUserFound = groups.values().stream()
-      .anyMatch(group -> group.getExternalId().equals(resource.getExternalId()));
+      .anyMatch(group -> resource.getExternalId().equals(group.getExternalId()));
     if (existingUserFound) {
       // HTTP leaking into data layer
       throw new UnableToCreateResourceException(Response.Status.CONFLICT, "Group '" + resource.getExternalId() + "' already exists.");