Use LDAP DN to match LDAP group to Gerrit group

Rather than relying only on the name of the group, use the full DN
of the group to match from LDAP to Gerrit's database.  This prevents
identically named groups under different trees from colliding with
each other and producing ambiguous results.

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

16 files changed, 102 insertions, 213 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 0c499323dd..17f356c464 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -489,7 +489,6 @@ Directory.
     #
     groupBase = ou=groups,dc=example,dc=com
     groupMemberPattern = (&(objectClass=group)(member=${dn}))
-    groupName = cn
 ====
 
 [[ldap.server]]ldap.server::
@@ -638,14 +637,6 @@ Scope of the search performed for group objects.  Must be one of:
 +
 Default is `subtree` as many directories have several levels.
 
-[[ldap.groupName]]ldap.groupName::
-+
-Name of an attribute on the group object which matches to the name
-of a group registered in the Gerrit database.  Typically this would
-be the display name of the group.
-+
-Default is `cn`.
-
 [[ldap.groupMemberPattern]]ldap.groupMemberPattern::
 +
 Query pattern to use when searching for the groups that a user
diff --git a/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java b/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
index 2fc65fa09f..4c151c59f6 100644
--- a/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
+++ b/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
@@ -32,12 +32,10 @@ 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;
@@ -64,9 +62,6 @@ 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;
   }
@@ -89,7 +84,6 @@ public class AccountGroupScreen extends AccountScreen {
     initName();
     initOwner();
     initDescription();
-    initRealmProperties();
     initMemberList();
   }
 
@@ -180,17 +174,6 @@ 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();
 
@@ -230,24 +213,6 @@ public class AccountGroupScreen extends AccountScreen {
     }
     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 {
diff --git a/src/main/java/com/google/gerrit/client/admin/AdminConstants.java b/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
index d4d41ff501..d51ff34eac 100644
--- a/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
+++ b/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
@@ -36,7 +36,6 @@ public interface AdminConstants extends Constants {
   String headingOwner();
   String headingDescription();
   String headingSubmitType();
-  String headingGroupProperties();
   String headingMembers();
   String headingCreateGroup();
   String headingAccessRights();
diff --git a/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties b/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
index 4fec723fa2..87fea6b349 100644
--- a/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
+++ b/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
@@ -17,7 +17,6 @@ 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
diff --git a/src/main/java/com/google/gerrit/client/admin/GroupDetail.java b/src/main/java/com/google/gerrit/client/admin/GroupDetail.java
index eda33bc707..c73a72a899 100644
--- a/src/main/java/com/google/gerrit/client/admin/GroupDetail.java
+++ b/src/main/java/com/google/gerrit/client/admin/GroupDetail.java
@@ -25,7 +25,6 @@ public class GroupDetail {
   protected AccountGroup group;
   protected List<AccountGroupMember> members;
   protected AccountGroup ownerGroup;
-  protected List<RealmProperty> realmProperties;
 
   public GroupDetail() {
   }
@@ -45,25 +44,4 @@ 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;
-    }
-  }
 }
diff --git a/src/main/java/com/google/gerrit/client/reviewdb/AccountGroup.java b/src/main/java/com/google/gerrit/client/reviewdb/AccountGroup.java
index 1d7bd0d8e5..9cd3dd25ba 100644
--- a/src/main/java/com/google/gerrit/client/reviewdb/AccountGroup.java
+++ b/src/main/java/com/google/gerrit/client/reviewdb/AccountGroup.java
@@ -46,6 +46,32 @@ public final class AccountGroup {
     }
   }
 
+  /** Distinguished name, within organization directory server. */
+  public static class ExternalNameKey extends
+      StringKey<com.google.gwtorm.client.Key<?>> {
+    private static final long serialVersionUID = 1L;
+
+    @Column
+    protected String name;
+
+    protected ExternalNameKey() {
+    }
+
+    public ExternalNameKey(final String n) {
+      name = n;
+    }
+
+    @Override
+    public String get() {
+      return name;
+    }
+
+    @Override
+    protected void set(String newValue) {
+      name = newValue;
+    }
+  }
+
   /** Synthetic key to link to within the database */
   public static class Id extends IntKey<com.google.gwtorm.client.Key<?>> {
     private static final long serialVersionUID = 1L;
@@ -102,6 +128,10 @@ public final class AccountGroup {
   @Column
   protected boolean automaticMembership;
 
+  /** Distinguished name in the directory server. */
+  @Column(notNull = false)
+  protected ExternalNameKey externalName;
+
   protected AccountGroup() {
   }
 
@@ -145,10 +175,18 @@ public final class AccountGroup {
   }
 
   public boolean isAutomaticMembership() {
-    return automaticMembership;
+    return automaticMembership || externalName != null;
   }
 
   public void setAutomaticMembership(final boolean auto) {
     automaticMembership = auto;
   }
+
+  public ExternalNameKey getExternalNameKey() {
+    return externalName;
+  }
+
+  public void setExternalNameKey(final ExternalNameKey k) {
+    externalName = k;
+  }
 }
diff --git a/src/main/java/com/google/gerrit/client/reviewdb/AccountGroupAccess.java b/src/main/java/com/google/gerrit/client/reviewdb/AccountGroupAccess.java
index c99818d362..308a15e1e0 100644
--- a/src/main/java/com/google/gerrit/client/reviewdb/AccountGroupAccess.java
+++ b/src/main/java/com/google/gerrit/client/reviewdb/AccountGroupAccess.java
@@ -29,6 +29,9 @@ public interface AccountGroupAccess extends
   @SecondaryKey("name")
   AccountGroup get(AccountGroup.NameKey name) throws OrmException;
 
+  @SecondaryKey("externalName")
+  AccountGroup get(AccountGroup.ExternalNameKey name) throws OrmException;
+
   @Query("ORDER BY name")
   ResultSet<AccountGroup> all() throws OrmException;
 
diff --git a/src/main/java/com/google/gerrit/server/account/DefaultRealm.java b/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
index eb5380a93a..5dbd4454c0 100644
--- a/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
+++ b/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
@@ -14,13 +14,10 @@
 
 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 {
@@ -67,9 +64,4 @@ public final class DefaultRealm implements Realm {
     }
     return null;
   }
-
-  @Override
-  public List<RealmProperty> getProperties(final AccountGroup group) {
-    return Collections.emptyList();
-  }
 }
diff --git a/src/main/java/com/google/gerrit/server/account/GroupCache.java b/src/main/java/com/google/gerrit/server/account/GroupCache.java
index 1f2474ded9..1a2ac2a0db 100644
--- a/src/main/java/com/google/gerrit/server/account/GroupCache.java
+++ b/src/main/java/com/google/gerrit/server/account/GroupCache.java
@@ -20,6 +20,8 @@ import com.google.gerrit.client.reviewdb.AccountGroup;
 public interface GroupCache {
   public AccountGroup get(AccountGroup.Id groupId);
 
+  public AccountGroup get(AccountGroup.ExternalNameKey externalName);
+
   public void evict(AccountGroup group);
 
   public void evictAfterRename(AccountGroup.NameKey oldName);
diff --git a/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java b/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java
index 4c4374473b..ad236ca94a 100644
--- a/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java
+++ b/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java
@@ -50,6 +50,7 @@ public class GroupCacheImpl implements GroupCache {
   private final AccountGroup.Id administrators;
   private final SelfPopulatingCache<AccountGroup.Id, AccountGroup> byId;
   private final SelfPopulatingCache<AccountGroup.NameKey, AccountGroup> byName;
+  private final SelfPopulatingCache<AccountGroup.ExternalNameKey, AccountGroup> byExternalName;
 
   @Inject
   GroupCacheImpl(
@@ -82,6 +83,16 @@ public class GroupCacheImpl implements GroupCache {
             return lookup(key);
           }
         };
+
+    byExternalName =
+        new SelfPopulatingCache<AccountGroup.ExternalNameKey, AccountGroup>(
+            (Cache) rawAny) {
+          @Override
+          public AccountGroup createEntry(final AccountGroup.ExternalNameKey key)
+              throws Exception {
+            return lookup(key);
+          }
+        };
   }
 
   private AccountGroup lookup(final AccountGroup.Id groupId)
@@ -118,6 +129,16 @@ public class GroupCacheImpl implements GroupCache {
     }
   }
 
+  private AccountGroup lookup(final AccountGroup.ExternalNameKey externalName)
+      throws OrmException {
+    final ReviewDb db = schema.open();
+    try {
+      return db.accountGroups().get(externalName);
+    } finally {
+      db.close();
+    }
+  }
+
   public AccountGroup get(final AccountGroup.Id groupId) {
     return byId.get(groupId);
   }
@@ -125,6 +146,7 @@ public class GroupCacheImpl implements GroupCache {
   public void evict(final AccountGroup group) {
     byId.remove(group.getId());
     byName.remove(group.getNameKey());
+    byExternalName.remove(group.getExternalNameKey());
   }
 
   public void evictAfterRename(final AccountGroup.NameKey oldName) {
@@ -134,4 +156,8 @@ public class GroupCacheImpl implements GroupCache {
   public AccountGroup lookup(final String groupName) {
     return byName.get(new AccountGroup.NameKey(groupName));
   }
+
+  public AccountGroup get(final AccountGroup.ExternalNameKey externalName) {
+    return byExternalName.get(externalName);
+  }
 }
diff --git a/src/main/java/com/google/gerrit/server/account/Realm.java b/src/main/java/com/google/gerrit/server/account/Realm.java
index 832e28edd5..129a977a8b 100644
--- a/src/main/java/com/google/gerrit/server/account/Realm.java
+++ b/src/main/java/com/google/gerrit/server/account/Realm.java
@@ -14,11 +14,9 @@
 
 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 {
@@ -40,7 +38,4 @@ 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);
 }
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 cf39357c12..94f04c1bb6 100644
--- a/src/main/java/com/google/gerrit/server/ldap/LdapRealm.java
+++ b/src/main/java/com/google/gerrit/server/ldap/LdapRealm.java
@@ -14,7 +14,6 @@
 
 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;
@@ -47,11 +46,9 @@ import java.util.ArrayList;
 import java.util.Arrays;
 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;
 
@@ -59,14 +56,12 @@ import javax.naming.Context;
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
 import javax.naming.directory.Attribute;
-import javax.naming.directory.Attributes;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.InitialDirContext;
 import javax.net.ssl.SSLSocketFactory;
 
 @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";
@@ -88,10 +83,8 @@ class LdapRealm implements Realm {
   private final SelfPopulatingCache<String, Account.Id> usernameCache;
 
   private final GroupCache groupCache;
-  private final String groupName;
   private boolean groupNeedsAccount;
   private final List<LdapQuery> groupMemberQueryList;
-  private final List<LdapQuery> groupByNameQueryList;
   private final SelfPopulatingCache<String, Set<AccountGroup.Id>> membershipCache;
 
   @Inject
@@ -115,7 +108,6 @@ class LdapRealm implements Realm {
     this.type = discoverLdapType();
 
     groupMemberQueryList = new ArrayList<LdapQuery>();
-    groupByNameQueryList = new ArrayList<LdapQuery>();
     accountQueryList = new ArrayList<LdapQuery>();
 
     final Set<String> groupAtts = new HashSet<String>();
@@ -123,8 +115,6 @@ class LdapRealm implements Realm {
 
     // Group query
     //
-    groupName = reqdef(config, "groupName", type.groupName());
-    groupAtts.add(groupName);
 
     final SearchScope groupScope = scope(config, "groupScope");
     final String groupMemberPattern =
@@ -148,9 +138,6 @@ class LdapRealm implements Realm {
 
         groupMemberQueryList.add(groupMemberQuery);
       }
-
-      groupByNameQueryList.add(new LdapQuery(groupBase, groupScope, "("
-          + groupName + "=${groupname})", LdapQuery.ALL_ATTRIBUTES));
     }
 
     membershipCache =
@@ -393,11 +380,7 @@ class LdapRealm implements Realm {
 
       for (LdapQuery groupMemberQuery : groupMemberQueryList) {
         for (LdapQuery.Result r : groupMemberQuery.query(ctx, params)) {
-          final String name = r.get(groupName);
-          final AccountGroup group = groupCache.lookup(name);
-          if (group != null && isLdapGroup(group)) {
-            actual.add(group.getId());
-          }
+          memberOfGroup(actual, r.getDN());
         }
       }
     }
@@ -407,16 +390,13 @@ class LdapRealm implements Realm {
         account = findAccount(ctx, username);
       }
 
+      final Set<String> groupDNs = new HashSet<String>();
       NamingEnumeration<?> groups = account.getAll(accountMemberField).getAll();
       while (groups.hasMore()) {
-        final String dn = (String) groups.next();
-
-        for (String name : groupsFor(ctx, dn)) {
-          AccountGroup group = groupCache.lookup(name);
-          if (group != null && isLdapGroup(group)) {
-            actual.add(group.getId());
-          }
-        }
+        recursivelyExpandGroups(groupDNs, ctx, (String) groups.next());
+      }
+      for (String dn : groupDNs) {
+        memberOfGroup(actual, dn);
       }
     }
 
@@ -427,35 +407,32 @@ class LdapRealm implements Realm {
     }
   }
 
-  private Set<String> groupsFor(final DirContext ctx, final String groupDN) {
-    final Set<String> groupNames = new HashSet<String>();
-    try {
-      // Determine this group's name.
-      //
-      final String attNames[] = {groupName, accountMemberField};
-      final Attributes groupAtts = ctx.getAttributes(groupDN, attNames);
-      final String name = (String) groupAtts.get(groupName).get();
-      groupNames.add(name);
+  private void memberOfGroup(final Set<AccountGroup.Id> actual, final String dn) {
+    final AccountGroup group;
 
+    group = groupCache.get(new AccountGroup.ExternalNameKey(dn));
+    if (group != null) {
+      actual.add(group.getId());
+    }
+  }
+
+  private void recursivelyExpandGroups(final Set<String> groupDNs,
+      final DirContext ctx, final String groupDN) {
+    if (groupDNs.add(groupDN)) {
       // Recursively identify the groups it is a member of.
       //
-      final Attribute in = ctx.getAttributes(groupDN).get(accountMemberField);
-      if (in != null) {
-        final NamingEnumeration<?> otherGroups = in.getAll();
-        while (otherGroups.hasMore()) {
-          final String otherDN = (String) otherGroups.next();
-          groupNames.addAll(groupsFor(ctx, otherDN));
+      try {
+        final Attribute in = ctx.getAttributes(groupDN).get(accountMemberField);
+        if (in != null) {
+          final NamingEnumeration<?> groups = in.getAll();
+          while (groups.hasMore()) {
+            recursivelyExpandGroups(groupDNs, ctx, (String) groups.next());
+          }
         }
+      } catch (NamingException e) {
+        log.warn("Could not find group " + groupDN, e);
       }
-    } catch (NamingException e) {
-      log.warn("Could not find group " + groupDN, e);
     }
-    return groupNames;
-  }
-
-  private boolean isLdapGroup(final AccountGroup group) {
-    return group.isAutomaticMembership()
-        && !authConfig.getRegisteredGroups().contains(group.getId());
   }
 
   private static String findId(final Collection<AccountExternalId> ids) {
@@ -472,71 +449,6 @@ 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 = new ArrayList<LdapQuery.Result>();
-        for (LdapQuery groupByNameQuery : groupByNameQueryList) {
-          q.addAll(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).map().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();
diff --git a/src/main/java/com/google/gerrit/server/ldap/LdapType.java b/src/main/java/com/google/gerrit/server/ldap/LdapType.java
index eb5185d37c..16c34cb58f 100644
--- a/src/main/java/com/google/gerrit/server/ldap/LdapType.java
+++ b/src/main/java/com/google/gerrit/server/ldap/LdapType.java
@@ -38,8 +38,6 @@ abstract class LdapType {
     return RFC_2307;
   }
 
-  abstract String groupName();
-
   abstract String groupMemberPattern();
 
   abstract String accountFullName();
@@ -54,11 +52,6 @@ abstract class LdapType {
 
   private static class Rfc2307 extends LdapType {
     @Override
-    String groupName() {
-      return "cn";
-    }
-
-    @Override
     String groupMemberPattern() {
       return "(memberUid=${username})";
     }
@@ -115,11 +108,6 @@ abstract class LdapType {
     }
 
     @Override
-    String groupName() {
-      return "cn";
-    }
-
-    @Override
     String groupMemberPattern() {
       return null; // Active Directory uses memberOf in the account
     }
diff --git a/src/main/java/com/google/gerrit/server/rpc/account/GroupDetailFactory.java b/src/main/java/com/google/gerrit/server/rpc/account/GroupDetailFactory.java
index 6bb4f1bea1..93c73be790 100644
--- a/src/main/java/com/google/gerrit/server/rpc/account/GroupDetailFactory.java
+++ b/src/main/java/com/google/gerrit/server/rpc/account/GroupDetailFactory.java
@@ -23,7 +23,6 @@ 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;
@@ -42,7 +41,6 @@ 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;
@@ -51,13 +49,11 @@ 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;
@@ -70,7 +66,6 @@ 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());
     }
diff --git a/src/main/webapp/WEB-INF/sql/upgrade018_019_mysql.sql b/src/main/webapp/WEB-INF/sql/upgrade018_019_mysql.sql
index 63e4fe305c..f281ed358b 100644
--- a/src/main/webapp/WEB-INF/sql/upgrade018_019_mysql.sql
+++ b/src/main/webapp/WEB-INF/sql/upgrade018_019_mysql.sql
@@ -10,4 +10,7 @@ VALUES
 UPDATE project_rights SET max_value = 2
 WHERE category_id = 'READ' AND max_value = 1;
 
+ALTER TABLE account_groups ADD external_name VARCHAR(255);
+ALTER TABLE account_groups ADD UNIQUE (external_name);
+
 UPDATE schema_version SET version_nbr = 19;
diff --git a/src/main/webapp/WEB-INF/sql/upgrade018_019_postgres.sql b/src/main/webapp/WEB-INF/sql/upgrade018_019_postgres.sql
index e436b1eab4..c641204295 100644
--- a/src/main/webapp/WEB-INF/sql/upgrade018_019_postgres.sql
+++ b/src/main/webapp/WEB-INF/sql/upgrade018_019_postgres.sql
@@ -14,6 +14,9 @@ VALUES
 UPDATE project_rights SET max_value = 2
 WHERE category_id = 'READ' AND max_value = 1;
 
+ALTER TABLE account_groups ADD external_name VARCHAR(255);
+ALTER TABLE account_groups ADD UNIQUE (external_name);
+
 UPDATE schema_version SET version_nbr = 19;
 
 COMMIT;