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 "Peter Kvokacka (JIRA)" <se...@james.apache.org> on 2011/09/12 12:59:09 UTC
[jira] [Created] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
----------------------------------------------------------------------------------------------------
Key: JAMES-1313
URL: https://issues.apache.org/jira/browse/JAMES-1313
Project: JAMES Server
Issue Type: Improvement
Components: UsersStore & UsersRepository
Affects Versions: 3.0-beta3
Reporter: Peter Kvokacka
Priority: Minor
Fix For: 3.0-beta3
Hello
I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all user in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
... X more, where X is size-1 of userBase subtree
I suggest a patch that (at least in my case) does simple search instead:
<repository name="LocalUsers"
class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
ldapHost="ldaps://ldap.mycomapny.local:1636"
principal="cn=admin"
credentials="***"
userBase="ou=people,dc=mycompany,dc=sk"
userIdAttribute="uid"
userObjectClass="inetOrgPerson"/>
SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
Peter
Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
===================================================================
--- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
+++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
@@ -351,16 +351,23 @@
* Propagated by the underlying LDAP communication layer.
*/
private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
- ReadOnlyLDAPUser result;
- Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
+ SearchControls sc = new SearchControls();
+ sc.setSearchScope(SearchControls.OBJECT_SCOPE);
+ sc.setReturningAttributes(new String[] {userIdAttribute});
+ sc.setCountLimit(1);
+
+ NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
+
+ if (!sr.hasMore())
+ return null;
+
+ Attributes userAttributes = sr.next().getAttributes();
Attribute userName = userAttributes.get(userIdAttribute);
+
+ return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
+ }
- result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
-
- return result;
- }
-
/*
* (non-Javadoc)
*
@@ -425,23 +432,14 @@
*/
public User getUserByName(String name) throws UsersRepositoryException {
try {
- Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
- while (userIt.hasNext()) {
- ReadOnlyLDAPUser u = userIt.next();
- if (u.getUserName().equals(name)) {
- return u;
- }
- }
-
+ return buildUser(userIdAttribute + "=" + name + "," + userBase);
} catch (NamingException e) {
log.error("Unable to retrieve user from ldap", e);
throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
-
+
}
- return null;
+ }
- }
-
/*
* (non-Javadoc)
*
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Resolved] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Norman Maurer (Resolved) (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Norman Maurer resolved JAMES-1313.
----------------------------------
Resolution: Fixed
Fix Version/s: (was: 3.0-beta3)
3.0-beta4
Assignee: Norman Maurer
Thanks again for the patch. I just committed it. Sorry for taking so long but I lost track of it :(
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Assignee: Norman Maurer
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta4
>
> Attachments: ReadOnlyUsersLDAPRepository.java.patch
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Commented] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Norman Maurer (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13102640#comment-13102640 ]
Norman Maurer commented on JAMES-1313:
--------------------------------------
Could you upload the patch as diff and check the ASF grant box ?
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta3
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Updated] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Peter Kvokacka (Updated) (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Peter Kvokacka updated JAMES-1313:
----------------------------------
Attachment: ReadOnlyUsersLDAPRepository.java.for3.0beta4_wholeFile
same functionality as in ReadOnlyUsersLDAPRepository.java.2.patch just in 3.0 beta4 but I couldn't make patch because I don't have all compile dependencies in 3.0 beta4 sorry guys don't have that much time to get beta4 works
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Assignee: Norman Maurer
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta4
>
> Attachments: ReadOnlyUsersLDAPRepository.java.2.patch, ReadOnlyUsersLDAPRepository.java.for3.0beta4_wholeFile, ReadOnlyUsersLDAPRepository.java.patch
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Commented] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Kevin Dion (Commented) (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13238345#comment-13238345 ]
Kevin Dion commented on JAMES-1313:
-----------------------------------
The restriction added by requiring the userIdAttribute to be the first part of the DN is overly restrictive, at least in some Windows environments. In Microsoft AD LDAPs, the dn is usually of the form 'cn=Charlie Brown,dc=domain,dc=org' where cn is the 'Common', or full name of the individual. The mail username would usually be specified elsewhere in the user object, and the flexibility of allowing for attributes other than the dn for login is essential in many instances.
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Assignee: Norman Maurer
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta4
>
> Attachments: ReadOnlyUsersLDAPRepository.java.patch
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Updated] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Peter Kvokacka (Updated) (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Peter Kvokacka updated JAMES-1313:
----------------------------------
Attachment: ReadOnlyUsersLDAPRepository.java.2.patch
patch created for 3.0 beta3 because I don't have time to get 3.0 beta4 work
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Assignee: Norman Maurer
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta4
>
> Attachments: ReadOnlyUsersLDAPRepository.java.2.patch, ReadOnlyUsersLDAPRepository.java.patch
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Commented] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Eric Charles (Commented) (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13244145#comment-13244145 ]
Eric Charles commented on JAMES-1313:
-------------------------------------
Peter patch.2 committed in http://svn.apache.org/viewvc?rev=1308311&view=rev.
@Kevin Could test test and tell us if it succeeds for you?
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Assignee: Norman Maurer
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta4
>
> Attachments: ReadOnlyUsersLDAPRepository.java.2.patch, ReadOnlyUsersLDAPRepository.java.for3.0beta4_wholeFile, ReadOnlyUsersLDAPRepository.java.patch
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Updated] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Peter Kvokacka (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Peter Kvokacka updated JAMES-1313:
----------------------------------
Description:
Hello
I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
... X more, where X is size-1 of userBase subtree
I suggest a patch that (at least in my case) does simple search instead:
<repository name="LocalUsers"
class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
ldapHost="ldaps://ldap.mycomapny.local:1636"
principal="cn=admin"
credentials="***"
userBase="ou=people,dc=mycompany,dc=sk"
userIdAttribute="uid"
userObjectClass="inetOrgPerson"/>
SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
Peter
Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
===================================================================
--- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
+++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
@@ -351,16 +351,23 @@
* Propagated by the underlying LDAP communication layer.
*/
private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
- ReadOnlyLDAPUser result;
- Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
+ SearchControls sc = new SearchControls();
+ sc.setSearchScope(SearchControls.OBJECT_SCOPE);
+ sc.setReturningAttributes(new String[] {userIdAttribute});
+ sc.setCountLimit(1);
+
+ NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
+
+ if (!sr.hasMore())
+ return null;
+
+ Attributes userAttributes = sr.next().getAttributes();
Attribute userName = userAttributes.get(userIdAttribute);
+
+ return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
+ }
- result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
-
- return result;
- }
-
/*
* (non-Javadoc)
*
@@ -425,23 +432,14 @@
*/
public User getUserByName(String name) throws UsersRepositoryException {
try {
- Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
- while (userIt.hasNext()) {
- ReadOnlyLDAPUser u = userIt.next();
- if (u.getUserName().equals(name)) {
- return u;
- }
- }
-
+ return buildUser(userIdAttribute + "=" + name + "," + userBase);
} catch (NamingException e) {
log.error("Unable to retrieve user from ldap", e);
throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
-
+
}
- return null;
+ }
- }
-
/*
* (non-Javadoc)
*
was:
Hello
I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all user in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
... X more, where X is size-1 of userBase subtree
I suggest a patch that (at least in my case) does simple search instead:
<repository name="LocalUsers"
class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
ldapHost="ldaps://ldap.mycomapny.local:1636"
principal="cn=admin"
credentials="***"
userBase="ou=people,dc=mycompany,dc=sk"
userIdAttribute="uid"
userObjectClass="inetOrgPerson"/>
SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
Peter
Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
===================================================================
--- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
+++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
@@ -351,16 +351,23 @@
* Propagated by the underlying LDAP communication layer.
*/
private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
- ReadOnlyLDAPUser result;
- Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
+ SearchControls sc = new SearchControls();
+ sc.setSearchScope(SearchControls.OBJECT_SCOPE);
+ sc.setReturningAttributes(new String[] {userIdAttribute});
+ sc.setCountLimit(1);
+
+ NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
+
+ if (!sr.hasMore())
+ return null;
+
+ Attributes userAttributes = sr.next().getAttributes();
Attribute userName = userAttributes.get(userIdAttribute);
+
+ return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
+ }
- result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
-
- return result;
- }
-
/*
* (non-Javadoc)
*
@@ -425,23 +432,14 @@
*/
public User getUserByName(String name) throws UsersRepositoryException {
try {
- Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
- while (userIt.hasNext()) {
- ReadOnlyLDAPUser u = userIt.next();
- if (u.getUserName().equals(name)) {
- return u;
- }
- }
-
+ return buildUser(userIdAttribute + "=" + name + "," + userBase);
} catch (NamingException e) {
log.error("Unable to retrieve user from ldap", e);
throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
-
+
}
- return null;
+ }
- }
-
/*
* (non-Javadoc)
*
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta3
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Updated] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Peter Kvokacka (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Peter Kvokacka updated JAMES-1313:
----------------------------------
Attachment: ReadOnlyUsersLDAPRepository.java.patch
I checked out trunk version of server/ldap and created the patch in eclipse. Is it ok or do you need it in different format? I've added a check for restriction in buildUser() method.
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta3
>
> Attachments: ReadOnlyUsersLDAPRepository.java.patch
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[jira] [Commented] (JAMES-1313) more effective getUserByName(String
name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
Posted by "Peter Kvokacka (Commented) (JIRA)" <se...@james.apache.org>.
[ https://issues.apache.org/jira/browse/JAMES-1313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13244140#comment-13244140 ]
Peter Kvokacka commented on JAMES-1313:
---------------------------------------
I'm not an LDAP expert. As I wrote, the previous patch I provided, works fine for me and has the restriction. As it seems it doesn't work for Kevin, I made quick changes to support his case as well. I tested it quickly on my james 3.0 beta3 setup and it works. I suggest these changes to beta4 code base. I don't have time to test it on beta4, because to get beta4 work can take some time, I suppose, because of many changes between beta3 and beta4.
The main idea of the patch is to replace search with potencialy huge result-set (all user in your userBase) with single search for one user. Therefore I created new method searchAndBuildUser() instead of buildUser() and I used it in getUserByName(). It makes ldap search like this
ldapContext.search(userBase,"(&(${userIdAttribute}=${name})(objectClass=${userObjectClass}))")
where userBase, userIdAttribure and userObjectClass are attributes from configuration and name is "mail user name" as Kevin named it. I'm not sure how quick would be the search on LDAP with many users, certainly longer than my previous suggestion, but still I think more efficient than original implementation.
I also return previous implementation of buildUser() as it is used in buildUserCollection() in case of listing all users where the previous implementation is perfectly fine.
Peter
> more effective getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository
> ----------------------------------------------------------------------------------------------------
>
> Key: JAMES-1313
> URL: https://issues.apache.org/jira/browse/JAMES-1313
> Project: JAMES Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: 3.0-beta3
> Reporter: Peter Kvokacka
> Assignee: Norman Maurer
> Priority: Minor
> Labels: patch
> Fix For: 3.0-beta4
>
> Attachments: ReadOnlyUsersLDAPRepository.java.2.patch, ReadOnlyUsersLDAPRepository.java.for3.0beta4_wholeFile, ReadOnlyUsersLDAPRepository.java.patch
>
>
> Hello
> I'd like to use james in my current project, but I find LDAP implementation of usersRepository to be not very effective.
> Especially method getUserByName(String name) in org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository seems to search all users in LDAP with userBase and after that it goes through the result in memory and looking for specific user. Which produce search like this with potencialy big resultset:
> SEARCH REQ conn=26 op=6 msgID=7 base="ou=people,dc=mycompany,dc=sk" scope=wholeSubtree filter="(objectClass=inetOrgPerson)" attrs="distinguishedName"
> SEARCH RES conn=26 op=6 msgID=7 result=0 nentries=438 etime=169
> SEARCH REQ conn=26 op=7 msgID=8 base="uid=somebody,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=*)" attrs="ALL"
> SEARCH RES conn=26 op=7 msgID=8 result=0 nentries=1 etime=1
> ... X more, where X is size-1 of userBase subtree
> I suggest a patch that (at least in my case) does simple search instead:
> <repository name="LocalUsers"
> class="org.apache.james.user.ldap.ReadOnlyUsersLDAPRepository"
> ldapHost="ldaps://ldap.mycomapny.local:1636"
> principal="cn=admin"
> credentials="***"
> userBase="ou=people,dc=mycompany,dc=sk"
> userIdAttribute="uid"
> userObjectClass="inetOrgPerson"/>
> SEARCH REQ conn=26 op=1 msgID=2 base="uid=test0123,ou=people,dc=mycompany,dc=sk" scope=baseObject filter="(objectClass=inetOrgPerson)" attrs="uid"
> SEARCH RES conn=26 op=1 msgID=2 result=0 nentries=1 etime=1
> There is only one assumption that distinguishedName for each entry in userBase is "userIdAttribute=$name,userBase", where $name is username. I don't think of it as of a strong restriction, but you should consider that and decide for yourself. It works just fine for me.
> Also it looks like getUserByNameCaseInsensitive(String name) is not used anywhere, so you can stick with current implementation for now.
> Peter
> Index: src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java
> ===================================================================
> --- src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (revision 1169673)
> +++ src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java (working copy)
> @@ -351,16 +351,23 @@
> * Propagated by the underlying LDAP communication layer.
> */
> private ReadOnlyLDAPUser buildUser(String userDN) throws NamingException {
> - ReadOnlyLDAPUser result;
>
> - Attributes userAttributes = ldapConnection.getLdapContext().getAttributes(userDN);
> + SearchControls sc = new SearchControls();
> + sc.setSearchScope(SearchControls.OBJECT_SCOPE);
> + sc.setReturningAttributes(new String[] {userIdAttribute});
> + sc.setCountLimit(1);
> +
> + NamingEnumeration<SearchResult> sr = ldapConnection.getLdapContext().search(userDN, "(objectClass=" + userObjectClass + ")", sc);
> +
> + if (!sr.hasMore())
> + return null;
> +
> + Attributes userAttributes = sr.next().getAttributes();
> Attribute userName = userAttributes.get(userIdAttribute);
> +
> + return new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> + }
>
> - result = new ReadOnlyLDAPUser(userName.get().toString(), userDN, ldapHost);
> -
> - return result;
> - }
> -
> /*
> * (non-Javadoc)
> *
> @@ -425,23 +432,14 @@
> */
> public User getUserByName(String name) throws UsersRepositoryException {
> try {
> - Iterator<ReadOnlyLDAPUser> userIt = buildUserCollection(getValidUsers()).iterator();
> - while (userIt.hasNext()) {
> - ReadOnlyLDAPUser u = userIt.next();
> - if (u.getUserName().equals(name)) {
> - return u;
> - }
> - }
> -
> + return buildUser(userIdAttribute + "=" + name + "," + userBase);
> } catch (NamingException e) {
> log.error("Unable to retrieve user from ldap", e);
> throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
> -
> +
> }
> - return null;
> + }
>
> - }
> -
> /*
> * (non-Javadoc)
> *
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org