diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java index 778dabe37e6..af9e081e0d7 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java +++ b/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 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 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 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."); } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpointsTests.java index 2562544442a..76f350c3e74 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneEndpointsTests.java +++ b/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 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 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(); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java index 34931c0980d..4cc3a45c8c8 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/IdentityZoneEndpointsIntegrationTests.java +++ b/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 response = client.exchange( + serverRunning.getUrl("/identity-zones"), + HttpMethod.POST, + new HttpEntity<>(idZone), + new ParameterizedTypeReference() {}, + id); + assertEquals(response.getBody(), HttpStatus.CREATED, response.getStatusCode()); + + List 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() {}, + id); + assertEquals(response.getBody() , HttpStatus.OK, response.getStatusCode()); + + List 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() {}, + id); + assertEquals(response.getBody() , HttpStatus.UNPROCESSABLE_ENTITY, response.getStatusCode()); + } + @Test public void testCreateZoneWithClient() { IdentityZone idZone = new IdentityZone();