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 bff9876270 Fixed: security.login.externalLoginKey.enabled never
checked by checkExternalLoginKey() and cannot be changed live (OFBIZ-13519)
bff9876270 is described below
commit bff9876270d806cae0fb6450fd5d4530c6fe59a2
Author: Mridul Pathak <[email protected]>
AuthorDate: Thu Sep 3 18:06:54 2026 +0530
Fixed: security.login.externalLoginKey.enabled never checked by
checkExternalLoginKey() and cannot be changed live (OFBIZ-13519)
isExternalLoginKeyEnabled() cached the property in a static field on first
read and never re-read it, so any later change made no difference until a
process restart. Separately, checkExternalLoginKey() -- the consumption path
wired into the default preprocessor chain used by most controller.xml files --
never called isExternalLoginKeyEnabled() at all; the flag was only ever checked
at the two key-generation call sites. Together this meant an admin disabling
the property got no change [...]
---
.../webapp/control/ExternalLoginKeysManager.java | 19 +++++++++----------
.../webapp/control/ExternalLoginKeysManagerTests.java | 12 ++++++++++--
2 files changed, 19 insertions(+), 12 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 46d64475b2..49dc13f6a9 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,7 +28,6 @@ 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;
@@ -54,9 +53,6 @@ public class ExternalLoginKeysManager {
// rejecting a second redemption against the same webapp.
private static final Map<String, ExternalLoginTicket> EXTERNAL_LOGIN_KEYS
= new ConcurrentHashMap<>();
- // This variable is set to empty so we know need to read from the
properties file.
- private static String isExternalLoginKeyEnabled = "";
-
/**
* 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
@@ -164,6 +160,12 @@ public class ExternalLoginKeysManager {
String externalKey = request.getParameter(EXTERNAL_LOGIN_KEY_ATTR);
if (externalKey == null) return "success";
+ if (!isExternalLoginKeyEnabled(request)) {
+ // Feature disabled: don't even look up the ticket, so nothing
about it is consumed.
+ LoginWorker.autoLoginSet(request, response);
+ return "success";
+ }
+
// Look up without removing: the same key is shared across every
cross-webapp link one
// render emits, so it must stay valid for whichever *other*
destination webapps the
// user still hasn't visited yet. redeemFor(), below, is what actually
stops replay --
@@ -239,12 +241,9 @@ public class ExternalLoginKeysManager {
* @return
*/
public static boolean isExternalLoginKeyEnabled(HttpServletRequest
request) {
- if (UtilValidate.isEmpty(isExternalLoginKeyEnabled)) {
- isExternalLoginKeyEnabled =
EntityUtilProperties.getPropertyValue("security",
- "security.login.externalLoginKey.enabled", "true",
- (Delegator) request.getAttribute("delegator"));
- }
- return "true".equals(isExternalLoginKeyEnabled);
+ return "true".equals(EntityUtilProperties.getPropertyValue("security",
+ "security.login.externalLoginKey.enabled", "true",
+ (Delegator) request.getAttribute("delegator")));
}
}
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 7976bcb669..2ac8fb7e26 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
@@ -35,6 +35,7 @@ import jakarta.servlet.http.HttpSession;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.GenericValue;
+import org.apache.ofbiz.entity.util.EntityUtilProperties;
import org.junit.jupiter.api.Test;
import org.mockito.MockedStatic;
@@ -94,7 +95,11 @@ public class ExternalLoginKeysManagerTests {
HttpServletResponse response = mock(HttpServletResponse.class);
when(request.getParameter("externalLoginKey")).thenReturn("ELunknown-key-not-in-map");
- try (MockedStatic<LoginWorker> loginWorker =
mockStatic(LoginWorker.class)) {
+ 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(request, response);
assertEquals("success", result);
@@ -123,8 +128,11 @@ public class ExternalLoginKeysManagerTests {
ServletContext servletContext = mock(ServletContext.class);
when(servletContext.getContextPath()).thenReturn("/partymgr");
- try (MockedStatic<LoginWorker> loginWorker =
mockStatic(LoginWorker.class)) {
+ 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");
// First redemption: a cookie-less client presents the freshly
minted key.
HttpServletRequest firstUse = mock(HttpServletRequest.class);
