Author: jaz
Date: Wed Apr 29 22:08:07 2009
New Revision: 769963
URL: http://svn.apache.org/viewvc?rev=769963&view=rev
Log:
Added check to prevent recursive permission lookup (when implementing dynamic
access logic using services) - JIRA OFBIZ-2381
Modified:
ofbiz/trunk/applications/securityext/servicedef/services.xml
ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java
ofbiz/trunk/applications/securityext/testdef/da/DynamicAccessTest.xml
ofbiz/trunk/applications/securityext/testdef/data/SecurityTestData.xml
ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java
Modified: ofbiz/trunk/applications/securityext/servicedef/services.xml
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/securityext/servicedef/services.xml?rev=769963&r1=769962&r2=769963&view=diff
==============================================================================
--- ofbiz/trunk/applications/securityext/servicedef/services.xml (original)
+++ ofbiz/trunk/applications/securityext/servicedef/services.xml Wed Apr 29
22:08:07 2009
@@ -138,4 +138,8 @@
location="component://securityext/testdef/da/DynamicAccessTest.xml"
invoke="testDa">
<implements service="dynamicAccessInterface"/>
</service>
+ <service name="dynamicAccessRecursiveTest" engine="simple" auth="false"
+
location="component://securityext/testdef/da/DynamicAccessTest.xml"
invoke="testDaRecursion">
+ <implements service="dynamicAccessInterface"/>
+ </service>
</services>
Modified:
ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java?rev=769963&r1=769962&r2=769963&view=diff
==============================================================================
---
ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java
(original)
+++
ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/test/AuthorizationTests.java
Wed Apr 29 22:08:07 2009
@@ -60,4 +60,9 @@
Debug.logInfo("Running testAutoGrantCleanup()", module);
assertFalse("User was auto-granted an unexpected permission",
security.hasPermission("user", "test:autogranted", null, true));
}
+
+ public void testDynamicAccessRecursion() throws Exception {
+ Debug.logInfo("Running testDynamicAccessRecursion()", module);
+ assertFalse("User was granted an unexpected permission",
security.hasPermission("user", "test:recursion", null, true));
+ }
}
Modified: ofbiz/trunk/applications/securityext/testdef/da/DynamicAccessTest.xml
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/securityext/testdef/da/DynamicAccessTest.xml?rev=769963&r1=769962&r2=769963&view=diff
==============================================================================
--- ofbiz/trunk/applications/securityext/testdef/da/DynamicAccessTest.xml
(original)
+++ ofbiz/trunk/applications/securityext/testdef/da/DynamicAccessTest.xml Wed
Apr 29 22:08:07 2009
@@ -32,4 +32,12 @@
</if-compare>
<field-to-result field="permissionGranted"/>
</simple-method>
+
+ <simple-method method-name="testDaRecursion" short-description="Test
recusion" login-required="false">
+ <set field="permissionGranted" value="false" type="Boolean"/>
+ <if-has-permission permission="test:recursion">
+ <set field="permissionGranted" value="true" type="Boolean"/>
+ </if-has-permission>
+ <field-to-result field="permissionGranted"/>
+ </simple-method>
</simple-methods>
\ No newline at end of file
Modified: ofbiz/trunk/applications/securityext/testdef/data/SecurityTestData.xml
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/securityext/testdef/data/SecurityTestData.xml?rev=769963&r1=769962&r2=769963&view=diff
==============================================================================
--- ofbiz/trunk/applications/securityext/testdef/data/SecurityTestData.xml
(original)
+++ ofbiz/trunk/applications/securityext/testdef/data/SecurityTestData.xml Wed
Apr 29 22:08:07 2009
@@ -22,6 +22,7 @@
<SecurityPermission permissionId="test:groovy1"
dynamicAccess="component://securityext/testdef/da/DaTest1.groovy"/>
<SecurityPermission permissionId="test:groovy2"
dynamicAccess="org.ofbiz.securityext.test.DaTest2.groovy"/>
<SecurityPermission permissionId="test:service"
dynamicAccess="service:dynamicAccessTestService"/>
+ <SecurityPermission permissionId="test:recursion"
dynamicAccess="service:dynamicAccessRecursiveTest"/>
<SecurityPermission permissionId="test:autogranted" dynamicAccess=""/>
<SecurityPermissionAutoGrant permissionId="test:groovy1"
grantPermission="test:autogranted"/>
</entity-engine-xml>
\ No newline at end of file
Modified:
ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java?rev=769963&r1=769962&r2=769963&view=diff
==============================================================================
---
ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java
(original)
+++
ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/AbtractAuthorization.java
Wed Apr 29 22:08:07 2009
@@ -17,6 +17,7 @@
* Used to manage Auto-Grant permissions for the current "request"
*/
private static ThreadLocal<List<String>> autoGrant = new
ThreadLocal<List<String>>();
+ private static ThreadLocal<String> origPermission = new
ThreadLocal<String>();
private static ThreadLocal<String> uid = new ThreadLocal<String>();
/**
@@ -69,18 +70,26 @@
// verify the ThreadLocal data; make sure it isn't stale (from
a thread pool)
String threadUid = uid.get();
- if (!userId.equals(threadUid)) {
+ if (threadUid != null && !userId.equals(threadUid)) {
+ origPermission.remove();
autoGrant.remove();
uid.remove();
+ threadUid = null;
}
-
+
+ // set the tracking values on thread local
+ if (UtilValidate.isEmpty(threadUid)) {
+ origPermission.set(permission);
+ uid.set(userId);
+ }
+
// split the permission string; so we can walk up the levels
String[] permSplit = expandedPermission.split(":");
StringBuffer joined = new StringBuffer();
int index = 1;
if (permSplit != null && permSplit.length > 1) {
- if (Debug.verboseOn()) Debug.logVerbose("Security 2.0
schema found -- walking tree : " + expandedPermission, module);
+ if (Debug.infoOn()) Debug.logInfo("Security 2.0 schema
found -- walking tree : " + expandedPermission, module);
// start walking
for (String perm : permSplit) {
if (permSplit.length >= index) {
@@ -112,10 +121,15 @@
}
// finally check dynamic permission (outside the loop)
- if (hasDynamicPermission(userId, expandedPermission, context)) {
- // permission granted
- handleAutoGrantPermissions(userId, expandedPermission,
context);
- return true;
+ String threadPerm = origPermission.get();
+ if (!permission.equals(threadPerm)) {
+ if (hasDynamicPermission(userId, expandedPermission,
context)) {
+ // permission granted
+ handleAutoGrantPermissions(userId,
expandedPermission, context);
+ return true;
+ }
+ } else {
+ Debug.logWarning("Recursive permission check detected; do
not call hasPermission() from a dynamic access implementation!", module);
}
} else {
// legacy mode; only call static permission check; no auto
grants
@@ -140,8 +154,7 @@
alreadyGranted.add(FlexibleStringExpander.expandString(toGrant, context));
}
}
- autoGrant.set(granted);
- uid.set(userId);
+ autoGrant.set(granted);
}
}
}
