commit 481e028e888283e1b2b930f25caf9be713a652b3
author Shawn O. Pearce <sop@google.com> Wed Nov 04 07:31:41 2009 -0800
committer Shawn O. Pearce <sop@google.com> Wed Nov 04 07:31:41 2009 -0800
tree 51bec602daa505709e6d61ca64f28d7f14a2f7cd
parent 1271771f94aa2bc7aa31ecface6dc52975e84d8e

Fix LDAP account lookup when user not in group

If a user is not a member of any group and the backing LDAP server
is an Active Directory instance we won't receive a 'memberOf'
attribute in the user's account data.  Instead of crashing with
NPE we should handle the case as though the list was empty.

I cleaned up the LdapQuery.Result code a little while checking the
other uses of data obtained from the directory, as we should always
handle a missing attribute gracefully.

Change-Id: Id2f030815bd597dcba5712f7def5d10818b4eb79
Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/server/ldap/LdapQuery.java

diff --git a/src/main/java/com/google/gerrit/server/ldap/LdapQuery.java b/src/main/java/com/google/gerrit/server/ldap/LdapQuery.java
index 33a2fad..3055fe6 100644
--- a/src/main/java/com/google/gerrit/server/ldap/LdapQuery.java
+++ b/src/main/java/com/google/gerrit/server/ldap/LdapQuery.java
@@ -80,29 +80,25 @@
 
   class Result {
     private final Map<String, Attribute> atts = new HashMap<String, Attribute>();
-    private final Map<String, String> attsSingle = new HashMap<String, String>();
 
-    Result(final SearchResult sr) throws NamingException {
+    Result(final SearchResult sr) {
       if (returnAttributes != null) {
         for (final String attName : returnAttributes) {
           final Attribute a = sr.getAttributes().get(attName);
           if (a != null && a.size() > 0) {
             atts.put(attName, a);
-            attsSingle.put(attName, String.valueOf(a.get(0)));
           }
         }
+
       } else {
         NamingEnumeration<? extends Attribute> e = sr.getAttributes().getAll();
         while (e.hasMoreElements()) {
           final Attribute a = e.nextElement();
           atts.put(a.getID(), a);
-          if (a.size() == 1) {
-            attsSingle.put(a.getID(), String.valueOf(a.get(0)));
-          }
         }
       }
+
       atts.put("dn", new BasicAttribute("dn", sr.getNameInNamespace()));
-      attsSingle.put("dn", sr.getNameInNamespace());
     }
 
     String getDN() throws NamingException {
@@ -110,15 +106,16 @@
     }
 
     String get(final String attName) throws NamingException {
-      return String.valueOf(atts.get(attName).get(0));
+      final Attribute att = getAll(attName);
+      return att != null && 0 < att.size() ? String.valueOf(att.get(0)) : null;
     }
 
     Attribute getAll(final String attName) {
       return atts.get(attName);
     }
 
-    Map<String, String> map() {
-      return Collections.unmodifiableMap(attsSingle);
+    Set<String> attributes() {
+      return Collections.unmodifiableSet(atts.keySet());
     }
 
     @Override

src/main/java/com/google/gerrit/server/ldap/LdapRealm.java

diff --git a/src/main/java/com/google/gerrit/server/ldap/LdapRealm.java b/src/main/java/com/google/gerrit/server/ldap/LdapRealm.java
index f3dbd1e..47aa33d 100644
--- a/src/main/java/com/google/gerrit/server/ldap/LdapRealm.java
+++ b/src/main/java/com/google/gerrit/server/ldap/LdapRealm.java
@@ -283,11 +283,18 @@
     }
   }
 
-  private static String apply(ParamertizedString p, LdapQuery.Result m) {
+  private static String apply(ParamertizedString p, LdapQuery.Result m)
+      throws NamingException {
     if (p == null) {
       return null;
     }
-    String r = p.replace(m.map());
+
+    final Map<String, String> values = new HashMap<String, String>();
+    for (final String name : m.attributes()) {
+      values.put(name, m.get(name));
+    }
+
+    String r = p.replace(values);
     return r.isEmpty() ? null : r;
   }
 
@@ -399,9 +406,12 @@
         account = findAccount(ctx, username);
       }
 
-      NamingEnumeration<?> groups = account.getAll(accountMemberField).getAll();
-      while (groups.hasMore()) {
-        recursivelyExpandGroups(groupDNs, ctx, (String) groups.next());
+      final Attribute groupAtt = account.getAll(accountMemberField);
+      if (groupAtt != null) {
+        final NamingEnumeration<?> groups = groupAtt.getAll();
+        while (groups.hasMore()) {
+          recursivelyExpandGroups(groupDNs, ctx, (String) groups.next());
+        }
       }
     }