Merge branch 'feature/move-idp-alias-handling-to-separate-class' into…

… feature/alias-id-and-alias-zid-for-users

model/src/main/java/org/cloudfoundry/identity/uaa/EntityWithAlias.java

@@ -1,5 +1,7 @@
 package org.cloudfoundry.identity.uaa;
 
+import java.util.Optional;
+
 import org.springframework.lang.Nullable;
 
 /**
@@ -19,4 +21,22 @@ public interface EntityWithAlias {
     String getAliasZid();
 
     void setAliasZid(String aliasZid);
+
+    /**
+     * Get a description of the entity including its alias properties, e.g., for logging.
+     */
+    default String getAliasDescription() {
+        return String.format(
+                "%s[id=%s,zid=%s,aliasId=%s,aliasZid=%s]",
+                getClass().getSimpleName(),
+                surroundWithSingleQuotesIfPresent(getId()),
+                surroundWithSingleQuotesIfPresent(getZoneId()),
+                surroundWithSingleQuotesIfPresent(getAliasId()),
+                surroundWithSingleQuotesIfPresent(getAliasZid())
+        );
+    }
+
+    private static String surroundWithSingleQuotesIfPresent(@Nullable final String input) {
+        return Optional.ofNullable(input).map(it -> "'" + it + "'").orElse(null);
+    }
 }

model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProvider.java

@@ -114,6 +114,7 @@ public IdentityProvider setName(String name) {
         return this;
     }
 
+    @Override
     public String getId() {
         return id;
     }

model/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderTest.java

@@ -91,4 +91,22 @@ void testEqualsAndHashCode() {
         assertThat(idp2.equals(idp1)).isFalse();
     }
 
+    @Test
+    void testGetAliasDescription() {
+        final String customZoneId = "custom-zone";
+        final String aliasIdpId = "id-of-alias-idp";
+
+        final IdentityProvider<OIDCIdentityProviderDefinition> idp = new IdentityProvider<>();
+        idp.setId("12345");
+        idp.setName("some-name");
+        idp.setOriginKey("some-origin");
+        idp.setAliasZid(customZoneId);
+        idp.setAliasId(aliasIdpId);
+        idp.setActive(true);
+        idp.setIdentityZoneId(UAA);
+
+        assertThat(idp.getAliasDescription()).isEqualTo(
+                "IdentityProvider[id='12345',zid='uaa',aliasId='id-of-alias-idp',aliasZid='custom-zone']"
+        );
+    }
 }

server/src/main/java/org/cloudfoundry/identity/uaa/alias/EntityAliasFailedException.java

@@ -0,0 +1,15 @@
+package org.cloudfoundry.identity.uaa.alias;
+
+import org.cloudfoundry.identity.uaa.error.UaaException;
+
+public class EntityAliasFailedException extends UaaException {
+    private static final String ERROR = "alias_entity_creation_failed";
+
+    public EntityAliasFailedException(
+            final String msg,
+            final int httpStatusCode,
+            final Throwable cause
+    ) {
+        super(cause, ERROR, msg, httpStatusCode);
+    }
+}

server/src/main/java/org/cloudfoundry/identity/uaa/EntityAliasHandler.java → server/src/main/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandler.java

@@ -1,41 +1,51 @@
-package org.cloudfoundry.identity.uaa;
+package org.cloudfoundry.identity.uaa.alias;
 
 import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UAA;
 import static org.springframework.util.StringUtils.hasText;
 
 import java.util.Optional;
 
-import org.cloudfoundry.identity.uaa.error.UaaException;
+import org.cloudfoundry.identity.uaa.EntityWithAlias;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning;
 import org.cloudfoundry.identity.uaa.zone.ZoneDoesNotExistsException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.dao.DataAccessException;
+import org.springframework.http.HttpStatus;
 import org.springframework.lang.NonNull;
 import org.springframework.lang.Nullable;
 
 public abstract class EntityAliasHandler<T extends EntityWithAlias> {
     private static final Logger LOGGER = LoggerFactory.getLogger(EntityAliasHandler.class);
 
     private final IdentityZoneProvisioning identityZoneProvisioning;
+    private final boolean aliasEntitiesEnabled;
 
-    protected EntityAliasHandler(final IdentityZoneProvisioning identityZoneProvisioning) {
+    protected EntityAliasHandler(
+            final IdentityZoneProvisioning identityZoneProvisioning,
+            final boolean aliasEntitiesEnabled
+    ) {
         this.identityZoneProvisioning = identityZoneProvisioning;
+        this.aliasEntitiesEnabled = aliasEntitiesEnabled;
     }
 
-    public boolean aliasPropertiesAreValid(
+    public final boolean aliasPropertiesAreValid(
             @NonNull final T requestBody,
             @Nullable final T existingEntity
     ) {
+        // if the entity already has an alias, the alias properties must not be changed
         final boolean entityAlreadyHasAlias = existingEntity != null && hasText(existingEntity.getAliasZid());
-
         if (entityAlreadyHasAlias) {
+            if (!aliasEntitiesEnabled) {
+                // if the feature is disabled, we only allow setting both alias properties to null
+                return !hasText(requestBody.getAliasId()) && !hasText(requestBody.getAliasZid());
+            }
+
             if (!hasText(existingEntity.getAliasId())) {
                 // at this point, we expect both properties to be set -> if not, the entity is in an inconsistent state
                 throw new IllegalStateException(String.format(
-                        "Both alias ID and alias ZID expected to be set for existing entity of type '%s' with ID '%s' in zone '%s'.",
-                        existingEntity.getClass().getSimpleName(),
-                        existingEntity.getId(),
-                        existingEntity.getZoneId()
+                        "Both alias ID and alias ZID expected to be set for existing entity %s.",
+                        existingEntity.getAliasDescription()
                 ));
             }
 
@@ -54,6 +64,12 @@ public boolean aliasPropertiesAreValid(
             return true;
         }
 
+        /* At this point, we know that a new alias entity should be created.
+         * -> check if the creation of alias entities is enabled */
+        if (!aliasEntitiesEnabled) {
+            return false;
+        }
+
         // the referenced zone must exist
         try {
             identityZoneProvisioning.retrieve(requestBody.getAliasZid());
@@ -95,13 +111,58 @@ public boolean aliasPropertiesAreValid(
      * is already set.
      *
      * @param originalEntity the original entity
-     * @return the original entity as well as the alias entity (if applicable) after the operation
+     * @return the original entity as well as the alias entity (if affected) after the operation
      * @throws EntityAliasFailedException if a new alias entity needs to be created, but the zone referenced in
      *                                    'aliasZid' does not exist
-     * @throws EntityAliasFailedException if 'aliasId' and 'aliasZid' are set in the original IdP, but the
+     * @throws EntityAliasFailedException if 'aliasId' and 'aliasZid' are set in the original entity, but the
      *                                    referenced alias entity could not be found
      */
-    public EntityAliasResult<T> ensureConsistencyOfAliasEntity(final T originalEntity) {
+    public final EntityAliasResult<T> ensureConsistencyOfAliasEntity(
+            @NonNull final T originalEntity,
+            @Nullable final T existingEntity
+    ) throws EntityAliasFailedException {
+        /* If the entity had an alias before the update and the alias feature is now turned off, we break the reference
+         * between the entity and its alias by setting aliasId and aliasZid to null for both of them. Then, all other
+         * changes are only applied to the original entity. */
+        final boolean entityHadAlias = existingEntity != null && hasText(existingEntity.getAliasZid());
+        final boolean referenceBreakRequired = entityHadAlias && !aliasEntitiesEnabled;
+        if (referenceBreakRequired) {
+            if (!hasText(existingEntity.getAliasId())) {
+                LOGGER.warn(
+                        "The state of the entity {} before the update had an aliasZid set, but no aliasId.",
+                        existingEntity.getAliasDescription()
+                );
+                return new EntityAliasResult<>(originalEntity, null);
+            }
+
+            final Optional<T> aliasEntityOpt = retrieveAliasEntity(existingEntity);
+            if (aliasEntityOpt.isEmpty()) {
+                LOGGER.warn(
+                        "The alias referenced in entity {} does not exist, therefore cannot break reference.",
+                        existingEntity.getAliasDescription()
+                );
+                return new EntityAliasResult<>(originalEntity, null);
+            }
+
+            final T aliasEntity = aliasEntityOpt.get();
+            aliasEntity.setAliasId(null);
+            aliasEntity.setAliasZid(null);
+
+            try {
+                updateEntity(aliasEntity, aliasEntity.getZoneId());
+            } catch (final DataAccessException e) {
+                throw new EntityAliasFailedException(
+                        String.format(
+                                "Could not break reference to alias in entity %s.",
+                                existingEntity.getAliasDescription()
+                        ), HttpStatus.UNPROCESSABLE_ENTITY.value(), e
+                );
+            }
+
+            // no change required in the original entity since its aliasId and aliasZid were already set to null
+            return new EntityAliasResult<>(originalEntity, aliasEntity);
+        }
+
         if (!hasText(originalEntity.getAliasZid())) {
             // no alias handling is necessary
             return new EntityAliasResult<>(originalEntity, null);
@@ -129,12 +190,11 @@ public EntityAliasResult<T> ensureConsistencyOfAliasEntity(final T originalEntit
         try {
             identityZoneProvisioning.retrieve(originalEntity.getAliasZid());
         } catch (final ZoneDoesNotExistsException e) {
-            throw new EntityAliasFailedException(String.format(
-                    "Could not create alias for entity (type: %s; ID: '%s') in alias zone '%s', as zone does not exist.",
-                    originalEntity.getClass().getSimpleName(),
-                    originalEntity.getId(),
-                    originalEntity.getAliasZid()
-            ), e);
+            final String errorMessage = String.format(
+                    "Could not create alias for %s, as alias zone does not exist.",
+                    originalEntity.getAliasDescription()
+            );
+            throw new EntityAliasFailedException(errorMessage, HttpStatus.UNPROCESSABLE_ENTITY.value(), e);
         }
 
         // create new alias entity in alias zid
@@ -143,7 +203,6 @@ public EntityAliasResult<T> ensureConsistencyOfAliasEntity(final T originalEntit
         // update alias ID in original entity
         originalEntity.setAliasId(persistedAliasEntity.getId());
         final T updatedOriginalEntity = updateEntity(originalEntity, originalEntity.getZoneId());
-
         return new EntityAliasResult<>(updatedOriginalEntity, persistedAliasEntity);
     }
 
@@ -166,15 +225,15 @@ private T buildAliasEntity(final T originalEntity) {
      */
     protected abstract T cloneEntity(final T originalEntity);
 
-    private Optional<T> retrieveAliasEntity(final T originalEntity) {
+    public final Optional<T> retrieveAliasEntity(final T originalEntity) {
         return retrieveEntity(originalEntity.getAliasId(), originalEntity.getAliasZid());
     }
 
     protected abstract Optional<T> retrieveEntity(final String id, final String zoneId);
 
     protected abstract T updateEntity(final T entity, final String zoneId);
 
-    protected abstract T createEntity(final T entity, final String zoneId);
+    protected abstract T createEntity(final T entity, final String zoneId) throws EntityAliasFailedException;
 
     protected static <T extends EntityWithAlias> boolean isCorrectAliasPair(final T entity1, final T entity2) {
         // check if both entities have an alias
@@ -192,12 +251,6 @@ protected static <T extends EntityWithAlias> boolean isCorrectAliasPair(final T
         return entity1ReferencesEntity2 && entity2ReferencesEntity1;
     }
 
-    public static class EntityAliasFailedException extends UaaException {
-        public EntityAliasFailedException(final String msg, final Throwable t) {
-            super(msg, t);
-        }
-    }
-
     public record EntityAliasResult<T extends EntityWithAlias>(
             @NonNull T originalEntity,
             @Nullable T aliasEntity

server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderAliasHandler.java

@@ -7,12 +7,16 @@
 import java.util.Optional;
 import java.util.Set;
 
-import org.cloudfoundry.identity.uaa.EntityAliasHandler;
+import org.cloudfoundry.identity.uaa.alias.EntityAliasFailedException;
+import org.cloudfoundry.identity.uaa.alias.EntityAliasHandler;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.dao.EmptyResultDataAccessException;
+import org.springframework.http.HttpStatus;
+import org.springframework.lang.NonNull;
 import org.springframework.stereotype.Component;
 
 @Component
@@ -26,16 +30,17 @@ public class IdentityProviderAliasHandler extends EntityAliasHandler<IdentityPro
 
     private final IdentityProviderProvisioning identityProviderProvisioning;
 
-    protected IdentityProviderAliasHandler(
+    public IdentityProviderAliasHandler(
             @Qualifier("identityZoneProvisioning") final IdentityZoneProvisioning identityZoneProvisioning,
-            final IdentityProviderProvisioning identityProviderProvisioning
+            final IdentityProviderProvisioning identityProviderProvisioning,
+            @Value("${login.aliasEntitiesEnabled:false}") final boolean aliasEntitiesEnabled
     ) {
-        super(identityZoneProvisioning);
+        super(identityZoneProvisioning, aliasEntitiesEnabled);
         this.identityProviderProvisioning = identityProviderProvisioning;
     }
 
     @Override
-    protected boolean additionalValidationChecksForNewAlias(final IdentityProvider<?> requestBody) {
+    protected boolean additionalValidationChecksForNewAlias(@NonNull final IdentityProvider<?> requestBody) {
         // check if aliases are supported for this IdP type
         return IDP_TYPES_ALIAS_SUPPORTED.contains(requestBody.getType());
     }
@@ -52,18 +57,14 @@ protected void setZoneId(final IdentityProvider<?> entity, final String zoneId)
 
     @Override
     protected IdentityProvider<?> cloneEntity(final IdentityProvider<?> originalEntity) {
-        final IdentityProvider aliasIdp = new IdentityProvider<>();
-        aliasIdp.setActive(originalEntity.isActive());
-        aliasIdp.setName(originalEntity.getName());
-        aliasIdp.setOriginKey(originalEntity.getOriginKey());
-        aliasIdp.setType(originalEntity.getType());
-        aliasIdp.setConfig(originalEntity.getConfig());
-        aliasIdp.setSerializeConfigRaw(originalEntity.isSerializeConfigRaw());
-        // reference the ID and zone ID of the initial IdP entry
-        aliasIdp.setAliasZid(originalEntity.getIdentityZoneId());
-        aliasIdp.setAliasId(originalEntity.getId());
-        aliasIdp.setIdentityZoneId(originalEntity.getAliasZid());
-        return aliasIdp;
+        final IdentityProvider clonedIdp = new IdentityProvider<>();
+        clonedIdp.setActive(originalEntity.isActive());
+        clonedIdp.setName(originalEntity.getName());
+        clonedIdp.setOriginKey(originalEntity.getOriginKey());
+        clonedIdp.setType(originalEntity.getType());
+        clonedIdp.setConfig(originalEntity.getConfig());
+        clonedIdp.setSerializeConfigRaw(originalEntity.isSerializeConfigRaw());
+        return clonedIdp;
     }
 
     @Override
@@ -85,6 +86,14 @@ protected IdentityProvider<?> updateEntity(final IdentityProvider<?> entity, fin
 
     @Override
     protected IdentityProvider<?> createEntity(final IdentityProvider<?> entity, final String zoneId) {
-        return identityProviderProvisioning.create(entity, zoneId);
+        try {
+            return identityProviderProvisioning.create(entity, zoneId);
+        } catch (final IdpAlreadyExistsException e) {
+            final String errorMessage = String.format(
+                    "Could not create %s. An IdP with this origin already exists in the alias zone.",
+                    entity.getAliasDescription()
+            );
+            throw new EntityAliasFailedException(errorMessage, HttpStatus.CONFLICT.value(), e);
+        }
     }
 }