fix: check for existing groups

server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java

@@ -46,8 +46,8 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.UUID;
-import java.util.stream.Collectors;
 
 import static java.util.Optional.ofNullable;
 import static org.springframework.http.HttpStatus.CONFLICT;
@@ -268,15 +268,12 @@ public ResponseEntity<IdentityZone> updateIdentityZone(
             body.setId(id);
             body = validator.validate(body, IdentityZoneValidator.Mode.MODIFY);
 
-            // check for groups which would be not allowed after the update
             UserConfig userConfig = body.getConfig().getUserConfig();
             if (!userConfig.allGroupsAllowed()) {
-                List<String> existingGroupNames = groupProvisioning.retrieveAll(body.getId())
-                                                    .stream()
-                                                    .map(ScimGroup::getDescription)
-                                                    .collect(Collectors.toList());
-                if (!userConfig.resultingAllowedGroups().containsAll(existingGroupNames)) {
-                    throw new UnprocessableEntityException("The identity zone details contains not-allowed groups.");
+                Set<String> allowedGroups = userConfig.resultingAllowedGroups();
+                // check for groups which would be not allowed after the update
+                if(groupProvisioning.retrieveAll(body.getId()).stream().anyMatch(g -> !allowedGroups.contains(g.getDisplayName()))) {
+                    throw new UnprocessableEntityException("The identity zone user configuration contains not-allowed groups.");
                 }
             }
 

server/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpointsTests.java

@@ -1,5 +1,6 @@
 package org.cloudfoundry.identity.uaa.zone;
 
+import org.cloudfoundry.identity.uaa.error.UaaException;
 import org.cloudfoundry.identity.uaa.extensions.PollutionPreventionExtension;
 import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
 import org.cloudfoundry.identity.uaa.saml.SamlKey;
@@ -15,9 +16,12 @@
 import org.springframework.validation.BindingResult;
 
 import java.util.List;
+import java.util.stream.Collectors;
 
+import static org.cloudfoundry.identity.uaa.util.AssertThrowsWithMessage.assertThrowsWithMessageThat;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -133,6 +137,38 @@ void restore_keys() {
 
     }
 
+    @Test
+    void extend_zone_allowed_groups_on_update() throws InvalidIdentityZoneDetailsException {
+        when(mockIdentityZoneValidator.validate(any(), any())).then(invocation -> invocation.getArgument(0));
+
+        IdentityZoneEndpoints spy = Mockito.spy(endpoints);
+        identityZone = createZone();
+        identityZone.getConfig().getUserConfig().setAllowedGroups(List.of("sps.write", "sps.read", "idps.write", "idps.read"));
+        when(mockIdentityZoneProvisioning.retrieveIgnoreActiveFlag(identityZone.getId())).thenReturn(identityZone);
+        when(mockIdentityZoneProvisioning.update(same(identityZone))).thenReturn(identityZone);
+        List<ScimGroup> existingScimGroups = List.of("sps.write", "sps.read").stream().
+            map(e -> new ScimGroup(e, e, identityZone.getId())).collect(Collectors.toList());
+        when(mockScimGroupProvisioning.retrieveAll(identityZone.getId())).thenReturn(existingScimGroups);
+        spy.updateIdentityZone(identityZone, identityZone.getId());
+        verify(spy, times(1)).createUserGroups(same(identityZone));
+    }
+
+    @Test
+    void reduce_zone_allowed_groups_on_update_should_fail() throws InvalidIdentityZoneDetailsException {
+        when(mockIdentityZoneValidator.validate(any(), any())).then(invocation -> invocation.getArgument(0));
+
+        identityZone = createZone();
+        identityZone.getConfig().getUserConfig().setAllowedGroups(List.of("clients.admin", "clients.write", "clients.read", "clients.secret"));
+        when(mockIdentityZoneProvisioning.retrieveIgnoreActiveFlag(identityZone.getId())).thenReturn(identityZone);
+        List<ScimGroup> existingScimGroups = List.of("sps.write", "sps.read", "idps.write", "idps.read",
+            "clients.admin", "clients.write", "clients.read", "clients.secret", "scim.write", "scim.read", "scim.create", "scim.userids",
+            "scim.zones", "groups.update", "password.write", "oauth.login", "uaa.admin").stream().
+            map(e -> new ScimGroup(e, e, identityZone.getId())).collect(Collectors.toList());
+        when(mockScimGroupProvisioning.retrieveAll(identityZone.getId())).thenReturn(existingScimGroups);
+        assertThrowsWithMessageThat(UaaException.class, () -> endpoints.updateIdentityZone(identityZone, identityZone.getId()),
+            is("The identity zone user configuration contains not-allowed groups."));
+    }
+
     private static IdentityZone createZone() {
         IdentityZone zone = MultitenancyFixture.identityZone("id", "subdomain");
         IdentityZoneConfiguration config = zone.getConfig();

uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java

@@ -144,6 +144,43 @@ public void testCreateZone() {
         assertNull(ObjectUtils.castInstance(identityProvider.getConfig(),UaaIdentityProviderDefinition.class).getPasswordPolicy());
     }
 
+    @Test
+    public void testUpdateZoneAllowedGroups() {
+        IdentityZone idZone = new IdentityZone();
+        String id = UUID.randomUUID().toString();
+        idZone.setId(id);
+        idZone.setSubdomain(id);
+        idZone.setName("testUpdateZone-"+id);
+        ResponseEntity<String> response = client.exchange(
+                serverRunning.getUrl("/identity-zones"),
+                HttpMethod.POST,
+                new HttpEntity<>(idZone),
+                new ParameterizedTypeReference<String>() {},
+                id);
+        assertEquals(response.getBody(), HttpStatus.CREATED, response.getStatusCode());
+
+        List<String> existingGroups = List.of("sps.write", "sps.read", "idps.write", "idps.read", "clients.admin", "clients.write", "clients.read",
+            "clients.secret", "scim.write", "scim.read", "scim.create", "scim.userids", "scim.zones", "groups.update", "password.write", "oauth.login", "uaa.admin");
+        idZone.getConfig().getUserConfig().setAllowedGroups(existingGroups);
+        response = client.exchange(
+                serverRunning.getUrl("/identity-zones/"+id),
+                HttpMethod.PUT,
+                new HttpEntity<>(idZone),
+                new ParameterizedTypeReference<String>() {},
+                id);
+        assertEquals(response.getBody() , HttpStatus.OK, response.getStatusCode());
+
+        List<String> notAllExistingGroups = List.of("clients.admin", "clients.write", "clients.read", "clients.secret");
+        idZone.getConfig().getUserConfig().setAllowedGroups(notAllExistingGroups);
+        response = client.exchange(
+            serverRunning.getUrl("/identity-zones/"+id),
+            HttpMethod.PUT,
+            new HttpEntity<>(idZone),
+            new ParameterizedTypeReference<String>() {},
+            id);
+        assertEquals(response.getBody() , HttpStatus.UNPROCESSABLE_ENTITY, response.getStatusCode());
+    }
+
     @Test
     public void testCreateZoneWithClient() {
         IdentityZone idZone = new IdentityZone();