From b51e355873921748979a39f88fc23cc77d9fd1aa Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 24 Apr 2024 16:38:04 +0200 Subject: [PATCH 01/15] Remove deletion of alias IdP from JdbcIdentityProviderProvisioning.deleteByIdentityZone --- .../uaa/provider/JdbcIdentityProviderProvisioning.java | 7 ++----- .../JdbcIdentityProviderProvisioningTests.java | 10 +++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index 89f8eedf1de..490ebf15a78 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -40,7 +40,7 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi public static final String DELETE_IDENTITY_PROVIDER_BY_ORIGIN_SQL = "delete from identity_provider where identity_zone_id=? and origin_key = ?"; - public static final String DELETE_IDENTITY_PROVIDER_BY_ZONE_SQL = "delete from identity_provider where identity_zone_id=? or alias_zid=?"; + public static final String DELETE_IDENTITY_PROVIDER_BY_ZONE_SQL = "delete from identity_provider where identity_zone_id=?"; public static final String IDENTITY_PROVIDER_BY_ID_QUERY = "select " + ID_PROVIDER_FIELDS + " from identity_provider " + "where id=? and identity_zone_id=?"; @@ -150,12 +150,9 @@ protected void validate(IdentityProvider provider) { } } - /** - * Delete all identity providers in the given zone as well as all alias identity providers of them. - */ @Override public int deleteByIdentityZone(String zoneId) { - return jdbcTemplate.update(DELETE_IDENTITY_PROVIDER_BY_ZONE_SQL, zoneId, zoneId); + return jdbcTemplate.update(DELETE_IDENTITY_PROVIDER_BY_ZONE_SQL, zoneId); } @Override diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java index 72688c4335a..55dbbfef88e 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java @@ -69,7 +69,7 @@ void deleteProvidersInZone() { } @Test - void deleteByIdentityZone_ShouldAlsoDeleteAliasIdentityProviders() { + void deleteByIdentityZone_ShouldNotDeleteAliasIdentityProviders() { final String originSuffix = generator.generate(); // IdP 1: created in custom zone, no alias @@ -105,13 +105,13 @@ void deleteByIdentityZone_ShouldAlsoDeleteAliasIdentityProviders() { // delete by zone final int rowsDeleted = jdbcIdentityProviderProvisioning.deleteByIdentityZone(otherZoneId1); - // number should also include the alias IdP - Assertions.assertThat(rowsDeleted).isEqualTo(3); + // number should not include the alias IdP + Assertions.assertThat(rowsDeleted).isEqualTo(2); - // check if all three entries are gone + // the two IdPs in the custom zone should be deleted, the alias should still be present assertIdentityProviderDoesNotExist(createdIdp1.getId(), otherZoneId1); assertIdentityProviderDoesNotExist(createdIdp2.getId(), otherZoneId1); - assertIdentityProviderDoesNotExist(createdIdp2Alias.getId(), uaaZoneId); + assertIdentityProviderExists(createdIdp2Alias.getId(), uaaZoneId); } private void assertIdentityProviderExists(final String id, final String zoneId) { From 5a7fd8ce0f884d39d8cc43ed760673790856547d Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 24 Apr 2024 17:08:15 +0200 Subject: [PATCH 02/15] Reject deletion of identity zone if an IdP with alias exists in the zone --- .../uaa/zone/IdentityZoneEndpoints.java | 11 ++++++++ .../uaa/zone/IdentityZoneEndpointsTests.java | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+) 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 2bdee34005c..a15af3d1245 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 @@ -336,6 +336,17 @@ public ResponseEntity deleteIdentityZone(@PathVariable String id) IdentityZone zone = zoneDao.retrieveIgnoreActiveFlag(id); // ignore the id in the body, the id in the path is the only one that matters IdentityZoneHolder.set(zone); + + /* reject deletion if an IdP with alias exists in the zone - checking for users with alias is not required + * here, since they can only exist if their origin IdP has an alias as well */ + final List idps = idpDao.retrieveAll(false, zone.getId()); + final boolean idpWithAliasExists = idps.stream() + .map(IdentityProvider::getAliasZid) + .anyMatch(UaaStringUtils::isNotEmpty); + if (idpWithAliasExists) { + return new ResponseEntity<>(UNPROCESSABLE_ENTITY); + } + if (publisher != null && zone != null) { publisher.publishEvent(new EntityDeletedEvent<>(zone, SecurityContextHolder.getContext().getAuthentication(), IdentityZoneHolder.getCurrentZoneId())); logger.debug("Zone - deleted id[" + zone.getId() + "]"); 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 76f350c3e74..1fb01ca3459 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 @@ -2,10 +2,12 @@ import org.cloudfoundry.identity.uaa.error.UaaException; import org.cloudfoundry.identity.uaa.extensions.PollutionPreventionExtension; +import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.saml.SamlKey; import org.cloudfoundry.identity.uaa.scim.ScimGroup; import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning; +import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; @@ -13,11 +15,14 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.validation.BindingResult; import java.util.List; import java.util.stream.Collectors; +import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UAA; import static org.cloudfoundry.identity.uaa.util.AssertThrowsWithMessage.assertThrowsWithMessageThat; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -169,6 +174,28 @@ void reduce_zone_allowed_groups_on_update_should_fail() throws InvalidIdentityZo is("The identity zone user configuration contains not-allowed groups.")); } + @Test + void deleteIdentityZone_ShouldReject_IfIdpWithAliasExists() { + final IdentityZone idz = new IdentityZone(); + final String idzId = new AlphanumericRandomValueStringGenerator(5).generate(); + idz.setName(idzId); + idz.setId(idzId); + idz.setSubdomain(idzId); + when(mockIdentityZoneProvisioning.retrieveIgnoreActiveFlag(idzId)).thenReturn(idz); + + // arrange IdP with alias exists in zone + final IdentityProvider idpWithoutAlias = mock(IdentityProvider.class); + when(idpWithoutAlias.getAliasZid()).thenReturn(""); + final IdentityProvider idpWithAlias = mock(IdentityProvider.class); + when(idpWithAlias.getAliasZid()).thenReturn(UAA); + when(mockIdentityProviderProvisioning.retrieveAll(false, idzId)) + .thenReturn(List.of(idpWithoutAlias, idpWithAlias)); + + final ResponseEntity response = endpoints.deleteIdentityZone(idzId); + assertNotNull(response); + assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, response.getStatusCode()); + } + private static IdentityZone createZone() { IdentityZone zone = MultitenancyFixture.identityZone("id", "subdomain"); IdentityZoneConfiguration config = zone.getConfig(); From a6966f80974549833cd0d615eaf8dcbf699ead7c Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 24 Apr 2024 17:10:10 +0200 Subject: [PATCH 03/15] Add new status code to identity zone deletion documentation --- uaa/slateCustomizations/source/index.html.md.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 490ae2f633f..ec9a848fccd 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -871,6 +871,7 @@ _Error Codes_ | 401 | Unauthorized - Invalid token | | 403 | Forbidden - Insufficient scope (zone admins can only delete their own zone) | | 404 | Not Found - Zone does not exist | +| 422 | Unprocessable Entity - at least one IdP with alias exists in the zone | # Identity Providers From ce46e69e7d21b665bc2c172823b5db535356ea76 Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Thu, 25 Apr 2024 16:04:32 +0200 Subject: [PATCH 04/15] Fix Sonar: remove unnecessary clause in if statement --- .../cloudfoundry/identity/uaa/zone/IdentityZoneEndpoints.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a15af3d1245..be5a897ed29 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 @@ -347,7 +347,7 @@ public ResponseEntity deleteIdentityZone(@PathVariable String id) return new ResponseEntity<>(UNPROCESSABLE_ENTITY); } - if (publisher != null && zone != null) { + if (publisher != null) { publisher.publishEvent(new EntityDeletedEvent<>(zone, SecurityContextHolder.getContext().getAuthentication(), IdentityZoneHolder.getCurrentZoneId())); logger.debug("Zone - deleted id[" + zone.getId() + "]"); return new ResponseEntity<>(removeKeys(zone), OK); From b74a6209add7c16190667ba1cd584c2e4f129926 Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Thu, 25 Apr 2024 17:16:15 +0200 Subject: [PATCH 05/15] Add unit test for zone deletion success case --- .../uaa/zone/IdentityZoneEndpointsTests.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) 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 1fb01ca3459..8bd170d3df2 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.audit.event.EntityDeletedEvent; import org.cloudfoundry.identity.uaa.error.UaaException; import org.cloudfoundry.identity.uaa.extensions.PollutionPreventionExtension; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; @@ -8,6 +9,7 @@ import org.cloudfoundry.identity.uaa.scim.ScimGroup; import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning; import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; @@ -15,10 +17,12 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.validation.BindingResult; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -59,9 +63,17 @@ class IdentityZoneEndpointsTests { @Mock private IdentityZoneEndpointClientRegistrationService mockIdentityZoneEndpointClientRegistrationService; + @Mock + private ApplicationEventPublisher mockApplicationEventPublisher; + @InjectMocks private IdentityZoneEndpoints endpoints; + @BeforeEach + void setUp() { + endpoints.setApplicationEventPublisher(mockApplicationEventPublisher); + } + @Test void create_zone() throws InvalidIdentityZoneDetailsException { when(mockIdentityZoneProvisioning.create(any())).then(invocation -> invocation.getArgument(0)); @@ -196,6 +208,31 @@ void deleteIdentityZone_ShouldReject_IfIdpWithAliasExists() { assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, response.getStatusCode()); } + @Test + void deleteIdentityZone_ShouldEmitEntityDeletedEvent_WhenNoAliasIdpExists() { + final IdentityZone idz = new IdentityZone(); + final String idzId = new AlphanumericRandomValueStringGenerator(5).generate(); + idz.setName(idzId); + idz.setId(idzId); + idz.setSubdomain(idzId); + when(mockIdentityZoneProvisioning.retrieveIgnoreActiveFlag(idzId)).thenReturn(idz); + + // arrange no IdP with alias exists in zone + final IdentityProvider idpWithoutAlias = mock(IdentityProvider.class); + when(idpWithoutAlias.getAliasZid()).thenReturn(""); + when(mockIdentityProviderProvisioning.retrieveAll(false, idzId)) + .thenReturn(Collections.singletonList(idpWithoutAlias)); + + final ResponseEntity response = endpoints.deleteIdentityZone(idzId); + assertNotNull(response); + assertEquals(HttpStatus.OK, response.getStatusCode()); + + final ArgumentCaptor> eventArgument = ArgumentCaptor.forClass(EntityDeletedEvent.class); + verify(mockApplicationEventPublisher).publishEvent(eventArgument.capture()); + final var capturedEvent = eventArgument.getValue(); + assertEquals(idz, capturedEvent.getDeleted()); + } + private static IdentityZone createZone() { IdentityZone zone = MultitenancyFixture.identityZone("id", "subdomain"); IdentityZoneConfiguration config = zone.getConfig(); From cda7f1c3d6220e7fa1368bff6a77d41865f9fc22 Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 15 May 2024 09:50:19 +0200 Subject: [PATCH 06/15] Add MockMVC test: deletion of zone should fail when IdP with alias exists in zone --- .../IdentityZoneEndpointsMockMvcTests.java | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java index 031b2173a0f..d23663a7c69 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneEndpointsMockMvcTests.java @@ -11,6 +11,7 @@ import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.cloudfoundry.identity.uaa.client.event.ClientCreateEvent; import org.cloudfoundry.identity.uaa.client.event.ClientDeleteEvent; +import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.login.util.RandomValueStringGenerator; import org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils; import org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils.IdentityZoneCreationResult; @@ -18,6 +19,7 @@ import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; +import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.scim.*; import org.cloudfoundry.identity.uaa.scim.event.GroupModifiedEvent; import org.cloudfoundry.identity.uaa.scim.event.UserModifiedEvent; @@ -63,7 +65,6 @@ import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.TokenFormat.OPAQUE; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.junit.Assert.*; @@ -1698,6 +1699,33 @@ void testDeleteZonePublishesEvent() throws Exception { assertThat(auditedIdentityZone, containsString(id)); } + @Test + void testDeleteZone_ShouldFail_WhenIdpWithAliasExistsInZone() throws Exception { + // create zone + final String idzId = generator.generate(); + createZone(idzId, HttpStatus.CREATED, identityClientToken, new IdentityZoneConfiguration()); + + // create IdP with alias (via DB, since alias feature is disabled by default) + final JdbcIdentityProviderProvisioning idpProvisioning = webApplicationContext + .getBean(JdbcIdentityProviderProvisioning.class); + final IdentityProvider idp = new IdentityProvider<>(); + idp.setName("some-idp"); + idp.setId(UUID.randomUUID().toString()); + idp.setIdentityZoneId(idzId); + idp.setOriginKey("some-origin-key"); + idp.setAliasZid(IdentityZone.getUaaZoneId()); + idp.setAliasId(UUID.randomUUID().toString()); + idp.setType(OriginKeys.OIDC10); + idpProvisioning.create(idp, idzId); + + // deleting zone should fail + mockMvc.perform( + delete("/identity-zones/" + idzId) + .header("Authorization", "Bearer " + identityClientToken) + .accept(APPLICATION_JSON) + ).andExpect(status().isUnprocessableEntity()); + } + @Test void testCreateAndDeleteLimitedClientInNewZoneUsingZoneEndpoint() throws Exception { String id = generator.generate(); From e19a904e6cfa922537175df2128ac2be491ee8db Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 15 May 2024 11:34:15 +0200 Subject: [PATCH 07/15] Add method idpWithAliasExistsInZone to JdbcIdentityProviderProvisioning --- .../JdbcIdentityProviderProvisioning.java | 19 ++++++++++++++++++- ...JdbcIdentityProviderProvisioningTests.java | 12 ++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index 490ebf15a78..9f6d314b441 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -1,5 +1,7 @@ package org.cloudfoundry.identity.uaa.provider; +import static java.sql.Types.VARCHAR; + import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.util.JsonUtils; @@ -8,10 +10,10 @@ import org.slf4j.LoggerFactory; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.DuplicateKeyException; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.stereotype.Component; -import org.springframework.util.Assert; import org.springframework.util.StringUtils; import java.sql.ResultSet; @@ -34,6 +36,8 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi public static final String IDENTITY_ACTIVE_PROVIDERS_QUERY = IDENTITY_PROVIDERS_QUERY + " and active=?"; + public static final String IDP_WITH_ALIAS_EXISTS_QUERY = "values (exists(select 1 from identity_provider idp where idp.identity_zone_id = ? and idp.alias_zid is not null and idp.alias_zid <> ''))"; + public static final String ID_PROVIDER_UPDATE_FIELDS = "version,lastmodified,name,type,config,active,alias_id,alias_zid".replace(",", "=?,") + "=?"; public static final String UPDATE_IDENTITY_PROVIDER_SQL = "update identity_provider set " + ID_PROVIDER_UPDATE_FIELDS + " where id=? and identity_zone_id=?"; @@ -56,6 +60,19 @@ public JdbcIdentityProviderProvisioning(JdbcTemplate jdbcTemplate) { this.jdbcTemplate = jdbcTemplate; } + public boolean idpWithAliasExistsInZone(final String zoneId) { + final Boolean idpWithAliasExists = jdbcTemplate.queryForObject( + IDP_WITH_ALIAS_EXISTS_QUERY, + new Object[]{zoneId}, + new int[]{VARCHAR}, + Boolean.class + ); + if (idpWithAliasExists == null) { + throw new IncorrectResultSizeDataAccessException(1); + } + return idpWithAliasExists; + } + @Override public IdentityProvider retrieve(String id, String zoneId) { return jdbcTemplate.queryForObject(IDENTITY_PROVIDER_BY_ID_QUERY, mapper, id, zoneId); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java index 55dbbfef88e..212f45d9fef 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java @@ -317,4 +317,16 @@ void retrieveIdentityProviderByOriginInDifferentZone() { IdentityProvider idp1 = jdbcIdentityProviderProvisioning.create(idp, otherZoneId1); assertThrows(EmptyResultDataAccessException.class, () -> jdbcIdentityProviderProvisioning.retrieveByOrigin(idp1.getOriginKey(), otherZoneId2)); } + + @Test + void testIdpWithAliasExistsInZone() { + final IdentityProvider idpWithAlias = MultitenancyFixture.identityProvider( + generator.generate(), + otherZoneId1 + ); + idpWithAlias.setAliasZid(IdentityZone.getUaaZoneId()); + idpWithAlias.setAliasId(UUID.randomUUID().toString()); + jdbcIdentityProviderProvisioning.create(idpWithAlias, otherZoneId1); + assertTrue(jdbcIdentityProviderProvisioning.idpWithAliasExistsInZone(otherZoneId1)); + } } From 59905c18b14d8ca4ebfef076bab8988d90d6fe20 Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 15 May 2024 11:36:43 +0200 Subject: [PATCH 08/15] Replace filtering for IdP with alias in Java code with new IdpProvisioning method --- .../uaa/zone/IdentityZoneEndpoints.java | 11 ++++------- .../uaa/zone/IdentityZoneEndpointsTests.java | 19 ++++--------------- 2 files changed, 8 insertions(+), 22 deletions(-) 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 be5a897ed29..60a19c872ea 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 @@ -7,7 +7,7 @@ import org.cloudfoundry.identity.uaa.error.UaaException; import org.cloudfoundry.identity.uaa.provider.ClientAlreadyExistsException; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; -import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; +import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.NoSuchClientException; import org.cloudfoundry.identity.uaa.provider.UaaIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.saml.SamlKey; @@ -68,7 +68,7 @@ public class IdentityZoneEndpoints implements ApplicationEventPublisherAware { private static final String ID_SUBDOMAIN_LOGGING = "[{}] subdomain [{}]"; private final IdentityZoneProvisioning zoneDao; - private final IdentityProviderProvisioning idpDao; + private final JdbcIdentityProviderProvisioning idpDao; private final IdentityZoneEndpointClientRegistrationService clientRegistrationService; private final ScimGroupProvisioning groupProvisioning; private final IdentityZoneValidator validator; @@ -77,7 +77,7 @@ public class IdentityZoneEndpoints implements ApplicationEventPublisherAware { private ApplicationEventPublisher publisher; public IdentityZoneEndpoints(final IdentityZoneProvisioning zoneDao, - final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning idpDao, + final @Qualifier("identityProviderProvisioning") JdbcIdentityProviderProvisioning idpDao, final IdentityZoneEndpointClientRegistrationService clientRegistrationService, final ScimGroupProvisioning groupProvisioning, final IdentityZoneValidator validator, @@ -339,10 +339,7 @@ public ResponseEntity deleteIdentityZone(@PathVariable String id) /* reject deletion if an IdP with alias exists in the zone - checking for users with alias is not required * here, since they can only exist if their origin IdP has an alias as well */ - final List idps = idpDao.retrieveAll(false, zone.getId()); - final boolean idpWithAliasExists = idps.stream() - .map(IdentityProvider::getAliasZid) - .anyMatch(UaaStringUtils::isNotEmpty); + final boolean idpWithAliasExists = idpDao.idpWithAliasExistsInZone(zone.getId()); if (idpWithAliasExists) { return new ResponseEntity<>(UNPROCESSABLE_ENTITY); } 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 8bd170d3df2..21571d00986 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 @@ -3,8 +3,7 @@ import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; import org.cloudfoundry.identity.uaa.error.UaaException; import org.cloudfoundry.identity.uaa.extensions.PollutionPreventionExtension; -import org.cloudfoundry.identity.uaa.provider.IdentityProvider; -import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; +import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.saml.SamlKey; import org.cloudfoundry.identity.uaa.scim.ScimGroup; import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning; @@ -22,11 +21,9 @@ import org.springframework.http.ResponseEntity; import org.springframework.validation.BindingResult; -import java.util.Collections; import java.util.List; import java.util.stream.Collectors; -import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UAA; import static org.cloudfoundry.identity.uaa.util.AssertThrowsWithMessage.assertThrowsWithMessageThat; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -58,7 +55,7 @@ class IdentityZoneEndpointsTests { private IdentityZoneValidator mockIdentityZoneValidator; @Mock - private IdentityProviderProvisioning mockIdentityProviderProvisioning; + private JdbcIdentityProviderProvisioning mockIdentityProviderProvisioning; @Mock private IdentityZoneEndpointClientRegistrationService mockIdentityZoneEndpointClientRegistrationService; @@ -196,12 +193,7 @@ void deleteIdentityZone_ShouldReject_IfIdpWithAliasExists() { when(mockIdentityZoneProvisioning.retrieveIgnoreActiveFlag(idzId)).thenReturn(idz); // arrange IdP with alias exists in zone - final IdentityProvider idpWithoutAlias = mock(IdentityProvider.class); - when(idpWithoutAlias.getAliasZid()).thenReturn(""); - final IdentityProvider idpWithAlias = mock(IdentityProvider.class); - when(idpWithAlias.getAliasZid()).thenReturn(UAA); - when(mockIdentityProviderProvisioning.retrieveAll(false, idzId)) - .thenReturn(List.of(idpWithoutAlias, idpWithAlias)); + when(mockIdentityProviderProvisioning.idpWithAliasExistsInZone(idzId)).thenReturn(true); final ResponseEntity response = endpoints.deleteIdentityZone(idzId); assertNotNull(response); @@ -218,10 +210,7 @@ void deleteIdentityZone_ShouldEmitEntityDeletedEvent_WhenNoAliasIdpExists() { when(mockIdentityZoneProvisioning.retrieveIgnoreActiveFlag(idzId)).thenReturn(idz); // arrange no IdP with alias exists in zone - final IdentityProvider idpWithoutAlias = mock(IdentityProvider.class); - when(idpWithoutAlias.getAliasZid()).thenReturn(""); - when(mockIdentityProviderProvisioning.retrieveAll(false, idzId)) - .thenReturn(Collections.singletonList(idpWithoutAlias)); + when(mockIdentityProviderProvisioning.idpWithAliasExistsInZone(idzId)).thenReturn(false); final ResponseEntity response = endpoints.deleteIdentityZone(idzId); assertNotNull(response); From 4f05c87330e3a3e1aadc8bbc8639f80904ed4575 Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 15 May 2024 15:04:05 +0200 Subject: [PATCH 09/15] Fix query in JdbcIdentityProviderProvisioning.idpWithAliasExistsInZone --- .../provider/JdbcIdentityProviderProvisioning.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index 9f6d314b441..cea8e4bbaf4 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -36,7 +36,7 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi public static final String IDENTITY_ACTIVE_PROVIDERS_QUERY = IDENTITY_PROVIDERS_QUERY + " and active=?"; - public static final String IDP_WITH_ALIAS_EXISTS_QUERY = "values (exists(select 1 from identity_provider idp where idp.identity_zone_id = ? and idp.alias_zid is not null and idp.alias_zid <> ''))"; + public static final String NUMBER_OF_IDPS_WITH_ALIAS_QUERY = "select count(*) from identity_provider idp where idp.identity_zone_id = ? and idp.alias_zid is not null and idp.alias_zid <> ''"; public static final String ID_PROVIDER_UPDATE_FIELDS = "version,lastmodified,name,type,config,active,alias_id,alias_zid".replace(",", "=?,") + "=?"; @@ -61,16 +61,16 @@ public JdbcIdentityProviderProvisioning(JdbcTemplate jdbcTemplate) { } public boolean idpWithAliasExistsInZone(final String zoneId) { - final Boolean idpWithAliasExists = jdbcTemplate.queryForObject( - IDP_WITH_ALIAS_EXISTS_QUERY, + final Integer numberOfIdpsWithAlias = jdbcTemplate.queryForObject( + NUMBER_OF_IDPS_WITH_ALIAS_QUERY, new Object[]{zoneId}, new int[]{VARCHAR}, - Boolean.class + Integer.class ); - if (idpWithAliasExists == null) { + if (numberOfIdpsWithAlias == null) { throw new IncorrectResultSizeDataAccessException(1); } - return idpWithAliasExists; + return numberOfIdpsWithAlias > 0; } @Override From 7f52786a5d607baf5fa8e68854e54ed64d45000c Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Tue, 4 Jun 2024 10:20:38 +0200 Subject: [PATCH 10/15] Replace count query with query limited to one row --- .../provider/JdbcIdentityProviderProvisioning.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index cea8e4bbaf4..0ec08ecf991 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -10,7 +10,6 @@ import org.slf4j.LoggerFactory; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.DuplicateKeyException; -import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.stereotype.Component; @@ -36,7 +35,7 @@ public class JdbcIdentityProviderProvisioning implements IdentityProviderProvisi public static final String IDENTITY_ACTIVE_PROVIDERS_QUERY = IDENTITY_PROVIDERS_QUERY + " and active=?"; - public static final String NUMBER_OF_IDPS_WITH_ALIAS_QUERY = "select count(*) from identity_provider idp where idp.identity_zone_id = ? and idp.alias_zid is not null and idp.alias_zid <> ''"; + public static final String IDP_WITH_ALIAS_EXISTS_QUERY = "select 1 from identity_provider idp where idp.identity_zone_id = ? and idp.alias_zid is not null and idp.alias_zid <> '' limit 1"; public static final String ID_PROVIDER_UPDATE_FIELDS = "version,lastmodified,name,type,config,active,alias_id,alias_zid".replace(",", "=?,") + "=?"; @@ -61,16 +60,16 @@ public JdbcIdentityProviderProvisioning(JdbcTemplate jdbcTemplate) { } public boolean idpWithAliasExistsInZone(final String zoneId) { - final Integer numberOfIdpsWithAlias = jdbcTemplate.queryForObject( - NUMBER_OF_IDPS_WITH_ALIAS_QUERY, + final Integer idpWithAliasExists = jdbcTemplate.queryForObject( + IDP_WITH_ALIAS_EXISTS_QUERY, new Object[]{zoneId}, new int[]{VARCHAR}, Integer.class ); - if (numberOfIdpsWithAlias == null) { - throw new IncorrectResultSizeDataAccessException(1); + if (idpWithAliasExists == null) { + return false; } - return numberOfIdpsWithAlias > 0; + return idpWithAliasExists == 1; } @Override From 780d9ff382342f67701bcb2a89cfdb1145a4179e Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Tue, 4 Jun 2024 10:54:49 +0200 Subject: [PATCH 11/15] Add indexes on identity_provider.alias_zid and users.alias_zid --- .../identity/uaa/db/hsqldb/V4_109__AliasZid_Indexes.sql | 2 ++ .../identity/uaa/db/mysql/V4_109__AliasZid_Indexes.sql | 2 ++ .../identity/uaa/db/postgresql/V4_109__AliasZid_Indexes.sql | 2 ++ 3 files changed, 6 insertions(+) create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_109__AliasZid_Indexes.sql create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_109__AliasZid_Indexes.sql create mode 100644 server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_109__AliasZid_Indexes.sql diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_109__AliasZid_Indexes.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_109__AliasZid_Indexes.sql new file mode 100644 index 00000000000..658d3531e20 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_109__AliasZid_Indexes.sql @@ -0,0 +1,2 @@ +CREATE INDEX identity_provider_alias_zid__idx ON identity_provider (alias_zid); +CREATE INDEX users_alias_zid__idx ON users (alias_zid); diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_109__AliasZid_Indexes.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_109__AliasZid_Indexes.sql new file mode 100644 index 00000000000..658d3531e20 --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_109__AliasZid_Indexes.sql @@ -0,0 +1,2 @@ +CREATE INDEX identity_provider_alias_zid__idx ON identity_provider (alias_zid); +CREATE INDEX users_alias_zid__idx ON users (alias_zid); diff --git a/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_109__AliasZid_Indexes.sql b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_109__AliasZid_Indexes.sql new file mode 100644 index 00000000000..7c8ee9a8b4b --- /dev/null +++ b/server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_109__AliasZid_Indexes.sql @@ -0,0 +1,2 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS identity_provider_alias_zid__idx on identity_provider (alias_zid); +CREATE INDEX CONCURRENTLY IF NOT EXISTS users_alias_zid__idx on users (alias_zid); From a3a72bb94e01770880387a75e9549c0a68cd5bb3 Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Tue, 4 Jun 2024 11:17:26 +0200 Subject: [PATCH 12/15] Fix tests --- .../JdbcIdentityProviderProvisioning.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index 0ec08ecf991..f1e982e2138 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -8,8 +8,10 @@ import org.cloudfoundry.identity.uaa.util.ObjectUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.dao.DataAccessException; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.DuplicateKeyException; +import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.stereotype.Component; @@ -60,12 +62,17 @@ public JdbcIdentityProviderProvisioning(JdbcTemplate jdbcTemplate) { } public boolean idpWithAliasExistsInZone(final String zoneId) { - final Integer idpWithAliasExists = jdbcTemplate.queryForObject( - IDP_WITH_ALIAS_EXISTS_QUERY, - new Object[]{zoneId}, - new int[]{VARCHAR}, - Integer.class - ); + final Integer idpWithAliasExists; + try { + idpWithAliasExists = jdbcTemplate.queryForObject( + IDP_WITH_ALIAS_EXISTS_QUERY, + new Object[]{zoneId}, + new int[]{VARCHAR}, + Integer.class + ); + } catch (final EmptyResultDataAccessException e) { + return false; + } if (idpWithAliasExists == null) { return false; } From a09808bd513b9925eec60039d7e25f73ef586a45 Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 5 Jun 2024 09:24:52 +0200 Subject: [PATCH 13/15] Rework --- .../JdbcIdentityProviderProvisioning.java | 25 ++++++------------- ...JdbcIdentityProviderProvisioningTests.java | 13 +++++++++- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index f1e982e2138..8b7430a6977 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -8,10 +8,8 @@ import org.cloudfoundry.identity.uaa.util.ObjectUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.dao.DataAccessException; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.DuplicateKeyException; -import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.stereotype.Component; @@ -62,21 +60,14 @@ public JdbcIdentityProviderProvisioning(JdbcTemplate jdbcTemplate) { } public boolean idpWithAliasExistsInZone(final String zoneId) { - final Integer idpWithAliasExists; - try { - idpWithAliasExists = jdbcTemplate.queryForObject( - IDP_WITH_ALIAS_EXISTS_QUERY, - new Object[]{zoneId}, - new int[]{VARCHAR}, - Integer.class - ); - } catch (final EmptyResultDataAccessException e) { - return false; - } - if (idpWithAliasExists == null) { - return false; - } - return idpWithAliasExists == 1; + final List result = jdbcTemplate.queryForList( + IDP_WITH_ALIAS_EXISTS_QUERY, + new Object[]{zoneId}, + new int[]{VARCHAR}, + Integer.class + ); + // if an IdP with alias is present, the list contains a single element, otherwise it is empty + return result.size() == 1; } @Override diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java index 2ee5891c804..d57818c6850 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioningTests.java @@ -5,6 +5,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -319,7 +320,7 @@ void retrieveIdentityProviderByOriginInDifferentZone() { } @Test - void testIdpWithAliasExistsInZone() { + void testIdpWithAliasExistsInZone_TrueCase() { final IdentityProvider idpWithAlias = MultitenancyFixture.identityProvider( generator.generate(), otherZoneId1 @@ -329,4 +330,14 @@ void testIdpWithAliasExistsInZone() { jdbcIdentityProviderProvisioning.create(idpWithAlias, otherZoneId1); assertTrue(jdbcIdentityProviderProvisioning.idpWithAliasExistsInZone(otherZoneId1)); } + + @Test + void testIdpWithAliasExistsInZone_FalseCase() { + final IdentityProvider idp = MultitenancyFixture.identityProvider( + generator.generate(), + otherZoneId2 + ); + jdbcIdentityProviderProvisioning.create(idp, otherZoneId2); + assertFalse(jdbcIdentityProviderProvisioning.idpWithAliasExistsInZone(otherZoneId2)); + } } From b77cecd6d09d04d5b2dc49ae5514a1573ad004ff Mon Sep 17 00:00:00 2001 From: Adrian Hoelzl Date: Wed, 5 Jun 2024 09:43:45 +0200 Subject: [PATCH 14/15] Move method idpWithAliasExistsInZone from JdbcIdentityProviderProvisioning to IdentityProviderProvisioning interface --- .../uaa/provider/IdentityProviderProvisioning.java | 2 ++ .../provider/JdbcIdentityProviderProvisioning.java | 1 + .../oauth/ExternalOAuthProviderConfigurator.java | 5 +++++ .../identity/uaa/zone/IdentityZoneEndpoints.java | 6 +++--- .../ExternalOAuthProviderConfiguratorTests.java | 13 +++++++++++++ 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java index 0349fe729f6..28c73ec1338 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderProvisioning.java @@ -21,6 +21,8 @@ public interface IdentityProviderProvisioning { IdentityProvider update(IdentityProvider identityProvider, String zoneId); + boolean idpWithAliasExistsInZone(String zoneId); + IdentityProvider retrieve(String id, String zoneId); List retrieveActive(String zoneId); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java index 8b7430a6977..6549b1bc203 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/JdbcIdentityProviderProvisioning.java @@ -59,6 +59,7 @@ public JdbcIdentityProviderProvisioning(JdbcTemplate jdbcTemplate) { this.jdbcTemplate = jdbcTemplate; } + @Override public boolean idpWithAliasExistsInZone(final String zoneId) { final List result = jdbcTemplate.queryForList( IDP_WITH_ALIAS_EXISTS_QUERY, diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java index 7ae1f6ec846..2dbcec883e8 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java @@ -135,6 +135,11 @@ public IdentityProvider update(IdentityProvider identityProvider, String zoneId) return providerProvisioning.update(identityProvider, zoneId); } + @Override + public boolean idpWithAliasExistsInZone(final String zoneId) { + return providerProvisioning.idpWithAliasExistsInZone(zoneId); + } + @Override public IdentityProvider retrieve(String id, String zoneId) { IdentityProvider p = providerProvisioning.retrieve(id, zoneId); 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 9b12a5d0351..21ebc8ada1f 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 @@ -7,7 +7,7 @@ import org.cloudfoundry.identity.uaa.error.UaaException; import org.cloudfoundry.identity.uaa.provider.ClientAlreadyExistsException; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; -import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; +import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.NoSuchClientException; import org.cloudfoundry.identity.uaa.provider.UaaIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.saml.SamlKey; @@ -68,7 +68,7 @@ public class IdentityZoneEndpoints implements ApplicationEventPublisherAware { private static final String ID_SUBDOMAIN_LOGGING = "[{}] subdomain [{}]"; private final IdentityZoneProvisioning zoneDao; - private final JdbcIdentityProviderProvisioning idpDao; + private final IdentityProviderProvisioning idpDao; private final IdentityZoneEndpointClientRegistrationService clientRegistrationService; private final ScimGroupProvisioning groupProvisioning; private final IdentityZoneValidator validator; @@ -77,7 +77,7 @@ public class IdentityZoneEndpoints implements ApplicationEventPublisherAware { private ApplicationEventPublisher publisher; public IdentityZoneEndpoints(final IdentityZoneProvisioning zoneDao, - final @Qualifier("identityProviderProvisioning") JdbcIdentityProviderProvisioning idpDao, + final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning idpDao, final IdentityZoneEndpointClientRegistrationService clientRegistrationService, final ScimGroupProvisioning groupProvisioning, final IdentityZoneValidator validator, diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java index e9624285b05..e7d4a8327d0 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java @@ -6,12 +6,15 @@ import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.RawExternalOAuthIdentityProviderDefinition; +import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; import org.cloudfoundry.identity.uaa.util.UaaRandomStringUtil; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.dao.IncorrectResultSizeDataAccessException; @@ -56,6 +59,8 @@ @ExtendWith(PollutionPreventionExtension.class) @ExtendWith(MockitoExtension.class) class ExternalOAuthProviderConfiguratorTests { + private static final AlphanumericRandomValueStringGenerator RANDOM_STRING_GENERATOR = + new AlphanumericRandomValueStringGenerator(6); private final String UAA_BASE_URL = "https://localhost:8443/uaa"; @@ -386,4 +391,12 @@ void testGetIdpAuthenticationUrlAndCheckTokenFormatParameter() { UriComponentsBuilder.fromUriString(authzUri).build().getQueryParams().toSingleValueMap(); assertThat(queryParams, hasEntry("token_format", "jwt")); } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testIdpWithAliasExistsInZone(final boolean resultFromDelegate) { + final String zoneId = RANDOM_STRING_GENERATOR.generate(); + when(mockIdentityProviderProvisioning.idpWithAliasExistsInZone(zoneId)).thenReturn(resultFromDelegate); + assertEquals(resultFromDelegate, configurator.idpWithAliasExistsInZone(zoneId)); + } } From 3acb204ec1291039d28db6030c7e2d01a7b79c0a Mon Sep 17 00:00:00 2001 From: d036670 Date: Mon, 10 Jun 2024 08:36:01 +0200 Subject: [PATCH 15/15] Add tests --- .../identity/uaa/scim/ScimUserTests.java | 8 +++ .../EntityAliasHandlerValidationTest.java | 7 +++ .../ScimUserEndpointsAliasTests.java | 55 ++++++++++++++++++- 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java b/model/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java index ae0bb3d8e8c..c590f564585 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserTests.java @@ -827,6 +827,14 @@ public void testPatchActive() { assertTrue(patchUser.isActive()); } + @Test + public void testScimUserAliasDeserialization() { + user.setAliasId("aliasId"); + user.setAliasZid("custom"); + String staticJson = "{\"id\":\"id\",\"externalId\":\"\",\"meta\":{\"version\":0},\"userName\":\"uname\",\"name\":{\"formatted\":\"gname fname\",\"familyName\":\"fname\",\"givenName\":\"gname\"},\"emails\":[{\"value\":\"test@example.org\",\"primary\":false}],\"phoneNumbers\":[{\"value\":\"0123456789\"}],\"displayName\":\"display\",\"title\":\"title\",\"locale\":\"en.UTF-8\",\"active\":true,\"verified\":true,\"origin\":\"\",\"aliasZid\":\"custom\",\"aliasId\":\"aliasId\",\"password\":\"password\",\"schemas\":[\"urn:scim:schemas:core:1.0\"]}"; + assertEquals(user, JsonUtils.readValue(staticJson, ScimUser.class)); + } + @Test public void testPatchVerified() { user.setVerified(false); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandlerValidationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandlerValidationTest.java index f4a44d3063e..6552d38f144 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandlerValidationTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandlerValidationTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.cloudfoundry.identity.uaa.alias.EntityAliasHandlerValidationTest.NoExistingAliasBase.ExistingEntityArgument.ENTITY_WITH_EMPTY_ALIAS_PROPS; +import static org.junit.Assert.assertFalse; import java.util.UUID; import java.util.stream.Stream; @@ -129,6 +130,12 @@ final void shouldReturnFalse_UpdatesOfEntitiesWithExistingAliasForbidden() { requestBody = buildEntityWithAliasProps(null, null); assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse(); } + + @Test + final void shouldReturnFalse_DefaultSetting() { + AliasEntitiesConfig aliasEntitiesConfig = new AliasEntitiesConfig(); + assertFalse(aliasEntitiesConfig.aliasEntitiesEnabled(false)); + } } protected abstract class ExistingAlias_AliasFeatureEnabled extends Base { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsAliasTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsAliasTests.java index 4321c879019..f77ab48f9f3 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsAliasTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsAliasTests.java @@ -13,6 +13,7 @@ import org.cloudfoundry.identity.uaa.scim.ScimUserAliasHandler; import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning; import org.cloudfoundry.identity.uaa.scim.exception.ScimException; +import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceConflictException; import org.cloudfoundry.identity.uaa.scim.validate.PasswordValidator; import org.cloudfoundry.identity.uaa.security.IsSelfCheck; import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; @@ -26,6 +27,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -123,7 +125,7 @@ void setUp() { lenient().when(transactionTemplate.execute(any())).then(invocationOnMock -> { final TransactionCallback callback = invocationOnMock.getArgument(0); - return callback.doInTransaction(mock(TransactionStatus.class)); + return callback != null ? callback.doInTransaction(mock(TransactionStatus.class)) : null; }); } @@ -177,6 +179,21 @@ void shouldThrow_WhenAliasPropertiesAreInvalid() { assertThat(exception.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST); } + @Test + void shouldThrow_WhenAliasIsNotPresent() { + final ScimUser user = buildScimUser(UAA, origin); + + when(scimUserAliasHandler.aliasPropertiesAreValid(user, null)).thenReturn(true); + when(scimUserAliasHandler.ensureConsistencyOfAliasEntity(user, null)).thenReturn(null); + + final MockHttpServletRequest req = new MockHttpServletRequest(); + final MockHttpServletResponse res = new MockHttpServletResponse(); + final IllegalStateException exception = assertThrows(IllegalStateException.class, () -> + scimUserEndpoints.createUser(user, req, res) + ); + assertThat(exception.getMessage()).isEqualTo("The persisted user is not present after handling the alias."); + } + @Test void shouldReturnOriginalUser() { final ScimUser user = buildScimUser(UAA, origin); @@ -246,6 +263,28 @@ void shouldThrow_IfAliasPropertiesAreInvalid() { assertThat(exception.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST); } + @Test + void shouldThrow_IfAliasIsLocked() { + when(scimUserAliasHandler.aliasPropertiesAreValid(originalUser, existingOriginalUser)) + .thenReturn(true); + when(transactionTemplate.execute(any())).then(invocationOnMock -> { + throw new OptimisticLockingFailureException("The alias is locked."); + }); + + final ScimResourceConflictException exception = assertThrows(ScimResourceConflictException.class, () -> + scimUserEndpoints.updateUser( + originalUser, + originalUser.getId(), + "*", + new MockHttpServletRequest(), + new MockHttpServletResponse(), + null + ) + ); + assertThat(exception.getMessage()).isEqualTo("The alias is locked."); + assertThat(exception.getStatus()).isEqualTo(HttpStatus.CONFLICT); + } + @Test void shouldAlsoUpdateAliasUserIfPresent() { when(scimUserAliasHandler.aliasPropertiesAreValid(originalUser, existingOriginalUser)) @@ -467,6 +506,20 @@ void shouldThrowException_IfUserHasExistingAlias() { .isEqualTo("Could not delete user with alias since alias entities are disabled."); assertThat(exception.getHttpStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); } + + @Test + void shouldDeleteUserIfPresent() { + ScimUser originalUser = buildScimUser("123456789", "uaa"); + when(scimUserProvisioning.retrieve(any(), any())).thenReturn(originalUser); + final ScimUser response = scimUserEndpoints.deleteUser( + "12345678", + null, + new MockHttpServletRequest(), + new MockHttpServletResponse() + ); + + assertScimUsersAreEqual(response, originalUser); + } } }