Alias ID and Alias ZID for Users (#2663)

* Checkout DB migrations for alias properties of Identity Provider table

* Add DB migrations for alias properties of users table

* Add alias properties to ScimUser class

* Add alias properties to create operations in JdbcScimUserProvisioning

* Add alias properties to update query in JdbcScimUserProvisioning

* Add alias properties to deactivate and delete user operations in JdbcScimUserProvisioning

* Add alias properties to change password query in JdbcScimUserProvisioning

* Add alias properties to update password change required query in JdbcScimUserProvisioning

* Fix unit tests

* Add hasMirroredUser method to ScimUser

* Improve check for number of updated records in change password handler

* Add tests for alias property handling in JdbcScimUserProvisioning

* Fix unit tests

* Add check whether the password change required flag was also updated for the mirrored user

* Move validity check of alias properties to separate class

* Move mirroring handling to separate reusable class

* Fix unit tests

* Add javadoc to EntityMirroringHandler

* Add mirroring handler for ScimUser class

* Add missing override annotations to JdbcScimUserProvisioning

* Fix wrong getter call in EntityMirroringHandler

* Fix access token cache in IdentityProviderEndpointsAliasMockMvcTests

* Make access token cache in IdentityProviderEndpointsAliasMockMvcTests non-static

* Add factory method for cloning Approval

* Add EntityMirroringResult class

* Add additional validation checks to EntityMirroringHandler

* Add mirrored entity to return value of EntityMirroringHandler.ensureConsistencyOfMirroredEntity

* Add mirroring handling to ScimUser create endpoint

* Add missing constructor parameters to ScimUserEndpoints

* Make ScimUser.getAliasZid return aliasZid instead of null

* Change wording from "mirrored" to "alias"

* Add tests for IdentityProviderAliasHandler

* Move unit tests for alias handling to separate class

* Add further MockMvc tests for Creation/Update of IdPs with alias properties

* Add tests for read operations

* Fix IdentityProviderEndpointsTest

* Fix Flyway migration

* Remove obsolete IdentityProviderEndpointsAliasTest

* Fix IdentityProviderAliasHandlerEnsureConsistencyTest

* Remove obsolete IdentityProviderAliasHandlerTest

* Add JsonIgnore annotation to EntityWithAlias#getAliasDescription

* Fix JdbcScimUserProvisioningTests

* Refactor

* Remove obsolete IdentityProviderEndpointsTestBase

* Revert changes to JdbcScimUserProvisioning

* Add skeleton of ScimUserEndpointsAliasMockMvcTests

* Introduce superclass for MockMvcTests of endpoints for entities with alias

* Add alias properties to ScimUser JSON deserialization

* Add MockMvc tests for ScimUser GET with disabled alias feature

* Fix JdbcScimUserProvisioningTests

* Add MockMvc tests for SCIM user create with alias

* Add more detailed comparison of original and alias user to ScimUserEndpointsAliasMockMvcTests

* Add 'aliasEntitiesEnabled' flag to ScimUserEndpoints

* Add alias logic to deletion of SCIM users

* Add MockMvc test about ignoring dangling reference during deletion

* Add MockMvc test about also deleting alias user if original is deleted

* Add MockMvc test about breaking reference to original user in alias user if original is deleted

* Add MockMvc tests for SCIM user PUT with alias

* Add MockMvc tests for SCIM User PUT with disabled alias feature

* Refactor

* Add MockMvc tests for SCIM user patch

* Fix assignment of groups and approvals of alias SCIM users

* Remove obsolete Approval.clone method

* Add check if alias user only has default groups of alias zone to ScimUserEndpointsAliasMockMvcTests

* Revert EntityAliasResult

* Fix missing import

* Fix unit tests

* Revert adding retrievePasswordForUser method to ScimUserProvisioning

* Revert making ScimUSer getters public

* Remove usages of no longer accessible ScimUser getters

* Reject user deletion if alias exists and alias feature is disabled

* Adjust MockMvc tests to new deletion behavior

* Adjust MockMvc test Update -> AliasFeatureDisabled -> ExistingAlias -> shouldAccept_OnlyAliasPropsSetToNull to new update behavior

* Adjust MockMvc test Update -> AliasFeatureDisabled -> ExistingAlias -> shouldAccept_AliasPropsSetToNullAndOtherPropsChanged to new update behavior

* Adjust MockMvc test Update -> AliasFeatureDisabled -> ExistingAlias -> shouldAccept_ShouldIgnoreAliasIdMissingInExistingUser to new update behavior

* Adjust MockMvc test Update -> AliasFeatureDisabled -> ExistingAlias -> shouldAccept_ShouldIgnoreDanglingRef to new update behavior

* Improve test case names

* Remove unused method

* Remove obsolete tests checking whether alias properties are ignored in SCIM user endpoints

* Use 400 status code instead of 422 for rejected deletions of SCIM users with alias

* Adjust endpoint docs for new alias fields

* Remove obsolete test cases for JdbcScimUserProvisioning

* Refactor alias handling in ScimUser create endpoint

* Adjust SCIM operation counters to not count operations performed on alias users

* Adjust ScimUserEndpointsAliasMockMvcTests.assertIsCorrectAliasPair to ScimUserAliasHandler.cloneEntity method

* Fix endpoint docs for users delete endpoint

* Add unit test for ScimUserEndpoints: should throw during creation if alias properties are invalid

* Add unit test for ScimUserEndpoints: should throw during deletion if user has alias and alias feature disabled

* Fix integration tests for postgresql

* Fix unit tests

* Remove unnecessary sync of approvals and groups from update endpoint

* Revert "Remove unnecessary sync of approvals and groups from update endpoint"

This reverts commit e75cff9.

* Add separate class for unit tests related to user alias fields

* Fix unit tests

* Move ScimUserEndpointsAliasTests from uaa to server bundle

* Add unit tests for ScimUser delete with alias

* Add unit tests for ScimUser update with alias

* Add unit tests for ScimUser update with alias: alias handler throws exception

* Add unit tests for ScimUser delete with alias: should ignore dangling reference

* Add unit tests for ScimUser update with alias: should throw ScimException if aliasHandler.ensureConsistency fails

* Use EntityAliasFailedException as cause if thrown by alias handler during ScimUser update

* Adjust endpoint docs for ScimUser create/update: 422 status code if alias creation/update fails

* Remove deletion of alias IdP from JdbcIdentityProviderProvisioning.deleteByIdentityZone

* Reject deletion of identity zone if an IdP with alias exists in the zone

* Add new status code to identity zone deletion documentation

* Fix ScimUser create: ensure zone ID is set before alias validity check

* Fix Sonar: remove unnecessary clause in if statement

* Remove changes from PR#2850

* Replace value annotation with aliasEntitiesEnabled bean in ScimUserEndpoints constructor

* Refactor

* Rework: use transaction and alias handling only when alias flag is enabled

* Rework: inject transaction template instead of creating it in the constructor of ScimUserEndpoints

* Fix Sonar: change collect(toList()) to toList()

* Import statement

order as before

---------

Co-authored-by: d036670 <markus.strehle@sap.com>

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

@@ -121,7 +121,7 @@ public String getId() {
 
     @Override
     public String getZoneId() {
-        return identityZoneId;
+        return getIdentityZoneId();
     }
 
     public IdentityProvider setId(String id) {

model/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUser.java

@@ -353,11 +353,9 @@ public int hashCode() {
 
     private String zoneId = null;
 
-    @JsonIgnore
     @Setter
     private String aliasZid = null;
 
-    @JsonIgnore
     @Setter
     private String aliasId = null;
 

model/src/main/java/org/cloudfoundry/identity/uaa/scim/impl/ScimUserJsonDeserializer.java

@@ -87,6 +87,10 @@ public ScimUser deserialize(JsonParser jp, DeserializationContext ctxt) throws I
                     user.setOrigin(jp.readValueAs(String.class));
                 } else if ("zoneId".equalsIgnoreCase(fieldName)) {
                     user.setZoneId(jp.readValueAs(String.class));
+                } else if ("aliasId".equalsIgnoreCase(fieldName)) {
+                    user.setAliasId(jp.readValueAs(String.class));
+                } else if ("aliasZid".equalsIgnoreCase(fieldName)) {
+                    user.setAliasZid(jp.readValueAs(String.class));
                 } else if ("salt".equalsIgnoreCase(fieldName)) {
                     user.setSalt(jp.readValueAs(String.class));
                 } else if ("passwordLastModified".equalsIgnoreCase(fieldName)) {

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

@@ -3,6 +3,7 @@
 import com.jayway.jsonpath.JsonPathException;
 import org.cloudfoundry.identity.uaa.account.UserAccountStatus;
 import org.cloudfoundry.identity.uaa.account.event.UserAccountUnlockedEvent;
+import org.cloudfoundry.identity.uaa.alias.EntityAliasFailedException;
 import org.cloudfoundry.identity.uaa.approval.Approval;
 import org.cloudfoundry.identity.uaa.approval.ApprovalStore;
 import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent;
@@ -26,6 +27,7 @@
 import org.cloudfoundry.identity.uaa.scim.ScimGroup;
 import org.cloudfoundry.identity.uaa.scim.ScimGroupMembershipManager;
 import org.cloudfoundry.identity.uaa.scim.ScimUser;
+import org.cloudfoundry.identity.uaa.scim.ScimUserAliasHandler;
 import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
 import org.cloudfoundry.identity.uaa.scim.exception.InvalidScimResourceException;
 import org.cloudfoundry.identity.uaa.scim.exception.ScimException;
@@ -58,9 +60,12 @@
 import org.springframework.jmx.export.annotation.ManagedResource;
 import org.springframework.jmx.support.MetricType;
 import org.springframework.lang.NonNull;
+import org.springframework.lang.Nullable;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Controller;
+import org.springframework.transaction.annotation.Transactional;
+import org.springframework.transaction.support.TransactionTemplate;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.ExceptionHandler;
@@ -84,12 +89,14 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
 import static org.cloudfoundry.identity.uaa.codestore.ExpiringCodeType.REGISTRATION;
+import static org.springframework.util.StringUtils.hasText;
 import static org.springframework.util.StringUtils.isEmpty;
 
 import lombok.Getter;
@@ -122,12 +129,21 @@ public class ScimUserEndpoints implements InitializingBean, ApplicationEventPubl
     private final ExpiringCodeStore codeStore;
     private final ApprovalStore approvalStore;
     private final ScimGroupMembershipManager membershipManager;
+    private final boolean aliasEntitiesEnabled;
     @Getter
     private final int userMaxCount;
     private final HttpMessageConverter<?>[] messageConverters;
+    /**
+     * Update operations performed on alias users are not considered.
+     */
     private final AtomicInteger scimUpdates;
+    /**
+     * Deletion operations performed on alias users are not considered.
+     */
     private final AtomicInteger scimDeletes;
     private final Map<String, AtomicInteger> errorCounts;
+    private final ScimUserAliasHandler aliasHandler;
+    private final TransactionTemplate transactionTemplate;
 
     private ApplicationEventPublisher publisher;
 
@@ -145,7 +161,11 @@ public ScimUserEndpoints(
             final ExpiringCodeStore codeStore,
             final ApprovalStore approvalStore,
             final ScimGroupMembershipManager membershipManager,
-            final @Value("${userMaxCount:500}") int userMaxCount) {
+            final ScimUserAliasHandler aliasHandler,
+            final TransactionTemplate transactionTemplate,
+            final @Qualifier("aliasEntitiesEnabled") boolean aliasEntitiesEnabled,
+            final @Value("${userMaxCount:500}") int userMaxCount
+    ) {
         if (userMaxCount <= 0) {
             throw new IllegalArgumentException(
                     String.format("Invalid \"userMaxCount\" value (got %d). Should be positive number.", userMaxCount)
@@ -161,11 +181,14 @@ public ScimUserEndpoints(
         this.passwordValidator = passwordValidator;
         this.codeStore = codeStore;
         this.approvalStore = approvalStore;
+        this.aliasEntitiesEnabled = aliasEntitiesEnabled;
         this.userMaxCount = userMaxCount;
         this.membershipManager = membershipManager;
         this.messageConverters = new HttpMessageConverter[] {
                 new ExceptionReportHttpMessageConverter()
         };
+        this.aliasHandler = aliasHandler;
+        this.transactionTemplate = transactionTemplate;
         scimUpdates = new AtomicInteger();
         scimDeletes = new AtomicInteger();
         errorCounts = new ConcurrentHashMap<>();
@@ -224,15 +247,52 @@ public ScimUser createUser(@RequestBody ScimUser user, HttpServletRequest reques
             passwordValidator.validate(user.getPassword());
         }
 
-        ScimUser scimUser = scimUserProvisioning.createUser(user, user.getPassword(), identityZoneManager.getCurrentIdentityZoneId());
+        user.setZoneId(identityZoneManager.getCurrentIdentityZoneId());
+
+        if (!aliasHandler.aliasPropertiesAreValid(user, null)) {
+            throw new ScimException("Alias ID and/or alias ZID are invalid.", HttpStatus.BAD_REQUEST);
+        }
+
+        final ScimUser scimUser;
+        if (aliasEntitiesEnabled) {
+            // create the user and an alias for it if necessary
+            scimUser = createScimUserWithAliasHandling(user);
+        } else {
+            // create the user without alias handling
+            scimUser = scimUserProvisioning.createUser(user, user.getPassword(), identityZoneManager.getCurrentIdentityZoneId());
+        }
+
         if (user.getApprovals() != null) {
             for (Approval approval : user.getApprovals()) {
                 approval.setUserId(scimUser.getId());
                 approvalStore.addApproval(approval, identityZoneManager.getCurrentIdentityZoneId());
             }
         }
-        scimUser = syncApprovals(syncGroups(scimUser));
-        addETagHeader(response, scimUser);
+        final ScimUser scimUserWithApprovalsAndGroups = syncApprovals(syncGroups(scimUser));
+        addETagHeader(response, scimUserWithApprovalsAndGroups);
+        return scimUserWithApprovalsAndGroups;
+    }
+
+    private ScimUser createScimUserWithAliasHandling(final ScimUser user) {
+        final ScimUser scimUser;
+        try {
+            scimUser = transactionTemplate.execute(txStatus -> {
+                final ScimUser originalScimUser = scimUserProvisioning.createUser(
+                        user,
+                        user.getPassword(),
+                        identityZoneManager.getCurrentIdentityZoneId()
+                );
+                return aliasHandler.ensureConsistencyOfAliasEntity(
+                        originalScimUser,
+                        null
+                );
+            });
+        } catch (final EntityAliasFailedException e) {
+            throw new ScimException(e.getMessage(), e, HttpStatus.resolve(e.getHttpStatus()));
+        }
+        if (scimUser == null) {
+            throw new IllegalStateException("The persisted user is not present after handling the alias.");
+        }
         return scimUser;
     }
 
@@ -257,15 +317,47 @@ public ScimUser updateUser(@RequestBody ScimUser user, @PathVariable String user
         int version = getVersion(userId, etag);
         user.setVersion(version);
 
+        final ScimUser existingScimUser = scimUserProvisioning.retrieve(
+                userId,
+                identityZoneManager.getCurrentIdentityZoneId()
+        );
+        if (!aliasHandler.aliasPropertiesAreValid(user, existingScimUser)) {
+            throw new ScimException("The fields 'aliasId' and/or 'aliasZid' are invalid.", HttpStatus.BAD_REQUEST);
+        }
+
+        final ScimUser scimUser;
         try {
-            ScimUser updated = scimUserProvisioning.update(userId, user, identityZoneManager.getCurrentIdentityZoneId());
-            scimUpdates.incrementAndGet();
-            ScimUser scimUser = syncApprovals(syncGroups(updated));
-            addETagHeader(httpServletResponse, scimUser);
-            return scimUser;
-        } catch (OptimisticLockingFailureException e) {
+            if (aliasEntitiesEnabled) {
+                // update user and create/update alias, if necessary
+                scimUser = updateUserWithAliasHandling(userId, user, existingScimUser);
+            } else {
+                // update user without alias handling
+                scimUser = scimUserProvisioning.update(userId, user, identityZoneManager.getCurrentIdentityZoneId());
+            }
+        } catch (final OptimisticLockingFailureException e) {
             throw new ScimResourceConflictException(e.getMessage());
+        } catch (final EntityAliasFailedException e) {
+            throw new ScimException(e.getMessage(), e, HttpStatus.resolve(e.getHttpStatus()));
         }
+
+        scimUpdates.incrementAndGet();
+        final ScimUser scimUserWithApprovalsAndGroups = syncApprovals(syncGroups(scimUser));
+        addETagHeader(httpServletResponse, scimUserWithApprovalsAndGroups);
+        return scimUserWithApprovalsAndGroups;
+    }
+
+    private ScimUser updateUserWithAliasHandling(final String userId, final ScimUser user, final ScimUser existingUser) {
+        return transactionTemplate.execute(txStatus -> {
+            final ScimUser updatedOriginalUser = scimUserProvisioning.update(
+                    userId,
+                    user,
+                    identityZoneManager.getCurrentIdentityZoneId()
+            );
+            return aliasHandler.ensureConsistencyOfAliasEntity(
+                    updatedOriginalUser,
+                    existingUser
+            );
+        });
     }
 
     @RequestMapping(value = "/Users/{userId}", method = RequestMethod.PATCH)
@@ -298,13 +390,23 @@ public ScimUser patchUser(@RequestBody ScimUser patch, @PathVariable String user
 
     @RequestMapping(value = "/Users/{userId}", method = RequestMethod.DELETE)
     @ResponseBody
+    @Transactional
     public ScimUser deleteUser(@PathVariable String userId,
                                @RequestHeader(value = "If-Match", required = false) String etag,
                                HttpServletRequest request,
                                HttpServletResponse httpServletResponse) {
         int version = etag == null ? -1 : getVersion(userId, etag);
         ScimUser user = getUser(userId, httpServletResponse);
         throwWhenUserManagementIsDisallowed(user.getOrigin(), request);
+
+        final boolean userHasAlias = hasText(user.getAliasZid());
+        if (userHasAlias && !aliasEntitiesEnabled) {
+            throw new UaaException(
+                    "Could not delete user with alias since alias entities are disabled.",
+                    HttpStatus.BAD_REQUEST.value()
+            );
+        }
+
         membershipManager.removeMembersByMemberId(userId, identityZoneManager.getCurrentIdentityZoneId());
         scimUserProvisioning.delete(userId, version, identityZoneManager.getCurrentIdentityZoneId());
         scimDeletes.incrementAndGet();
@@ -315,8 +417,35 @@ public ScimUser deleteUser(@PathVariable String userId,
                             SecurityContextHolder.getContext().getAuthentication(),
                             identityZoneManager.getCurrentIdentityZoneId())
             );
-            logger.debug("User delete event sent[" + userId + "]");
+            logger.debug("User delete event sent[{}]", userId);
+        }
+
+        if (!userHasAlias) {
+            // no further action necessary
+            return user;
         }
+
+        // also delete alias user, if present
+        final Optional<ScimUser> aliasUserOpt = aliasHandler.retrieveAliasEntity(user);
+        if (aliasUserOpt.isEmpty()) {
+            // ignore dangling reference to alias user
+            logger.warn("Attempted to delete alias of user '{}', but it was not present.", user.getId());
+            return user;
+        }
+        final ScimUser aliasUser = aliasUserOpt.get();
+        membershipManager.removeMembersByMemberId(aliasUser.getId(), aliasUser.getZoneId());
+        scimUserProvisioning.delete(aliasUser.getId(), aliasUser.getVersion(), aliasUser.getZoneId());
+        if (publisher != null) {
+            publisher.publishEvent(
+                    new EntityDeletedEvent<>(
+                            aliasUser,
+                            SecurityContextHolder.getContext().getAuthentication(),
+                            aliasUser.getZoneId()
+                    )
+            );
+            logger.debug("User delete event sent[{}]", userId);
+        }
+
         return user;
     }
 
@@ -413,7 +542,7 @@ public SearchResults<?> findUsers(
             }
         } catch (IllegalArgumentException e) {
             String msg = "Invalid filter expression: [" + filter + "]";
-            if (StringUtils.hasText(sortBy)) {
+            if (hasText(sortBy)) {
                 msg += " [" + sortBy + "]";
             }
             throw new ScimException(HtmlUtils.htmlEscape(msg), HttpStatus.BAD_REQUEST);
@@ -469,9 +598,10 @@ public UserAccountStatus updateAccountStatus(@RequestBody UserAccountStatus stat
         return status;
     }
 
-    private ScimUser syncGroups(ScimUser user) {
+    @Nullable
+    private ScimUser syncGroups(@Nullable ScimUser user) {
         if (user == null) {
-            return user;
+            return null;
         }
 
         Set<ScimGroup> directGroups = membershipManager.getGroupsWithMember(user.getId(), false, identityZoneManager.getCurrentIdentityZoneId());
@@ -489,6 +619,9 @@ private ScimUser syncGroups(ScimUser user) {
         return user;
     }
 
+    /**
+     * Look up the approvals for the given user and keep only those that are currently active.
+     */
     private ScimUser syncApprovals(ScimUser user) {
         if (user == null || approvalStore == null) {
             return user;

server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java

@@ -22,9 +22,9 @@
 import java.sql.Types;
 import java.util.Arrays;
 import java.util.Calendar;
+import java.util.Collections;
 import java.util.Date;
 import java.util.GregorianCalendar;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
@@ -294,12 +294,8 @@ public ScimUser create(final ScimUser user, String zoneId) {
             });
         } catch (DuplicateKeyException e) {
             String userOrigin = hasText(user.getOrigin()) ? user.getOrigin() : OriginKeys.UAA;
-            ScimUser existingUser = retrieveByUsernameAndOriginAndZone(user.getUserName(), userOrigin, zoneId).get(0);
-            Map<String,Object> userDetails = new HashMap<>();
-            userDetails.put("active", existingUser.isActive());
-            userDetails.put("verified", existingUser.isVerified());
-            userDetails.put("user_id", existingUser.getId());
-            throw new ScimResourceAlreadyExistsException("Username already in use: " + existingUser.getUserName(), userDetails);
+            Map<String,Object> userDetails = Collections.singletonMap("origin", userOrigin);
+            throw new ScimResourceAlreadyExistsException("Username already in use: " + user.getUserName(), userDetails);
         }
         return retrieve(id, zoneId);
     }

server/src/test/java/org/cloudfoundry/identity/uaa/scim/bootstrap/ScimUserBootstrapTests.java

@@ -111,7 +111,12 @@ void init() throws SQLException {
                 null,
                 null,
                 null,
-                jdbcScimGroupMembershipManager, 5);
+                jdbcScimGroupMembershipManager,
+                null,
+                null,
+                false,
+                5
+        );
         IdentityZoneHolder.get().getConfig().getUserConfig().setDefaultGroups(emptyList());
     }