Show simple properties of an LDAP group

To help administrators verify their group configurations we now show
some basic non-repeating properties of an LDAP group on the group
page within the admin section.

Bug: GERRIT-276
Change-Id: Ia415f003964401ca4a5eb92d711ae93d936c986e
Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java

@@ -32,10 +32,12 @@ import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.user.client.ui.Button;
 import com.google.gwt.user.client.ui.CheckBox;
 import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwt.user.client.ui.Grid;
 import com.google.gwt.user.client.ui.Panel;
 import com.google.gwt.user.client.ui.SuggestBox;
 import com.google.gwt.user.client.ui.VerticalPanel;
 import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter;
+import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
 import com.google.gwtexpui.globalkey.client.NpTextArea;
 import com.google.gwtexpui.globalkey.client.NpTextBox;
 import com.google.gwtjsonrpc.client.VoidResult;
@@ -62,6 +64,9 @@ public class AccountGroupScreen extends AccountScreen {
   private AddMemberBox addMemberBox;
   private Button delMember;
 
+  private Panel realmProperties;
+  private Grid realmPropertiesTable;
+
   public AccountGroupScreen(final AccountGroup.Id toShow) {
     groupId = toShow;
   }
@@ -69,13 +74,13 @@ public class AccountGroupScreen extends AccountScreen {
   @Override
   protected void onLoad() {
     super.onLoad();
-    Util.GROUP_SVC.groupDetail(groupId,
-        new ScreenLoadCallback<GroupDetail>(this) {
-          @Override
-          protected void preDisplay(final GroupDetail result) {
-            display(result);
-          }
-        });
+    Util.GROUP_SVC.groupDetail(groupId, new ScreenLoadCallback<GroupDetail>(
+        this) {
+      @Override
+      protected void preDisplay(final GroupDetail result) {
+        display(result);
+      }
+    });
   }
 
   @Override
@@ -84,6 +89,7 @@ public class AccountGroupScreen extends AccountScreen {
     initName();
     initOwner();
     initDescription();
+    initRealmProperties();
     initMemberList();
   }
 
@@ -174,6 +180,17 @@ public class AccountGroupScreen extends AccountScreen {
     new TextSaveButtonListener(descTxt, saveDesc);
   }
 
+  private void initRealmProperties() {
+    realmPropertiesTable = new Grid(0, 0);
+    realmPropertiesTable.setStyleName("gerrit-InfoBlock");
+
+    realmProperties = new FlowPanel();
+    realmProperties.add(new SmallHeading(Util.C.headingGroupProperties()));
+    realmProperties.add(realmPropertiesTable);
+
+    add(realmProperties);
+  }
+
   private void initMemberList() {
     addMemberBox = new AddMemberBox();
 
@@ -212,6 +229,25 @@ public class AccountGroupScreen extends AccountScreen {
       ownerTxt.setText(Util.M.deletedGroup(group.getOwnerGroupId().get()));
     }
     descTxt.setText(group.getDescription());
+
+    final List<GroupDetail.RealmProperty> propertyList = result.realmProperties;
+    if (!propertyList.isEmpty()) {
+      final int cnt = propertyList.size();
+      final CellFormatter fmt = realmPropertiesTable.getCellFormatter();
+      realmProperties.setVisible(true);
+      realmPropertiesTable.resize(cnt, 2);
+      for (int i = 0; i < cnt; i++) {
+        fmt.addStyleName(i, 0, "header");
+        realmPropertiesTable.setText(i, 0, propertyList.get(i).name);
+        realmPropertiesTable.setText(i, 1, propertyList.get(i).value);
+      }
+      fmt.addStyleName(0, 0, "topmost");
+      fmt.addStyleName(0, 1, "topmost");
+      fmt.addStyleName(cnt - 1, 0, "bottomheader");
+    } else {
+      realmProperties.setVisible(false);
+    }
+
     if (group.isAutomaticMembership()) {
       memberPanel.setVisible(false);
     } else {

src/main/java/com/google/gerrit/client/admin/AdminConstants.java

@@ -36,6 +36,7 @@ public interface AdminConstants extends Constants {
   String headingOwner();
   String headingDescription();
   String headingSubmitType();
+  String headingGroupProperties();
   String headingMembers();
   String headingCreateGroup();
   String headingAccessRights();

src/main/java/com/google/gerrit/client/admin/AdminConstants.properties

@@ -17,6 +17,7 @@ useSignedOffBy = Require <a href="http://gerrit.googlecode.com/svn/documentation
 headingOwner = Owners
 headingDescription = Description
 headingSubmitType = Change Submit Action
+headingGroupProperties = Group Properties
 headingMembers = Members
 headingCreateGroup = Create New Group
 headingAccessRights = Access Rights

src/main/java/com/google/gerrit/client/admin/GroupDetail.java

@@ -25,6 +25,7 @@ public class GroupDetail {
   protected AccountGroup group;
   protected List<AccountGroupMember> members;
   protected AccountGroup ownerGroup;
+  protected List<RealmProperty> realmProperties;
 
   public GroupDetail() {
   }
@@ -44,4 +45,25 @@ public class GroupDetail {
   public void setOwnerGroup(AccountGroup g) {
     ownerGroup = g;
   }
+
+  public void setRealmProperties(List<RealmProperty> p) {
+    realmProperties = p;
+  }
+
+  public static class RealmProperty {
+    protected String name;
+    protected String value;
+
+    protected RealmProperty() {
+    }
+
+    public RealmProperty(final String n, final String v) {
+      name = n;
+      value = v;
+    }
+
+    public String getName() {
+      return name;
+    }
+  }
 }

src/main/java/com/google/gerrit/server/account/DefaultRealm.java

@@ -14,10 +14,13 @@
 
 package com.google.gerrit.server.account;
 
+import com.google.gerrit.client.admin.GroupDetail.RealmProperty;
 import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.inject.Inject;
 
+import java.util.Collections;
+import java.util.List;
 import java.util.Set;
 
 public final class DefaultRealm implements Realm {
@@ -64,4 +67,9 @@ public final class DefaultRealm implements Realm {
     }
     return null;
   }
+
+  @Override
+  public List<RealmProperty> getProperties(final AccountGroup group) {
+    return Collections.emptyList();
+  }
 }

src/main/java/com/google/gerrit/server/account/Realm.java

@@ -14,9 +14,11 @@
 
 package com.google.gerrit.server.account;
 
+import com.google.gerrit.client.admin.GroupDetail;
 import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.AccountGroup;
 
+import java.util.List;
 import java.util.Set;
 
 public interface Realm {
@@ -38,4 +40,7 @@ public interface Realm {
    * user by that email address.
    */
   public Account.Id lookup(String accountName);
+
+  /** Obtain detailed properties about this group, for display to owners. */
+  public List<GroupDetail.RealmProperty> getProperties(AccountGroup group);
 }

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.ldap;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -29,6 +30,8 @@ import javax.naming.directory.SearchResult;
 
 /** Supports issuing parameterized queries against an LDAP data source. */
 class LdapQuery {
+  static final Set<String> ALL_ATTRIBUTES = null;
+
   private final String base;
   private final SearchScope searchScope;
   private final String pattern;
@@ -65,8 +68,12 @@ class LdapQuery {
     this.patternArgs = new String[a.size()];
     a.toArray(this.patternArgs);
 
-    this.returnAttributes = new String[returnAttributes.size()];
-    returnAttributes.toArray(this.returnAttributes);
+    if (returnAttributes != null) {
+      this.returnAttributes = new String[returnAttributes.size()];
+      returnAttributes.toArray(this.returnAttributes);
+    } else {
+      this.returnAttributes = null;
+    }
   }
 
   String[] getParameters() {
@@ -107,10 +114,20 @@ class LdapQuery {
     private final Map<String, String> atts = new HashMap<String, String>();
 
     Result(final SearchResult sr) throws NamingException {
-      for (final String attName : returnAttributes) {
-        final Attribute a = sr.getAttributes().get(attName);
-        if (a != null && a.size() > 0) {
-          atts.put(attName, String.valueOf(a.get(0)));
+      if (returnAttributes != null) {
+        for (final String attName : returnAttributes) {
+          final Attribute a = sr.getAttributes().get(attName);
+          if (a != null && a.size() > 0) {
+            atts.put(attName, String.valueOf(a.get(0)));
+          }
+        }
+      } else {
+        NamingEnumeration<? extends Attribute> e = sr.getAttributes().getAll();
+        while (e.hasMoreElements()) {
+          final Attribute a = e.nextElement();
+          if (a.size() == 1) {
+            atts.put(a.getID(), String.valueOf(a.get(0)));
+          }
         }
       }
       atts.put("dn", sr.getNameInNamespace());
@@ -119,5 +136,14 @@ class LdapQuery {
     String get(final String attName) {
       return atts.get(attName);
     }
+
+    Set<String> keySet() {
+      return Collections.unmodifiableSet(atts.keySet());
+    }
+
+    @Override
+    public String toString() {
+      return atts.get("dn");
+    }
   }
 }

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.ldap;
 
+import com.google.gerrit.client.admin.GroupDetail.RealmProperty;
 import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.AccountExternalId;
 import com.google.gerrit.client.reviewdb.AccountGroup;
@@ -38,11 +39,14 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.spearce.jgit.lib.Config;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
 
@@ -53,6 +57,7 @@ import javax.naming.directory.InitialDirContext;
 
 @Singleton
 class LdapRealm implements Realm {
+  private static final String GROUPNAME = "groupname";
   private static final Logger log = LoggerFactory.getLogger(LdapRealm.class);
   private static final String LDAP = "com.sun.jndi.ldap.LdapCtxFactory";
   private static final String USERNAME = "username";
@@ -73,6 +78,7 @@ class LdapRealm implements Realm {
   private final String groupName;
   private boolean groupNeedsAccount;
   private final LdapQuery groupMemberQuery;
+  private final LdapQuery groupByNameQuery;
   private final SelfPopulatingCache<String, Set<AccountGroup.Id>> membershipCache;
 
   @Inject
@@ -106,6 +112,9 @@ class LdapRealm implements Realm {
       throw new IllegalArgumentException(
           "No variables in ldap.groupMemberPattern");
     }
+    groupByNameQuery =
+        new LdapQuery(groupBase, groupScope,
+            "(" + groupName + "=${groupname})", LdapQuery.ALL_ATTRIBUTES);
 
     membershipCache =
         new SelfPopulatingCache<String, Set<AccountGroup.Id>>(rawGroup) {
@@ -327,6 +336,67 @@ class LdapRealm implements Realm {
     return usernameCache.get(accountName);
   }
 
+  @Override
+  public List<RealmProperty> getProperties(final AccountGroup group) {
+    if (!isLdapGroup(group)) {
+      return Collections.emptyList();
+    }
+
+    try {
+      final DirContext ctx = open();
+      try {
+        final Map<String, String> params = new HashMap<String, String>();
+        params.put(GROUPNAME, group.getName());
+
+        final List<RealmProperty> props = new ArrayList<RealmProperty>();
+        final List<LdapQuery.Result> q = groupByNameQuery.query(ctx, params);
+        switch (q.size()) {
+          case 0:
+            log.warn("Group \"" + group.getName() + "\" not found in LDAP.");
+            props.add(new RealmProperty("error", "NOT FOUND"));
+            break;
+
+          case 1:
+            for (final String name : q.get(0).keySet()) {
+              props.add(new RealmProperty(name, q.get(0).get(name)));
+            }
+            Collections.sort(props, new Comparator<RealmProperty>() {
+              @Override
+              public int compare(final RealmProperty a, final RealmProperty b) {
+                int sort = classOf(a) - classOf(b);
+                if (sort == 0) sort = a.getName().compareTo(b.getName());
+                return sort;
+              }
+
+              private int classOf(final RealmProperty p) {
+                final String n = p.getName();
+                if ("dn".equals(n) || "distinguishedName".equals(n)) return 0;
+                if ("cn".equals(n)) return 1;
+                return 5000;
+              }
+            });
+            break;
+
+          default:
+            log.warn("Group \"" + group.getName()
+                + "\" has multiple matches in LDAP: " + q);
+            props.add(new RealmProperty("error", "MULTIPLE MATCHES"));
+            break;
+        }
+        return props;
+      } finally {
+        try {
+          ctx.close();
+        } catch (NamingException e) {
+          log.warn("Cannot close LDAP query handle", e);
+        }
+      }
+    } catch (NamingException e) {
+      log.error("Cannot query LDAP directory for group " + group.getName(), e);
+      return Collections.emptyList();
+    }
+  }
+
   private Account.Id queryForUsername(final String username) {
     try {
       final ReviewDb db = schema.open();

src/main/java/com/google/gerrit/server/rpc/account/GroupDetailFactory.java

@@ -23,6 +23,7 @@ import com.google.gerrit.server.account.AccountInfoCacheFactory;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.account.NoSuchGroupException;
+import com.google.gerrit.server.account.Realm;
 import com.google.gerrit.server.rpc.Handler;
 import com.google.gwtorm.client.OrmException;
 import com.google.inject.Inject;
@@ -41,6 +42,7 @@ class GroupDetailFactory extends Handler<GroupDetail> {
   private final ReviewDb db;
   private final GroupControl.Factory groupControl;
   private final GroupCache groupCache;
+  private final Realm realm;
   private final AccountInfoCacheFactory aic;
 
   private final AccountGroup.Id groupId;
@@ -49,11 +51,13 @@ class GroupDetailFactory extends Handler<GroupDetail> {
   @Inject
   GroupDetailFactory(final ReviewDb db,
       final GroupControl.Factory groupControl, final GroupCache groupCache,
+      final Realm realm,
       final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
       @Assisted final AccountGroup.Id groupId) {
     this.db = db;
     this.groupControl = groupControl;
     this.groupCache = groupCache;
+    this.realm = realm;
     this.aic = accountInfoCacheFactory.create();
 
     this.groupId = groupId;
@@ -66,6 +70,7 @@ class GroupDetailFactory extends Handler<GroupDetail> {
     final GroupDetail detail = new GroupDetail();
     detail.setGroup(group);
     detail.setOwnerGroup(groupCache.get(group.getOwnerGroupId()));
+    detail.setRealmProperties(realm.getProperties(group));
     if (!group.isAutomaticMembership()) {
       detail.setMembers(loadMembers());
     }