Change update behavior for SCIM user aliases: no longer propagate tim…

…estamps from original user to alias user
cloudfoundry · Apr 11, 2024 · 96af351 · 96af351
1 parent 9ed1580
commit 96af351
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 2 deletions.
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/alias/EntityAliasHandler.java
@@ -155,6 +155,7 @@ public final T ensureConsistencyOfAliasEntity(
         // update the existing alias entity
         if (existingAliasEntity != null) {
             setId(aliasEntity, existingAliasEntity.getId());
+            setPropertiesFromExistingAliasEntity(aliasEntity, existingAliasEntity);
             updateEntity(aliasEntity, originalEntity.getAliasZid());
             return originalEntity;
         }
@@ -178,6 +179,12 @@ public final T ensureConsistencyOfAliasEntity(
         return updateEntity(originalEntity, originalEntity.getZoneId());
     }
 
+    /**
+     * Set properties from the existing alias entity in the new alias entity before it is updated. Can be used if
+     * certain properties should differ between the original and the alias entity.
+     */
+    protected abstract void setPropertiesFromExistingAliasEntity(final T newAliasEntity, final T existingAliasEntity);
+
     private T buildAliasEntity(final T originalEntity) {
         final T aliasEntity = cloneEntity(originalEntity);
         aliasEntity.setAliasId(originalEntity.getId());

diff --git a/...er/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderAliasHandler.java b/...er/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderAliasHandler.java
@@ -45,6 +45,14 @@ protected boolean additionalValidationChecksForNewAlias(@NonNull final IdentityP
         return IDP_TYPES_ALIAS_SUPPORTED.contains(requestBody.getType());
     }
 
+    @Override
+    protected void setPropertiesFromExistingAliasEntity(
+            final IdentityProvider<?> newAliasEntity,
+            final IdentityProvider<?> existingAliasEntity
+    ) {
+        // not required for identity providers
+    }
+
     @Override
     protected void setId(final IdentityProvider<?> entity, final String id) {
         entity.setId(id);

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUserAliasHandler.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimUserAliasHandler.java
@@ -53,6 +53,17 @@ protected boolean additionalValidationChecksForNewAlias(final ScimUser requestBo
         return EntityAliasHandler.isValidAliasPair(idpInCurrentZone.get(), idpInAliasZone.get());
     }
 
+    @Override
+    protected void setPropertiesFromExistingAliasEntity(
+            final ScimUser newAliasEntity,
+            final ScimUser existingAliasEntity
+    ) {
+        // these three timestamps should not be overwritten by the timestamps of the original user
+        newAliasEntity.setPasswordLastModified(existingAliasEntity.getPasswordLastModified());
+        newAliasEntity.setLastLogonTime(existingAliasEntity.getLastLogonTime());
+        newAliasEntity.setPreviousLogonTime(existingAliasEntity.getPreviousLogonTime());
+    }
+
     private Optional<IdentityProvider<?>> retrieveIdpByOrigin(final String originKey, final String zoneId) {
         final IdentityProvider<?> idpInAliasZone;
         try {
@@ -98,7 +109,9 @@ protected ScimUser cloneEntity(final ScimUser originalEntity) {
         aliasUser.setAliasId(null);
         aliasUser.setAliasZid(null);
 
-        // these timestamps will be overwritten during creation
+        /* these timestamps will be overwritten:
+         *  - creation: with current timestamp during persistence (JdbcScimUserProvisioning)
+         *  - update: with values from existing alias entity */
         aliasUser.setPasswordLastModified(null);
         aliasUser.setLastLogonTime(null);
         aliasUser.setPreviousLogonTime(null);

diff --git a/...st/java/org/cloudfoundry/identity/uaa/scim/ScimUserAliasHandlerEnsureConsistencyTest.java b/...st/java/org/cloudfoundry/identity/uaa/scim/ScimUserAliasHandlerEnsureConsistencyTest.java
@@ -9,7 +9,9 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.sql.Timestamp;
 import java.util.Collections;
+import java.util.Date;
 import java.util.Objects;
 import java.util.UUID;
 
@@ -100,7 +102,10 @@ protected boolean entitiesAreEqual(final ScimUser entity1, final ScimUser entity
                 && Objects.equals(entity1.getAliasZid(), entity2.getAliasZid())
                 && Objects.equals(entity1.getZoneId(), entity2.getZoneId())
                 && Objects.equals(entity1.getPassword(), entity2.getPassword())
-                && Objects.equals(entity1.getSalt(), entity2.getSalt());
+                && Objects.equals(entity1.getSalt(), entity2.getSalt())
+                && Objects.equals(entity1.getPreviousLogonTime(), entity2.getPreviousLogonTime())
+                && Objects.equals(entity1.getLastLogonTime(), entity2.getLastLogonTime())
+                && Objects.equals(entity1.getPasswordLastModified(), entity2.getPasswordLastModified());
     }
 
     @Override
@@ -148,6 +153,12 @@ void shouldPropagateChangesToExistingAlias() {
                 final String aliasId = UUID.randomUUID().toString();
                 final ScimUser existingUser = buildEntityWithAliasProperties(aliasId, customZoneId);
 
+                // set timestamps of original user
+                final Timestamp timestampOriginalUser = new Timestamp(new Date().getTime());
+                existingUser.setPasswordLastModified(timestampOriginalUser);
+                existingUser.setPreviousLogonTime(timestampOriginalUser.getTime());
+                existingUser.setLastLogonTime(timestampOriginalUser.getTime());
+
                 final ScimUser originalUser = shallowCloneEntity(existingUser);
                 final String newGivenName = "some-new-name";
                 originalUser.setName(new ScimUser.Name(newGivenName, originalUser.getFamilyName()));
@@ -158,6 +169,13 @@ void shouldPropagateChangesToExistingAlias() {
                 aliasUser.setZoneId(existingUser.getAliasZid());
                 aliasUser.setAliasId(existingUser.getId());
                 aliasUser.setAliasZid(existingUser.getZoneId());
+
+                // set timestamps of alias user to a different value
+                final Timestamp timestampAliasUser = new Timestamp(new Date().getTime() + 5000);
+                aliasUser.setPasswordLastModified(timestampAliasUser);
+                aliasUser.setPreviousLogonTime(timestampAliasUser.getTime());
+                aliasUser.setLastLogonTime(timestampAliasUser.getTime());
+
                 arrangeEntityExists(aliasUser.getId(), aliasUser.getZoneId(), aliasUser);
 
                 final ScimUser result = aliasHandler.ensureConsistencyOfAliasEntity(
@@ -175,6 +193,11 @@ void shouldPropagateChangesToExistingAlias() {
                 assertThat(capturedUser.getId()).isEqualTo(aliasId);
                 assertThat(capturedUser.getZoneId()).isEqualTo(customZoneId);
                 assertThat(capturedUser.getGivenName()).isEqualTo(newGivenName);
+
+                // check if the alias timestamps were left unchanged even though the original user has different ones
+                assertThat(capturedUser.getPasswordLastModified()).isNotNull().isEqualTo(timestampAliasUser);
+                assertThat(capturedUser.getPreviousLogonTime()).isNotNull().isEqualTo(timestampAliasUser.getTime());
+                assertThat(capturedUser.getLastLogonTime()).isNotNull().isEqualTo(timestampAliasUser.getTime());
             }
 
             @Test