Reject IdZ deletion if an IdP with alias exists in the zone

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);

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<IdentityProvider> retrieveActive(String zoneId);

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;
@@ -11,7 +13,6 @@
 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,13 +35,15 @@ 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 = "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(",", "=?,") + "=?";
 
     public static final String UPDATE_IDENTITY_PROVIDER_SQL = "update identity_provider set " + ID_PROVIDER_UPDATE_FIELDS + " where id=? and identity_zone_id=?";
 
     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=?";
 
@@ -56,6 +59,18 @@ public JdbcIdentityProviderProvisioning(JdbcTemplate jdbcTemplate) {
         this.jdbcTemplate = jdbcTemplate;
     }
 
+    @Override
+    public boolean idpWithAliasExistsInZone(final String zoneId) {
+        final List<Integer> 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
     public IdentityProvider retrieve(String id, String zoneId) {
         return jdbcTemplate.queryForObject(IDENTITY_PROVIDER_BY_ID_QUERY, mapper, id, zoneId);
@@ -150,12 +165,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

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);

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

@@ -336,7 +336,15 @@ public ResponseEntity<IdentityZone> 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);
-            if (publisher != null && zone != null) {
+
+            /* 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 boolean idpWithAliasExists = idpDao.idpWithAliasExistsInZone(zone.getId());
+            if (idpWithAliasExists) {
+                return new ResponseEntity<>(UNPROCESSABLE_ENTITY);
+            }
+
+            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);

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);

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);

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);

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 {

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;
@@ -69,7 +70,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 +106,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) {
@@ -317,4 +318,26 @@ void retrieveIdentityProviderByOriginInDifferentZone() {
         IdentityProvider idp1 = jdbcIdentityProviderProvisioning.create(idp, otherZoneId1);
         assertThrows(EmptyResultDataAccessException.class, () -> jdbcIdentityProviderProvisioning.retrieveByOrigin(idp1.getOriginKey(), otherZoneId2));
     }
+
+    @Test
+    void testIdpWithAliasExistsInZone_TrueCase() {
+        final IdentityProvider<AbstractIdentityProviderDefinition> idpWithAlias = MultitenancyFixture.identityProvider(
+                generator.generate(),
+                otherZoneId1
+        );
+        idpWithAlias.setAliasZid(IdentityZone.getUaaZoneId());
+        idpWithAlias.setAliasId(UUID.randomUUID().toString());
+        jdbcIdentityProviderProvisioning.create(idpWithAlias, otherZoneId1);
+        assertTrue(jdbcIdentityProviderProvisioning.idpWithAliasExistsInZone(otherZoneId1));
+    }
+
+    @Test
+    void testIdpWithAliasExistsInZone_FalseCase() {
+        final IdentityProvider<AbstractIdentityProviderDefinition> idp = MultitenancyFixture.identityProvider(
+                generator.generate(),
+                otherZoneId2
+        );
+        jdbcIdentityProviderProvisioning.create(idp, otherZoneId2);
+        assertFalse(jdbcIdentityProviderProvisioning.idpWithAliasExistsInZone(otherZoneId2));
+    }
 }

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));
+    }
 }

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);
+            }
         }
     }