Author: rjung
Date: Mon Apr 27 18:54:55 2009
New Revision: 769102
URL: http://svn.apache.org/viewvc?rev=769102&view=rev
Log:
BZ 46925: Improve search for nested roles in
JNDIRealm by replacing recursive search with
iterative search. Patch provided by Stefan Zoerner.
Modified:
tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java?rev=769102&r1=769101&r2=769102&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java (original)
+++ tomcat/trunk/java/org/apache/catalina/realm/JNDIRealm.java Mon Apr 27
18:54:55 2009
@@ -144,9 +144,10 @@
* authenticated by setting the <code>commonRole</code> property to the
* name of this role. The role doesn't have to exist in the directory.</li>
*
- * <li>If the directory server contains nested roles, you can search for roles
- * recursively by setting <code>roleRecursionLimit</code> to some positive
value.
- * The default value is <code>0</code>, so role searches do not recurse.</li>
+ * <li>If the directory server contains nested roles, you can search for them
+ * by setting <code>roleNested</code> to <code>true</code>.
+ * The default value is <code>false</code>, so role searches will not find
+ * nested roles.</li>
*
* <li>Note that the standard <code><security-role-ref></code> element in
* the web application deployment descriptor allows applications to refer
@@ -319,14 +320,6 @@
*/
protected MessageFormat[] userPatternFormatArray = null;
-
- /**
- * The maximum recursion depth when resolving roles recursively.
- * By default we don't resolve roles recursively.
- */
- protected int roleRecursionLimit = 0;
-
-
/**
* The base element for role searches.
*/
@@ -364,6 +357,12 @@
* Should we search the entire subtree for matching memberships?
*/
protected boolean roleSubtree = false;
+
+ /**
+ * Should we look for nested group in order to determine roles?
+ */
+ protected boolean roleNested = false;
+
/**
* An alternate URL, to which, we should connect if connectionURL fails.
@@ -654,28 +653,6 @@
/**
- * Return the maximum recursion depth for role searches.
- */
- public int getRoleRecursionLimit() {
-
- return (this.roleRecursionLimit);
-
- }
-
-
- /**
- * Set the maximum recursion depth for role searches.
- *
- * @param roleRecursionLimit The new recursion limit
- */
- public void setRoleRecursionLimit(int roleRecursionLimit) {
-
- this.roleRecursionLimit = roleRecursionLimit;
-
- }
-
-
- /**
* Return the base element for role searches.
*/
public String getRoleBase() {
@@ -765,6 +742,28 @@
this.roleSubtree = roleSubtree;
}
+
+ /**
+ * Return the "The nested group search flag" flag.
+ */
+ public boolean getRoleNested() {
+
+ return (this.roleNested);
+
+ }
+
+
+ /**
+ * Set the "search subtree for roles" flag.
+ *
+ * @param roleNested The nested group search flag
+ */
+ public void setRoleNested(boolean roleNested) {
+
+ this.roleNested = roleNested;
+
+ }
+
/**
@@ -1548,71 +1547,6 @@
return (validated);
}
-
- /**
- * Add roles to a user and search for other roles containing them
themselves.
- * We search recursively with a limited depth.
- * By default the depth is 0, and we only use direct roles.
- * The search needs to use the distinguished role names,
- * but to return the role names.
- *
- * @param depth Recursion depth, starting at zero
- * @param context The directory context we are searching
- * @param recursiveMap The cumulative result map of role names and DNs.
- * @param recursiveSet The cumulative result set of role names.
- * @param groupName The role name to add to the list.
- * @param groupDName The distinguished name of the role.
- *
- * @exception NamingException if a directory server error occurs
- */
- private void getRolesRecursive(int depth, DirContext context, Map<String,
String> recursiveMap, Set<String> recursiveSet,
- String groupName, String groupDName)
throws NamingException {
- if (containerLog.isTraceEnabled())
- containerLog.trace("Recursive search depth " + depth + " for group
'" + groupDName + " (" + groupName + ")'");
- // Adding the given group to the result set if not already found
- if (!recursiveSet.contains(groupDName)) {
- recursiveSet.add(groupDName);
- recursiveMap.put(groupDName, groupName);
- if (depth >= roleRecursionLimit) {
- if (roleRecursionLimit > 0)
- containerLog.warn("Terminating recursive role search
because of recursion limit " +
- roleRecursionLimit + ", results might be
incomplete");
- return;
- }
- // Prepare the parameters for searching groups
- String filter = roleFormat.format(new String[] { groupDName });
- SearchControls controls = new SearchControls();
- controls.setSearchScope(roleSubtree ? SearchControls.SUBTREE_SCOPE
: SearchControls.ONELEVEL_SCOPE);
- controls.setReturningAttributes(new String[] { roleName });
- if (containerLog.isTraceEnabled()) {
- containerLog.trace("Recursive search in role base '" +
roleBase + "' for attribute '" + roleName + "'" +
- " with filter expression '" + filter + "'");
- }
- // Searching groups that assign the given group
- NamingEnumeration<SearchResult> results =
- context.search(roleBase, filter, controls);
- if (results != null) {
- // Iterate over the resulting groups
- try {
- while (results.hasMore()) {
- SearchResult result = results.next();
- Attributes attrs = result.getAttributes();
- if (attrs == null)
- continue;
- String dname = getDistinguishedName(context, roleBase,
result);
- String name = getAttributeValue(roleName, attrs);
- if (name != null && dname != null) {
- getRolesRecursive(depth+1, context, recursiveMap,
recursiveSet, name, dname);
- }
- }
- } catch (PartialResultException ex) {
- if (!adCompat)
- throw ex;
- }
- }
- }
- }
-
/**
* Return a List of roles associated with the given User. Any
* roles present in the user's directory entry are supplemented by
@@ -1656,7 +1590,7 @@
// Are we configured to do role searches?
if ((roleFormat == null) || (roleName == null))
return (list);
-
+
// Set up parameters for an appropriate search
String filter = roleFormat.format(new String[] {
doRFC2254Encoding(dn), username });
SearchControls controls = new SearchControls();
@@ -1693,30 +1627,60 @@
Set<String> keys = groupMap.keySet();
if (containerLog.isTraceEnabled()) {
containerLog.trace(" Found " + keys.size() + " direct roles");
- for (Iterator<String> i = keys.iterator(); i.hasNext();) {
- Object k = i.next();
- containerLog.trace( " Found direct role " + k + " -> " +
groupMap.get(k));
+ for (String key: keys) {
+ containerLog.trace( " Found direct role " + key + " -> " +
groupMap.get(key));
}
}
- HashSet<String> recursiveSet = new HashSet<String>();
- HashMap<String, String> recursiveMap = new HashMap<String, String>();
+ // if nested group search is enabled, perform searches for nested
groups until no new group is found
+ if (getRoleNested()) {
- for (Iterator<String> i = keys.iterator(); i.hasNext();) {
- String k = i.next();
- getRolesRecursive(0, context, recursiveMap, recursiveSet,
groupMap.get(k), k);
- }
+ // The following efficient algorithm is known as memberOf
Algorithm, as described in "Practices in
+ // Directory Groups". It avoids group slurping and handles cyclic
group memberships as well.
+ // See http://middleware.internet2.edu/dir/ for details
+
+ Set<String> newGroupDNs = new HashSet<String>(groupMap.keySet());
+ while (!newGroupDNs.isEmpty()) {
+ Set<String> newThisRound = new HashSet<String>(); // Stores
the groups we find in this iteration
- HashSet<String> resultSet = new HashSet<String>(list);
- resultSet.addAll(recursiveMap.values());
+ for (String groupDN : newGroupDNs) {
+ filter = roleFormat.format(new String[] { groupDN });
- if (containerLog.isTraceEnabled()) {
- containerLog.trace(" Returning " + resultSet.size() + " roles");
- for (Iterator<String> i = resultSet.iterator(); i.hasNext();)
- containerLog.trace( " Found role " + i.next());
+ if (containerLog.isTraceEnabled()) {
+ containerLog.trace("Perform a nested group search with
base "+ roleBase + " and filter " + filter);
+ }
+
+ results = context.search(roleBase, filter, controls);
+
+ try {
+ while (results.hasMore()) {
+ SearchResult result = results.next();
+ Attributes attrs = result.getAttributes();
+ if (attrs == null)
+ continue;
+ String dname = getDistinguishedName(context,
roleBase, result);
+ String name = getAttributeValue(roleName, attrs);
+ if (name != null && dname != null &&
!groupMap.keySet().contains(dname)) {
+ groupMap.put(dname, name);
+ newThisRound.add(dname);
+
+ if (containerLog.isTraceEnabled()) {
+ containerLog.trace(" Found nested role "
+ dname + " -> " + name);
+ }
+
+ }
+ }
+ } catch (PartialResultException ex) {
+ if (!adCompat)
+ throw ex;
+ }
+ }
+
+ newGroupDNs = newThisRound;
+ }
}
- return new ArrayList<String>(resultSet);
+ return new ArrayList<String>(groupMap.values());
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=769102&r1=769101&r2=769102&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Apr 27 18:54:55 2009
@@ -34,6 +34,10 @@
<subsection name="Catalina">
<changelog>
<update>
+ <bug>46925</bug>: Replace recusive search for nested roles with
+ iterative search. Patch provided by Stefan Zoerner. (rjung)
+ </update>
+ <update>
Switch from AnnotationProcessor to InstanceManager. Patch provided by
David Jecks with modifications by Remy. (remm/fhanik)
</update>
@@ -78,7 +82,7 @@
</add>
<fix>
<rev>713953</rev> Include name of attribute when logging failure of
- session attribute serializatyion. (mturk)
+ session attribute serialization. (mturk)
</fix>
<update>
Improve JNDI realm compatability with Active Directory. (rjung)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
