Merge pull request #2967 from cloudfoundry/fix/alias-validation-zonei…

…d-null

Fix Alias Validation when no Identity Zone ID is set in the Request Body

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

@@ -33,6 +33,10 @@ public final boolean aliasPropertiesAreValid(
             @NonNull final T requestBody,
             @Nullable final T existingEntity
     ) {
+        if (!hasText(requestBody.getZoneId())) {
+            throw new IllegalArgumentException("The zone ID of the request body must not be empty.");
+        }
+
         // if the entity already has an alias, the alias properties must not be changed
         final boolean entityAlreadyHasAlias = existingEntity != null && hasText(existingEntity.getAliasZid());
         if (entityAlreadyHasAlias) {

server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java

@@ -317,6 +317,8 @@ public ScimUser updateUser(@RequestBody ScimUser user, @PathVariable String user
         int version = getVersion(userId, etag);
         user.setVersion(version);
 
+        user.setZoneId(identityZoneManager.getCurrentIdentityZoneId());
+
         final ScimUser existingScimUser = scimUserProvisioning.retrieve(
                 userId,
                 identityZoneManager.getCurrentIdentityZoneId()

server/src/test/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandlerValidationTest.java

@@ -1,6 +1,7 @@
 package org.cloudfoundry.identity.uaa.alias;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 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;
@@ -9,6 +10,7 @@
 import java.util.stream.Stream;
 
 import org.cloudfoundry.identity.uaa.EntityWithAlias;
+import org.cloudfoundry.identity.uaa.zone.IdentityZone;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -21,11 +23,15 @@ public abstract class EntityAliasHandlerValidationTest<T extends EntityWithAlias
 
     protected abstract EntityAliasHandler<T> buildAliasHandler(final boolean aliasEntitiesEnabled);
 
-    protected abstract T buildEntityWithAliasProps(@Nullable final String aliasId, @Nullable final String aliasZid);
+    protected abstract T buildEntityWithAliasProps(
+            @Nullable final String zoneId,
+            @Nullable final String aliasId,
+            @Nullable final String aliasZid
+    );
 
     protected abstract void changeNonAliasProperties(@NonNull final T entity);
 
-    protected abstract void setZoneId(@NonNull final T entity, @NonNull final String zoneId);
+    protected abstract void setZoneId(@NonNull final T entity, @Nullable final String zoneId);
 
     protected abstract void arrangeZoneExists(@NonNull final String zoneId);
 
@@ -47,19 +53,37 @@ protected abstract class NoExistingAliasBase extends Base {
         @ParameterizedTest
         @MethodSource("existingEntityArgNoAlias")
         final void shouldReturnFalse_AliasIdSetInReqBody(final ExistingEntityArgument existingEntityArg) {
-            final T requestBody = buildEntityWithAliasProps(UUID.randomUUID().toString(), null);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), UUID.randomUUID().toString(), null);
             final T existingEntity = resolveExistingEntityArgument(existingEntityArg);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
         }
 
         @ParameterizedTest
         @MethodSource("existingEntityArgNoAlias")
         final void shouldReturnTrue_BothAliasPropsEmptyInReqBody(final ExistingEntityArgument existingEntityArg) {
-            final T requestBody = buildEntityWithAliasProps(null, null);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, null);
             final T existingEntity = resolveExistingEntityArgument(existingEntityArg);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isTrue();
         }
 
+        /**
+         * For some endpoints, we allow the identity zone ID of to be empty in the request body. However, the field is
+         * required during alias validation. We therefore rely on the endpoint methods to set the identity zone ID to
+         * the one resolved from the token.
+         */
+        @ParameterizedTest
+        @MethodSource("existingEntityArgNoAlias")
+        final void shouldThrowIllegalArgumentException_ZoneIdEmptyInReqBody(final ExistingEntityArgument existingEntityArg) {
+            // alias property values are not important here, the zoneId is checked before them
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, null);
+            setZoneId(requestBody, null);
+
+            final T existingEntity = resolveExistingEntityArgument(existingEntityArg);
+            assertThatIllegalArgumentException().isThrownBy(() ->
+                    aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)
+            ).withMessage("The zone ID of the request body must not be empty.");
+        }
+
         /**
          * Provider for the 'existingEntity' argument for cases where no alias should exist, i.e., either an original
          * entity with empty alias properties or no existing entity.
@@ -70,7 +94,7 @@ protected static Stream<ExistingEntityArgument> existingEntityArgNoAlias() {
 
         protected final T resolveExistingEntityArgument(@NonNull final ExistingEntityArgument existingEntityArgument) {
             if (existingEntityArgument == ENTITY_WITH_EMPTY_ALIAS_PROPS) {
-                return buildEntityWithAliasProps(null, null);
+                return buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, null);
             }
             return null;
         }
@@ -92,42 +116,42 @@ final void shouldReturnFalse_UpdatesOfEntitiesWithExistingAliasForbidden() {
             final String initialAliasId = UUID.randomUUID().toString();
             final String initialAliasZid = CUSTOM_ZONE_ID;
 
-            final T existingEntity = buildEntityWithAliasProps(initialAliasId, initialAliasZid);
+            final T existingEntity = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), initialAliasId, initialAliasZid);
 
             // (1) both alias props left unchanged
-            T requestBody = buildEntityWithAliasProps(initialAliasId, initialAliasZid);
+            T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), initialAliasId, initialAliasZid);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (2) alias ID unchanged, alias ZID changed
-            requestBody = buildEntityWithAliasProps(initialAliasId, "some-other-zid");
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), initialAliasId, "some-other-zid");
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (3) alias ID unchanged, alias ZID removed
-            requestBody = buildEntityWithAliasProps(initialAliasId, null);
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), initialAliasId, null);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (4) alias ID changed, alias ZID unchanged
-            requestBody = buildEntityWithAliasProps("some-other-id", initialAliasZid);
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), "some-other-id", initialAliasZid);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (5) alias ID changed, alias ZID changed
-            requestBody = buildEntityWithAliasProps("some-other-id", "some-other-zid");
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), "some-other-id", "some-other-zid");
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (6) alias ID changed, alias ZID removed
-            requestBody = buildEntityWithAliasProps("some-other-id", null);
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), "some-other-id", null);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (7) alias ID removed, alias ZID unchanged
-            requestBody = buildEntityWithAliasProps(null, initialAliasZid);
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, initialAliasZid);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (8) alias ID removed, alias ZID changed
-            requestBody = buildEntityWithAliasProps(null, "some-other-zid");
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, "some-other-zid");
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
 
             // (9) alias ID removed, alias ZID removed
-            requestBody = buildEntityWithAliasProps(null, null);
+            requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, null);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
         }
 
@@ -146,9 +170,9 @@ protected final boolean isAliasFeatureEnabled() {
 
         @Test
         final void shouldThrow_AliasIdEmptyInExisting() {
-            final T existingEntity = buildEntityWithAliasProps(null, CUSTOM_ZONE_ID);
+            final T existingEntity = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, CUSTOM_ZONE_ID);
 
-            final T requestBody = buildEntityWithAliasProps(null, CUSTOM_ZONE_ID);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, CUSTOM_ZONE_ID);
             changeNonAliasProperties(requestBody);
 
             assertThatIllegalStateException().isThrownBy(() ->
@@ -161,9 +185,9 @@ final void shouldReturnFalse_AliasPropsChangedInReqBody() {
             final String initialAliasId = UUID.randomUUID().toString();
             final String initialAliasZid = CUSTOM_ZONE_ID;
 
-            final T existingEntity = buildEntityWithAliasProps(initialAliasId, initialAliasZid);
+            final T existingEntity = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), initialAliasId, initialAliasZid);
 
-            final T requestBody = buildEntityWithAliasProps(initialAliasId, initialAliasZid);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), initialAliasId, initialAliasZid);
             changeNonAliasProperties(requestBody);
 
             final Runnable resetRequestBody = () -> {
@@ -206,9 +230,9 @@ final void shouldReturnFalse_AliasPropsChangedInReqBody() {
         @Test
         final void shouldReturnTrue_AliasPropsUnchangedInReqBody() {
             final String aliasId = UUID.randomUUID().toString();
-            final T existingEntity = buildEntityWithAliasProps(aliasId, CUSTOM_ZONE_ID);
+            final T existingEntity = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), aliasId, CUSTOM_ZONE_ID);
 
-            final T requestBody = buildEntityWithAliasProps(aliasId, CUSTOM_ZONE_ID);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), aliasId, CUSTOM_ZONE_ID);
             changeNonAliasProperties(requestBody);
 
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isTrue();
@@ -227,7 +251,7 @@ final void shouldReturnFalse_AliasZoneDoesNotExist(final ExistingEntityArgument
             final String aliasZid = UUID.randomUUID().toString();
             arrangeZoneDoesNotExist(aliasZid);
 
-            final T requestBody = buildEntityWithAliasProps(null, aliasZid);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, aliasZid);
 
             final T existingEntity = resolveExistingEntityArgument(existingEntityArg);
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
@@ -239,7 +263,7 @@ final void shouldReturnFalse_ZidAndAliasZidAreEqual(final ExistingEntityArgument
             final String aliasZid = UUID.randomUUID().toString();
             arrangeZoneExists(aliasZid);
 
-            final T requestBody = buildEntityWithAliasProps(null, aliasZid);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, aliasZid);
             setZoneId(requestBody, aliasZid);
 
             final T existingEntity = resolveExistingEntityArgument(existingEntityArg);
@@ -252,7 +276,7 @@ final void shouldReturnFalse_NeitherOfZidAndAliasZidIsUaa(final ExistingEntityAr
             final String aliasZid = UUID.randomUUID().toString();
             arrangeZoneExists(aliasZid);
 
-            final T requestBody = buildEntityWithAliasProps(null, aliasZid);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, aliasZid);
             setZoneId(requestBody, UUID.randomUUID().toString());
 
             final T existingEntity = resolveExistingEntityArgument(existingEntityArg);
@@ -271,10 +295,11 @@ final void shouldReturnFalse_OnlyAliasZidSetInReqBody() {
             final String initialAliasZid = CUSTOM_ZONE_ID;
 
             final T existingEntity = buildEntityWithAliasProps(
+                    IdentityZone.getUaaZoneId(),
                     UUID.randomUUID().toString(),
                     initialAliasZid
             );
-            final T requestBody = buildEntityWithAliasProps(null, initialAliasZid);
+            final T requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, initialAliasZid);
 
             assertThat(aliasHandler.aliasPropertiesAreValid(requestBody, existingEntity)).isFalse();
         }

server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderAliasHandlerValidationTest.java

@@ -15,6 +15,7 @@
 
 import org.cloudfoundry.identity.uaa.alias.EntityAliasHandler;
 import org.cloudfoundry.identity.uaa.alias.EntityAliasHandlerValidationTest;
+import org.cloudfoundry.identity.uaa.zone.IdentityZone;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning;
 import org.cloudfoundry.identity.uaa.zone.ZoneDoesNotExistsException;
 import org.junit.jupiter.api.Nested;
@@ -44,14 +45,19 @@ protected EntityAliasHandler<IdentityProvider<?>> buildAliasHandler(final boolea
     }
 
     @Override
-    protected IdentityProvider<?> buildEntityWithAliasProps(@Nullable final String aliasId, @Nullable final String aliasZid) {
+    protected IdentityProvider<?> buildEntityWithAliasProps(
+            @Nullable final String zoneId,
+            @Nullable final String aliasId,
+            @Nullable final String aliasZid
+    ) {
         final IdentityProvider<AbstractIdentityProviderDefinition> idp = new IdentityProvider<>();
         idp.setName("example");
         idp.setOriginKey("example");
         idp.setType(OIDC10);
         idp.setIdentityZoneId(UAA);
         idp.setAliasId(aliasId);
         idp.setAliasZid(aliasZid);
+        setZoneId(idp, zoneId);
         return idp;
     }
 
@@ -61,7 +67,7 @@ protected void changeNonAliasProperties(@NonNull final IdentityProvider<?> entit
     }
 
     @Override
-    protected void setZoneId(@NonNull final IdentityProvider<?> entity, @NonNull final String zoneId) {
+    protected void setZoneId(@NonNull final IdentityProvider<?> entity, @Nullable final String zoneId) {
         entity.setIdentityZoneId(zoneId);
     }
 
@@ -89,7 +95,7 @@ void shouldReturnFalse_AliasNotSupportedForIdpType(
                 final String aliasZid = UUID.randomUUID().toString();
                 arrangeZoneExists(aliasZid);
 
-                final IdentityProvider<?> requestBody = buildEntityWithAliasProps(null, aliasZid);
+                final IdentityProvider<?> requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, aliasZid);
                 requestBody.setIdentityZoneId(UAA);
                 requestBody.setType(typeAliasNotSupported);
 
@@ -113,7 +119,7 @@ void shouldReturnTrue_AliasSupportedForIdpType(
                 final String aliasZid = UUID.randomUUID().toString();
                 arrangeZoneExists(aliasZid);
 
-                final IdentityProvider<?> requestBody = buildEntityWithAliasProps(null, aliasZid);
+                final IdentityProvider<?> requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, aliasZid);
                 requestBody.setIdentityZoneId(UAA);
                 requestBody.setType(typeAliasSupported);
 

server/src/test/java/org/cloudfoundry/identity/uaa/scim/ScimUserAliasHandlerValidationTest.java

@@ -14,6 +14,7 @@
 import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
 import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
 import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator;
+import org.cloudfoundry.identity.uaa.zone.IdentityZone;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning;
 import org.cloudfoundry.identity.uaa.zone.ZoneDoesNotExistsException;
 import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager;
@@ -51,7 +52,7 @@ protected EntityAliasHandler<ScimUser> buildAliasHandler(final boolean aliasEnti
     }
 
     @Override
-    protected ScimUser buildEntityWithAliasProps(final String aliasId, final String aliasZid) {
+    protected ScimUser buildEntityWithAliasProps(final String zoneId, final String aliasId, final String aliasZid) {
         final ScimUser scimUser = new ScimUser();
 
         scimUser.setDisplayName("Some Displayname");
@@ -62,6 +63,7 @@ protected ScimUser buildEntityWithAliasProps(final String aliasId, final String
         scimUser.setAliasId(aliasId);
         scimUser.setAliasZid(aliasZid);
 
+        setZoneId(scimUser, zoneId);
         return scimUser;
     }
 
@@ -121,7 +123,7 @@ private void shouldReturnFalse_IfIdpHasNoAlias(
 
                 arrangeCurrentIdz(zone1);
 
-                final ScimUser requestBody = buildEntityWithAliasProps(null, zone2);
+                final ScimUser requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, zone2);
                 requestBody.setZoneId(zone1);
                 final String origin = RANDOM_STRING_GENERATOR.generate();
                 requestBody.setOrigin(origin);
@@ -166,7 +168,7 @@ private void shouldReturnFalse_IfIdpOfOriginalUserDoesNotExist(
 
                 arrangeCurrentIdz(zone1);
 
-                final ScimUser requestBody = buildEntityWithAliasProps(null, zone2);
+                final ScimUser requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, zone2);
                 requestBody.setZoneId(zone1);
                 final String origin = RANDOM_STRING_GENERATOR.generate();
                 requestBody.setOrigin(origin);
@@ -201,7 +203,7 @@ void shouldReturnFalse_IfIdpHasAliasToDifferentZoneThanUser(
                 // should always be true
                 assertThat(aliasZidIdp).isNotEqualTo(customZoneId);
 
-                final ScimUser requestBody = buildEntityWithAliasProps(null, customZoneId);
+                final ScimUser requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, customZoneId);
                 requestBody.setZoneId(UAA);
                 final String origin = RANDOM_STRING_GENERATOR.generate();
                 requestBody.setOrigin(origin);
@@ -249,7 +251,7 @@ private void shouldReturnFalse_IfIdpOfOriginalUserHasEmptyAliasId(
 
                 arrangeCurrentIdz(zone1);
 
-                final ScimUser requestBody = buildEntityWithAliasProps(null, zone2);
+                final ScimUser requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, zone2);
                 requestBody.setZoneId(zone1);
                 final String origin = RANDOM_STRING_GENERATOR.generate();
                 requestBody.setOrigin(origin);
@@ -298,7 +300,7 @@ private void shouldReturnTrue_IfIdpHasAliasToSameZoneAsUser(
 
                 arrangeCurrentIdz(zone1);
 
-                final ScimUser requestBody = buildEntityWithAliasProps(null, zone2);
+                final ScimUser requestBody = buildEntityWithAliasProps(IdentityZone.getUaaZoneId(), null, zone2);
                 requestBody.setZoneId(zone1);
                 final String origin = RANDOM_STRING_GENERATOR.generate();
                 requestBody.setOrigin(origin);