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 45e8de777f Improved: Bind externalLoginKey to its intended destination
webapps instead of validating against any webapp on the server (OFBIZ-13522)
45e8de777f is described below
commit 45e8de777fc1f22d5050a9afe8c81e3056ffa82b
Author: Mridul Pathak <[email protected]>
AuthorDate: Fri Sep 4 13:22:33 2026 +0530
Improved: Bind externalLoginKey to its intended destination webapps instead
of validating against any webapp on the server (OFBIZ-13522)
ExternalLoginTicket previously carried a nullable targetContextPath that
every mint site left null, making isValidFor treat any webapp on the server as
a valid redemption target for a captured key, not just the webapps the render
that minted it actually linked to. It now carries a mutable allow-list of
context paths that is grown as concrete destinations become known, at
WidgetWorker#buildHyperlinkUri's inter-app branch, the duplicate inter-app
branch in HtmlTreeRenderer#renderLink, a [...]
---
.../webapp/control/ExternalLoginKeysManager.java | 82 +++++++++++++++++++---
.../ofbiz/webapp/control/RequestHandler.java | 6 +-
.../control/ExternalLoginKeysManagerTests.java | 77 ++++++++++++++++++++
.../java/org/apache/ofbiz/widget/WidgetWorker.java | 2 +
.../ofbiz/widget/renderer/ScreenRenderer.java | 31 ++++++++
.../widget/renderer/html/HtmlTreeRenderer.java | 2 +
6 files changed, 190 insertions(+), 10 deletions(-)
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
index 49dc13f6a9..4c32567b36 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
@@ -28,6 +28,7 @@ import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.DelegatorFactory;
import org.apache.ofbiz.entity.GenericValue;
@@ -55,23 +56,28 @@ public class ExternalLoginKeysManager {
/**
* A minted external login key, bound to the UserLogin it authenticates
and to a deadline.
- * Optionally bound to a target context path too, for mint sites that know
which webapp the
- * key is destined for; existing mint sites do not, so that field is null
and the check is
- * skipped for them.
+ * Also bound to the set of destination webapps it is actually attached
to: a mint site does
+ * not know in advance every webapp its render will link to (one render
mints one key and
+ * hands it to several cross-webapp links, possibly several different
destinations), so
+ * destinations are registered as they become known -- see #allowFor --
rather than fixed at
+ * construction time. A ticket with no registered destinations yet
authenticates nowhere.
*/
private static final class ExternalLoginTicket {
private final GenericValue userLogin;
- private final String targetContextPath;
private final long expiresAtMillis;
+ // Context paths this ticket is allowed to authenticate into. Grown by
#allowFor as the
+ // concrete destinations of this render's cross-webapp links become
known; checked by
+ // #isValidFor so a leaked key is only ever redeemable against a
webapp this render
+ // actually intended to reach, not any webapp on the server.
+ private final Set<String> allowedContextPaths =
ConcurrentHashMap.newKeySet();
// Context paths this ticket has already been redeemed for. A single
render shares one
// key across every cross-webapp link it emits, so the same key
legitimately needs to
// authenticate into several different webapps; this set is what stops
it authenticating
// into the *same* webapp twice, which is the actual replay this
ticket must prevent.
private final Set<String> redeemedContextPaths =
ConcurrentHashMap.newKeySet();
- ExternalLoginTicket(GenericValue userLogin, String targetContextPath) {
+ ExternalLoginTicket(GenericValue userLogin) {
this.userLogin = userLogin;
- this.targetContextPath = targetContextPath;
this.expiresAtMillis = System.currentTimeMillis() +
EXTERNAL_LOGIN_KEY_TTL_MILLIS;
}
@@ -79,11 +85,19 @@ public class ExternalLoginKeysManager {
return userLogin;
}
+ /**
+ * Allows this ticket to authenticate into the given destination
webapp.
+ * @param contextPath the destination webapp's context path, e.g.
"/partymgr"
+ */
+ void allowFor(String contextPath) {
+ allowedContextPaths.add(contextPath);
+ }
+
boolean isValidFor(HttpServletRequest request) {
if (System.currentTimeMillis() > expiresAtMillis) {
return false;
}
- return targetContextPath == null ||
targetContextPath.equals(request.getServletContext().getContextPath());
+ return
allowedContextPaths.contains(request.getServletContext().getContextPath());
}
/**
@@ -131,7 +145,7 @@ public class ExternalLoginKeysManager {
request.setAttribute(EXTERNAL_LOGIN_KEY_ATTR, externalKey);
session.setAttribute(EXTERNAL_LOGIN_KEY_ATTR, externalKey);
- EXTERNAL_LOGIN_KEYS.put(externalKey, new
ExternalLoginTicket(userLogin, null));
+ EXTERNAL_LOGIN_KEYS.put(externalKey, new
ExternalLoginTicket(userLogin));
return externalKey;
}
}
@@ -147,6 +161,58 @@ public class ExternalLoginKeysManager {
}
}
+ /**
+ * Allows an already-minted key to authenticate into a destination webapp,
given that
+ * webapp's context path directly. Used at the point a concrete
destination is already
+ * known, e.g. the set of webapps a rendered app-switcher menu can link to.
+ * @param externalLoginKey the minted key, as returned by {@link
#getExternalLoginKey}
+ * @param contextPath the destination webapp's context path, e.g.
"/partymgr"
+ */
+ public static void registerDestination(String externalLoginKey, String
contextPath) {
+ if (UtilValidate.isEmpty(externalLoginKey) ||
UtilValidate.isEmpty(contextPath)) {
+ return;
+ }
+ ExternalLoginTicket ticket = EXTERNAL_LOGIN_KEYS.get(externalLoginKey);
+ if (ticket != null) {
+ ticket.allowFor(contextPath);
+ }
+ }
+
+ /**
+ * Allows an already-minted key to authenticate into the destination
webapp targeted by a
+ * relative inter-app link, e.g. {@code
"/partymgr/control/viewprofile?partyId=10000"} or
+ * {@code "/partymgr/control"}. The destination webapp's context path is
parsed out of the
+ * target; a target that isn't a routed control-servlet request registers
nothing, since
+ * nothing would ever redeem a ticket against it anyway.
+ * @param externalLoginKey the minted key, as returned by {@link
#getExternalLoginKey}
+ * @param interAppTarget the relative target of an inter-app link
+ */
+ public static void registerInterAppDestination(String externalLoginKey,
String interAppTarget) {
+ registerDestination(externalLoginKey, contextRootOf(interAppTarget));
+ }
+
+ /**
+ * Parses the destination webapp's context path out of a relative
inter-app target, by
+ * taking everything before the "/control" path segment.
+ * @param target the relative target, e.g.
"/partymgr/control/viewprofile?partyId=10000"
+ * @return the context path, e.g. "/partymgr" (possibly empty, for a
root-mounted webapp),
+ * or null if the target has no "/control" path segment to anchor on
+ */
+ private static String contextRootOf(String target) {
+ if (target == null) {
+ return null;
+ }
+ int idx = target.indexOf("/control");
+ while (idx >= 0) {
+ int afterIdx = idx + "/control".length();
+ if (afterIdx == target.length() || target.charAt(afterIdx) == '/'
|| target.charAt(afterIdx) == '?') {
+ return target.substring(0, idx);
+ }
+ idx = target.indexOf("/control", idx + 1);
+ }
+ return null;
+ }
+
/**
* OFBiz controller event that performs the user authentication using the
authentication token.
* The method is designed to be used in a chain of controller preprocessor
event: it always return "success"
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
index 1aa558bcf8..4dc7024a25 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
@@ -1535,10 +1535,12 @@ public final class RequestHandler {
}
if (addExternalKeyParam) {
+ String externalLoginKey =
ExternalLoginKeysManager.getExternalLoginKey(request);
+
ExternalLoginKeysManager.registerInterAppDestination(externalLoginKey,
targetControlPath);
if (url.contains("?")) {
- url += "&externalLoginKey=" +
ExternalLoginKeysManager.getExternalLoginKey(request);
+ url += "&externalLoginKey=" + externalLoginKey;
} else {
- url += "?externalLoginKey=" +
ExternalLoginKeysManager.getExternalLoginKey(request);
+ url += "?externalLoginKey=" + externalLoginKey;
}
}
diff --git
a/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManagerTests.java
b/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManagerTests.java
index 2ac8fb7e26..c24f8bbfdd 100644
---
a/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManagerTests.java
+++
b/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManagerTests.java
@@ -123,6 +123,9 @@ public class ExternalLoginKeysManagerTests {
when(mintRequest.getSession()).thenReturn(mintSession);
when(mintRequest.getAttribute("userLogin")).thenReturn(userLogin);
String key = ExternalLoginKeysManager.getExternalLoginKey(mintRequest);
+ // The mint site attaches the key to a concrete link, e.g. a partymgr
menu item,
+ // registering that destination -- exactly as
WidgetWorker/RequestHandler do.
+ ExternalLoginKeysManager.registerDestination(key, "/partymgr");
// Both redemptions target the same webapp, so the second one is the
actual replay case.
ServletContext servletContext = mock(ServletContext.class);
@@ -165,4 +168,78 @@ public class ExternalLoginKeysManagerTests {
loginWorker.verify(() -> LoginWorker.autoLoginSet(replay,
replayResponse));
}
}
+
+ @Test
+ public void
checkExternalLoginKeyRejectsRedemptionAgainstAnUnregisteredDestination() {
+ // Mint a key and register it against partymgr only -- e.g. a page
that linked to
+ // partymgr, never to webtools.
+ GenericValue userLogin = mock(GenericValue.class);
+ HttpServletRequest mintRequest = mock(HttpServletRequest.class);
+ HttpSession mintSession = mock(HttpSession.class);
+ when(mintRequest.getSession()).thenReturn(mintSession);
+ when(mintRequest.getAttribute("userLogin")).thenReturn(userLogin);
+ String key = ExternalLoginKeysManager.getExternalLoginKey(mintRequest);
+ ExternalLoginKeysManager.registerDestination(key, "/partymgr");
+
+ // A captured key is presented against a webapp this render never
linked to.
+ ServletContext webtoolsContext = mock(ServletContext.class);
+ when(webtoolsContext.getContextPath()).thenReturn("/webtools");
+ HttpServletRequest attempt = mock(HttpServletRequest.class);
+ HttpServletResponse attemptResponse = mock(HttpServletResponse.class);
+ when(attempt.getParameter("externalLoginKey")).thenReturn(key);
+ when(attempt.getServletContext()).thenReturn(webtoolsContext);
+
+ try (MockedStatic<LoginWorker> loginWorker =
mockStatic(LoginWorker.class);
+ MockedStatic<EntityUtilProperties> props =
mockStatic(EntityUtilProperties.class)) {
+ props.when(() -> EntityUtilProperties.getPropertyValue(
+ "security", "security.login.externalLoginKey.enabled",
"true", null)).thenReturn("true");
+
+ String result =
ExternalLoginKeysManager.checkExternalLoginKey(attempt, attemptResponse);
+
+ assertEquals("success", result);
+ loginWorker.verify(() -> LoginWorker.doBasicLogin(any(), any(),
any()), never());
+ loginWorker.verify(() -> LoginWorker.autoLoginSet(attempt,
attemptResponse));
+ }
+ }
+
+ @Test
+ public void
registerInterAppDestinationParsesContextRootFromControlServletTarget() {
+ Delegator delegator = mock(Delegator.class);
+ when(delegator.getDelegatorName()).thenReturn("default");
+ GenericValue userLogin = mock(GenericValue.class);
+ when(userLogin.getDelegator()).thenReturn(delegator);
+ when(userLogin.getString("userLoginId")).thenReturn("demoadmin");
+
+ HttpServletRequest mintRequest = mock(HttpServletRequest.class);
+ HttpSession mintSession = mock(HttpSession.class);
+ when(mintRequest.getSession()).thenReturn(mintSession);
+ when(mintRequest.getAttribute("userLogin")).thenReturn(userLogin);
+ String key = ExternalLoginKeysManager.getExternalLoginKey(mintRequest);
+
+ // As WidgetWorker/HtmlTreeRenderer see it: a relative inter-app link
target, not a
+ // bare context path.
+ ExternalLoginKeysManager.registerInterAppDestination(key,
"/partymgr/control/viewprofile?partyId=10000");
+
+ ServletContext servletContext = mock(ServletContext.class);
+ when(servletContext.getContextPath()).thenReturn("/partymgr");
+ HttpServletRequest attempt = mock(HttpServletRequest.class);
+ HttpServletResponse attemptResponse = mock(HttpServletResponse.class);
+ HttpSession attemptSession = mock(HttpSession.class);
+ when(attempt.getParameter("externalLoginKey")).thenReturn(key);
+ when(attempt.getServletContext()).thenReturn(servletContext);
+ when(attempt.getAttribute("delegator")).thenReturn(delegator);
+ when(attempt.getSession()).thenReturn(attemptSession);
+
+ try (MockedStatic<LoginWorker> loginWorker =
mockStatic(LoginWorker.class);
+ MockedStatic<EntityUtilProperties> props =
mockStatic(EntityUtilProperties.class)) {
+ loginWorker.when(() -> LoginWorker.checkLogout(any(),
any())).thenReturn(userLogin);
+ props.when(() -> EntityUtilProperties.getPropertyValue(
+ "security", "security.login.externalLoginKey.enabled",
"true", delegator)).thenReturn("true");
+
+ String result =
ExternalLoginKeysManager.checkExternalLoginKey(attempt, attemptResponse);
+
+ assertEquals("success", result);
+ loginWorker.verify(() -> LoginWorker.doBasicLogin(userLogin,
attempt, attemptResponse), times(1));
+ }
+ }
}
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
index e32037954d..4eeed077c7 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
@@ -34,6 +34,7 @@ import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.service.LocalDispatcher;
import org.apache.ofbiz.webapp.control.ConfigXMLReader;
+import org.apache.ofbiz.webapp.control.ExternalLoginKeysManager;
import org.apache.ofbiz.webapp.control.RequestHandler;
import org.apache.ofbiz.webapp.taglib.ContentUrlTag;
import org.apache.ofbiz.widget.model.CommonWidgetModels;
@@ -108,6 +109,7 @@ public final class WidgetWorker {
if (!isAbsoluteTarget(localRequestName)) {
String externalLoginKey = (String)
request.getAttribute("externalLoginKey");
additionalParameters.put("externalLoginKey", externalLoginKey);
+
ExternalLoginKeysManager.registerInterAppDestination(externalLoginKey,
localRequestName);
}
} else if ("content".equals(targetType)) {
uriString = getContentUrl(localRequestName, request);
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/ScreenRenderer.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/ScreenRenderer.java
index 72bf8bf2d4..a6ff3fbd70 100644
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/ScreenRenderer.java
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/ScreenRenderer.java
@@ -35,6 +35,7 @@ import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
import javax.xml.parsers.ParserConfigurationException;
+import org.apache.ofbiz.base.component.ComponentConfig.WebappInfo;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.GeneralException;
import org.apache.ofbiz.base.util.UtilDateTime;
@@ -54,6 +55,7 @@ import org.apache.ofbiz.security.Security;
import org.apache.ofbiz.service.DispatchContext;
import org.apache.ofbiz.service.GenericServiceException;
import org.apache.ofbiz.service.LocalDispatcher;
+import org.apache.ofbiz.webapp.WebAppCache;
import org.apache.ofbiz.webapp.control.ExternalLoginKeysManager;
import org.apache.ofbiz.webapp.control.LoginWorker;
import org.apache.ofbiz.webapp.website.WebSiteWorker;
@@ -308,6 +310,12 @@ public class ScreenRenderer {
boolean externalLoginKeyEnabled =
ExternalLoginKeysManager.isExternalLoginKeyEnabled(request);
if (externalLoginKeyEnabled) {
externalLoginKey =
ExternalLoginKeysManager.getExternalLoginKey(request);
+ // Themes build the app-switcher menu directly in FTL
(string-concatenated hrefs,
+ // bypassing WidgetWorker/RequestHandler entirely), so those
destinations can never be
+ // registered at the point the key is embedded. Pre-register them
here instead, using
+ // the same lookup the templates themselves use, so a key exposed
to a template is
+ // only ever valid for the webapps that app-switcher can actually
link to.
+ registerAppBarDestinations(externalLoginKey, servletContext);
}
String externalKeyParam = externalLoginKey == null ? "" :
"&externalLoginKey=" + externalLoginKey;
context.put("externalLoginKey", externalLoginKey);
@@ -368,6 +376,29 @@ public class ScreenRenderer {
context.push();
}
+ /**
+ * Allows the given key to authenticate into every webapp the app-switcher
menu (built
+ * directly in theme FTL templates, from this same lookup) can link to for
the current
+ * server, so the app-switcher keeps working under the destination-scoped
externalLoginKey.
+ * @param externalLoginKey the minted key, as returned by {@link
ExternalLoginKeysManager#getExternalLoginKey}
+ * @param servletContext the current webapp's servlet context, used to
find the running server's id
+ */
+ private static void registerAppBarDestinations(String externalLoginKey,
ServletContext servletContext) {
+ if (servletContext == null) {
+ return;
+ }
+ String serverId = (String) servletContext.getAttribute("_serverId");
+ if (serverId == null) {
+ return;
+ }
+ WebAppCache webAppCache = WebAppCache.getShared();
+ for (String menuName : UtilMisc.toList("main", "secondary")) {
+ for (WebappInfo webappInfo :
webAppCache.getAppBarWebInfos(serverId, menuName)) {
+ ExternalLoginKeysManager.registerDestination(externalLoginKey,
webappInfo.getContextRoot());
+ }
+ }
+ }
+
/**
* Gets context.
* @return the context
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/html/HtmlTreeRenderer.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/html/HtmlTreeRenderer.java
index 341e157f85..fe68c63fb2 100644
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/html/HtmlTreeRenderer.java
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/html/HtmlTreeRenderer.java
@@ -30,6 +30,7 @@ import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.StringUtil;
import org.apache.ofbiz.base.util.UtilGenerics;
import org.apache.ofbiz.base.util.UtilValidate;
+import org.apache.ofbiz.webapp.control.ExternalLoginKeysManager;
import org.apache.ofbiz.webapp.control.RequestHandler;
import org.apache.ofbiz.webapp.taglib.ContentUrlTag;
import org.apache.ofbiz.widget.WidgetWorker;
@@ -259,6 +260,7 @@ public class HtmlTreeRenderer extends HtmlWidgetRenderer
implements TreeStringRe
writer.append("?externalLoginKey=");
}
writer.append(externalLoginKey);
+
ExternalLoginKeysManager.registerInterAppDestination(externalLoginKey, target);
}
} else {
writer.append(target);
