Skip to content

Commit 9f3d86b

Browse files
ilgrossoilgrosso
committed
[SYNCOPE-1988] Reviewing ConnInstanceTO and ResourceTO generation to strip out confidential property values (#1491)
1 parent ee5dff7 commit 9f3d86b

19 files changed

Lines changed: 161 additions & 140 deletions

File tree

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

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.syncope.common.lib.to.PlainSchemaTO;
3535
import org.apache.syncope.common.lib.types.AttrSchemaType;
3636
import org.apache.syncope.common.lib.types.ClientExceptionType;
37+
import org.apache.syncope.common.lib.types.ConnConfProperty;
3738
import org.apache.syncope.common.lib.types.IdMEntitlement;
3839
import org.apache.syncope.core.persistence.api.dao.ConnInstanceDAO;
3940
import org.apache.syncope.core.persistence.api.dao.ExternalResourceDAO;
@@ -109,7 +110,7 @@ public ConnInstanceTO create(final ConnInstanceTO connInstanceTO) {
109110
connInstanceTO.getAdminRealm());
110111
securityChecks(effectiveRealms, connInstanceTO.getAdminRealm(), null);
111112

112-
return binder.getConnInstanceTO(doSave(binder.getConnInstance(connInstanceTO)));
113+
return binder.getConnInstanceTO(doSave(binder.create(connInstanceTO)));
113114
}
114115

115116
@PreAuthorize("hasRole('" + IdMEntitlement.CONNECTOR_UPDATE + "')")
@@ -163,10 +164,8 @@ public List<ConnInstanceTO> list(final String lang) {
163164
public ConnInstanceTO read(final String key, final String lang) {
164165
CurrentLocale.set(StringUtils.isBlank(lang) ? Locale.ENGLISH : Locale.of(lang));
165166

166-
ConnInstance connInstance = connInstanceDAO.authFind(key);
167-
if (connInstance == null) {
168-
throw new NotFoundException("Connector '" + key + '\'');
169-
}
167+
ConnInstance connInstance = Optional.ofNullable(connInstanceDAO.authFind(key)).
168+
orElseThrow(() -> new NotFoundException("Connector '" + key + '\''));
170169

171170
return binder.getConnInstanceTO(connInstance);
172171
}
@@ -195,7 +194,7 @@ public List<ConnIdBundle> getBundles(final String lang) {
195194

196195
ConfigurationProperties properties = connIdBundleManager.getConfigurationProperties(bundle);
197196
connBundleTO.getProperties().addAll(properties.getPropertyNames().stream().
198-
map(propName -> binder.build(properties.getProperty(propName))).
197+
map(propName -> ConnInstanceDataBinder.build(properties.getProperty(propName))).
199198
toList());
200199

201200
return connBundleTO;
@@ -208,13 +207,12 @@ public List<ConnIdBundle> getBundles(final String lang) {
208207
public List<ConnIdObjectClass> buildObjectClassInfo(
209208
final ConnInstanceTO connInstanceTO, final boolean includeSpecial) {
210209

211-
ConnInstanceTO actual = connInstanceDAO.findById(connInstanceTO.getKey()).
212-
map(binder::getConnInstanceTO).
213-
orElse(connInstanceTO);
210+
ConnInstance connInstance = Optional.ofNullable(connInstanceDAO.authFind(connInstanceTO.getKey())).
211+
orElseThrow(() -> new NotFoundException("Connector '" + connInstanceTO.getKey() + '\''));
214212

215213
Set<ObjectClassInfo> objectClassInfo = connectorManager.createConnector(
216214
connectorManager.buildConnInstanceOverride(
217-
actual, Optional.of(connInstanceTO.getConf()), Optional.empty())).
215+
connInstance, Optional.of(connInstanceTO.getConf()), Optional.empty())).
218216
getObjectClassInfo();
219217

220218
return objectClassInfo.stream().map(info -> {
@@ -244,24 +242,22 @@ public List<ConnIdObjectClass> buildObjectClassInfo(
244242
+ "or hasRole('" + IdMEntitlement.CONNECTOR_UPDATE + "'))")
245243
@Transactional(readOnly = true)
246244
public void check(final ConnInstanceTO connInstanceTO) {
247-
if (connInstanceTO.getAdminRealm() == null) {
248-
throw SyncopeClientException.build(ClientExceptionType.InvalidRealm);
249-
}
245+
Optional.ofNullable(connInstanceTO.getKey()).flatMap(connInstanceDAO::findById).ifPresent(connInstance -> {
246+
List<ConnConfProperty> newConf =
247+
ConnInstanceDataBinder.newConf(connInstance.getConf(), connInstanceTO.getConf());
248+
connInstanceTO.getConf().clear();
249+
connInstanceTO.getConf().addAll(newConf);
250+
});
250251

251-
connectorManager.createConnector(binder.getConnInstance(connInstanceTO)).test();
252+
connectorManager.createConnector(binder.create(connInstanceTO)).test();
252253
}
253254

254255
@PreAuthorize("hasRole('" + IdMEntitlement.CONNECTOR_READ + "')")
255256
@Transactional(readOnly = true)
256257
public ConnInstanceTO readByResource(final String resourceName, final String lang) {
257-
CurrentLocale.set(StringUtils.isBlank(lang) ? Locale.ENGLISH : Locale.of(lang));
258-
259258
ExternalResource resource = resourceDAO.findById(resourceName).
260259
orElseThrow(() -> new NotFoundException("Resource " + resourceName));
261-
ConnInstanceTO connInstance = binder.getConnInstanceTO(
262-
connectorManager.getConnector(resource).getConnInstance());
263-
connInstance.setKey(resource.getConnector().getKey());
264-
return connInstance;
260+
return read(resource.getConnector().getKey(), lang);
265261
}
266262

267263
@PreAuthorize("hasRole('" + IdMEntitlement.CONNECTOR_RELOAD + "')")

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ public ResourceLogic resourceLogic(
121121
final AnyTypeDAO anyTypeDAO,
122122
final ExternalResourceDAO resourceDAO,
123123
final ConnInstanceDAO connInstanceDAO,
124-
final ConnInstanceDataBinder connInstanceDataBinder,
125124
final ConnectorManager connectorManager,
126125
final OutboundMatcher outboundMatcher,
127126
final MappingManager mappingManager) {
@@ -131,7 +130,6 @@ public ResourceLogic resourceLogic(
131130
anyTypeDAO,
132131
connInstanceDAO,
133132
resourceDataBinder,
134-
connInstanceDataBinder,
135133
outboundMatcher,
136134
mappingManager,
137135
connectorManager,

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import org.apache.syncope.core.provisioning.api.Connector;
4949
import org.apache.syncope.core.provisioning.api.ConnectorManager;
5050
import org.apache.syncope.core.provisioning.api.MappingManager;
51-
import org.apache.syncope.core.provisioning.api.data.ConnInstanceDataBinder;
5251
import org.apache.syncope.core.provisioning.api.data.ResourceDataBinder;
5352
import org.apache.syncope.core.provisioning.java.pushpull.OutboundMatcher;
5453
import org.apache.syncope.core.provisioning.java.utils.ConnObjectUtils;
@@ -75,8 +74,6 @@ public class ResourceLogic extends AbstractTransactionalLogic<ResourceTO> {
7574

7675
protected final ResourceDataBinder binder;
7776

78-
protected final ConnInstanceDataBinder connInstanceDataBinder;
79-
8077
protected final OutboundMatcher outboundMatcher;
8178

8279
protected final MappingManager mappingManager;
@@ -90,7 +87,6 @@ public ResourceLogic(
9087
final AnyTypeDAO anyTypeDAO,
9188
final ConnInstanceDAO connInstanceDAO,
9289
final ResourceDataBinder binder,
93-
final ConnInstanceDataBinder connInstanceDataBinder,
9490
final OutboundMatcher outboundMatcher,
9591
final MappingManager mappingManager,
9692
final ConnectorManager connectorManager,
@@ -100,7 +96,6 @@ public ResourceLogic(
10096
this.anyTypeDAO = anyTypeDAO;
10197
this.connInstanceDAO = connInstanceDAO;
10298
this.binder = binder;
103-
this.connInstanceDataBinder = connInstanceDataBinder;
10499
this.outboundMatcher = outboundMatcher;
105100
this.mappingManager = mappingManager;
106101
this.connectorManager = connectorManager;
@@ -436,7 +431,7 @@ public void check(final ResourceTO resourceTO) {
436431

437432
connectorManager.createConnector(
438433
connectorManager.buildConnInstanceOverride(
439-
connInstanceDataBinder.getConnInstanceTO(connInstance),
434+
connInstance,
440435
resourceTO.getConfOverride(),
441436
resourceTO.getCapabilitiesOverride())).
442437
test();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ public void setLatestSyncToken() {
211211
assertNull(resource.getProvision(AnyTypeKind.USER.name()).orElseThrow().getSyncToken());
212212

213213
ResourceLogic resourceLogic = new ResourceLogic(
214-
resourceDAO, anyTypeDAO, null, null, null, null, null, connectorManager, null);
214+
resourceDAO, anyTypeDAO, null, null, null, null, connectorManager, null);
215215

216216
resourceLogic.setLatestSyncToken(resource.getKey(), AnyTypeKind.USER.name());
217217
entityManager.flush();

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.List;
2222
import java.util.Optional;
2323
import java.util.Set;
24-
import org.apache.syncope.common.lib.to.ConnInstanceTO;
2524
import org.apache.syncope.common.lib.types.ConnConfProperty;
2625
import org.apache.syncope.common.lib.types.ConnectorCapability;
2726
import org.apache.syncope.core.persistence.api.entity.ConnInstance;
@@ -41,7 +40,7 @@ public void unregisterConnector(final ExternalResource resource) {
4140

4241
@Override
4342
public ConnInstance buildConnInstanceOverride(
44-
final ConnInstanceTO connInstance,
43+
final ConnInstance connInstance,
4544
final Optional<List<ConnConfProperty>> confOverride,
4645
final Optional<Set<ConnectorCapability>> capabilitiesOverride) {
4746

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.List;
2222
import java.util.Optional;
2323
import java.util.Set;
24-
import org.apache.syncope.common.lib.to.ConnInstanceTO;
2524
import org.apache.syncope.common.lib.types.ConnConfProperty;
2625
import org.apache.syncope.common.lib.types.ConnectorCapability;
2726
import org.apache.syncope.core.persistence.api.entity.ConnInstance;
@@ -41,7 +40,7 @@ public void unregisterConnector(final ExternalResource resource) {
4140

4241
@Override
4342
public ConnInstance buildConnInstanceOverride(
44-
final ConnInstanceTO connInstance,
43+
final ConnInstance connInstance,
4544
final Optional<List<ConnConfProperty>> confOverride,
4645
final Optional<Set<ConnectorCapability>> capabilitiesOverride) {
4746

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.List;
2222
import java.util.Optional;
2323
import java.util.Set;
24-
import org.apache.syncope.common.lib.to.ConnInstanceTO;
2524
import org.apache.syncope.common.lib.types.ConnConfProperty;
2625
import org.apache.syncope.common.lib.types.ConnectorCapability;
2726
import org.apache.syncope.core.persistence.api.entity.ConnInstance;
@@ -43,7 +42,7 @@ public interface ConnectorManager {
4342
* @return connector instance override over base connector instance, configuration and capabilities
4443
*/
4544
ConnInstance buildConnInstanceOverride(
46-
ConnInstanceTO connInstance,
45+
ConnInstance connInstance,
4746
Optional<List<ConnConfProperty>> confOverride,
4847
Optional<Set<ConnectorCapability>> capabilitiesOverride);
4948

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

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,81 @@
1818
*/
1919
package org.apache.syncope.core.provisioning.api.data;
2020

21+
import java.util.ArrayList;
22+
import java.util.Collection;
23+
import java.util.List;
2124
import org.apache.syncope.common.lib.to.ConnInstanceTO;
2225
import org.apache.syncope.common.lib.types.ConnConfPropSchema;
26+
import org.apache.syncope.common.lib.types.ConnConfProperty;
2327
import org.apache.syncope.core.persistence.api.entity.ConnInstance;
28+
import org.identityconnectors.common.security.GuardedString;
2429
import org.identityconnectors.framework.api.ConfigurationProperty;
30+
import org.identityconnectors.framework.impl.api.ConfigurationPropertyImpl;
2531

2632
public interface ConnInstanceDataBinder {
2733

28-
ConnConfPropSchema build(ConfigurationProperty property);
34+
static ConnConfPropSchema build(final ConfigurationProperty property) {
35+
ConnConfPropSchema connConfPropSchema = new ConnConfPropSchema();
2936

30-
ConnInstance getConnInstance(ConnInstanceTO connInstanceTO);
37+
connConfPropSchema.setName(property.getName());
38+
connConfPropSchema.setDisplayName(property.getDisplayName(property.getName()));
39+
connConfPropSchema.setHelpMessage(property.getHelpMessage(property.getName()));
40+
connConfPropSchema.setRequired(property.isRequired());
41+
connConfPropSchema.setType(property.getType().getName());
42+
connConfPropSchema.setOrder(((ConfigurationPropertyImpl) property).getOrder());
43+
connConfPropSchema.setConfidential(property.isConfidential());
44+
45+
if (property.getValue() != null) {
46+
if (property.getValue().getClass().isArray()) {
47+
connConfPropSchema.getDefaultValues().addAll(List.of((Object[]) property.getValue()));
48+
} else if (property.getValue() instanceof Collection<?> collection) {
49+
connConfPropSchema.getDefaultValues().addAll(collection);
50+
} else {
51+
connConfPropSchema.getDefaultValues().add(property.getValue());
52+
}
53+
}
54+
55+
return connConfPropSchema;
56+
}
57+
58+
static List<ConnConfProperty> newConf(
59+
final List<ConnConfProperty> previousConf,
60+
final List<ConnConfProperty> toConf) {
61+
62+
List<ConnConfProperty> newConf = new ArrayList<>();
63+
toConf.forEach(property -> {
64+
if (property.getSchema().isConfidential()
65+
|| GuardedString.class.getName().equals(property.getSchema().getType())) {
66+
67+
if (property.getValues().isEmpty()) {
68+
// no values provided, keep existing
69+
previousConf.stream().
70+
filter(p -> p.getSchema().getName().equals(property.getSchema().getName())).
71+
findFirst().ifPresent(newConf::add);
72+
} else {
73+
// translate confidential properties' cleartext values into GuardedStrings
74+
ConnConfProperty newProperty = new ConnConfProperty();
75+
newProperty.setSchema(property.getSchema());
76+
newProperty.setOverridable(property.isOverridable());
77+
property.getValues().forEach(value -> {
78+
if (value instanceof String string) {
79+
newProperty.getValues().add(new GuardedString(string.toCharArray()));
80+
} else {
81+
newProperty.getValues().add(value);
82+
}
83+
});
84+
}
85+
}
86+
87+
newConf.add(property);
88+
});
89+
90+
return newConf;
91+
}
3192

3293
ConnInstanceTO getConnInstanceTO(ConnInstance connInstance);
3394

95+
ConnInstance create(ConnInstanceTO connInstanceTO);
96+
3497
ConnInstance update(ConnInstanceTO connInstanceTO);
3598
}

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import org.apache.syncope.core.provisioning.api.Connector;
3535
import org.apache.syncope.core.provisioning.api.TimeoutException;
3636
import org.apache.syncope.core.provisioning.api.pushpull.ReconFilterBuilder;
37-
import org.identityconnectors.common.CollectionUtil;
3837
import org.identityconnectors.common.security.GuardedByteArray;
3938
import org.identityconnectors.common.security.GuardedString;
4039
import org.identityconnectors.framework.api.APIConfiguration;
@@ -107,7 +106,7 @@ public ConnectorFacadeProxy(final ConnInstance connInstance, final AsyncConnecto
107106
// set connector configuration according to conninstance's
108107
ConfigurationProperties properties = apiConfig.getConfigurationProperties();
109108
connInstance.getConf().stream().
110-
filter(property -> !CollectionUtil.isEmpty(property.getValues())).
109+
filter(property -> !property.getValues().isEmpty()).
111110
forEach(property -> properties.setPropertyValue(
112111
property.getSchema().getName(),
113112
getPropertyValue(property.getSchema().getType(), property.getValues())));
@@ -521,9 +520,15 @@ private static Object getPropertyValue(final String propType, final List<?> valu
521520
Class<?> propertySchemaClass = ClassUtils.forName(propType, ClassUtils.getDefaultClassLoader());
522521

523522
if (GuardedString.class.equals(propertySchemaClass)) {
524-
value = new GuardedString(values.getFirst().toString().toCharArray());
523+
if (values.getFirst() instanceof GuardedString) {
524+
value = values.getFirst();
525+
} else {
526+
value = new GuardedString(values.getFirst().toString().toCharArray());
527+
}
525528
} else if (GuardedByteArray.class.equals(propertySchemaClass)) {
526-
value = new GuardedByteArray((byte[]) values.getFirst());
529+
if (values.getFirst() instanceof byte[] byteArray) {
530+
value = new GuardedByteArray(byteArray);
531+
}
527532
} else if (Character.class.equals(propertySchemaClass) || Character.TYPE.equals(propertySchemaClass)) {
528533
value = values.getFirst() == null || values.getFirst().toString().isEmpty()
529534
? null : values.getFirst().toString().charAt(0);

0 commit comments

Comments
 (0)