This is an automated email from the ASF dual-hosted git repository.
mridulpathak pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 53a8767b54 Improved: Validate tenant ID before creating a per-tenant
delegator (#1647)
53a8767b54 is described below
commit 53a8767b54a621a26be7a0044ca20fcbb4fad21d
Author: Mridul Pathak <[email protected]>
AuthorDate: Mon Aug 17 10:12:33 2026 +0530
Improved: Validate tenant ID before creating a per-tenant delegator (#1647)
ContextFilter and LoginWorker now confirm a request-supplied userTenantId
matches an enabled Tenant record before asking DelegatorFactory to build a
delegator for it, instead of doing so unconditionally and only reacting to the
failure afterward. DelegatorFactory also now evicts a delegator name from its
cache when creation fails, so a bad or transient lookup no longer permanently
occupies a cache entry.
Thanks: Pavel Kohout, Aisle Research for reporting.
---
.../org/apache/ofbiz/entity/DelegatorFactory.java | 5 +++-
.../java/org/apache/ofbiz/webapp/WebAppUtil.java | 27 ++++++++++++++++++++++
.../apache/ofbiz/webapp/control/ContextFilter.java | 3 +++
.../apache/ofbiz/webapp/control/LoginWorker.java | 4 ++++
4 files changed, 38 insertions(+), 1 deletion(-)
diff --git
a/framework/entity/src/main/java/org/apache/ofbiz/entity/DelegatorFactory.java
b/framework/entity/src/main/java/org/apache/ofbiz/entity/DelegatorFactory.java
index c810e0798f..1bc38ed436 100644
---
a/framework/entity/src/main/java/org/apache/ofbiz/entity/DelegatorFactory.java
+++
b/framework/entity/src/main/java/org/apache/ofbiz/entity/DelegatorFactory.java
@@ -39,10 +39,13 @@ public abstract class DelegatorFactory implements
Factory<Delegator, String> {
Runtime.getRuntime().availableProcessors(), 10, true);
public static Delegator getDelegator(String delegatorName) {
- Future<Delegator> future = getDelegatorFuture(delegatorName);
+ String cacheKey = delegatorName == null ? "default" : delegatorName;
+ Future<Delegator> future = getDelegatorFuture(cacheKey);
try {
return future.get();
} catch (ExecutionException | InterruptedException e) {
+ // do not let a failed lookup permanently occupy the cache under
its (possibly caller-supplied) key
+ DELEGATORS.remove(cacheKey, future);
Debug.logError(e, MODULE);
return null;
}
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java
index 3b25858934..03b125abbe 100644
--- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java
+++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java
@@ -27,6 +27,7 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
+import java.util.regex.Pattern;
import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletRequest;
@@ -41,6 +42,9 @@ import org.apache.ofbiz.base.util.UtilXml.LocalResolver;
import org.apache.ofbiz.base.util.cache.UtilCache;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.DelegatorFactory;
+import org.apache.ofbiz.entity.GenericEntityException;
+import org.apache.ofbiz.entity.GenericValue;
+import org.apache.ofbiz.entity.util.EntityQuery;
import org.apache.ofbiz.security.Security;
import org.apache.ofbiz.security.SecurityConfigurationException;
import org.apache.ofbiz.security.SecurityFactory;
@@ -68,6 +72,8 @@ public final class WebAppUtil {
public static final String CONTROL_MOUNT_POINT = "control";
private static final Path WEB_APP_FILE_NAME = Paths.get("WEB-INF",
"web.xml");
private static final UtilCache<Path, WebXml> WEB_XML_CACHE =
UtilCache.createUtilCache("webapp.WebXml");
+ // matches the "id" field type's VARCHAR(20) column width used by the
Tenant entity's primary key
+ private static final Pattern TENANT_ID_PATTERN =
Pattern.compile("[A-Za-z0-9_-]{1,20}");
/**
* Returns the control servlet path. The path consists of the web
application's mount-point
@@ -222,6 +228,27 @@ public final class WebAppUtil {
return delegator;
}
+ /**
+ * Returns true if the given tenant ID belongs to an enabled
<code>Tenant</code> record, as looked up through
+ * the given base (non-tenant-specific) delegator. Callers must check this
before building a per-tenant
+ * delegator name from request-supplied input, since {@link
DelegatorFactory} caches every delegator name it is
+ * asked for, including ones that fail to resolve.
+ * @param baseDelegator
+ * @param tenantId
+ */
+ public static boolean isValidTenantId(Delegator baseDelegator, String
tenantId) {
+ if (UtilValidate.isEmpty(tenantId) ||
!TENANT_ID_PATTERN.matcher(tenantId).matches()) {
+ return false;
+ }
+ try {
+ GenericValue tenant =
EntityQuery.use(baseDelegator).from("Tenant").where("tenantId",
tenantId).queryOne();
+ return tenant != null && !"Y".equals(tenant.getString("disabled"));
+ } catch (GenericEntityException e) {
+ Debug.logError(e, "Error looking up tenant ID " + tenantId,
MODULE);
+ return false;
+ }
+ }
+
public static Security getSecurity(ServletContext servletContext) {
Security security = (Security) servletContext.getAttribute("security");
if (security == null) {
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
index 39672d3256..20b0303113 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
@@ -139,6 +139,9 @@ public class ContextFilter implements Filter {
if (UtilValidate.isEmpty(tenantId)) {
tenantId = httpRequest.getParameter("userTenantId");
}
+ if (UtilValidate.isNotEmpty(tenantId) &&
!WebAppUtil.isValidTenantId(baseDelegator, tenantId)) {
+ tenantId = null;
+ }
if (UtilValidate.isNotEmpty(tenantId)) {
// if the request path is a root mount then redirect to
the initial path
if ("".equals(httpRequest.getContextPath()) &&
"".equals(httpRequest.getServletPath())) {
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
index 92ffb1c1be..457a4aef8c 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
@@ -476,8 +476,12 @@ public final class LoginWorker {
if (delegatorNameHashIndex == -1 || (currentDelegatorTenantId !=
null && !tenantId.equals(currentDelegatorTenantId))) {
// make that tenant active, setup a new delegator and a new
dispatcher
String delegatorName = delegator.getDelegatorBaseName() + "#"
+ tenantId;
+ Delegator baseDelegator =
DelegatorFactory.getDelegator(delegator.getDelegatorBaseName());
try {
+ if (!WebAppUtil.isValidTenantId(baseDelegator, tenantId)) {
+ throw new NullPointerException("Tenant [" + tenantId +
"] not found");
+ }
// after this line the delegator is replaced with the new
per-tenant delegator
delegator = DelegatorFactory.getDelegator(delegatorName);
dispatcher =
WebAppUtil.makeWebappDispatcher(servletContext, delegator);
