[SYNCOPE-1988] Reviewing ConnInstanceTO and ResourceTO generation to strip out confidential property values

core/idm/logic/src/main/java/org/apache/syncope/core/logic/ConnectorLogic.java

@@ -34,6 +34,7 @@
 import org.apache.syncope.common.lib.to.PlainSchemaTO;
 import org.apache.syncope.common.lib.types.AttrSchemaType;
 import org.apache.syncope.common.lib.types.ClientExceptionType;
+import org.apache.syncope.common.lib.types.ConnConfProperty;
 import org.apache.syncope.common.lib.types.IdMEntitlement;
 import org.apache.syncope.core.persistence.api.dao.ConnInstanceDAO;
 import org.apache.syncope.core.persistence.api.dao.ExternalResourceDAO;
@@ -109,7 +110,7 @@ public ConnInstanceTO create(final ConnInstanceTO connInstanceTO) {
                 connInstanceTO.getAdminRealm());
         securityChecks(effectiveRealms, connInstanceTO.getAdminRealm(), null);
 
-        return binder.getConnInstanceTO(doSave(binder.getConnInstance(connInstanceTO)));
+        return binder.getConnInstanceTO(doSave(binder.create(connInstanceTO)));
     }
 
     @PreAuthorize("hasRole('" + IdMEntitlement.CONNECTOR_UPDATE + "')")
@@ -163,10 +164,8 @@ public List<ConnInstanceTO> list(final String lang) {
     public ConnInstanceTO read(final String key, final String lang) {
         CurrentLocale.set(StringUtils.isBlank(lang) ? Locale.ENGLISH : Locale.of(lang));
 
-        ConnInstance connInstance = connInstanceDAO.authFind(key);
-        if (connInstance == null) {
-            throw new NotFoundException("Connector '" + key + '\'');
-        }
+        ConnInstance connInstance = Optional.ofNullable(connInstanceDAO.authFind(key)).
+                orElseThrow(() -> new NotFoundException("Connector '" + key + '\''));
 
         return binder.getConnInstanceTO(connInstance);
     }
@@ -195,7 +194,7 @@ public List<ConnIdBundle> getBundles(final String lang) {
 
                     ConfigurationProperties properties = connIdBundleManager.getConfigurationProperties(bundle);
                     connBundleTO.getProperties().addAll(properties.getPropertyNames().stream().
-                            map(propName -> binder.build(properties.getProperty(propName))).
+                            map(propName -> ConnInstanceDataBinder.build(properties.getProperty(propName))).
                             toList());
 
                     return connBundleTO;
@@ -208,13 +207,12 @@ public List<ConnIdBundle> getBundles(final String lang) {
     public List<ConnIdObjectClass> buildObjectClassInfo(
             final ConnInstanceTO connInstanceTO, final boolean includeSpecial) {
 
-        ConnInstanceTO actual = connInstanceDAO.findById(connInstanceTO.getKey()).
-                map(binder::getConnInstanceTO).
-                orElse(connInstanceTO);
+        ConnInstance connInstance = Optional.ofNullable(connInstanceDAO.authFind(connInstanceTO.getKey())).
+                orElseThrow(() -> new NotFoundException("Connector '" + connInstanceTO.getKey() + '\''));
 
         Set<ObjectClassInfo> objectClassInfo = connectorManager.createConnector(
                 connectorManager.buildConnInstanceOverride(
-                        actual, Optional.of(connInstanceTO.getConf()), Optional.empty())).
+                        connInstance, Optional.of(connInstanceTO.getConf()), Optional.empty())).
                 getObjectClassInfo();
 
         return objectClassInfo.stream().map(info -> {
@@ -244,24 +242,22 @@ public List<ConnIdObjectClass> buildObjectClassInfo(
             + "or hasRole('" + IdMEntitlement.CONNECTOR_UPDATE + "'))")
     @Transactional(readOnly = true)
     public void check(final ConnInstanceTO connInstanceTO) {
-        if (connInstanceTO.getAdminRealm() == null) {
-            throw SyncopeClientException.build(ClientExceptionType.InvalidRealm);
-        }
+        Optional.ofNullable(connInstanceTO.getKey()).flatMap(connInstanceDAO::findById).ifPresent(connInstance -> {
+            List<ConnConfProperty> newConf =
+                    ConnInstanceDataBinder.newConf(connInstance.getConf(), connInstanceTO.getConf());
+            connInstanceTO.getConf().clear();
+            connInstanceTO.getConf().addAll(newConf);
+        });
 
-        connectorManager.createConnector(binder.getConnInstance(connInstanceTO)).test();
+        connectorManager.createConnector(binder.create(connInstanceTO)).test();
     }
 
     @PreAuthorize("hasRole('" + IdMEntitlement.CONNECTOR_READ + "')")
     @Transactional(readOnly = true)
     public ConnInstanceTO readByResource(final String resourceName, final String lang) {
-        CurrentLocale.set(StringUtils.isBlank(lang) ? Locale.ENGLISH : Locale.of(lang));
-
         ExternalResource resource = resourceDAO.findById(resourceName).
                 orElseThrow(() -> new NotFoundException("Resource " + resourceName));
-        ConnInstanceTO connInstance = binder.getConnInstanceTO(
-                connectorManager.getConnector(resource).getConnInstance());
-        connInstance.setKey(resource.getConnector().getKey());
-        return connInstance;
+        return read(resource.getConnector().getKey(), lang);
     }
 
     @PreAuthorize("hasRole('" + IdMEntitlement.CONNECTOR_RELOAD + "')")

core/idm/logic/src/main/java/org/apache/syncope/core/logic/IdMLogicContext.java

@@ -124,7 +124,6 @@ public ResourceLogic resourceLogic(
             final AnyTypeDAO anyTypeDAO,
             final ExternalResourceDAO resourceDAO,
             final ConnInstanceDAO connInstanceDAO,
-            final ConnInstanceDataBinder connInstanceDataBinder,
             final ConnectorManager connectorManager,
             final OutboundMatcher outboundMatcher,
             final MappingManager mappingManager) {
@@ -134,7 +133,6 @@ public ResourceLogic resourceLogic(
                 anyTypeDAO,
                 connInstanceDAO,
                 resourceDataBinder,
-                connInstanceDataBinder,
                 outboundMatcher,
                 mappingManager,
                 connectorManager,

core/idm/logic/src/main/java/org/apache/syncope/core/logic/ResourceLogic.java

@@ -48,7 +48,6 @@
 import org.apache.syncope.core.provisioning.api.Connector;
 import org.apache.syncope.core.provisioning.api.ConnectorManager;
 import org.apache.syncope.core.provisioning.api.MappingManager;
-import org.apache.syncope.core.provisioning.api.data.ConnInstanceDataBinder;
 import org.apache.syncope.core.provisioning.api.data.ResourceDataBinder;
 import org.apache.syncope.core.provisioning.java.pushpull.OutboundMatcher;
 import org.apache.syncope.core.provisioning.java.utils.ConnObjectUtils;
@@ -75,8 +74,6 @@ public class ResourceLogic extends AbstractTransactionalLogic<ResourceTO> {
 
     protected final ResourceDataBinder binder;
 
-    protected final ConnInstanceDataBinder connInstanceDataBinder;
-
     protected final OutboundMatcher outboundMatcher;
 
     protected final MappingManager mappingManager;
@@ -90,7 +87,6 @@ public ResourceLogic(
             final AnyTypeDAO anyTypeDAO,
             final ConnInstanceDAO connInstanceDAO,
             final ResourceDataBinder binder,
-            final ConnInstanceDataBinder connInstanceDataBinder,
             final OutboundMatcher outboundMatcher,
             final MappingManager mappingManager,
             final ConnectorManager connectorManager,
@@ -100,7 +96,6 @@ public ResourceLogic(
         this.anyTypeDAO = anyTypeDAO;
         this.connInstanceDAO = connInstanceDAO;
         this.binder = binder;
-        this.connInstanceDataBinder = connInstanceDataBinder;
         this.outboundMatcher = outboundMatcher;
         this.mappingManager = mappingManager;
         this.connectorManager = connectorManager;
@@ -430,7 +425,7 @@ public void check(final ResourceTO resourceTO) {
 
         connectorManager.createConnector(
                 connectorManager.buildConnInstanceOverride(
-                        connInstanceDataBinder.getConnInstanceTO(connInstance),
+                        connInstance,
                         resourceTO.getConfOverride(),
                         resourceTO.getCapabilitiesOverride())).
                 test();

core/idm/logic/src/test/java/org/apache/syncope/core/logic/ResourceLogicTest.java

@@ -211,7 +211,7 @@ public void setLatestSyncToken() {
         assertNull(resource.getProvision(AnyTypeKind.USER.name()).orElseThrow().getSyncToken());
 
         ResourceLogic resourceLogic = new ResourceLogic(
-                resourceDAO, anyTypeDAO, null, null, null, null, null, connectorManager, null);
+                resourceDAO, anyTypeDAO, null, null, null, null, connectorManager, null);
 
         resourceLogic.setLatestSyncToken(resource.getKey(), AnyTypeKind.USER.name());
         entityManager.flush();

core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/DummyConnectorManager.java

@@ -21,7 +21,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import org.apache.syncope.common.lib.to.ConnInstanceTO;
 import org.apache.syncope.common.lib.types.ConnConfProperty;
 import org.apache.syncope.common.lib.types.ConnectorCapability;
 import org.apache.syncope.core.persistence.api.entity.ConnInstance;
@@ -41,7 +40,7 @@ public void unregisterConnector(final ExternalResource resource) {
 
     @Override
     public ConnInstance buildConnInstanceOverride(
-            final ConnInstanceTO connInstance,
+            final ConnInstance connInstance,
             final Optional<List<ConnConfProperty>> confOverride,
             final Optional<Set<ConnectorCapability>> capabilitiesOverride) {
 

core/persistence-neo4j/src/test/java/org/apache/syncope/core/persistence/neo4j/DummyConnectorManager.java

@@ -21,7 +21,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import org.apache.syncope.common.lib.to.ConnInstanceTO;
 import org.apache.syncope.common.lib.types.ConnConfProperty;
 import org.apache.syncope.common.lib.types.ConnectorCapability;
 import org.apache.syncope.core.persistence.api.entity.ConnInstance;
@@ -41,7 +40,7 @@ public void unregisterConnector(final ExternalResource resource) {
 
     @Override
     public ConnInstance buildConnInstanceOverride(
-            final ConnInstanceTO connInstance,
+            final ConnInstance connInstance,
             final Optional<List<ConnConfProperty>> confOverride,
             final Optional<Set<ConnectorCapability>> capabilitiesOverride) {
 

core/provisioning-api/src/main/java/org/apache/syncope/core/provisioning/api/ConnectorManager.java

@@ -21,7 +21,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import org.apache.syncope.common.lib.to.ConnInstanceTO;
 import org.apache.syncope.common.lib.types.ConnConfProperty;
 import org.apache.syncope.common.lib.types.ConnectorCapability;
 import org.apache.syncope.core.persistence.api.entity.ConnInstance;
@@ -43,7 +42,7 @@ public interface ConnectorManager {
      * @return connector instance override over base connector instance, configuration and capabilities
      */
     ConnInstance buildConnInstanceOverride(
-            ConnInstanceTO connInstance,
+            ConnInstance connInstance,
             Optional<List<ConnConfProperty>> confOverride,
             Optional<Set<ConnectorCapability>> capabilitiesOverride);
 

core/provisioning-api/src/main/java/org/apache/syncope/core/provisioning/api/data/ConnInstanceDataBinder.java

@@ -18,18 +18,81 @@
  */
 package org.apache.syncope.core.provisioning.api.data;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
 import org.apache.syncope.common.lib.to.ConnInstanceTO;
 import org.apache.syncope.common.lib.types.ConnConfPropSchema;
+import org.apache.syncope.common.lib.types.ConnConfProperty;
 import org.apache.syncope.core.persistence.api.entity.ConnInstance;
+import org.identityconnectors.common.security.GuardedString;
 import org.identityconnectors.framework.api.ConfigurationProperty;
+import org.identityconnectors.framework.impl.api.ConfigurationPropertyImpl;
 
 public interface ConnInstanceDataBinder {
 
-    ConnConfPropSchema build(ConfigurationProperty property);
+    static ConnConfPropSchema build(final ConfigurationProperty property) {
+        ConnConfPropSchema connConfPropSchema = new ConnConfPropSchema();
 
-    ConnInstance getConnInstance(ConnInstanceTO connInstanceTO);
+        connConfPropSchema.setName(property.getName());
+        connConfPropSchema.setDisplayName(property.getDisplayName(property.getName()));
+        connConfPropSchema.setHelpMessage(property.getHelpMessage(property.getName()));
+        connConfPropSchema.setRequired(property.isRequired());
+        connConfPropSchema.setType(property.getType().getName());
+        connConfPropSchema.setOrder(((ConfigurationPropertyImpl) property).getOrder());
+        connConfPropSchema.setConfidential(property.isConfidential());
+
+        if (property.getValue() != null) {
+            if (property.getValue().getClass().isArray()) {
+                connConfPropSchema.getDefaultValues().addAll(List.of((Object[]) property.getValue()));
+            } else if (property.getValue() instanceof Collection<?> collection) {
+                connConfPropSchema.getDefaultValues().addAll(collection);
+            } else {
+                connConfPropSchema.getDefaultValues().add(property.getValue());
+            }
+        }
+
+        return connConfPropSchema;
+    }
+
+    static List<ConnConfProperty> newConf(
+            final List<ConnConfProperty> previousConf,
+            final List<ConnConfProperty> toConf) {
+
+        List<ConnConfProperty> newConf = new ArrayList<>();
+        toConf.forEach(property -> {
+            if (property.getSchema().isConfidential()
+                    || GuardedString.class.getName().equals(property.getSchema().getType())) {
+
+                if (property.getValues().isEmpty()) {
+                    // no values provided, keep existing
+                    previousConf.stream().
+                            filter(p -> p.getSchema().getName().equals(property.getSchema().getName())).
+                            findFirst().ifPresent(newConf::add);
+                } else {
+                    // translate confidential properties' cleartext values into GuardedStrings
+                    ConnConfProperty newProperty = new ConnConfProperty();
+                    newProperty.setSchema(property.getSchema());
+                    newProperty.setOverridable(property.isOverridable());
+                    property.getValues().forEach(value -> {
+                        if (value instanceof String string) {
+                            newProperty.getValues().add(new GuardedString(string.toCharArray()));
+                        } else {
+                            newProperty.getValues().add(value);
+                        }
+                    });
+                }
+            }
+
+            newConf.add(property);
+        });
+
+        return newConf;
+    }
 
     ConnInstanceTO getConnInstanceTO(ConnInstance connInstance);
 
+    ConnInstance create(ConnInstanceTO connInstanceTO);
+
     ConnInstance update(ConnInstanceTO connInstanceTO);
 }

core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ConnectorFacadeProxy.java

@@ -32,7 +32,6 @@
 import org.apache.syncope.core.provisioning.api.Connector;
 import org.apache.syncope.core.provisioning.api.TimeoutException;
 import org.apache.syncope.core.provisioning.api.pushpull.ReconFilterBuilder;
-import org.identityconnectors.common.CollectionUtil;
 import org.identityconnectors.common.security.GuardedByteArray;
 import org.identityconnectors.common.security.GuardedString;
 import org.identityconnectors.framework.api.APIConfiguration;
@@ -95,7 +94,7 @@ public ConnectorFacadeProxy(
         // set connector configuration according to conninstance's
         ConfigurationProperties properties = apiConfig.getConfigurationProperties();
         connInstance.getConf().stream().
-                filter(property -> !CollectionUtil.isEmpty(property.getValues())).
+                filter(property -> !property.getValues().isEmpty()).
                 forEach(property -> properties.setPropertyValue(
                 property.getSchema().getName(),
                 getPropertyValue(property.getSchema().getType(), property.getValues())));
@@ -509,9 +508,15 @@ private static Object getPropertyValue(final String propType, final List<?> valu
             Class<?> propertySchemaClass = ClassUtils.forName(propType, ClassUtils.getDefaultClassLoader());
 
             if (GuardedString.class.equals(propertySchemaClass)) {
-                value = new GuardedString(values.getFirst().toString().toCharArray());
+                if (values.getFirst() instanceof GuardedString) {
+                    value = values.getFirst();
+                } else {
+                    value = new GuardedString(values.getFirst().toString().toCharArray());
+                }
             } else if (GuardedByteArray.class.equals(propertySchemaClass)) {
-                value = new GuardedByteArray((byte[]) values.getFirst());
+                if (values.getFirst() instanceof byte[] byteArray) {
+                    value = new GuardedByteArray(byteArray);
+                }
             } else if (Character.class.equals(propertySchemaClass) || Character.TYPE.equals(propertySchemaClass)) {
                 value = values.getFirst() == null || values.getFirst().toString().isEmpty()
                         ? null : values.getFirst().toString().charAt(0);