This is an automated email from the ASF dual-hosted git repository. andreapatricelli pushed a commit to branch 3_0_X in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/3_0_X by this push: new e5d3a7e178 [SYNCOPE-1751] moves random password generation to propagation task execution (#441) e5d3a7e178 is described below commit e5d3a7e178531c944c085c6833ead3d3022451f7 Author: Andrea Patricelli <andreapatrice...@apache.org> AuthorDate: Fri Apr 14 15:23:58 2023 +0200 [SYNCOPE-1751] moves random password generation to propagation task execution (#441) * [SYNCOPE-1751] moves random password generation to effective create on external resource --------- Co-authored-by: Francesco Chicchiriccò <ilgro...@apache.org> --- .../core/persistence/jpa/inner/UserTest.java | 6 +- .../provisioning/java/DefaultMappingManager.java | 9 -- .../provisioning/java/ProvisioningContext.java | 8 +- .../AbstractPropagationTaskExecutor.java | 176 ++++++++++++++------- .../PriorityPropagationTaskExecutor.java | 8 +- .../provisioning/java/utils/ConnObjectUtils.java | 16 +- .../java/DefaultMappingManagerTest.java | 8 +- .../spring/security/DefaultPasswordGenerator.java | 14 +- .../core/spring/security/PasswordGenerator.java | 3 +- .../syncope/fit/core/LinkedAccountITCase.java | 4 +- .../syncope/fit/core/PropagationTaskITCase.java | 34 ++++ 11 files changed, 199 insertions(+), 87 deletions(-) diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java index 4d1467d22d..21ca8564b0 100644 --- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java +++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java @@ -259,7 +259,8 @@ public class UserTest extends AbstractTest { @Test public void testPasswordGenerator() { - String password = passwordGenerator.generate(resourceDAO.find("ws-target-resource-nopropagation")); + String password = passwordGenerator.generate(resourceDAO.find("ws-target-resource-nopropagation"), + List.of(realmDAO.getRoot())); assertNotNull(password); User user = userDAO.find("c9b2dec2-00a7-4855-97c0-d854842b4b24"); @@ -270,7 +271,8 @@ public class UserTest extends AbstractTest { @Test public void passwordGeneratorFailing() { assertThrows(IllegalArgumentException.class, () -> { - String password = passwordGenerator.generate(resourceDAO.find("ws-target-resource-nopropagation")); + String password = passwordGenerator.generate(resourceDAO.find("ws-target-resource-nopropagation"), + List.of(realmDAO.getRoot())); assertNotNull(password); User user = userDAO.find("c9b2dec2-00a7-4855-97c0-d854842b4b24"); diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java index 4669e9c7ea..ee7c7ecadd 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/DefaultMappingManager.java @@ -95,7 +95,6 @@ import org.apache.syncope.core.provisioning.api.utils.FormatUtils; import org.apache.syncope.core.provisioning.java.utils.ConnObjectUtils; import org.apache.syncope.core.provisioning.java.utils.MappingUtils; import org.apache.syncope.core.spring.security.Encryptor; -import org.apache.syncope.core.spring.security.PasswordGenerator; import org.identityconnectors.framework.common.FrameworkUtil; import org.identityconnectors.framework.common.objects.Attribute; import org.identityconnectors.framework.common.objects.AttributeBuilder; @@ -135,8 +134,6 @@ public class DefaultMappingManager implements MappingManager { protected final VirAttrCache virAttrCache; - protected final PasswordGenerator passwordGenerator; - protected final AnyUtilsFactory anyUtilsFactory; protected final IntAttrNameParser intAttrNameParser; @@ -153,7 +150,6 @@ public class DefaultMappingManager implements MappingManager { final DerAttrHandler derAttrHandler, final VirAttrHandler virAttrHandler, final VirAttrCache virAttrCache, - final PasswordGenerator passwordGenerator, final AnyUtilsFactory anyUtilsFactory, final IntAttrNameParser intAttrNameParser) { @@ -168,7 +164,6 @@ public class DefaultMappingManager implements MappingManager { this.derAttrHandler = derAttrHandler; this.virAttrHandler = virAttrHandler; this.virAttrCache = virAttrCache; - this.passwordGenerator = passwordGenerator; this.anyUtilsFactory = anyUtilsFactory; this.intAttrNameParser = intAttrNameParser; } @@ -532,10 +527,6 @@ public class DefaultMappingManager implements MappingManager { } } - if (passwordAttrValue == null && resource.isRandomPwdIfNotProvided()) { - passwordAttrValue = passwordGenerator.generate(resource); - } - return passwordAttrValue; } diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java index c8ba816f9f..8491a7f7d9 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java @@ -447,7 +447,6 @@ public class ProvisioningContext { @ConditionalOnMissingBean @Bean public MappingManager mappingManager( - final PasswordGenerator passwordGenerator, final AnyUtilsFactory anyUtilsFactory, final AnyTypeDAO anyTypeDAO, final UserDAO userDAO, @@ -474,7 +473,6 @@ public class ProvisioningContext { derAttrHandler, virAttrHandler, virAttrCache, - passwordGenerator, anyUtilsFactory, intAttrNameParser); } @@ -559,12 +557,14 @@ public class ProvisioningContext { final TaskDAO taskDAO, final ExternalResourceDAO resourceDAO, final PlainSchemaDAO plainSchemaDAO, + final RealmDAO realmDAO, final NotificationManager notificationManager, final AuditManager auditManager, final TaskDataBinder taskDataBinder, final OutboundMatcher outboundMatcher, final PlainAttrValidationManager validator, - final ApplicationEventPublisher publisher) { + final ApplicationEventPublisher publisher, + final PasswordGenerator passwordGenerator) { return new PriorityPropagationTaskExecutor( connectorManager, @@ -575,6 +575,7 @@ public class ProvisioningContext { taskDAO, resourceDAO, plainSchemaDAO, + realmDAO, notificationManager, auditManager, taskDataBinder, @@ -583,6 +584,7 @@ public class ProvisioningContext { outboundMatcher, validator, publisher, + passwordGenerator, propagationTaskExecutorAsyncExecutor); } diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/AbstractPropagationTaskExecutor.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/AbstractPropagationTaskExecutor.java index 4ced18a1b6..4f320d857e 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/AbstractPropagationTaskExecutor.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/AbstractPropagationTaskExecutor.java @@ -35,6 +35,7 @@ import org.apache.syncope.common.lib.to.ExecTO; import org.apache.syncope.common.lib.to.Item; import org.apache.syncope.common.lib.to.OrgUnit; import org.apache.syncope.common.lib.to.Provision; +import org.apache.syncope.common.lib.types.AnyTypeKind; import org.apache.syncope.common.lib.types.AuditElements; import org.apache.syncope.common.lib.types.AuditElements.Result; import org.apache.syncope.common.lib.types.ExecStatus; @@ -46,8 +47,10 @@ import org.apache.syncope.core.persistence.api.dao.AnyObjectDAO; import org.apache.syncope.core.persistence.api.dao.ExternalResourceDAO; import org.apache.syncope.core.persistence.api.dao.GroupDAO; import org.apache.syncope.core.persistence.api.dao.PlainSchemaDAO; +import org.apache.syncope.core.persistence.api.dao.RealmDAO; import org.apache.syncope.core.persistence.api.dao.TaskDAO; import org.apache.syncope.core.persistence.api.dao.UserDAO; +import org.apache.syncope.core.persistence.api.entity.AnyUtils; import org.apache.syncope.core.persistence.api.entity.AnyUtilsFactory; import org.apache.syncope.core.persistence.api.entity.ExternalResource; import org.apache.syncope.core.persistence.api.entity.policy.PropagationPolicy; @@ -73,6 +76,7 @@ import org.apache.syncope.core.provisioning.java.utils.ConnObjectUtils; import org.apache.syncope.core.provisioning.java.utils.MappingUtils; import org.apache.syncope.core.spring.implementation.ImplementationManager; import org.apache.syncope.core.spring.security.AuthContextUtils; +import org.apache.syncope.core.spring.security.PasswordGenerator; import org.identityconnectors.framework.common.exceptions.ConnectorException; import org.identityconnectors.framework.common.objects.Attribute; import org.identityconnectors.framework.common.objects.AttributeBuilder; @@ -82,6 +86,7 @@ import org.identityconnectors.framework.common.objects.ConnectorObject; import org.identityconnectors.framework.common.objects.ConnectorObjectBuilder; import org.identityconnectors.framework.common.objects.Name; import org.identityconnectors.framework.common.objects.ObjectClass; +import org.identityconnectors.framework.common.objects.OperationalAttributes; import org.identityconnectors.framework.common.objects.SyncDeltaType; import org.identityconnectors.framework.common.objects.Uid; import org.slf4j.Logger; @@ -118,6 +123,8 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask protected final PlainSchemaDAO plainSchemaDAO; + protected final RealmDAO realmDAO; + protected final NotificationManager notificationManager; protected final AuditManager auditManager; @@ -134,6 +141,8 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask protected final ApplicationEventPublisher publisher; + protected final PasswordGenerator passwordGenerator; + protected final Map<String, PropagationActions> perContextActions = new ConcurrentHashMap<>(); public AbstractPropagationTaskExecutor( @@ -145,6 +154,7 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask final TaskDAO taskDAO, final ExternalResourceDAO resourceDAO, final PlainSchemaDAO plainSchemaDAO, + final RealmDAO realmDAO, final NotificationManager notificationManager, final AuditManager auditManager, final TaskDataBinder taskDataBinder, @@ -152,7 +162,8 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask final TaskUtilsFactory taskUtilsFactory, final OutboundMatcher outboundMatcher, final PlainAttrValidationManager validator, - final ApplicationEventPublisher publisher) { + final ApplicationEventPublisher publisher, + final PasswordGenerator passwordGenerator) { this.connectorManager = connectorManager; this.connObjectUtils = connObjectUtils; @@ -162,6 +173,7 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask this.taskDAO = taskDAO; this.resourceDAO = resourceDAO; this.plainSchemaDAO = plainSchemaDAO; + this.realmDAO = realmDAO; this.notificationManager = notificationManager; this.auditManager = auditManager; this.taskDataBinder = taskDataBinder; @@ -170,6 +182,7 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask this.outboundMatcher = outboundMatcher; this.validator = validator; this.publisher = publisher; + this.passwordGenerator = passwordGenerator; } @Override @@ -194,20 +207,86 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask return result; } + protected void checkMandatoryMissing( + final PropagationTaskInfo taskInfo, + final Set<Attribute> attrs, + final boolean enablePasswordCheck) { + + // check if there is any missing or null / empty mandatory attribute + Set<Object> mandatoryAttrNames = new HashSet<>(); + Optional.ofNullable(AttributeUtil.find(PropagationManager.MANDATORY_MISSING_ATTR_NAME, attrs)). + ifPresent(missing -> { + attrs.remove(missing); + + if (taskInfo.getOperation() == ResourceOperation.CREATE) { + // SYNCOPE-1751 remove __PASSWORD__ if enablePasswordCheck is false, this is needed to support + // LinkedAccount update propagation without password + mandatoryAttrNames.addAll(enablePasswordCheck + ? missing.getValue() + : missing.getValue().stream() + .filter(v -> !OperationalAttributes.PASSWORD_NAME.equals(v)) + .collect(Collectors.toList())); + } + }); + Optional.ofNullable(AttributeUtil.find(PropagationManager.MANDATORY_NULL_OR_EMPTY_ATTR_NAME, attrs)). + ifPresent(nullOrEmpty -> { + attrs.remove(nullOrEmpty); + + mandatoryAttrNames.addAll(nullOrEmpty.getValue()); + }); + if (!mandatoryAttrNames.isEmpty()) { + throw new IllegalArgumentException( + "Not attempted because there are mandatory attributes without value(s): " + mandatoryAttrNames); + } + } + protected Uid doCreate( final PropagationTaskInfo taskInfo, - final Set<Attribute> attributes, final Connector connector, - final AtomicReference<Boolean> propagationAttempted) { + final AtomicReference<Boolean> propagationAttempted, + final List<PropagationActions> actions) { + + Set<Attribute> attrs = taskInfo.getPropagationData().getAttributes(); + + if (AnyTypeKind.USER == taskInfo.getAnyTypeKind() + && AttributeUtil.getPasswordValue(attrs) == null + && taskInfo.getResource().isRandomPwdIfNotProvided()) { + + // generate random password + attrs.add(AttributeBuilder.buildPassword( + passwordGenerator.generate(taskInfo.getResource(), + realmDAO.findAncestors(userDAO.find(taskInfo.getEntityKey()).getRealm())). + toCharArray())); + + // remove __PASSWORD__ from MANDATORY_MISSING attribute + Set<Object> newMandatoryMissingAttrValues = new HashSet<>(); + Optional.ofNullable(AttributeUtil.find(PropagationManager.MANDATORY_MISSING_ATTR_NAME, attrs)). + ifPresent(missing -> { + newMandatoryMissingAttrValues.addAll( + missing.getValue().stream(). + filter(v -> !OperationalAttributes.PASSWORD_NAME.equals(v)). + collect(Collectors.toList())); + attrs.remove(missing); + }); + if (!newMandatoryMissingAttrValues.isEmpty()) { + attrs.add(AttributeBuilder.build( + PropagationManager.MANDATORY_MISSING_ATTR_NAME, newMandatoryMissingAttrValues)); + } + } + + actions.forEach(action -> action.before(taskInfo)); - LOG.debug("Create {} on {}", attributes, taskInfo.getResource().getKey()); + checkMandatoryMissing(taskInfo, attrs, true); - Uid result = connector.create(taskInfo.getObjectClass(), attributes, null, propagationAttempted); + LOG.debug("Create {} on {}", attrs, taskInfo.getResource().getKey()); + + Uid result = connector.create(taskInfo.getObjectClass(), attrs, null, propagationAttempted); taskInfo.getResource().getProvisionByAnyType(taskInfo.getAnyType()). filter(provision -> provision.getUidOnCreate() != null). ifPresent(provision -> { - anyUtilsFactory.getInstance(taskInfo.getAnyTypeKind()).addAttr( + AnyUtils anyUtils = anyUtilsFactory.getInstance(taskInfo.getAnyTypeKind()); + anyUtils.addAttr( validator, taskInfo.getEntityKey(), plainSchemaDAO.find(provision.getUidOnCreate()), @@ -215,7 +294,7 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask publisher.publishEvent(new AnyLifecycleEvent<>( this, SyncDeltaType.UPDATE, - userDAO.find(taskInfo.getEntityKey()), + anyUtils.dao().find(taskInfo.getEntityKey()), AuthContextUtils.getDomain())); }); @@ -224,15 +303,21 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask protected Uid doUpdate( final PropagationTaskInfo taskInfo, - final Set<Attribute> attributes, final Connector connector, final ConnectorObject beforeObj, - final AtomicReference<Boolean> propagationAttempted) { + final AtomicReference<Boolean> propagationAttempted, + final List<PropagationActions> actions) { + + actions.forEach(action -> action.before(taskInfo)); + + Set<Attribute> attrs = taskInfo.getPropagationData().getAttributes(); - LOG.debug("Update {} on {}", attributes, taskInfo.getResource().getKey()); + checkMandatoryMissing(taskInfo, attrs, false); + + LOG.debug("Update {} on {}", attrs, taskInfo.getResource().getKey()); // 1. check if rename is really required - Name newName = AttributeUtil.getNameFromAttributes(attributes); + Name newName = AttributeUtil.getNameFromAttributes(attrs); LOG.debug("Rename required with value {}", newName); @@ -241,7 +326,7 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask && !newName.getNameValue().equals(beforeObj.getUid().getUidValue())) { LOG.debug("Remote object name unchanged"); - attributes.remove(newName); + attrs.remove(newName); } // 2. check whether anything is actually needing to be propagated, i.e. if there is attribute @@ -250,7 +335,7 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask if (beforeObj != null) { originalAttrMap = beforeObj.getAttributes().stream(). collect(Collectors.toMap(attr -> attr.getName().toUpperCase(), Function.identity())); - Map<String, Attribute> updateAttrMap = attributes.stream(). + Map<String, Attribute> updateAttrMap = attrs.stream(). collect(Collectors.toMap(attr -> attr.getName().toUpperCase(), Function.identity())); // Only compare attribute from beforeObj that are also being updated @@ -258,14 +343,14 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask } Uid result; - if (!originalAttrMap.isEmpty() && originalAttrMap.values().equals(attributes)) { - LOG.debug("Don't need to propagate anything: {} is equal to {}", originalAttrMap.values(), attributes); - result = AttributeUtil.getUidAttribute(attributes); + if (!originalAttrMap.isEmpty() && originalAttrMap.values().equals(attrs)) { + LOG.debug("Don't need to propagate anything: {} is equal to {}", originalAttrMap.values(), attrs); + result = AttributeUtil.getUidAttribute(attrs); } else { - LOG.debug("Attributes to update: {}", attributes); + LOG.debug("Attributes to update: {}", attrs); // 3. provision entry - LOG.debug("Update {} on {}", attributes, taskInfo.getResource().getKey()); + LOG.debug("Update {} on {}", attrs, taskInfo.getResource().getKey()); result = connector.update( Optional.ofNullable(beforeObj). @@ -275,7 +360,7 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask map(ConnectorObject::getUid). orElseGet(() -> new Uid(taskInfo.getOldConnObjectKey() == null ? taskInfo.getConnObjectKey() : taskInfo.getOldConnObjectKey())), - attributes, + attrs, null, propagationAttempted); } @@ -287,7 +372,10 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask final PropagationTaskInfo taskInfo, final Set<AttributeDelta> modifications, final Connector connector, - final AtomicReference<Boolean> propagationAttempted) { + final AtomicReference<Boolean> propagationAttempted, + final List<PropagationActions> actions) { + + actions.forEach(action -> action.before(taskInfo)); Uid uid = new Uid(taskInfo.getConnObjectKey()); @@ -308,46 +396,24 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask final boolean fetchRemoteObj, final ConnectorObject beforeObj, final Connector connector, - final AtomicReference<Boolean> propagationAttempted) { + final AtomicReference<Boolean> propagationAttempted, + final List<PropagationActions> actions) { PropagationData propagationData = taskInfo.getPropagationData(); if (propagationData.getAttributeDeltas() == null) { - Set<Attribute> attrs = propagationData.getAttributes(); - - // check if there is any missing or null / empty mandatory attribute - Set<Object> mandatoryAttrNames = new HashSet<>(); - Optional.ofNullable(AttributeUtil.find(PropagationManager.MANDATORY_MISSING_ATTR_NAME, attrs)). - ifPresent(missing -> { - attrs.remove(missing); - - if (taskInfo.getOperation() == ResourceOperation.CREATE) { - mandatoryAttrNames.addAll(missing.getValue()); - } - }); - Optional.ofNullable(AttributeUtil.find(PropagationManager.MANDATORY_NULL_OR_EMPTY_ATTR_NAME, attrs)). - ifPresent(nullOrEmpty -> { - attrs.remove(nullOrEmpty); - - mandatoryAttrNames.addAll(nullOrEmpty.getValue()); - }); - if (!mandatoryAttrNames.isEmpty()) { - throw new IllegalArgumentException( - "Not attempted because there are mandatory attributes without value(s): " + mandatoryAttrNames); - } - if (beforeObj != null) { - return doUpdate(taskInfo, attrs, connector, beforeObj, propagationAttempted); + return doUpdate(taskInfo, connector, beforeObj, propagationAttempted, actions); } if (fetchRemoteObj || taskInfo.getOperation() == ResourceOperation.CREATE) { - return doCreate(taskInfo, attrs, connector, propagationAttempted); + return doCreate(taskInfo, connector, propagationAttempted, actions); } - return doUpdate(taskInfo, attrs, connector, beforeObj, propagationAttempted); + return doUpdate(taskInfo, connector, beforeObj, propagationAttempted, actions); } - return doUpdateDelta(taskInfo, propagationData.getAttributeDeltas(), connector, propagationAttempted); + return doUpdateDelta(taskInfo, propagationData.getAttributeDeltas(), connector, propagationAttempted, actions); } protected Uid delete( @@ -355,9 +421,13 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask final boolean fetchRemoteObj, final ConnectorObject beforeObj, final Connector connector, - final AtomicReference<Boolean> propagationAttempted) { + final AtomicReference<Boolean> propagationAttempted, + final List<PropagationActions> actions) { + + actions.forEach(action -> action.before(taskInfo)); Uid result; + if (fetchRemoteObj && beforeObj == null) { LOG.debug("{} not found on {}: ignoring delete", taskInfo.getConnObjectKey(), taskInfo.getResource().getKey()); @@ -543,18 +613,14 @@ public abstract class AbstractPropagationTaskExecutor implements PropagationTask beforeObj = taskInfo.getBeforeObj().get(); } - for (PropagationActions action : actions) { - action.before(taskInfo); - } - switch (taskInfo.getOperation()) { case CREATE: case UPDATE: - uid = createOrUpdate(taskInfo, fetchRemoteObj, beforeObj, connector, propagationAttempted); + uid = createOrUpdate(taskInfo, fetchRemoteObj, beforeObj, connector, propagationAttempted, actions); break; case DELETE: - uid = delete(taskInfo, fetchRemoteObj, beforeObj, connector, propagationAttempted); + uid = delete(taskInfo, fetchRemoteObj, beforeObj, connector, propagationAttempted, actions); break; default: diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/PriorityPropagationTaskExecutor.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/PriorityPropagationTaskExecutor.java index cd8c6aae38..7313d1a1c5 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/PriorityPropagationTaskExecutor.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/PriorityPropagationTaskExecutor.java @@ -33,6 +33,7 @@ import org.apache.syncope.core.persistence.api.dao.AnyObjectDAO; import org.apache.syncope.core.persistence.api.dao.ExternalResourceDAO; import org.apache.syncope.core.persistence.api.dao.GroupDAO; import org.apache.syncope.core.persistence.api.dao.PlainSchemaDAO; +import org.apache.syncope.core.persistence.api.dao.RealmDAO; import org.apache.syncope.core.persistence.api.dao.TaskDAO; import org.apache.syncope.core.persistence.api.dao.UserDAO; import org.apache.syncope.core.persistence.api.entity.AnyUtilsFactory; @@ -51,6 +52,7 @@ import org.apache.syncope.core.provisioning.api.propagation.PropagationTaskInfo; import org.apache.syncope.core.provisioning.java.pushpull.OutboundMatcher; import org.apache.syncope.core.provisioning.java.utils.ConnObjectUtils; import org.apache.syncope.core.spring.ApplicationContextProvider; +import org.apache.syncope.core.spring.security.PasswordGenerator; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.context.ApplicationEventPublisher; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; @@ -97,6 +99,7 @@ public class PriorityPropagationTaskExecutor extends AbstractPropagationTaskExec final TaskDAO taskDAO, final ExternalResourceDAO resourceDAO, final PlainSchemaDAO plainSchemaDAO, + final RealmDAO realmDAO, final NotificationManager notificationManager, final AuditManager auditManager, final TaskDataBinder taskDataBinder, @@ -105,6 +108,7 @@ public class PriorityPropagationTaskExecutor extends AbstractPropagationTaskExec final OutboundMatcher outboundMatcher, final PlainAttrValidationManager validator, final ApplicationEventPublisher publisher, + final PasswordGenerator passwordGenerator, final ThreadPoolTaskExecutor taskExecutor) { super(connectorManager, @@ -115,6 +119,7 @@ public class PriorityPropagationTaskExecutor extends AbstractPropagationTaskExec taskDAO, resourceDAO, plainSchemaDAO, + realmDAO, notificationManager, auditManager, taskDataBinder, @@ -122,7 +127,8 @@ public class PriorityPropagationTaskExecutor extends AbstractPropagationTaskExec taskUtilsFactory, outboundMatcher, validator, - publisher); + publisher, + passwordGenerator); this.taskExecutor = taskExecutor; } diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java index cdca631417..2c8195e1e4 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/utils/ConnObjectUtils.java @@ -46,7 +46,6 @@ import org.apache.syncope.core.persistence.api.dao.ExternalResourceDAO; import org.apache.syncope.core.persistence.api.dao.RealmDAO; import org.apache.syncope.core.persistence.api.dao.UserDAO; import org.apache.syncope.core.persistence.api.entity.AnyUtilsFactory; -import org.apache.syncope.core.persistence.api.entity.Realm; import org.apache.syncope.core.persistence.api.entity.policy.PasswordPolicy; import org.apache.syncope.core.persistence.api.entity.task.PullTask; import org.apache.syncope.core.persistence.api.entity.user.User; @@ -200,18 +199,19 @@ public class ConnObjectUtils { UserCR userCR = (UserCR) anyCR; List<PasswordPolicy> passwordPolicies = new ArrayList<>(); - Realm realm = realmDAO.findByFullPath(userCR.getRealm()); - if (realm != null) { - realmDAO.findAncestors(realm).stream(). - filter(ancestor -> ancestor.getPasswordPolicy() != null). - forEach(ancestor -> passwordPolicies.add(ancestor.getPasswordPolicy())); - } - + // add resource policies userCR.getResources().stream(). map(resourceDAO::find). filter(r -> r != null && r.getPasswordPolicy() != null). forEach(r -> passwordPolicies.add(r.getPasswordPolicy())); + // add realm policies + Optional.ofNullable(realmDAO.findByFullPath(userCR.getRealm())). + ifPresent(realm -> realmDAO.findAncestors(realm).stream(). + filter(ancestor -> ancestor.getPasswordPolicy() != null + && !passwordPolicies.contains(ancestor.getPasswordPolicy())). + forEach(ancestor -> passwordPolicies.add(ancestor.getPasswordPolicy()))); + userCR.setPassword(passwordGenerator.generate(passwordPolicies)); } diff --git a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/DefaultMappingManagerTest.java b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/DefaultMappingManagerTest.java index e6e6a0cb71..7c1ffc962b 100644 --- a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/DefaultMappingManagerTest.java +++ b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/DefaultMappingManagerTest.java @@ -121,6 +121,8 @@ public class DefaultMappingManagerTest extends AbstractTest { assertNull(AttributeUtil.getPasswordValue(attrs.getRight())); // 3. with no clear-text password but random password generation enabled + // after SYNCOPE-1751 no password is going to be generated by DefaultMappingManager, + // but by AbstractPropagationTaskExecutor ldap.setRandomPwdIfNotProvided(true); ldap = resourceDAO.save(ldap); entityManager().flush(); @@ -133,7 +135,7 @@ public class DefaultMappingManagerTest extends AbstractTest { Boolean.TRUE, ldap, provision); - assertNotEquals(encPassword, SecurityUtil.decrypt(AttributeUtil.getPasswordValue(attrs.getRight()))); + assertNull(AttributeUtil.getPasswordValue(attrs.getRight())); // 4. with no clear-text password and random password generation disabled ldap.setRandomPwdIfNotProvided(false); @@ -217,6 +219,8 @@ public class DefaultMappingManagerTest extends AbstractTest { assertNull(AttributeUtil.getPasswordValue(attrs)); // 4. without account password, no clear-text password but random password generation enabled + // after SYNCOPE-1751 no password is going to be generated by DefaultMappingManager, + // but by AbstractPropagationTaskExecutor ldap.setRandomPwdIfNotProvided(true); ldap = resourceDAO.save(ldap); entityManager().flush(); @@ -228,7 +232,7 @@ public class DefaultMappingManagerTest extends AbstractTest { null, true, provision); - assertNotEquals(encPassword, SecurityUtil.decrypt(AttributeUtil.getPasswordValue(attrs))); + assertNull(AttributeUtil.getPasswordValue(attrs)); // 5. without account password, no clear-text password and random password generation disabled ldap.setRandomPwdIfNotProvided(false); diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultPasswordGenerator.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultPasswordGenerator.java index 44daa2a894..32954abff1 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultPasswordGenerator.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultPasswordGenerator.java @@ -21,11 +21,13 @@ package org.apache.syncope.core.spring.security; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import org.apache.syncope.common.lib.policy.DefaultPasswordRuleConf; import org.apache.syncope.core.persistence.api.entity.ExternalResource; import org.apache.syncope.core.persistence.api.entity.Implementation; +import org.apache.syncope.core.persistence.api.entity.Realm; import org.apache.syncope.core.persistence.api.entity.policy.PasswordPolicy; import org.apache.syncope.core.provisioning.api.rules.PasswordRule; import org.apache.syncope.core.spring.implementation.ImplementationManager; @@ -56,12 +58,16 @@ public class DefaultPasswordGenerator implements PasswordGenerator { @Transactional(readOnly = true) @Override - public String generate(final ExternalResource resource) { + public String generate(final ExternalResource resource, final List<Realm> realms) { List<PasswordPolicy> policies = new ArrayList<>(); - if (resource.getPasswordPolicy() != null) { - policies.add(resource.getPasswordPolicy()); - } + // add resource policy + Optional.ofNullable(resource.getPasswordPolicy()).ifPresent(policies::add); + + // add realm policies + realms.forEach(r -> Optional.ofNullable(r.getPasswordPolicy()). + filter(p -> !policies.contains(p)). + ifPresent(policies::add)); return generate(policies); } diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/PasswordGenerator.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/PasswordGenerator.java index f271036f48..758ea159f4 100644 --- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/PasswordGenerator.java +++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/PasswordGenerator.java @@ -20,11 +20,12 @@ package org.apache.syncope.core.spring.security; import java.util.List; import org.apache.syncope.core.persistence.api.entity.ExternalResource; +import org.apache.syncope.core.persistence.api.entity.Realm; import org.apache.syncope.core.persistence.api.entity.policy.PasswordPolicy; public interface PasswordGenerator { - String generate(ExternalResource resource); + String generate(ExternalResource resource, List<Realm> realms); String generate(List<PasswordPolicy> policies); } diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/LinkedAccountITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/LinkedAccountITCase.java index b257aec88a..d0734369cb 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/LinkedAccountITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/LinkedAccountITCase.java @@ -80,7 +80,7 @@ public class LinkedAccountITCase extends AbstractITCase { // 1. create user with linked account UserCR userCR = UserITCase.getSample( "linkedAccount" + RandomStringUtils.randomNumeric(5) + "@syncope.apache.org"); - String connObjectKeyValue = "uid=" + userCR.getUsername() + ",ou=People,o=isp"; + String connObjectKeyValue = "firstAccountOf" + userCR.getUsername(); String privilege = APPLICATION_SERVICE.read("mightyApp").getPrivileges().get(0).getKey(); LinkedAccountTO account = new LinkedAccountTO.Builder(RESOURCE_NAME_LDAP, connObjectKeyValue).build(); @@ -107,7 +107,7 @@ public class LinkedAccountITCase extends AbstractITCase { assertEquals(user.getPlainAttr("email").get().getValues(), ldapObj.getAttr("mail").get().getValues()); assertEquals("LINKED_SURNAME", ldapObj.getAttr("sn").get().getValues().get(0)); - // 3. remove linked account from user + // 3. update linked account UserUR userUR = new UserUR(); userUR.setKey(user.getKey()); diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java index c6a02e4013..379bb9f0aa 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java @@ -103,6 +103,7 @@ import org.apache.syncope.fit.core.reference.DateToLongItemTransformer; import org.identityconnectors.framework.common.objects.Attribute; import org.identityconnectors.framework.common.objects.AttributeUtil; import org.identityconnectors.framework.common.objects.Name; +import org.identityconnectors.framework.common.objects.OperationalAttributes; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.springframework.dao.DataAccessException; @@ -1008,4 +1009,37 @@ public class PropagationTaskITCase extends AbstractTaskITCase { } } } + + @Test + public void issueSYNCOPE1751() { + // 1. Create a Group with a resource assigned + GroupTO groupTO = + createGroup(new GroupCR.Builder(SyncopeConstants.ROOT_REALM, "SYNCOPEGROUP1751-" + getUUIDString()) + .resource(RESOURCE_NAME_LDAP) + .build()).getEntity(); + // 2. Create a user + String username = "SYNCOPEUSER1750" + getUUIDString(); + UserTO userTO = createUser( + new UserCR.Builder(SyncopeConstants.ROOT_REALM, username) + .plainAttrs(new Attr.Builder("userId").value(username + "@syncope.org").build(), + new Attr.Builder("fullname").value(username).build(), + new Attr.Builder("surname").value(username).build()) + .build()).getEntity(); + // 3. Update the user assigning the group previously created -> group-based provisioning + userTO = updateUser( + new UserUR.Builder(userTO.getKey()).membership(new MembershipUR.Builder(groupTO.getKey()).build()) + .build()).getEntity(); + // since the resource is flagged to generate random pwd must populate the password on effective create on the + // resource, even if it is an update on Syncope + PagedResult<TaskTO> propTasks = TASK_SERVICE.search( + new TaskQuery.Builder(TaskType.PROPAGATION).resource(RESOURCE_NAME_LDAP).anyTypeKind(AnyTypeKind.USER) + .entityKey(userTO.getKey()).build()); + assertFalse(propTasks.getResult().isEmpty()); + assertEquals(1, propTasks.getSize()); + PropagationData propagationData = + POJOHelper.deserialize(PropagationTaskTO.class.cast(propTasks.getResult().get(0)).getPropagationData(), + PropagationData.class); + assertTrue(propagationData.getAttributes().stream() + .anyMatch(a -> OperationalAttributes.PASSWORD_NAME.equals(a.getName()))); + } }