This is an automated email from the ASF dual-hosted git repository.
ilgrosso pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/master by this push:
new f9e4c8da6b [SYNCOPE-1697] Improving input and validation of
CommandArgs instances (#395)
f9e4c8da6b is described below
commit f9e4c8da6ba71d316b780dec9a285deaffc85b5e
Author: Francesco Chicchiriccò <ilgro...@users.noreply.github.com>
AuthorDate: Tue Nov 29 15:16:34 2022 +0100
[SYNCOPE-1697] Improving input and validation of CommandArgs instances
(#395)
---
.../syncope/client/console/panels/BeanPanel.java | 128 ++++++++++++++++-----
.../syncope/client/console/panels/BeanPanel.html | 14 ++-
.../common/lib/types/EntityViolationType.java | 72 +++++++-----
.../apache/syncope/core/logic/CommandLogic.java | 25 +++-
.../syncope/core/logic/IdRepoLogicContext.java | 5 +-
.../core/logic/job/MacroRunJobDelegate.java | 17 ++-
.../validation/InvalidEntityException.java | 27 ++---
.../fit/core/reference/TestCommandArgs.java | 15 ++-
.../org/apache/syncope/fit/core/CommandITCase.java | 17 +++
.../org/apache/syncope/fit/core/MacroITCase.java | 16 ++-
.../apache/syncope/fit/core/MembershipITCase.java | 3 +-
pom.xml | 2 +-
12 files changed, 248 insertions(+), 93 deletions(-)
diff --git
a/client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/BeanPanel.java
b/client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/BeanPanel.java
index 6e6d5b028a..bb945460b5 100644
---
a/client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/BeanPanel.java
+++
b/client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/BeanPanel.java
@@ -18,6 +18,10 @@
*/
package org.apache.syncope.client.console.panels;
+import
de.agilecoders.wicket.core.markup.html.bootstrap.components.PopoverBehavior;
+import
de.agilecoders.wicket.core.markup.html.bootstrap.components.PopoverConfig;
+import
de.agilecoders.wicket.core.markup.html.bootstrap.components.TooltipConfig;
+import io.swagger.v3.oas.annotations.media.Schema;
import java.io.Serializable;
import java.lang.reflect.Field;
import java.lang.reflect.ParameterizedType;
@@ -31,6 +35,7 @@ import java.util.Optional;
import java.util.stream.Collectors;
import org.apache.commons.lang3.time.DateFormatUtils;
import org.apache.commons.lang3.tuple.Pair;
+import org.apache.commons.lang3.tuple.Triple;
import org.apache.syncope.client.console.SyncopeConsoleSession;
import org.apache.syncope.client.console.SyncopeWebApplication;
import org.apache.syncope.client.console.panels.search.AnyObjectSearchPanel;
@@ -50,7 +55,6 @@ import
org.apache.syncope.client.ui.commons.markup.html.form.AjaxPalettePanel;
import
org.apache.syncope.client.ui.commons.markup.html.form.AjaxSpinnerFieldPanel;
import
org.apache.syncope.client.ui.commons.markup.html.form.AjaxTextFieldPanel;
import org.apache.syncope.client.ui.commons.markup.html.form.FieldPanel;
-import org.apache.syncope.common.lib.Schema;
import org.apache.syncope.common.lib.report.SearchCondition;
import org.apache.syncope.common.lib.search.AbstractFiqlSearchConditionBuilder;
import org.apache.syncope.common.lib.to.SchemaTO;
@@ -61,9 +65,11 @@ import
org.apache.wicket.core.util.lang.PropertyResolverConverter;
import org.apache.wicket.markup.html.basic.Label;
import org.apache.wicket.markup.html.list.ListItem;
import org.apache.wicket.markup.html.list.ListView;
+import org.apache.wicket.markup.html.panel.Fragment;
import org.apache.wicket.markup.html.panel.Panel;
import org.apache.wicket.model.IModel;
import org.apache.wicket.model.LoadableDetachableModel;
+import org.apache.wicket.model.Model;
import org.apache.wicket.model.PropertyModel;
import org.apache.wicket.model.ResourceModel;
import org.apache.wicket.model.util.ListModel;
@@ -126,9 +132,37 @@ public class BeanPanel<T extends Serializable> extends
Panel {
private static final long serialVersionUID = 9101744072914090143L;
+ private void setRequired(final ListItem<String> item, final
boolean required) {
+ if (required) {
+ Fragment fragment = new Fragment("required",
"requiredFragment", this);
+ fragment.add(new Label("requiredLabel", "*"));
+ item.replace(fragment);
+ }
+ }
+
+ private void setDescription(final ListItem<String> item, final
String description) {
+ Fragment fragment = new Fragment("description",
"descriptionFragment", this);
+ fragment.add(new Label("descriptionLabel", Model.of()).add(new
PopoverBehavior(
+ Model.<String>of(),
+ Model.of(description),
+ new
PopoverConfig().withPlacement(TooltipConfig.Placement.right)) {
+
+ private static final long serialVersionUID =
-7867802555691605021L;
+
+ @Override
+ protected String createRelAttribute() {
+ return "description";
+ }
+ }).setRenderBodyOnly(false));
+ item.replace(fragment);
+ }
+
@SuppressWarnings({ "unchecked", "rawtypes" })
@Override
protected void populateItem(final ListItem<String> item) {
+ item.add(new Fragment("required", "emptyFragment", this));
+ item.add(new Fragment("description", "emptyFragment", this));
+
String fieldName = item.getModelObject();
item.add(new Label("fieldName", new ResourceModel(fieldName,
fieldName)));
@@ -138,13 +172,11 @@ public class BeanPanel<T extends Serializable> extends
Panel {
return;
}
- SearchCondition scondAnnot =
field.getAnnotation(SearchCondition.class);
- Schema schemaAnnot = field.getAnnotation(Schema.class);
-
BeanWrapper wrapper =
PropertyAccessorFactory.forBeanPropertyAccess(bean.getObject());
Panel panel;
+ SearchCondition scondAnnot =
field.getAnnotation(SearchCondition.class);
if (scondAnnot != null) {
String fiql = (String) wrapper.getPropertyValue(fieldName);
@@ -171,32 +203,33 @@ public class BeanPanel<T extends Serializable> extends
Panel {
builder =
SyncopeClient.getAnyObjectSearchConditionBuilder(scondAnnot.type());
}
- if (BeanPanel.this.sCondWrapper != null) {
- BeanPanel.this.sCondWrapper.put(fieldName,
Pair.of(builder, clauses));
- }
+ Optional.ofNullable(BeanPanel.this.sCondWrapper).
+ ifPresent(scw -> scw.put(fieldName,
Pair.of(builder, clauses)));
} else if (List.class.equals(field.getType())) {
Class<?> listItemType = field.getGenericType() instanceof
ParameterizedType
? (Class<?>) ((ParameterizedType)
field.getGenericType()).getActualTypeArguments()[0]
: String.class;
- if (listItemType.equals(String.class) && schemaAnnot !=
null) {
+ org.apache.syncope.common.lib.Schema schema =
+
field.getAnnotation(org.apache.syncope.common.lib.Schema.class);
+ if (listItemType.equals(String.class) && schema != null) {
List<SchemaTO> choices = new ArrayList<>();
- for (SchemaType type : schemaAnnot.type()) {
+ for (SchemaType type : schema.type()) {
switch (type) {
case PLAIN:
choices.addAll(
-
SchemaRestClient.getSchemas(SchemaType.PLAIN, schemaAnnot.anyTypeKind()));
+
SchemaRestClient.getSchemas(SchemaType.PLAIN, schema.anyTypeKind()));
break;
case DERIVED:
choices.addAll(
-
SchemaRestClient.getSchemas(SchemaType.DERIVED, schemaAnnot.anyTypeKind()));
+
SchemaRestClient.getSchemas(SchemaType.DERIVED, schema.anyTypeKind()));
break;
case VIRTUAL:
choices.addAll(
-
SchemaRestClient.getSchemas(SchemaType.VIRTUAL, schemaAnnot.anyTypeKind()));
+
SchemaRestClient.getSchemas(SchemaType.VIRTUAL, schema.anyTypeKind()));
break;
default:
@@ -214,17 +247,29 @@ public class BeanPanel<T extends Serializable> extends
Panel {
new PropertyModel<>(bean.getObject(),
fieldName),
new
ListModel(List.of(listItemType.getEnumConstants()))).hideLabel();
} else {
+ Triple<FieldPanel, Boolean, Optional<String>> single =
+ buildSinglePanel(bean.getObject(),
field.getType(), field, "value");
+
+ setRequired(item, single.getMiddle());
+ single.getRight().ifPresent(description ->
setDescription(item, description));
+
panel = new MultiFieldPanel.Builder<>(
new PropertyModel<>(bean.getObject(),
fieldName)).build(
"value",
fieldName,
- buildSinglePanel(bean.getObject(),
listItemType, fieldName, "panel")).hideLabel();
+ single.getLeft()).hideLabel();
}
} else if (Map.class.equals(field.getType())) {
panel = new AjaxGridFieldPanel(
"value", fieldName, new PropertyModel<>(bean,
fieldName)).hideLabel();
} else {
- panel = buildSinglePanel(bean.getObject(),
field.getType(), fieldName, "value").hideLabel();
+ Triple<FieldPanel, Boolean, Optional<String>> single =
+ buildSinglePanel(bean.getObject(),
field.getType(), field, "value");
+
+ setRequired(item, single.getMiddle());
+ single.getRight().ifPresent(description ->
setDescription(item, description));
+
+ panel = single.getLeft().hideLabel();
}
item.add(panel.setRenderBodyOnly(true));
@@ -233,33 +278,34 @@ public class BeanPanel<T extends Serializable> extends
Panel {
}
@SuppressWarnings({ "unchecked", "rawtypes" })
- private static FieldPanel buildSinglePanel(
- final Serializable bean, final Class<?> type, final String
fieldName, final String id) {
+ private static Triple<FieldPanel, Boolean, Optional<String>>
buildSinglePanel(
+ final Serializable bean, final Class<?> type, final Field field,
final String id) {
- PropertyModel model = new PropertyModel<>(bean, fieldName);
+ PropertyModel model = new PropertyModel<>(bean, field.getName());
- FieldPanel result;
+ FieldPanel panel;
if (ClassUtils.isAssignable(Boolean.class, type)) {
- result = new AjaxCheckBoxPanel(id, fieldName, model);
+ panel = new AjaxCheckBoxPanel(id, field.getName(), model);
} else if (ClassUtils.isAssignable(Number.class, type)) {
- result = new AjaxSpinnerFieldPanel.Builder<>().build(
- id, fieldName, (Class<Number>)
ClassUtils.resolvePrimitiveIfNecessary(type), model);
+ panel = new AjaxSpinnerFieldPanel.Builder<>().build(
+ id, field.getName(), (Class<Number>)
ClassUtils.resolvePrimitiveIfNecessary(type), model);
} else if (Date.class.equals(type)) {
- result = new AjaxDateTimeFieldPanel(id, fieldName, model,
+ panel = new AjaxDateTimeFieldPanel(id, field.getName(), model,
DateFormatUtils.ISO_8601_EXTENDED_DATETIME_TIME_ZONE_FORMAT);
} else if (OffsetDateTime.class.equals(type)) {
- result = new AjaxDateTimeFieldPanel(id, fieldName, new
DateOps.WrappedDateModel(model),
+ panel = new AjaxDateTimeFieldPanel(id, field.getName(), new
DateOps.WrappedDateModel(model),
DateFormatUtils.ISO_8601_EXTENDED_DATETIME_TIME_ZONE_FORMAT);
} else if (type.isEnum()) {
- result = new AjaxDropDownChoicePanel(id, fieldName,
model).setChoices(List.of(type.getEnumConstants()));
+ panel = new AjaxDropDownChoicePanel(id, field.getName(), model).
+ setChoices(List.of(type.getEnumConstants()));
} else if (Duration.class.equals(type)) {
- result = new AjaxTextFieldPanel(id, fieldName, new IModel<>() {
+ panel = new AjaxTextFieldPanel(id, field.getName(), new IModel<>()
{
private static final long serialVersionUID =
807008909842554829L;
@Override
public String getObject() {
- return
Optional.ofNullable(PropertyResolver.getValue(fieldName, bean)).
+ return
Optional.ofNullable(PropertyResolver.getValue(field.getName(), bean)).
map(Object::toString).orElse(null);
}
@@ -268,15 +314,37 @@ public class BeanPanel<T extends Serializable> extends
Panel {
PropertyResolverConverter prc = new
PropertyResolverConverter(
SyncopeWebApplication.get().getConverterLocator(),
SyncopeConsoleSession.get().getLocale());
- PropertyResolver.setValue(fieldName, bean,
Duration.parse(object), prc);
+ PropertyResolver.setValue(field.getName(), bean,
Duration.parse(object), prc);
}
});
} else {
// treat as String if nothing matched above
- result = new AjaxTextFieldPanel(id, fieldName, model);
+ panel = new AjaxTextFieldPanel(id, field.getName(), model);
+ }
+
+ boolean required = false;
+ Optional<String> description = Optional.empty();
+
+ io.swagger.v3.oas.annotations.media.Schema schema =
+
field.getAnnotation(io.swagger.v3.oas.annotations.media.Schema.class);
+ if (schema != null) {
+ panel.setReadOnly(schema.accessMode() ==
Schema.AccessMode.READ_ONLY);
+
+ required = schema.requiredMode() == Schema.RequiredMode.REQUIRED;
+ panel.setRequired(required);
+
+
Optional.ofNullable(schema.example()).ifPresent(panel::setPlaceholder);
+
+ description = Optional.ofNullable(schema.description());
+
+ if (panel instanceof AjaxTextFieldPanel
+ && panel.getModelObject() == null
+ && schema.defaultValue() != null) {
+
+ ((AjaxTextFieldPanel)
panel).setModelObject(schema.defaultValue());
+ }
}
- result.hideLabel();
- return result;
+ return Triple.of(panel, required, description);
}
}
diff --git
a/client/idrepo/console/src/main/resources/org/apache/syncope/client/console/panels/BeanPanel.html
b/client/idrepo/console/src/main/resources/org/apache/syncope/client/console/panels/BeanPanel.html
index 3d584102ab..2d3290b384 100644
---
a/client/idrepo/console/src/main/resources/org/apache/syncope/client/console/panels/BeanPanel.html
+++
b/client/idrepo/console/src/main/resources/org/apache/syncope/client/console/panels/BeanPanel.html
@@ -19,8 +19,20 @@ under the License.
<html xmlns="http://www.w3.org/1999/xhtml";
xmlns:wicket="http://wicket.apache.org";>
<wicket:panel>
<div wicket:id="propView">
+ <wicket:fragment wicket:id="requiredFragment">
+ <span wicket:id="requiredLabel"/>
+ </wicket:fragment>
+
+ <wicket:fragment wicket:id="descriptionFragment">
+ <span wicket:id="descriptionLabel" class="fas fa-info-circle"
style="cursor: pointer"/>
+ </wicket:fragment>
+
+ <wicket:fragment wicket:id="emptyFragment">
+ </wicket:fragment>
+
<div class="form-group inforow">
- <label wicket:id="fieldName">[LABEL]</label>
+ <label wicket:id="fieldName">[LABEL]</label><span
wicket:id="required"/>
+ <span wicket:id="description"/>
<span wicket:id="value">[value]</span>
</div>
</div>
diff --git
a/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/EntityViolationType.java
b/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/EntityViolationType.java
index 7dc9345096..513a0a1dbb 100644
---
a/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/EntityViolationType.java
+++
b/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/EntityViolationType.java
@@ -20,40 +20,38 @@ package org.apache.syncope.common.lib.types;
public enum EntityViolationType {
- Standard(""),
- InvalidAnyType("org.apache.syncope.core.persistence.validation.anytype"),
-
InvalidADynMemberships("org.apache.syncope.core.persistence.validation.group.adynmemberships"),
-
InvalidConnInstanceLocation("org.apache.syncope.core.persistence.validation.conninstance.location"),
-
InvalidConnPoolConf("org.apache.syncope.core.persistence.validation.conninstance.poolConf"),
- InvalidMapping("org.apache.syncope.core.persistence.validation.mapping"),
- InvalidKey("org.apache.syncope.core.persistence.validation.key"),
- InvalidName("org.apache.syncope.core.persistence.validation.name"),
-
InvalidPassword("org.apache.syncope.core.persistence.validation.user.password"),
- InvalidPolicy("org.apache.syncope.core.persistence.validation.policy"),
-
InvalidPropagationTask("org.apache.syncope.core.persistence.validation.propagationtask"),
- InvalidRealm("org.apache.syncope.core.persistence.validation.realm"),
- InvalidDynRealm("org.apache.syncope.core.persistence.validation.dynrealm"),
- InvalidReport("org.apache.syncope.core.persistence.validation.report"),
-
InvalidResource("org.apache.syncope.core.persistence.validation.externalresource"),
-
InvalidGroupOwner("org.apache.syncope.core.persistence.validation.group.owner"),
-
InvalidSchemaEncrypted("org.apache.syncope.core.persistence.validation.schema.encrypted"),
-
InvalidSchemaEnum("org.apache.syncope.core.persistence.validation.schema.enum"),
-
InvalidSchemaMultivalueUnique("org.apache.syncope.core.persistence.validation.schema.multivalueUnique"),
-
InvalidSchedTask("org.apache.syncope.core.persistence.validation.schedtask"),
-
InvalidProvisioningTask("org.apache.syncope.core.persistence.validation.provisioningtask"),
-
InvalidPlainAttr("org.apache.syncope.core.persistence.validation.plainattr"),
-
InvalidUsername("org.apache.syncope.core.persistence.validation.user.username"),
-
InvalidValueList("org.apache.syncope.core.persistence.validation.attr.valueList"),
-
InvalidRemediation("org.apache.syncope.core.persistence.validation.remediation"),
-
MoreThanOneNonNull("org.apache.syncope.core.persistence.validation.attrvalue.moreThanOneNonNull");
+ Standard,
+ InvalidAnyType,
+ InvalidADynMemberships,
+ InvalidConnInstanceLocation,
+ InvalidConnPoolConf,
+ InvalidMapping,
+ InvalidKey,
+ InvalidName,
+ InvalidPassword,
+ InvalidPolicy,
+ InvalidPropagationTask,
+ InvalidRealm,
+ InvalidDynRealm,
+ InvalidReport,
+ InvalidResource,
+ InvalidGroupOwner,
+ InvalidSchemaEncrypted,
+ InvalidSchemaEnum,
+ InvalidSchemaMultivalueUnique,
+ InvalidSchedTask,
+ InvalidProvisioningTask,
+ InvalidPlainAttr,
+ InvalidUsername,
+ InvalidValueList,
+ InvalidRemediation,
+ MoreThanOneNonNull;
private String message;
private String propertyPath;
- EntityViolationType(final String message) {
- this.message = message;
- }
+ private Object invalidValue;
public void setMessage(final String message) {
this.message = message;
@@ -71,4 +69,20 @@ public enum EntityViolationType {
this.propertyPath = propertyPath;
}
+ public void setInvalidValue(final Object invalidValue) {
+ this.invalidValue = invalidValue;
+ }
+
+ public Object getInvalidValue() {
+ return invalidValue;
+ }
+
+ @Override
+ public String toString() {
+ return name() + "{"
+ + "message=" + message
+ + ", propertyPath=" + propertyPath
+ + ", invalidValue=" + invalidValue
+ + '}';
+ }
}
diff --git
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/CommandLogic.java
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/CommandLogic.java
index 907a343e12..02a5b25705 100644
---
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/CommandLogic.java
+++
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/CommandLogic.java
@@ -23,8 +23,12 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
+import javax.validation.ConstraintViolation;
+import javax.validation.ValidationException;
+import javax.validation.Validator;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.syncope.common.lib.SyncopeClientException;
import org.apache.syncope.common.lib.command.CommandArgs;
@@ -34,6 +38,7 @@ import
org.apache.syncope.common.lib.types.ClientExceptionType;
import org.apache.syncope.common.lib.types.IdRepoEntitlement;
import org.apache.syncope.common.lib.types.IdRepoImplementationType;
import org.apache.syncope.core.logic.api.Command;
+import
org.apache.syncope.core.persistence.api.attrvalue.validation.InvalidEntityException;
import org.apache.syncope.core.persistence.api.dao.ImplementationDAO;
import org.apache.syncope.core.persistence.api.dao.NotFoundException;
import org.apache.syncope.core.persistence.api.entity.Implementation;
@@ -45,10 +50,13 @@ public class CommandLogic extends AbstractLogic<EntityTO> {
protected final ImplementationDAO implementationDAO;
+ protected final Validator validator;
+
protected final Map<String, Command<?>> perContextCommands = new
ConcurrentHashMap<>();
- public CommandLogic(final ImplementationDAO implementationDAO) {
+ public CommandLogic(final ImplementationDAO implementationDAO, final
Validator validator) {
this.implementationDAO = implementationDAO;
+ this.validator = validator;
}
@PreAuthorize("hasRole('" + IdRepoEntitlement.IMPLEMENTATION_LIST + "')")
@@ -110,6 +118,21 @@ public class CommandLogic extends AbstractLogic<EntityTO> {
throw sce;
}
+ if (command.getArgs() != null) {
+ try {
+ Set<ConstraintViolation<Object>> violations =
validator.validate(command.getArgs());
+ if (!violations.isEmpty()) {
+ throw new
InvalidEntityException(command.getArgs().getClass().getName(), violations);
+ }
+ } catch (ValidationException e) {
+ LOG.error("While validating {}", command.getArgs(), e);
+
+ SyncopeClientException sce =
SyncopeClientException.build(ClientExceptionType.InvalidValues);
+ sce.getElements().add(e.getMessage());
+ throw sce;
+ }
+ }
+
try {
return runnable.run(command.getArgs());
} catch (Exception e) {
diff --git
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/IdRepoLogicContext.java
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/IdRepoLogicContext.java
index 9e004bd247..9904587bf4 100644
---
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/IdRepoLogicContext.java
+++
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/IdRepoLogicContext.java
@@ -20,6 +20,7 @@ package org.apache.syncope.core.logic;
import java.util.ArrayList;
import java.util.List;
+import javax.validation.Validator;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.LoggerContext;
@@ -267,8 +268,8 @@ public class IdRepoLogicContext {
@ConditionalOnMissingBean
@Bean
- public CommandLogic commandLogic(final ImplementationDAO
implementationDAO) {
- return new CommandLogic(implementationDAO);
+ public CommandLogic commandLogic(final ImplementationDAO
implementationDAO, final Validator validator) {
+ return new CommandLogic(implementationDAO, validator);
}
@ConditionalOnMissingBean
diff --git
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/job/MacroRunJobDelegate.java
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/job/MacroRunJobDelegate.java
index 9dab5397aa..781edbd3a2 100644
---
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/job/MacroRunJobDelegate.java
+++
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/job/MacroRunJobDelegate.java
@@ -19,7 +19,10 @@
package org.apache.syncope.core.logic.job;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
import org.apache.syncope.common.lib.command.CommandArgs;
import org.apache.syncope.core.logic.api.Command;
import org.apache.syncope.core.persistence.api.dao.ImplementationDAO;
@@ -38,6 +41,9 @@ public class MacroRunJobDelegate extends
AbstractSchedTaskJobDelegate<MacroTask>
@Autowired
protected ImplementationDAO implementationDAO;
+ @Autowired
+ protected Validator validator;
+
protected final Map<String, Command<?>> perContextCommands = new
ConcurrentHashMap<>();
@SuppressWarnings("unchecked")
@@ -67,12 +73,19 @@ public class MacroRunJobDelegate extends
AbstractSchedTaskJobDelegate<MacroTask>
output.append(command).append(' ').append(args);
} else {
try {
+ if (task.getCommandArgs().get(i) != null) {
+ Set<ConstraintViolation<Object>> violations =
validator.validate(task.getCommandArgs().get(i));
+ if (!violations.isEmpty()) {
+ LOG.error("Errors while validating {}: {}",
task.getCommandArgs().get(i), violations);
+ throw new
IllegalArgumentException(task.getCommandArgs().get(i).getClass().getName());
+ }
+ }
+
output.append(runnable.run(task.getCommandArgs().get(i)));
} catch (Exception e) {
if (task.isContinueOnError()) {
output.append("Continuing on error:
<").append(e.getMessage()).append('>');
- LOG.error("While running {} with args {}, continuing
on error",
- command.getKey(), args, e);
+ LOG.error("While running {} with args {}, continuing
on error", command.getKey(), args, e);
} else {
throw new RuntimeException("While running " +
command.getKey(), e);
}
diff --git
a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/attrvalue/validation/InvalidEntityException.java
b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/attrvalue/validation/InvalidEntityException.java
index 9e70892098..8870b4ea36 100644
---
a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/attrvalue/validation/InvalidEntityException.java
+++
b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/attrvalue/validation/InvalidEntityException.java
@@ -23,6 +23,7 @@ import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
import javax.validation.ConstraintViolation;
import javax.validation.ValidationException;
import org.apache.commons.lang3.StringUtils;
@@ -76,28 +77,22 @@ public class InvalidEntityException extends
ValidationException {
this.entityClassSimpleName = entityClassSimpleName;
violations.forEach(violation -> {
- int firstComma = violation.getMessageTemplate().indexOf(';');
-
- final String key = violation.getMessageTemplate().substring(
- 0, firstComma > 0 ? firstComma :
violation.getMessageTemplate().length());
-
- final String message =
violation.getMessageTemplate().substring(firstComma > 0 ? firstComma + 1 : 0);
+ String key =
StringUtils.substringBefore(violation.getMessageTemplate(), ";").trim();
+ String message =
StringUtils.substringAfter(violation.getMessageTemplate(), ";").trim();
EntityViolationType entityViolationType;
try {
- entityViolationType = EntityViolationType.valueOf(key.trim());
+ entityViolationType = EntityViolationType.valueOf(key);
} catch (IllegalArgumentException e) {
entityViolationType = EntityViolationType.Standard;
}
-
- entityViolationType.setMessage(message.trim());
-
+ entityViolationType.setMessage(message);
entityViolationType.setPropertyPath(violation.getPropertyPath().toString());
+ entityViolationType.setInvalidValue(violation.getInvalidValue());
if
(!this.violations.containsKey(violation.getLeafBean().getClass())) {
this.violations.put(violation.getLeafBean().getClass(),
EnumSet.noneOf(EntityViolationType.class));
}
-
this.violations.get(violation.getLeafBean().getClass()).add(entityViolationType);
});
}
@@ -116,12 +111,8 @@ public class InvalidEntityException extends
ValidationException {
@Override
public String getMessage() {
- StringBuilder sb = new StringBuilder();
-
- violations.forEach(
- (key, value) -> sb.append(key.getSimpleName()).append('
').append(value.toString()).append(", "));
- sb.delete(sb.lastIndexOf(", "), sb.length());
-
- return sb.toString();
+ return violations.entrySet().stream().
+ map(entry -> entry.getKey().getSimpleName() + " " +
entry.getValue().toString()).
+ collect(Collectors.joining(","));
}
}
diff --git
a/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestCommandArgs.java
b/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestCommandArgs.java
index 0aa7fe4e5e..3f83a3949c 100644
---
a/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestCommandArgs.java
+++
b/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestCommandArgs.java
@@ -18,17 +18,26 @@
*/
package org.apache.syncope.fit.core.reference;
+import io.swagger.v3.oas.annotations.media.Schema;
+import javax.validation.constraints.NotEmpty;
import org.apache.syncope.common.lib.command.CommandArgs;
public class TestCommandArgs extends CommandArgs {
private static final long serialVersionUID = 1408260716514938521L;
- private String parentRealm = "/even/two";
+ @NotEmpty
+ @Schema(description = "parent realm", example = "/even/two", defaultValue
= "/",
+ requiredMode = Schema.RequiredMode.REQUIRED)
+ private String parentRealm = "/";
- private String realmName = "realm123";
+ @NotEmpty
+ @Schema(description = "new realm name", example = "realm123", requiredMode
= Schema.RequiredMode.REQUIRED)
+ private String realmName;
- private String printerName = "printer123";
+ @NotEmpty
+ @Schema(description = "printer name", example = "printer123", requiredMode
= Schema.RequiredMode.REQUIRED)
+ private String printerName;
public String getParentRealm() {
return parentRealm;
diff --git
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/CommandITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/CommandITCase.java
index cc3d1dd7e8..cb2cfc72bd 100644
---
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/CommandITCase.java
+++
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/CommandITCase.java
@@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import javax.ws.rs.core.Response;
import org.apache.syncope.common.lib.SyncopeClientException;
@@ -30,6 +31,7 @@ import org.apache.syncope.common.lib.command.CommandTO;
import org.apache.syncope.common.lib.to.AnyObjectTO;
import org.apache.syncope.common.lib.to.ImplementationTO;
import org.apache.syncope.common.lib.to.PagedResult;
+import org.apache.syncope.common.lib.types.ClientExceptionType;
import org.apache.syncope.common.lib.types.IdRepoImplementationType;
import org.apache.syncope.common.lib.types.ImplementationEngine;
import org.apache.syncope.common.rest.api.RESTHeaders;
@@ -76,11 +78,26 @@ public class CommandITCase extends AbstractITCase {
assertTrue(command.getArgs() instanceof TestCommandArgs);
}
+ @Test
+ public void argsValidationFailure() {
+ CommandTO command = COMMAND_SERVICE.search(new CommandQuery.Builder().
+
keyword(TestCommand.class.getSimpleName()).page(1).size(1).build()).getResult().get(0);
+
+ try {
+ COMMAND_SERVICE.run(command);
+ fail();
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidValues, e.getType());
+ }
+ }
+
@Test
public void runCommand() {
CommandTO command = COMMAND_SERVICE.search(
new
CommandQuery.Builder().page(1).size(1).build()).getResult().get(0);
TestCommandArgs args = ((TestCommandArgs) command.getArgs());
+ args.setParentRealm("/even/two");
+ args.setRealmName("realm123");
args.setPrinterName("printer124");
CommandOutput output = COMMAND_SERVICE.run(command);
diff --git
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MacroITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MacroITCase.java
index aebedba892..1391335b49 100644
---
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MacroITCase.java
+++
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MacroITCase.java
@@ -57,6 +57,14 @@ public class MacroITCase extends AbstractITCase {
private static String MACRO_TASK_KEY;
+ private static final TestCommandArgs TCA = new TestCommandArgs();
+
+ static {
+ TCA.setParentRealm("/odd");
+ TCA.setRealmName("macro");
+ TCA.setPrinterName("aprinter112");
+ }
+
@BeforeAll
public static void testCommandsSetup() throws Exception {
CommandITCase.testCommandSetup();
@@ -87,8 +95,7 @@ public class MacroITCase extends AbstractITCase {
task.setActive(true);
task.setRealm("/odd");
task.getCommands().add(new
CommandTO.Builder("GroovyCommand").build());
- task.getCommands().add(new
CommandTO.Builder(TestCommand.class.getSimpleName()).
- args(new TestCommandArgs()).build());
+ task.getCommands().add(new
CommandTO.Builder(TestCommand.class.getSimpleName()).args(TCA).build());
Response response = TASK_SERVICE.create(TaskType.MACRO, task);
task = getObject(response.getLocation(), TaskService.class,
MacroTaskTO.class);
@@ -124,10 +131,9 @@ public class MacroITCase extends AbstractITCase {
ExecTO exec = TASK_SERVICE.read(TaskType.MACRO, MACRO_TASK_KEY,
true).getExecutions().get(preExecs);
assertEquals(ExecStatus.SUCCESS.name(), exec.getStatus());
- TestCommandArgs args = new TestCommandArgs();
- AnyObjectTO printer = ANY_OBJECT_SERVICE.read(args.getPrinterName());
+ AnyObjectTO printer = ANY_OBJECT_SERVICE.read(TCA.getPrinterName());
assertNotNull(printer);
- assertEquals(args.getParentRealm() + "/" + args.getRealmName(),
printer.getRealm());
+ assertEquals(TCA.getParentRealm() + "/" + TCA.getRealmName(),
printer.getRealm());
assertFalse(REALM_SERVICE.list(printer.getRealm()).isEmpty());
}
diff --git
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
index 36ecb8a460..b0e52ef103 100644
---
a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
+++
b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MembershipITCase.java
@@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import com.fasterxml.jackson.core.JsonProcessingException;
import javax.ws.rs.core.Response;
import org.apache.syncope.client.lib.SyncopeClient;
import org.apache.syncope.common.lib.Attr;
@@ -62,7 +63,7 @@ import org.springframework.jdbc.core.JdbcTemplate;
public class MembershipITCase extends AbstractITCase {
@Test
- public void misc() {
+ public void misc() throws JsonProcessingException {
UserCR userCR = UserITCase.getUniqueSample("m...@apache.org");
userCR.setRealm("/even/two");
userCR.getPlainAttrs().add(new
Attr.Builder("aLong").value("1976").build());
diff --git a/pom.xml b/pom.xml
index e92241a2d9..3027aad8ce 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1923,7 +1923,7 @@ under the License.
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
- <version>10.4</version>
+ <version>10.5.0</version>
</dependency>
</dependencies>
<configuration>
