Hi,

I agree with you. This search is suboptimal.
But the memberOf attribute is not really available on every LDAP directory. An other way should be find to allow to restric on a group.

Regards,
Raphaël Ouazana.

Le 2016-03-11 21:36, Robert Munn a écrit :
Hi all. I’ve been using James for awhile and have made a couple of
patches in the code that have been put into the trunk.

I would like to suggest that the “restriction" attribute of the LDAP
users repository be deprecated in favor of the “filter” attribute I
added.  O(1) v. O(n) performance is the reason I am suggesting that
the restriction attribute be deprecated. Sorry if I am going about
this wrong, I would just hate for new James implementors to by hobbled
by unnecessary performance limitations.


If you look at the source code in ReadOnlyUsersLDAPRepository.java,
you can see the flaws in the restriction attribute:

1. It only works with LDAP groups, it provides no other way to filter users. 2. Algorithm for filtering by group is O(n) for n number of users in the group.



By comparison, the filter attribute takes advantage of the LDAP
server’s internal query mechanism, so it’s advantages are:

1. Can filter by any valid filter string supported by the LDAP server.
Group membership is supported, but so could any other restriction
desired, such as active flag, etc.
2. Search algorithm is O(1) for filtering a user by group.

Essentially, if you use the filter attribute to filter by group, you
can skip the iterator code associated with the restriction attribute.
A filter string by group in my implementation looks like this:

(objectClass=inetOrgPerson)(memberOf=cn=EmailUsers,ou=groups,DC=domain,DC=com)

which is basically what you would put in the memberAttribute and group
attribute of restriction. Testing a valid filter string against LDAP
is easy using any standard LDAP query tool.



I have attached some of the code from ReadOnlyUsersLDAPRepository.java
to illustrate what I am saying.


        // config for restriction

        HierarchicalConfiguration restrictionConfig = null;
        // Check if we have a restriction we can use
        // See JAMES-1204
if (configuration.containsKey("restriction[@memberAttribute]")) { restrictionConfig = configuration.configurationAt("restriction");
        }
restriction = new ReadOnlyLDAPGroupRestriction(restrictionConfig);

       //config for filter

       filter = configuration.getString("[@filter]”);

    // main LDAP query
    private ReadOnlyLDAPUser searchAndBuildUser(String name) throws
NamingException {
      SearchControls sc = new SearchControls();
      sc.setSearchScope(SearchControls.SUBTREE_SCOPE);
      sc.setReturningAttributes(new String[] { userIdAttribute });
      sc.setCountLimit(1);

      StringBuilder builderFilter = new StringBuilder("(&(");
builderFilter.append(userIdAttribute).append("=").append(name).append(")") .append("(objectClass=").append(userObjectClass).append(")");

     if(StringUtils.isNotEmpty(filter)){
         builderFilter.append(filter).append(")");
         }
     else{
         builderFilter.append(")");
     }

      NamingEnumeration<SearchResult> sr =
ldapContext.search(userBase, builderFilter.toString(),
          sc);

      if (!sr.hasMore())
        return null;

      SearchResult r = sr.next();
      Attribute userName = r.getAttributes().get(userIdAttribute);

      if (!restriction.isActivated()
          || userInGroupsMembershipList(r.getNameInNamespace(),
restriction.getGroupMembershipLists(ldapContext)))
        return new ReadOnlyLDAPUser(userName.get().toString(),
r.getNameInNamespace(), ldapContext);

      return null;
    }

    // restriction filtering algorithm
    private boolean userInGroupsMembershipList(String userDN,
            Map<String, Collection<String>> groupMembershipList) {
        boolean result = false;

        Collection<Collection<String>> memberLists =
groupMembershipList.values();
        Iterator<Collection<String>> memberListsIterator =
memberLists.iterator();

        while (memberListsIterator.hasNext() && !result) {
Collection<String> groupMembers = memberListsIterator.next();
            result = groupMembers.contains(userDN);
        }

        return result;
    }



In addition, I just wanted to say thanks to everyone who has
contributed to James. It’s an awesome project made better by all of
your efforts.

Robert

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to