You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@syncope.apache.org by il...@apache.org on 2022/02/05 14:48:18 UTC
[syncope] branch master updated: [SYNCOPE-1663] Now returning appropriate error
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 9e29c51 [SYNCOPE-1663] Now returning appropriate error
9e29c51 is described below
commit 9e29c518beaa07dc3161cd23b48c6568e595733a
Author: Francesco Chicchiriccò <il...@apache.org>
AuthorDate: Sat Feb 5 13:16:01 2022 +0100
[SYNCOPE-1663] Now returning appropriate error
---
.../common/lib/types/ClientExceptionType.java | 2 +-
.../cxf/service/ReconciliationServiceImpl.java | 2 +-
.../core/rest/cxf/service/ResourceServiceImpl.java | 2 +-
.../core/rest/cxf/service/AbstractAnyService.java | 28 ++++++----
.../rest/cxf/service/AbstractSearchService.java | 2 +-
.../rest/cxf/service/AnyObjectServiceImpl.java | 2 +-
.../persistence/api/search/FilterConverter.java | 2 +-
.../api/search/SearchCondConverter.java | 2 +-
.../api/search/FilterConverterTest.java | 14 ++---
.../api/search/SearchCondConverterTest.java | 1 -
.../persistence/jpa/dao/MyJPAJSONAnySearchDAO.java | 7 +--
.../persistence/jpa/dao/PGJPAJSONAnySearchDAO.java | 3 +-
.../persistence/jpa/dao/AbstractAnySearchDAO.java | 24 +++------
.../core/persistence/jpa/dao/JPAAnySearchDAO.java | 53 ++++---------------
.../core/persistence/jpa/outer/AnySearchTest.java | 2 +-
.../java/data/DynRealmDataBinderImpl.java | 2 +-
.../java/data/GroupDataBinderImpl.java | 2 +-
.../provisioning/java/data/RoleDataBinderImpl.java | 2 +-
.../core/spring/security/AuthDataAccessor.java | 25 +++++----
.../jpa/dao/ElasticsearchAnySearchDAO.java | 59 ++++------------------
.../org/apache/syncope/fit/core/GroupITCase.java | 6 ++-
.../org/apache/syncope/fit/core/SearchITCase.java | 42 ++++++++++-----
22 files changed, 114 insertions(+), 170 deletions(-)
diff --git a/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java b/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java
index 0e170dc..8dc10d5 100644
--- a/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java
+++ b/common/idrepo/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java
@@ -53,7 +53,7 @@ public enum ClientExceptionType {
InvalidAnyObject(Response.Status.BAD_REQUEST),
InvalidGroup(Response.Status.BAD_REQUEST),
InvalidSchemaDefinition(Response.Status.BAD_REQUEST),
- InvalidSearchExpression(Response.Status.BAD_REQUEST),
+ InvalidSearchParameters(Response.Status.BAD_REQUEST),
InvalidPageOrSize(Response.Status.BAD_REQUEST),
InvalidPropagationTaskExecReport(Response.Status.BAD_REQUEST),
InvalidPlainSchema(Response.Status.BAD_REQUEST),
diff --git a/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java b/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java
index 5b40610..3bf25f9 100644
--- a/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java
+++ b/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java
@@ -88,7 +88,7 @@ public class ReconciliationServiceImpl extends AbstractSearchService implements
} catch (Exception e) {
LOG.error("Invalid FIQL expression: {}", reconQuery.getFiql(), e);
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(reconQuery.getFiql());
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java b/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java
index 1acbf88..fa4e7a8 100644
--- a/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java
+++ b/core/idm/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java
@@ -127,7 +127,7 @@ public class ResourceServiceImpl extends AbstractService implements ResourceServ
} catch (Exception e) {
LOG.error("Invalid FIQL expression: {}", query.getFiql(), e);
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(query.getFiql());
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java b/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java
index d64cfab..b1ab5bf 100644
--- a/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java
+++ b/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java
@@ -26,7 +26,9 @@ import java.util.stream.Collectors;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.commons.lang3.tuple.Pair;
+import org.apache.syncope.common.lib.SyncopeClientException;
import org.apache.syncope.common.lib.SyncopeConstants;
import org.apache.syncope.common.lib.request.AnyCR;
import org.apache.syncope.common.lib.request.AnyUR;
@@ -38,6 +40,7 @@ import org.apache.syncope.common.lib.to.AnyTO;
import org.apache.syncope.common.lib.Attr;
import org.apache.syncope.common.lib.to.PagedResult;
import org.apache.syncope.common.lib.to.ProvisioningResult;
+import org.apache.syncope.common.lib.types.ClientExceptionType;
import org.apache.syncope.common.lib.types.PatchOperation;
import org.apache.syncope.common.lib.types.ResourceAssociationAction;
import org.apache.syncope.common.lib.types.ResourceDeassociationAction;
@@ -134,15 +137,22 @@ public abstract class AbstractAnyService<TO extends AnyTO, CR extends AnyCR, UR
? null
: getSearchCond(anyQuery.getFiql(), realm);
- Pair<Integer, List<TO>> result = getAnyLogic().search(
- searchCond,
- anyQuery.getPage(),
- anyQuery.getSize(),
- getOrderByClauses(anyQuery.getOrderBy()),
- isAssignableCond ? SyncopeConstants.ROOT_REALM : realm,
- anyQuery.getDetails());
-
- return buildPagedResult(result.getRight(), anyQuery.getPage(), anyQuery.getSize(), result.getLeft());
+ try {
+ Pair<Integer, List<TO>> result = getAnyLogic().search(
+ searchCond,
+ anyQuery.getPage(),
+ anyQuery.getSize(),
+ getOrderByClauses(anyQuery.getOrderBy()),
+ isAssignableCond ? SyncopeConstants.ROOT_REALM : realm,
+ anyQuery.getDetails());
+
+ return buildPagedResult(result.getRight(), anyQuery.getPage(), anyQuery.getSize(), result.getLeft());
+ } catch (IllegalArgumentException e) {
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
+ sce.getElements().add(anyQuery.getFiql());
+ sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
+ throw sce;
+ }
}
protected Date findLastChange(final String key) {
diff --git a/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractSearchService.java b/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractSearchService.java
index 7b0ccbb..97afca1 100644
--- a/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractSearchService.java
+++ b/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractSearchService.java
@@ -44,7 +44,7 @@ public abstract class AbstractSearchService extends AbstractService {
} catch (Exception e) {
LOG.error("Invalid FIQL expression: {}", fiql, e);
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(fiql);
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java b/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java
index b63643a..10d4909 100644
--- a/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java
+++ b/core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java
@@ -86,7 +86,7 @@ public class AnyObjectServiceImpl extends AbstractAnyService<AnyObjectTO, AnyObj
if (StringUtils.isBlank(anyQuery.getFiql())
|| -1 == anyQuery.getFiql().indexOf(SpecialAttr.TYPE.toString())) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(SpecialAttr.TYPE.toString() + " is required in the FIQL string");
throw sce;
}
diff --git a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java
index b57c854..e90f339 100644
--- a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java
+++ b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java
@@ -51,7 +51,7 @@ public final class FilterConverter {
return visitor.getQuery();
} catch (Exception e) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(fiql);
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java
index da4095c..83c1486 100644
--- a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java
+++ b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java
@@ -55,7 +55,7 @@ public final class SearchCondConverter {
return visitor.getQuery();
} catch (Exception e) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(fiql);
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java
index d314db0..84f73d8 100644
--- a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java
+++ b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java
@@ -117,7 +117,7 @@ public class FilterConverterTest {
FilterConverter.convert(fiql);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -130,7 +130,7 @@ public class FilterConverterTest {
FilterConverter.convert(fiql);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -143,7 +143,7 @@ public class FilterConverterTest {
FilterConverter.convert(fiql);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -178,7 +178,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.DYNREALMS + "==realm");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -188,7 +188,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.DYNREALMS + "!=realm");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -198,7 +198,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.RESOURCES + "==resource");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -208,7 +208,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.RESOURCES + "!=resource");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
diff --git a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java
index eafb313..111aa67 100644
--- a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java
+++ b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java
@@ -329,5 +329,4 @@ public class SearchCondConverterTest {
assertEquals(SearchCond.getLeaf(cond), SearchCondConverter.convert(VISITOR, fiql));
}
-
}
diff --git a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java
index d3a83a9..0a01536 100644
--- a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java
+++ b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java
@@ -247,12 +247,7 @@ public class MyJPAJSONAnySearchDAO extends JPAAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- Pair<PlainSchema, PlainAttrValue> checked;
- try {
- checked = check(cond, svs.anyTypeKind);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Pair<PlainSchema, PlainAttrValue> checked = check(cond, svs.anyTypeKind);
// normalize NULL / NOT NULL checks
if (not) {
diff --git a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java
index 514b436..646fd92 100644
--- a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java
+++ b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java
@@ -612,8 +612,7 @@ public class PGJPAJSONAnySearchDAO extends JPAAnySearchDAO {
Realm realm = realmDAO.findByFullPath(cond.getExpression());
if (realm == null) {
- LOG.warn("Invalid Realm full path: {}", cond.getExpression());
- return EMPTY_QUERY;
+ throw new IllegalArgumentException("Invalid Realm full path: " + cond.getExpression());
}
cond.setExpression(realm.getKey());
}
diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java
index 38b5d55..f4bd37a 100644
--- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java
+++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java
@@ -189,8 +189,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
PlainSchema schema = plainSchemaDAO.find(cond.getSchema());
if (schema == null) {
- LOG.warn("Ignoring invalid schema '{}'", cond.getSchema());
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Invalid schema " + cond.getSchema());
}
PlainAttrValue attrValue = schema.isUniqueConstraint()
@@ -205,8 +204,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
((JPAPlainSchema) schema).validator().validate(cond.getExpression(), attrValue);
}
} catch (ValidationException e) {
- LOG.error("Could not validate expression '" + cond.getExpression() + '\'', e);
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not validate expression " + cond.getExpression());
}
return Pair.of(schema, attrValue);
@@ -221,8 +219,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
Field anyField = anyUtils.getField(computed.getSchema());
if (anyField == null) {
- LOG.warn("Ignoring invalid field '{}'", computed.getSchema());
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Invalid schema " + computed.getSchema());
}
// Keeps track of difference between entity's getKey() and JPA @Id fields
if ("key".equals(computed.getSchema())) {
@@ -268,8 +265,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
try {
((JPAPlainSchema) schema).validator().validate(computed.getExpression(), attrValue);
} catch (ValidationException e) {
- LOG.error("Could not validate expression '" + computed.getExpression() + '\'', e);
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not validate expression " + computed.getExpression());
}
}
@@ -285,8 +281,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
? Optional.ofNullable(groupDAO.findKey(cond.getGroup())).map(List::of).orElseGet(List::of)
: groupDAO.findKeysByNamePattern(cond.getGroup());
if (matching.isEmpty()) {
- LOG.error("Could not find group(s) for '{}'", cond.getGroup());
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find group(s) for " + cond.getGroup());
}
return matching;
@@ -301,8 +296,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
rightAnyObjectKey = Optional.ofNullable(anyObject).map(Entity::getKey).orElse(null);
}
if (rightAnyObjectKey == null) {
- LOG.error("Could not find any object for '" + cond.getAnyObject() + '\'');
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find any object for " + cond.getAnyObject());
}
return rightAnyObjectKey;
@@ -311,8 +305,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
protected Realm check(final AssignableCond cond) {
Realm realm = realmDAO.findByFullPath(cond.getRealmFullPath());
if (realm == null) {
- LOG.error("Could not find realm for '" + cond.getRealmFullPath() + '\'');
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find realm for " + cond.getRealmFullPath());
}
return realm;
@@ -330,8 +323,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
memberKey = Optional.ofNullable(member).map(Entity::getKey).orElse(null);
}
if (memberKey == null) {
- LOG.error("Could not find user or any object for '" + cond.getMember() + '\'');
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find user or any object for " + cond.getMember());
}
return memberKey;
diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java
index 7b04807..185074a 100644
--- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java
+++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java
@@ -72,8 +72,6 @@ import org.apache.syncope.core.persistence.api.entity.Realm;
*/
public class JPAAnySearchDAO extends AbstractAnySearchDAO {
- protected static final String EMPTY_QUERY = "SELECT any_id FROM user_search WHERE 1=2";
-
public JPAAnySearchDAO(
final RealmDAO realmDAO,
final DynRealmDAO dynRealmDAO,
@@ -248,9 +246,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
if (parameters.get(i) instanceof Date) {
query.setParameter(i + 1, (Date) parameters.get(i), TemporalType.TIMESTAMP);
} else if (parameters.get(i) instanceof Boolean) {
- query.setParameter(i + 1, ((Boolean) parameters.get(i))
- ? 1
- : 0);
+ query.setParameter(i + 1, ((Boolean) parameters.get(i)) ? 1 : 0);
} else {
query.setParameter(i + 1, parameters.get(i));
}
@@ -461,7 +457,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
}
if (orderByUniquePlainSchemas.size() > 1 || orderByNonUniquePlainSchemas.size() > 1) {
SyncopeClientException invalidSearch =
- SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
invalidSearch.getElements().add("Order by more than one attribute is not allowed; "
+ "remove one from " + (orderByUniquePlainSchemas.size() > 1
? orderByUniquePlainSchemas : orderByNonUniquePlainSchemas));
@@ -661,12 +657,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- String rightAnyObjectKey;
- try {
- rightAnyObjectKey = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ String rightAnyObjectKey = check(cond);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE ");
@@ -691,12 +682,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- List<String> groupKeys;
- try {
- groupKeys = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ List<String> groupKeys = check(cond);
String where = groupKeys.stream().
map(key -> "group_id=?" + setParameter(parameters, key)).
@@ -858,12 +844,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- Realm realm;
- try {
- realm = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Realm realm = check(cond);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE (");
@@ -889,12 +870,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- String memberKey;
- try {
- memberKey = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ String memberKey = check(cond);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE ");
@@ -1062,12 +1038,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- Pair<PlainSchema, PlainAttrValue> checked;
- try {
- checked = check(cond, svs.anyTypeKind);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Pair<PlainSchema, PlainAttrValue> checked = check(cond, svs.anyTypeKind);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ");
switch (cond.getType()) {
@@ -1118,18 +1089,12 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
Realm realm = realmDAO.findByFullPath(cond.getExpression());
if (realm == null) {
- LOG.warn("Invalid Realm full path: {}", cond.getExpression());
- return EMPTY_QUERY;
+ throw new IllegalArgumentException("Invalid Realm full path: " + cond.getExpression());
}
cond.setExpression(realm.getKey());
}
- Triple<PlainSchema, PlainAttrValue, AnyCond> checked;
- try {
- checked = check(cond, svs.anyTypeKind);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Triple<PlainSchema, PlainAttrValue, AnyCond> checked = check(cond, svs.anyTypeKind);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE ");
diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java
index 180d97c..b371889 100644
--- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java
+++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java
@@ -188,7 +188,7 @@ public class AnySearchTest extends AbstractTest {
searchDAO.search(searchCondition, orderByClauses, AnyTypeKind.USER);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java
index f0447ec..587d1a7 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java
@@ -62,7 +62,7 @@ public class DynRealmDataBinderImpl implements DynRealmDataBinder {
protected void setDynMembership(final DynRealm dynRealm, final AnyType anyType, final String dynMembershipFIQL) {
SearchCond dynMembershipCond = SearchCondConverter.convert(searchCondVisitor, dynMembershipFIQL);
if (!dynMembershipCond.isValid()) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(dynMembershipFIQL);
throw sce;
}
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
index 390a8d7..6ec503e 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
@@ -124,7 +124,7 @@ public class GroupDataBinderImpl extends AbstractAnyDataBinder implements GroupD
protected void setDynMembership(final Group group, final AnyType anyType, final String dynMembershipFIQL) {
SearchCond dynMembershipCond = SearchCondConverter.convert(searchCondVisitor, dynMembershipFIQL);
if (!dynMembershipCond.isValid()) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(dynMembershipFIQL);
throw sce;
}
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java
index 32401fc..bb6a867 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java
@@ -75,7 +75,7 @@ public class RoleDataBinderImpl implements RoleDataBinder {
protected void setDynMembership(final Role role, final String dynMembershipFIQL) {
SearchCond dynMembershipCond = SearchCondConverter.convert(searchCondVisitor, dynMembershipFIQL);
if (!dynMembershipCond.isValid()) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(dynMembershipFIQL);
throw sce;
}
diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java b/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java
index 6cd89b4..6211f33 100644
--- a/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java
+++ b/core/spring/src/main/java/org/apache/syncope/core/spring/security/AuthDataAccessor.java
@@ -214,21 +214,24 @@ public class AuthDataAccessor {
public Triple<User, Boolean, String> authenticate(final String domain, final Authentication authentication) {
User user = null;
- List<String> authAttrValues = List.of(confParamOps.get(domain,
- "authentication.attributes", new String[] { "username" }, String[].class));
- for (int i = 0; user == null && i < authAttrValues.size(); i++) {
- if ("username".equals(authAttrValues.get(i))) {
+ String[] authAttrValues = confParamOps.get(
+ domain, "authentication.attributes", new String[] { "username" }, String[].class);
+ for (int i = 0; user == null && i < authAttrValues.length; i++) {
+ if ("username".equals(authAttrValues[i])) {
user = userDAO.findByUsername(authentication.getName());
} else {
AttrCond attrCond = new AttrCond(AttrCond.Type.EQ);
- attrCond.setSchema(authAttrValues.get(i));
+ attrCond.setSchema(authAttrValues[i]);
attrCond.setExpression(authentication.getName());
- List<User> users = anySearchDAO.search(SearchCond.getLeaf(attrCond), AnyTypeKind.USER);
- if (users.size() == 1) {
- user = users.get(0);
- } else {
- LOG.warn("Value {} provided for {} does not uniquely identify a user",
- authentication.getName(), authAttrValues.get(i));
+ try {
+ List<User> users = anySearchDAO.search(SearchCond.getLeaf(attrCond), AnyTypeKind.USER);
+ if (users.size() == 1) {
+ user = users.get(0);
+ } else {
+ LOG.warn("Search condition {} does not uniquely match a user", attrCond);
+ }
+ } catch (IllegalArgumentException e) {
+ LOG.error("While searching user for authentication via {}", attrCond, e);
}
}
}
diff --git a/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java b/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java
index bd014ba..26406b8 100644
--- a/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java
+++ b/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java
@@ -24,8 +24,6 @@ import co.elastic.clients.elasticsearch._types.FieldValue;
import co.elastic.clients.elasticsearch._types.SearchType;
import co.elastic.clients.elasticsearch._types.SortOptions;
import co.elastic.clients.elasticsearch._types.SortOrder;
-import co.elastic.clients.elasticsearch._types.query_dsl.MatchAllQuery;
-import co.elastic.clients.elasticsearch._types.query_dsl.MatchNoneQuery;
import co.elastic.clients.elasticsearch._types.query_dsl.Query;
import co.elastic.clients.elasticsearch._types.query_dsl.QueryBuilders;
import co.elastic.clients.elasticsearch.core.CountRequest;
@@ -88,12 +86,6 @@ import org.springframework.util.CollectionUtils;
*/
public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
- protected static final Query MATCH_NONE_QUERY =
- new Query.Builder().matchNone(new MatchNoneQuery.Builder().build()).build();
-
- protected static final Query MATCH_ALL_QUERY =
- new Query.Builder().matchAll(new MatchAllQuery.Builder().build()).build();
-
protected static final char[] ELASTICSEARCH_REGEX_CHARS = new char[] {
'.', '?', '+', '*', '|', '{', '}', '[', ']', '(', ')', '"', '\\', '&' };
@@ -369,7 +361,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
if (query == null) {
- query = MATCH_NONE_QUERY;
+ throw new IllegalArgumentException("Cannot construct QueryBuilder");
}
if (cond.getType() == SearchCond.Type.NOT_LEAF) {
@@ -408,12 +400,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected Query getQuery(final RelationshipCond cond) {
- String rightAnyObjectKey;
- try {
- rightAnyObjectKey = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY;
- }
+ String rightAnyObjectKey = check(cond);
return new Query.Builder().term(QueryBuilders.term().
field("relationships").value(FieldValue.of(rightAnyObjectKey)).build()).
@@ -421,12 +408,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected Query getQuery(final MembershipCond cond) {
- List<String> groupKeys;
- try {
- groupKeys = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY;
- }
+ List<String> groupKeys = check(cond);
List<Query> membershipQueries = groupKeys.stream().
map(key -> new Query.Builder().term(QueryBuilders.term().
@@ -440,12 +422,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected Query getQuery(final AssignableCond cond) {
- Realm realm;
- try {
- realm = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY;
- }
+ Realm realm = check(cond);
List<Query> queries = new ArrayList<>();
if (cond.isFromGroup()) {
@@ -486,12 +463,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected Query getQuery(final MemberCond cond) {
- String memberKey;
- try {
- memberKey = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY;
- }
+ String memberKey = check(cond);
return new Query.Builder().term(QueryBuilders.term().
field("members").value(FieldValue.of(memberKey)).build()).
@@ -513,7 +485,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
? attrValue.getDateValue().getTime()
: attrValue.getValue();
- Query query = MATCH_NONE_QUERY;
+ Query query = null;
switch (cond.getType()) {
case ISNOTNULL:
@@ -602,12 +574,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected Query getQuery(final AttrCond cond, final AnyTypeKind kind) {
- Pair<PlainSchema, PlainAttrValue> checked;
- try {
- checked = check(cond, kind);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY;
- }
+ Pair<PlainSchema, PlainAttrValue> checked = check(cond, kind);
return fillAttrQuery(checked.getLeft(), checked.getRight(), cond);
}
@@ -618,23 +585,17 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
Realm realm = realmDAO.find(cond.getExpression());
if (realm == null) {
- LOG.warn("Invalid Realm key: {}", cond.getExpression());
- return MATCH_NONE_QUERY;
+ throw new IllegalArgumentException("Invalid Realm key: " + cond.getExpression());
}
cond.setExpression(realm.getFullPath());
}
- Triple<PlainSchema, PlainAttrValue, AnyCond> checked;
- try {
- checked = check(cond, kind);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY;
- }
+ Triple<PlainSchema, PlainAttrValue, AnyCond> checked = check(cond, kind);
return fillAttrQuery(checked.getLeft(), checked.getMiddle(), checked.getRight());
}
protected Query getQueryForCustomConds(final SearchCond cond, final AnyTypeKind kind) {
- return MATCH_ALL_QUERY;
+ return null;
}
}
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
index 3d8b64d..7c1f0b5 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
@@ -254,10 +254,12 @@ public class GroupITCase extends AbstractITCase {
@Test
public void patch() {
GroupCR createReq = getBasicSample("patch");
- createReq.setUDynMembershipCond("(($groups==3;$resources!=ws-target-resource-1);aLong==1)");
+ createReq.setUDynMembershipCond(
+ "(($groups==ebf97068-aa4b-4a85-9f01-680e8c4cf227;$resources!=ws-target-resource-1);aLong==1)");
createReq.getADynMembershipConds().put(
PRINTER,
- "(($groups==7;cool==ss);$resources==ws-target-resource-2);$type==PRINTER");
+ "(($groups==ece66293-8f31-4a84-8e8d-23da36e70846;cool==ss);$resources==ws-target-resource-2);"
+ + "$type==PRINTER");
GroupTO created = createGroup(createReq).getEntity();
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java
index c9f0e79..58a6a9d 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java
@@ -430,12 +430,15 @@ public class SearchITCase extends AbstractITCase {
UserTO userTO = createUser(userCR).getEntity();
assertNotNull(userTO.getSecurityQuestion());
- PagedResult<UserTO> matchingUsers = userService.search(
- new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
- fiql(SyncopeClient.getUserSearchConditionBuilder().
- is("securityAnswer").equalTo(securityAnswer).query()).build());
- assertNotNull(matchingUsers);
- assertTrue(matchingUsers.getResult().isEmpty());
+ try {
+ userService.search(
+ new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql(SyncopeClient.getUserSearchConditionBuilder().
+ is("securityAnswer").equalTo(securityAnswer).query()).build());
+ fail();
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
+ }
}
@Test
@@ -769,7 +772,7 @@ public class SearchITCase extends AbstractITCase {
fail();
}
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -806,11 +809,26 @@ public class SearchITCase extends AbstractITCase {
@Test
public void issueSYNCOPE1648() {
- PagedResult<UserTO> matching = userService.search(
- new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
- fiql(SyncopeClient.getUserSearchConditionBuilder().
- is("username").notEqualTo("verdi").query()).
- build());
+ PagedResult<UserTO> matching = userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql(SyncopeClient.getUserSearchConditionBuilder().
+ is("username").notEqualTo("verdi").query()).build());
assertTrue(matching.getResult().stream().noneMatch(user -> "verdi".equals(user.getUsername())));
}
+
+ @Test
+ public void issueSYNCOPE1663() {
+ try {
+ userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql("lastChangeDate=ge=2022-01-25T17:00:06Z").build());
+ fail();
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
+ assertTrue(e.getElements().stream().
+ anyMatch(elem -> elem.contains("Could not validate expression 2022-01-25T17:00:06Z")));
+ }
+
+ PagedResult<UserTO> matching = userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql("lastChangeDate=ge=2022-01-25T17:00:06+0000").build());
+ assertNotNull(matching);
+ }
}