You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Robert Munn <ro...@gmail.com> on 2016/03/11 21:36:25 UTC
LDAP users repository feature deprecation
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
Re: LDAP users repository feature deprecation
Posted by Ioan Eugen Stan <st...@gmail.com>.
Great. How can we help you integrate it?
Regards,
2016-03-11 22:36 GMT+02:00 Robert Munn <ro...@gmail.com>:
> 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
>
>
>
--
Ioan Eugen Stan
0720 898 747
Re: LDAP users repository feature deprecation
Posted by Raphaël Ouazana-Sustowski <ra...@linagora.com>.
You could also provide the parameter of a search request that checks the
user is belonging (or not) to a group.
Regards,
Raphaël Ouazana.
Le 2016-03-14 17:55, Robert Munn a écrit :
> 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
>> <ra...@linagora.com> 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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: LDAP users repository feature deprecation
Posted by Robert Munn <ro...@gmail.com>.
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 <ra...@linagora.com> 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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: LDAP users repository feature deprecation
Posted by Raphaël Ouazana-Sustowski <ra...@linagora.com>.
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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org