Hi Peter,
Best to bring your comment on the JAMES-1313 so we can keep trace of it:
- For now I have to tag your mail in my folder, but there's a high risk
it will get forgotten...
- You talk about a patch but the mailing list removes attachment. I you
have a patch, you have to upload it via JAMES-1313 and grant ASF for th
rights.
Doing such, the information is centralized and chance to come to a
solution will be higher.
Thx again,
Eric
On 28/03/12 16:42, Peter Kvokacka wrote:
Hello guys
I'm not an LDAP expert. As I wrote, the 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 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 by the search on LDAP with many users, certainly
longer then my previous suggestion, but still I think more efficient
than original implementation.
public User getUserByName(String name) throws UsersRepositoryException {
try {
return searchAndBuildUser(name);
} catch (NamingException e) {
log.error("Unable to retrieve user from ldap", e);
throw new UsersRepositoryException("Unable to retrieve user from ldap", e);
}
}
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("))");
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;
}
Another idea would be to implement some cache mechanism which will cache
result of buildUserCollection() and do LDAP search only if necessary.
Peter
On 26. 3. 2012 14:58, Kevin Dion (Commented) (JIRA) wrote:
[
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
--
eric | http://about.echarles.net | @echarles
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
