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 77a9d92e64 Replace Security.setDelegator with a SecurityProvider SPI
77a9d92e64 is described below
commit 77a9d92e644c4c10e030680439aa78f64803fd32
Author: Mridul Pathak <[email protected]>
AuthorDate: Wed Oct 28 12:17:33 2026 +0530
Replace Security.setDelegator with a SecurityProvider SPI
A Security instance used to be created through its no-argument constructor
and then given its delegator through the deprecated setDelegator method, so
SecurityFactory needed @SuppressWarnings("deprecation") and every
implementation carried a mutable delegator field and a getDelegator method.
SecurityFactory now loads a SecurityProvider through ServiceLoader and asks it
to create the Security instance for a delegator, and both implementations take
the delegator in their constructor and [...]
Implementations register a SecurityProvider in
META-INF/services/org.apache.ofbiz.security.SecurityProvider instead of the
Security class itself, as securityext now does with
OFBizApplicationSecurityProvider. The deprecated getDelegator, setDelegator,
findUserLoginSecurityGroupByUserLoginId and securityGroupPermissionExists
methods are removed from the Security interface; the last two are now private
helpers of the implementations, which were their only callers.
---
.../security/OFBizApplicationSecurity.java | 33 ++++----------
.../security/OFBizApplicationSecurityProvider.java | 35 +++++++++++++++
... => org.apache.ofbiz.security.SecurityProvider} | 2 +-
.../java/org/apache/ofbiz/security/Security.java | 33 +-------------
.../org/apache/ofbiz/security/SecurityFactory.java | 51 +++++++---------------
.../apache/ofbiz/security/SecurityProvider.java | 40 +++++++++++++++++
6 files changed, 101 insertions(+), 93 deletions(-)
diff --git
a/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
index a6e06f4305..891c6a571a 100644
---
a/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
+++
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurity.java
@@ -26,7 +26,6 @@ import java.util.Map;
import jakarta.servlet.http.HttpSession;
-import org.apache.ofbiz.base.util.Assert;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilMisc;
import org.apache.ofbiz.entity.Delegator;
@@ -44,14 +43,16 @@ public class OFBizApplicationSecurity implements Security {
private static final String MODULE =
OFBizApplicationSecurity.class.getName();
- private Delegator delegator = null;
+ private final Delegator delegator;
private static final Map<String, Map<String, String>> SIMPLE_ROLE_ENT =
UtilMisc.toMap(
"ORDERMGR", UtilMisc.<String, String>toMap("name", "OrderRole",
"pkey", "orderId"),
"FACILITY", UtilMisc.<String, String>toMap("name",
"FacilityParty", "pkey", "facilityId"),
"MARKETING", UtilMisc.<String, String>toMap("name",
"MarketingCampaignRole", "pkey", "marketingCampaignId"));
- public OFBizApplicationSecurity() { }
+ public OFBizApplicationSecurity(Delegator delegator) {
+ this.delegator = delegator;
+ }
@Override
public void clearUserData(GenericValue userLogin) {
@@ -61,9 +62,8 @@ public class OFBizApplicationSecurity implements Security {
}
}
- @Override
- @Deprecated
- public Iterator<GenericValue>
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
+ // Uses the UserLoginSecurityGroup cache to find the userLogin's security
groups.
+ private Iterator<GenericValue>
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
try {
List<GenericValue> collection =
EntityUtil.filterByDate(EntityQuery.use(delegator).from("UserLoginSecurityGroup")
.where("userLoginId",
userLoginId).cache(true).queryList());
@@ -74,12 +74,6 @@ public class OFBizApplicationSecurity implements Security {
}
}
- @Override
- @Deprecated
- public Delegator getDelegator() {
- return this.delegator;
- }
-
@Override
public boolean hasEntityPermission(String entity, String action,
GenericValue userLogin) {
if (userLogin == null || entity == null || action == null) return
false;
@@ -224,9 +218,8 @@ public class OFBizApplicationSecurity implements Security {
return hasRolePermission(application, action, primaryKey, role,
userLogin);
}
- @Override
- @Deprecated
- public boolean securityGroupPermissionExists(String groupId, String
permission) {
+ // Caches the groupId, permission pair instead of the userLoginId,
permission pair to keep the cache small.
+ private boolean securityGroupPermissionExists(String groupId, String
permission) {
try {
return EntityQuery.use(delegator).from("SecurityGroupPermission")
.where("groupId", groupId, "permissionId",
permission).cache(true).filterByDate().queryFirst() != null;
@@ -235,14 +228,4 @@ public class OFBizApplicationSecurity implements Security {
return false;
}
}
-
- @Override
- @Deprecated
- public void setDelegator(Delegator delegator) {
- if (this.delegator != null) {
- throw new IllegalStateException("This object has been initialized
already.");
- }
- Assert.notNull("delegator", delegator);
- this.delegator = delegator;
- }
}
diff --git
a/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurityProvider.java
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurityProvider.java
new file mode 100644
index 0000000000..22c9f7f24f
--- /dev/null
+++
b/applications/securityext/src/main/java/org/apache/ofbiz/securityext/security/OFBizApplicationSecurityProvider.java
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ofbiz.securityext.security;
+
+import org.apache.ofbiz.entity.Delegator;
+import org.apache.ofbiz.security.Security;
+import org.apache.ofbiz.security.SecurityProvider;
+
+/**
+ * Creates {@link OFBizApplicationSecurity} instances. Registered in
+ * {@code META-INF/services/org.apache.ofbiz.security.SecurityProvider}.
+ */
+public final class OFBizApplicationSecurityProvider implements
SecurityProvider {
+
+ @Override
+ public Security create(Delegator delegator) {
+ return new OFBizApplicationSecurity(delegator);
+ }
+}
diff --git
a/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.Security
b/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.SecurityProvider
similarity index 91%
rename from
applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.Security
rename to
applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.SecurityProvider
index b412a28997..06ad12597c 100644
---
a/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.Security
+++
b/applications/securityext/src/main/resources/META-INF/services/org.apache.ofbiz.security.SecurityProvider
@@ -15,4 +15,4 @@
# specific language governing permissions and limitations
# under the License.
-org.apache.ofbiz.securityext.security.OFBizApplicationSecurity
+org.apache.ofbiz.securityext.security.OFBizApplicationSecurityProvider
diff --git
a/framework/security/src/main/java/org/apache/ofbiz/security/Security.java
b/framework/security/src/main/java/org/apache/ofbiz/security/Security.java
index d82ac21326..bb0963e28f 100644
--- a/framework/security/src/main/java/org/apache/ofbiz/security/Security.java
+++ b/framework/security/src/main/java/org/apache/ofbiz/security/Security.java
@@ -18,50 +18,21 @@
*******************************************************************************/
package org.apache.ofbiz.security;
-import java.util.Iterator;
import java.util.List;
import jakarta.servlet.http.HttpSession;
-import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.GenericValue;
/**
* Security interface. This interface defines authorization-related methods.
* <p>Implementations must be thread-safe because the instance
* will be shared among multiple threads.</p>
- * <p>Note that the API is changing. New versions might not reference a
- * <code>Delegator</code>. Therefore, implementations requiring a
<code>Delegator</code>
- * reference should get one from the <code>GenericValue</code> method
argument.</p>
+ * <p>Instances are created by a {@link SecurityProvider} and are bound to the
+ * <code>Delegator</code> they were created for.</p>
*/
public interface Security {
- @Deprecated
- Delegator getDelegator();
-
- @Deprecated
- void setDelegator(Delegator delegator);
-
- /**
- * Uses userLoginSecurityGroupByUserLoginId cache to speed up the finding
of the userLogin's security group list.
- * @param userLoginId The userLoginId to find security groups by
- * @return An iterator made from the Collection either cached or retrieved
from the database through the
- * UserLoginSecurityGroup Delegator.
- */
- @Deprecated
- Iterator<GenericValue> findUserLoginSecurityGroupByUserLoginId(String
userLoginId);
-
- /**
- * Finds whether or not a SecurityGroupPermission row exists given a
groupId and permission.
- * The groupId, permission pair is cached instead of the userLoginId,
permission pair to keep the cache small and to
- * make it more changeable.
- * @param groupId The ID of the group
- * @param permission The name of the permission
- * @return boolean specifying whether or not a SecurityGroupPermission row
exists
- */
- @Deprecated
- boolean securityGroupPermissionExists(String groupId, String permission);
-
/**
* Checks to see if the currently logged in userLogin has the passed
permission.
* @param permission Name of the permission to check.
diff --git
a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
index af560418d0..5f1125bf9f 100644
---
a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
+++
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityFactory.java
@@ -43,31 +43,26 @@ import org.apache.ofbiz.entity.util.EntityUtil;
public final class SecurityFactory {
private static final String MODULE = SecurityFactory.class.getName();
- // The default implementation stores a Delegator reference, so we will
cache by delegator name.
- // The goal is to remove Delegator references in the Security interface,
then we can use a singleton
- // and eliminate the cache.
+ // Security instances are bound to a delegator, so they are cached by
delegator name.
private static final UtilCache<String, Security> AUTHORIZATION_CACHE =
UtilCache.createUtilCache("security.AuthorizationCache");
/**
- * Returns a <code>Security</code> instance. The method uses Java's
- * <a
href="http://docs.oracle.com/javase/6/docs/api/java/util/ServiceLoader.html"><code>ServiceLoader</code></a>
- * to get a <code>Security</code> instance. If no instance is found, a
default implementation is used.
- * The default implementation is based on/backed by the OFBiz entity
engine.
+ * Returns the <code>Security</code> instance bound to
<code>delegator</code>. The instance is created by
+ * the first {@link SecurityProvider} that Java's {@link ServiceLoader}
finds. If no provider is found, a
+ * default implementation is used. The default implementation is based
on/backed by the OFBiz entity engine.
* @param delegator The delegator
* @return A <code>Security</code> instance
*/
- @SuppressWarnings("deprecation")
public static Security getInstance(Delegator delegator) throws
SecurityConfigurationException {
Assert.notNull("delegator", delegator);
Security security =
AUTHORIZATION_CACHE.get(delegator.getDelegatorName());
if (security == null) {
- Iterator<Security> iterator =
ServiceLoader.load(Security.class).iterator();
+ Iterator<SecurityProvider> iterator =
ServiceLoader.load(SecurityProvider.class).iterator();
if (iterator.hasNext()) {
- security = iterator.next();
+ security = iterator.next().create(delegator);
} else {
- security = new OFBizSecurity();
+ security = new OFBizSecurity(delegator);
}
- security.setDelegator(delegator);
security =
AUTHORIZATION_CACHE.putIfAbsentAndGet(delegator.getDelegatorName(), security);
if (Debug.verboseOn()) {
Debug.logVerbose("Security implementation " +
security.getClass().getName() + " created for delegator "
@@ -81,9 +76,11 @@ public final class SecurityFactory {
private static final class OFBizSecurity implements Security {
- private Delegator delegator = null;
+ private final Delegator delegator;
- private OFBizSecurity() { }
+ private OFBizSecurity(Delegator delegator) {
+ this.delegator = delegator;
+ }
@Override
public void clearUserData(GenericValue userLogin) {
@@ -93,9 +90,8 @@ public final class SecurityFactory {
}
}
- @Override
- @Deprecated
- public Iterator<GenericValue>
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
+ // Uses the UserLoginSecurityGroup cache to find the userLogin's
security groups.
+ private Iterator<GenericValue>
findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
try {
List<GenericValue> collection =
EntityUtil.filterByDate(EntityQuery.use(delegator).from("UserLoginSecurityGroup")
.where("userLoginId",
userLoginId).cache(true).queryList());
@@ -106,12 +102,6 @@ public final class SecurityFactory {
}
}
- @Override
- @Deprecated
- public Delegator getDelegator() {
- return this.delegator;
- }
-
@Override
public boolean hasEntityPermission(String entity, String action,
GenericValue userLogin) {
if (userLogin == null || entity == null || action == null) return
false;
@@ -238,9 +228,8 @@ public final class SecurityFactory {
return hasRolePermission(application, action, primaryKey, role,
userLogin);
}
- @Override
- @Deprecated
- public boolean securityGroupPermissionExists(String groupId, String
permission) {
+ // Caches the groupId, permission pair instead of the userLoginId,
permission pair to keep the cache small.
+ private boolean securityGroupPermissionExists(String groupId, String
permission) {
try {
return
EntityQuery.use(delegator).from("SecurityGroupPermission")
.where("groupId", groupId, "permissionId",
permission).cache(true).filterByDate().queryFirst() != null;
@@ -249,15 +238,5 @@ public final class SecurityFactory {
return false;
}
}
-
- @Override
- @Deprecated
- public void setDelegator(Delegator delegator) {
- if (this.delegator != null) {
- throw new IllegalStateException("This object has been
initialized already.");
- }
- Assert.notNull("delegator", delegator);
- this.delegator = delegator;
- }
}
}
diff --git
a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityProvider.java
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityProvider.java
new file mode 100644
index 0000000000..f7cf64fdf9
--- /dev/null
+++
b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityProvider.java
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ofbiz.security;
+
+import org.apache.ofbiz.entity.Delegator;
+
+/**
+ * Service provider interface that creates {@link Security} instances.
+ * <p>An implementation is registered in a
+ * {@code META-INF/services/org.apache.ofbiz.security.SecurityProvider} file
and
+ * discovered by {@link SecurityFactory} through {@link
java.util.ServiceLoader},
+ * so it must have a public no-argument constructor.</p>
+ */
+public interface SecurityProvider {
+
+ /**
+ * Creates a <code>Security</code> instance bound to the given delegator.
+ * <p>{@link SecurityFactory} caches the returned instance per delegator
and
+ * shares it among threads.</p>
+ * @param delegator The delegator the instance reads security data from,
never <code>null</code>
+ * @return A new <code>Security</code> instance
+ */
+ Security create(Delegator delegator);
+}