OK, that’s the part I didn’t understand. So in those situations there is no alternative to returning the list of group members from the LDAP store. A hash table of userDNs would provide a quicker search than the iterator over the collection. I haven’t looked at the code in enough detail, though. Is ReadOnlyUsersLDAPRepository a Singleton? Could someone recommend a place in the code for me to start to see how the authentication system fits together?
Robert > On Mar 14, 2016, at 7:37 AM, Raphaël Ouazana-Sustowski > <[email protected]> wrote: > > 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]
