Add native LDAP support to Gerrit

User account information such as full name and email address
is now obtained from LDAP upon initial account registration.

Group membership is obtained on the fly, but held in a cache
in Gerrit for up to 1 hour.  This permits users who are new
to immediately have access to their LDAP groups, while any
existing users may have to wait up to 1 hour for the cache
to expire and any group modifications to be loaded.

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

src/main/java/com/google/gerrit/client/Gerrit.java

@@ -314,6 +314,7 @@ public class Gerrit implements EntryPoint {
     } else {
       switch (getConfig().getLoginType()) {
         case HTTP:
+        case HTTP_LDAP:
           break;
 
         case OPENID:

src/main/java/com/google/gerrit/client/reviewdb/LoginType.java

@@ -21,12 +21,24 @@ public enum LoginType {
   /**
    * Login relies upon the container/web server security.
    * <p>
-   * The container or web server must populate an HTTP header with the some
+   * The container or web server must populate an HTTP header with a unique name
-   * user token. Gerrit will implicitly trust the value of this header to
+   * for the current user. Gerrit will implicitly trust the value of this header
-   * supply the unique identity.
+   * to supply the unique identity.
    */
   HTTP,
 
+  /**
+   * Login relies upon the container/web server security, but also uses LDAP.
+   * <p>
+   * Like {@link #HTTP}, the container or web server must populate an HTTP
+   * header with a unique name for the current user. Gerrit will implicitly
+   * trust the value of this header to supply the unique identity.
+   * <p>
+   * In addition to trusting the HTTP headers, Gerrit will obtain basic user
+   * registration (name and email) from LDAP, and some group memberships.
+   */
+  HTTP_LDAP,
+
   /** Development mode to enable becoming anyone you want. */
   DEVELOPMENT_BECOME_ANY_ACCOUNT;
 }

src/main/java/com/google/gerrit/server/AnonymousUser.java

@@ -16,7 +16,7 @@ package com.google.gerrit.server;
 
 import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.Change;
-import com.google.gerrit.client.reviewdb.SystemConfig;
+import com.google.gerrit.server.config.AuthConfig;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
@@ -26,17 +26,14 @@ import java.util.Set;
 /** An anonymous user who has not yet authenticated. */
 @Singleton
 public class AnonymousUser extends CurrentUser {
-  private final Set<AccountGroup.Id> effectiveGroups;
-
   @Inject
-  AnonymousUser(final SystemConfig cfg) {
+  AnonymousUser(final AuthConfig auth) {
-    super(cfg);
+    super(auth);
-    effectiveGroups = Collections.singleton(cfg.anonymousGroupId);
   }
 
   @Override
   public Set<AccountGroup.Id> getEffectiveGroups() {
-    return effectiveGroups;
+    return authConfig.getAnonymousGroups();
   }
 
   @Override

src/main/java/com/google/gerrit/server/CurrentUser.java

@@ -16,7 +16,7 @@ package com.google.gerrit.server;
 
 import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.Change;
-import com.google.gerrit.client.reviewdb.SystemConfig;
+import com.google.gerrit.server.config.AuthConfig;
 import com.google.inject.servlet.RequestScoped;
 
 import java.util.Set;
@@ -30,10 +30,10 @@ import java.util.Set;
  * @see IdentifiedUser
  */
 public abstract class CurrentUser {
-  protected final SystemConfig systemConfig;
+  protected final AuthConfig authConfig;
 
-  protected CurrentUser(final SystemConfig cfg) {
+  protected CurrentUser(final AuthConfig authConfig) {
-    systemConfig = cfg;
+    this.authConfig = authConfig;
   }
 
   /**
@@ -54,6 +54,6 @@ public abstract class CurrentUser {
 
   @Deprecated
   public final boolean isAdministrator() {
-    return getEffectiveGroups().contains(systemConfig.adminGroupId);
+    return getEffectiveGroups().contains(authConfig.getAdministratorsGroup());
   }
 }

src/main/java/com/google/gerrit/server/IdentifiedUser.java

@@ -19,9 +19,10 @@ import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.Change;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.reviewdb.StarredChange;
-import com.google.gerrit.client.reviewdb.SystemConfig;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.Nullable;
 import com.google.gwtorm.client.OrmException;
 import com.google.inject.Inject;
@@ -48,18 +49,20 @@ public class IdentifiedUser extends CurrentUser {
   /** Create an IdentifiedUser, ignoring any per-request state. */
   @Singleton
   public static class GenericFactory {
-    private final SystemConfig systemConfig;
+    private final AuthConfig authConfig;
     private final AccountCache accountCache;
+    private final Realm realm;
 
     @Inject
-    GenericFactory(final SystemConfig systemConfig,
+    GenericFactory(final AuthConfig authConfig,
-        final AccountCache accountCache) {
+        final AccountCache accountCache, final Realm realm) {
-      this.systemConfig = systemConfig;
+      this.authConfig = authConfig;
       this.accountCache = accountCache;
+      this.realm = realm;
     }
 
     public IdentifiedUser create(final Account.Id id) {
-      return new IdentifiedUser(systemConfig, accountCache, null, null, id);
+      return new IdentifiedUser(authConfig, accountCache, realm, null, null, id);
     }
   }
 
@@ -71,46 +74,55 @@ public class IdentifiedUser extends CurrentUser {
    */
   @Singleton
   public static class RequestFactory {
-    private final SystemConfig systemConfig;
+    private final AuthConfig authConfig;
     private final AccountCache accountCache;
+    private final Realm realm;
     private final Provider<SocketAddress> remotePeerProvider;
     private final Provider<ReviewDb> dbProvider;
 
     @Inject
-    RequestFactory(final SystemConfig systemConfig,
+    RequestFactory(final AuthConfig authConfig,
-        final AccountCache accountCache,
+        final AccountCache accountCache, final Realm realm,
         final @RemotePeer Provider<SocketAddress> remotePeerProvider,
         final Provider<ReviewDb> dbProvider) {
-      this.systemConfig = systemConfig;
+      this.authConfig = authConfig;
       this.accountCache = accountCache;
+      this.realm = realm;
       this.remotePeerProvider = remotePeerProvider;
       this.dbProvider = dbProvider;
     }
 
     public IdentifiedUser create(final Account.Id id) {
-      return new IdentifiedUser(systemConfig, accountCache, remotePeerProvider,
+      return new IdentifiedUser(authConfig, accountCache, realm,
-          dbProvider, id);
+          remotePeerProvider, dbProvider, id);
     }
   }
 
   private static final Logger log =
       LoggerFactory.getLogger(IdentifiedUser.class);
 
+  private final Realm realm;
   private final AccountCache accountCache;
+
   @Nullable
   private final Provider<SocketAddress> remotePeerProvider;
+
   @Nullable
   private final Provider<ReviewDb> dbProvider;
+
   private final Account.Id accountId;
 
   private AccountState state;
+  private Set<String> emailAddresses;
+  private Set<AccountGroup.Id> effectiveGroups;
   private Set<Change.Id> starredChanges;
 
-  private IdentifiedUser(final SystemConfig systemConfig,
+  private IdentifiedUser(final AuthConfig authConfig,
-      final AccountCache accountCache,
+      final AccountCache accountCache, final Realm realm,
       @Nullable final Provider<SocketAddress> remotePeerProvider,
       @Nullable final Provider<ReviewDb> dbProvider, final Account.Id id) {
-    super(systemConfig);
+    super(authConfig);
+    this.realm = realm;
     this.accountCache = accountCache;
     this.remotePeerProvider = remotePeerProvider;
     this.dbProvider = dbProvider;
@@ -134,12 +146,23 @@ public class IdentifiedUser extends CurrentUser {
   }
 
   public Set<String> getEmailAddresses() {
-    return state().getEmailAddresses();
+    if (emailAddresses == null) {
+      emailAddresses = state().getEmailAddresses();
+    }
+    return emailAddresses;
   }
 
   @Override
   public Set<AccountGroup.Id> getEffectiveGroups() {
-    return state().getEffectiveGroups();
+    if (effectiveGroups == null) {
+      if (authConfig.isIdentityTrustable(state().getExternalIds())) {
+        effectiveGroups = realm.groups(state());
+
+      } else {
+        effectiveGroups = authConfig.getRegisteredGroups();
+      }
+    }
+    return effectiveGroups;
   }
 
   @Override

src/main/java/com/google/gerrit/server/ProjectAccess.java

@@ -1,39 +0,0 @@
-// Copyright (C) 2009 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server;
-
-import com.google.gerrit.client.reviewdb.Project;
-import com.google.gerrit.server.project.ProjectState;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-/**
- * Access controls for a {@link CurrentUser} within a {@link Project}.
- */
-public class ProjectAccess {
-  interface Factory {
-    ProjectAccess create(CurrentUser u, ProjectState entry);
-  }
-
-  private final CurrentUser user;
-  private final ProjectState entry;
-
-  @Inject
-  ProjectAccess(@Assisted final CurrentUser u,
-      @Assisted final ProjectState e) {
-    user = u;
-    entry = e;
-  }
-}

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

@@ -19,7 +19,6 @@ import com.google.gerrit.client.reviewdb.AccountExternalId;
 import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.AccountGroupMember;
 import com.google.gerrit.client.reviewdb.ReviewDb;
-import com.google.gerrit.client.reviewdb.SystemConfig;
 import com.google.gerrit.server.cache.Cache;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.cache.SelfPopulatingCache;
@@ -32,9 +31,9 @@ import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 
 /** Caches important (but small) account state to avoid database hits. */
@@ -55,24 +54,17 @@ public class AccountCache {
   }
 
   private final SchemaFactory<ReviewDb> schema;
-  private final AuthConfig authConfig;
   private final SelfPopulatingCache<Account.Id, AccountState> self;
 
   private final Set<AccountGroup.Id> registered;
   private final Set<AccountGroup.Id> anonymous;
 
   @Inject
-  AccountCache(final SchemaFactory<ReviewDb> sf, final SystemConfig cfg,
+  AccountCache(final SchemaFactory<ReviewDb> sf, final AuthConfig auth,
-      final AuthConfig ac,
       @Named(CACHE_NAME) final Cache<Account.Id, AccountState> rawCache) {
     schema = sf;
-    authConfig = ac;
+    registered = auth.getAnonymousGroups();
-
+    anonymous = auth.getRegisteredGroups();
-    final HashSet<AccountGroup.Id> r = new HashSet<AccountGroup.Id>(2);
-    r.add(cfg.anonymousGroupId);
-    r.add(cfg.registeredGroupId);
-    registered = Collections.unmodifiableSet(r);
-    anonymous = Collections.singleton(cfg.anonymousGroupId);
 
     self = new SelfPopulatingCache<Account.Id, AccountState>(rawCache) {
       @Override
@@ -97,35 +89,23 @@ public class AccountCache {
         return missingAccount(who);
       }
 
-      final List<AccountExternalId> ids =
+      final Collection<AccountExternalId> externalIds =
-          db.accountExternalIds().byAccount(who).toList();
+          Collections.unmodifiableCollection(db.accountExternalIds().byAccount(
-      Set<String> emails = new HashSet<String>();
+              who).toList());
-      for (AccountExternalId id : ids) {
-        if (id.getEmailAddress() != null && !id.getEmailAddress().isEmpty()) {
-          emails.add(id.getEmailAddress());
-        }
-      }
 
-      Set<AccountGroup.Id> actual = new HashSet<AccountGroup.Id>();
+      Set<AccountGroup.Id> internalGroups = new HashSet<AccountGroup.Id>();
       for (AccountGroupMember g : db.accountGroupMembers().byAccount(who)) {
-        actual.add(g.getAccountGroupId());
+        internalGroups.add(g.getAccountGroupId());
       }
 
-      if (actual.isEmpty()) {
+      if (internalGroups.isEmpty()) {
-        actual = registered;
+        internalGroups = registered;
       } else {
-        actual.addAll(registered);
+        internalGroups.addAll(registered);
-        actual = Collections.unmodifiableSet(actual);
+        internalGroups = Collections.unmodifiableSet(internalGroups);
       }
 
-      final Set<AccountGroup.Id> effective;
+      return new AccountState(account, internalGroups, externalIds);
-      if (authConfig.isIdentityTrustable(ids)) {
-        effective = actual;
-      } else {
-        effective = registered;
-      }
-
-      return new AccountState(account, actual, effective, emails);
     } finally {
       db.close();
     }
@@ -133,8 +113,8 @@ public class AccountCache {
 
   private AccountState missingAccount(final Account.Id accountId) {
     final Account account = new Account(accountId);
-    final Set<String> emails = Collections.emptySet();
+    final Collection<AccountExternalId> ids = Collections.emptySet();
-    return new AccountState(account, anonymous, anonymous, emails);
+    return new AccountState(account, anonymous, ids);
   }
 
   public AccountState get(final Account.Id accountId) {

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

@@ -34,18 +34,18 @@ public class AccountManager {
   private final SchemaFactory<ReviewDb> schema;
   private final AccountCache byIdCache;
   private final AccountByEmailCache byEmailCache;
-  private final EmailExpander emailExpander;
   private final AuthConfig authConfig;
+  private final Realm realm;
 
   @Inject
   AccountManager(final SchemaFactory<ReviewDb> schema,
       final AccountCache byIdCache, final AccountByEmailCache byEmailCache,
-      final EmailExpander emailExpander, final AuthConfig authConfig) {
+      final AuthConfig authConfig, final Realm accountMapper) {
     this.schema = schema;
     this.byIdCache = byIdCache;
     this.byEmailCache = byEmailCache;
-    this.emailExpander = emailExpander;
     this.authConfig = authConfig;
+    this.realm = accountMapper;
   }
 
   /**
@@ -74,7 +74,8 @@ public class AccountManager {
    * @throws AccountException the account does not exist, and cannot be created,
    *         or exists, but cannot be located.
    */
-  public AuthResult authenticate(final AuthRequest who) throws AccountException {
+  public AuthResult authenticate(AuthRequest who) throws AccountException {
+    who = realm.authenticate(who);
     try {
       final ReviewDb db = schema.open();
       try {
@@ -180,22 +181,8 @@ public class AccountManager {
     final Account account = new Account(newId);
     final AccountExternalId extId = createId(newId, who);
 
-    if (who.getLocalUser() != null && who.getEmailAddress() == null) {
-      // A SCHEMA_GERRIT account was authenticated by an external SSO
-      // solution. The external identity string actually contains a
-      // name that we can uniquely refer to the user by, so set
-      // account information based upon that name.
-      //
-      final String user = who.getLocalUser();
-      if (emailExpander.canExpand(user)) {
-        extId.setEmailAddress(emailExpander.expand(user));
-      }
-      account.setSshUserName(user);
-    } else if (who.getEmailAddress() != null) {
-      extId.setEmailAddress(who.getEmailAddress());
-    }
-
     extId.setLastUsedOn();
+    extId.setEmailAddress(who.getEmailAddress());
     account.setFullName(who.getDisplayName());
     account.setPreferredEmail(extId.getEmailAddress());
 

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

@@ -15,22 +15,23 @@
 package com.google.gerrit.server.account;
 
 import com.google.gerrit.client.reviewdb.Account;
+import com.google.gerrit.client.reviewdb.AccountExternalId;
 import com.google.gerrit.client.reviewdb.AccountGroup;
 
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.Set;
 
 public class AccountState {
   private final Account account;
-  private final Set<AccountGroup.Id> actualGroups;
+  private final Set<AccountGroup.Id> internalGroups;
-  private final Set<AccountGroup.Id> effectiveGroups;
+  private final Collection<AccountExternalId> externalIds;
-  private final Set<String> emails;
 
-  AccountState(final Account a, final Set<AccountGroup.Id> actual,
+  AccountState(final Account account, final Set<AccountGroup.Id> actualGroups,
-      final Set<AccountGroup.Id> effective, final Set<String> e) {
+      final Collection<AccountExternalId> externalIds) {
-    this.account = a;
+    this.account = account;
-    this.actualGroups = actual;
+    this.internalGroups = actualGroups;
-    this.effectiveGroups = effective;
+    this.externalIds = externalIds;
-    this.emails = e;
   }
 
   /** Get the cached account metadata. */
@@ -48,34 +49,22 @@ public class AccountState {
    * validated by Gerrit directly.
    */
   public Set<String> getEmailAddresses() {
+    final Set<String> emails = new HashSet<String>();
+    for (final AccountExternalId e : externalIds) {
+      if (e.getEmailAddress() != null && !e.getEmailAddress().isEmpty()) {
+        emails.add(e.getEmailAddress());
+      }
+    }
     return emails;
   }
 
-  /**
+  /** The external identities that identify the account holder. */
-   * Get the set of groups the user has been declared a member of.
+  public Collection<AccountExternalId> getExternalIds() {
-   * <p>
+    return externalIds;
-   * The returned set is the complete set of the user's groups. This can be a
-   * superset of {@link #getEffectiveGroups()} if the user's account is not
-   * sufficiently trusted to enable additional access.
-   *
-   * @return active groups for this user.
-   */
-  public Set<AccountGroup.Id> getActualGroups() {
-    return actualGroups;
   }
 
-  /**
+  /** The set of groups maintained directly within the Gerrit database. */
-   * Get the set of groups the user is currently a member of.
+  public Set<AccountGroup.Id> getInternalGroups() {
-   * <p>
+    return internalGroups;
-   * The returned set may be a subset of {@link #getActualGroups()}. If the
-   * user's account is currently deemed to be untrusted then the effective group
-   * set is only the anonymous and registered user groups. To enable additional
-   * groups (and gain their granted permissions) the user must update their
-   * account to use only trusted authentication providers.
-   *
-   * @return active groups for this user.
-   */
-  public Set<AccountGroup.Id> getEffectiveGroups() {
-    return effectiveGroups;
   }
 }

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

@@ -0,0 +1,43 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account;
+
+import com.google.gerrit.client.reviewdb.AccountGroup;
+import com.google.inject.Inject;
+
+import java.util.Set;
+
+public final class DefaultRealm implements Realm {
+  private final EmailExpander emailExpander;
+
+  @Inject
+  DefaultRealm(final EmailExpander emailExpander) {
+    this.emailExpander = emailExpander;
+  }
+
+  @Override
+  public AuthRequest authenticate(final AuthRequest who) {
+    if (who.getEmailAddress() == null && who.getLocalUser() != null
+        && emailExpander.canExpand(who.getLocalUser())) {
+      who.setEmailAddress(emailExpander.expand(who.getLocalUser()));
+    }
+    return who;
+  }
+
+  @Override
+  public Set<AccountGroup.Id> groups(final AccountState who) {
+    return who.getInternalGroups();
+  }
+}

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

@@ -16,10 +16,10 @@ package com.google.gerrit.server.account;
 
 import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.ReviewDb;
-import com.google.gerrit.client.reviewdb.SystemConfig;
 import com.google.gerrit.server.cache.Cache;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.cache.SelfPopulatingCache;
+import com.google.gerrit.server.config.AuthConfig;
 import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.SchemaFactory;
 import com.google.inject.Inject;
@@ -37,26 +37,29 @@ public class GroupCache {
     return new CacheModule() {
       @Override
       protected void configure() {
-        final TypeLiteral<Cache<AccountGroup.Id, AccountGroup>> type =
+        final TypeLiteral<Cache<com.google.gwtorm.client.Key<?>, AccountGroup>> byId =
-            new TypeLiteral<Cache<AccountGroup.Id, AccountGroup>>() {};
+            new TypeLiteral<Cache<com.google.gwtorm.client.Key<?>, AccountGroup>>() {};
-        core(type, CACHE_NAME);
+        core(byId, CACHE_NAME);
         bind(GroupCache.class);
       }
     };
   }
 
   private final SchemaFactory<ReviewDb> schema;
-  private final SelfPopulatingCache<AccountGroup.Id, AccountGroup> byId;
-
   private final AccountGroup.Id administrators;
+  private final SelfPopulatingCache<AccountGroup.Id, AccountGroup> byId;
+  private final SelfPopulatingCache<AccountGroup.NameKey, AccountGroup> byName;
 
   @Inject
-  GroupCache(final SchemaFactory<ReviewDb> sf, final SystemConfig cfg,
+  GroupCache(
-      @Named(CACHE_NAME) final Cache<AccountGroup.Id, AccountGroup> rawCache) {
+      final SchemaFactory<ReviewDb> sf,
+      final AuthConfig authConfig,
+      @Named(CACHE_NAME) final Cache<com.google.gwtorm.client.Key<?>, AccountGroup> rawAny) {
     schema = sf;
-    administrators = cfg.adminGroupId;
+    administrators = authConfig.getAdministratorsGroup();
 
-    byId = new SelfPopulatingCache<AccountGroup.Id, AccountGroup>(rawCache) {
+    byId =
+        new SelfPopulatingCache<AccountGroup.Id, AccountGroup>((Cache) rawAny) {
           @Override
           public AccountGroup createEntry(final AccountGroup.Id key)
               throws Exception {
@@ -68,10 +71,16 @@ public class GroupCache {
             return missingGroup(key);
           }
         };
-  }
 
-  public final AccountGroup.Id getAdministrators() {
+    byName =
-    return administrators;
+        new SelfPopulatingCache<AccountGroup.NameKey, AccountGroup>(
+            (Cache) rawAny) {
+          @Override
+          public AccountGroup createEntry(final AccountGroup.NameKey key)
+              throws Exception {
+            return lookup(key);
+          }
+        };
   }
 
   private AccountGroup lookup(final AccountGroup.Id groupId)
@@ -98,28 +107,30 @@ public class GroupCache {
     return g;
   }
 
-  @SuppressWarnings("unchecked")
+  private AccountGroup lookup(final AccountGroup.NameKey groupName)
-  public AccountGroup get(final AccountGroup.Id groupId) {
+      throws OrmException {
-    return byId.get(groupId);
-  }
-
-  public void evict(final AccountGroup.Id groupId) {
-    byId.remove(groupId);
-  }
-
-  public AccountGroup lookup(final String groupName) throws OrmException {
     final ReviewDb db = schema.open();
     try {
-      final AccountGroup.NameKey nameKey = new AccountGroup.NameKey(groupName);
+      return db.accountGroups().get(groupName);
-
-      final AccountGroup group = db.accountGroups().get(nameKey);
-      if (group != null) {
-        return group;
-      } else {
-        return null;
-      }
     } finally {
       db.close();
     }
   }
+
+  public AccountGroup get(final AccountGroup.Id groupId) {
+    return byId.get(groupId);
+  }
+
+  public void evict(final AccountGroup group) {
+    byId.remove(group.getId());
+    byName.remove(group.getNameKey());
+  }
+
+  public void evictAfterRename(final AccountGroup.NameKey oldName) {
+    byName.remove(oldName);
+  }
+
+  public AccountGroup lookup(final String groupName) {
+    return byName.get(new AccountGroup.NameKey(groupName));
+  }
 }

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

@@ -0,0 +1,25 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account;
+
+import com.google.gerrit.client.reviewdb.AccountGroup;
+
+import java.util.Set;
+
+public interface Realm {
+  public AuthRequest authenticate(AuthRequest who) throws AccountException;
+
+  public Set<AccountGroup.Id> groups(AccountState who);
+}

src/main/java/com/google/gerrit/server/config/AuthConfig.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.config;
 
 import com.google.gerrit.client.reviewdb.AccountExternalId;
+import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.LoginType;
 import com.google.gerrit.client.reviewdb.SystemConfig;
 import com.google.gwtjsonrpc.server.SignedToken;
@@ -25,6 +26,9 @@ import com.google.inject.Singleton;
 import org.spearce.jgit.lib.Config;
 
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
 
 /** Authentication related settings from {@code gerrit.config}. */
 @Singleton
@@ -35,6 +39,10 @@ public class AuthConfig {
   private final String[] trusted;
   private final SignedToken emailReg;
 
+  private final AccountGroup.Id administratorGroup;
+  private final Set<AccountGroup.Id> anonymousGroups;
+  private final Set<AccountGroup.Id> registeredGroups;
+
   private final boolean allowGoogleAccountUpgrade;
 
   @Inject
@@ -46,6 +54,13 @@ public class AuthConfig {
     trusted = toTrusted(cfg);
     emailReg = new SignedToken(5 * 24 * 60 * 60, s.registerEmailPrivateKey);
 
+    final HashSet<AccountGroup.Id> r = new HashSet<AccountGroup.Id>(2);
+    r.add(s.anonymousGroupId);
+    r.add(s.registeredGroupId);
+    registeredGroups = Collections.unmodifiableSet(r);
+    anonymousGroups = Collections.singleton(s.anonymousGroupId);
+    administratorGroup = s.adminGroupId;
+
     allowGoogleAccountUpgrade =
         cfg.getBoolean("auth", "allowgoogleaccountupgrade", false);
   }
@@ -104,10 +119,26 @@ public class AuthConfig {
     return allowGoogleAccountUpgrade;
   }
 
+  /** Identity of the magic group with full powers. */
+  public AccountGroup.Id getAdministratorsGroup() {
+    return administratorGroup;
+  }
+
+  /** Groups that all users, including anonymous users, belong to. */
+  public Set<AccountGroup.Id> getAnonymousGroups() {
+    return anonymousGroups;
+  }
+
+  /** Groups that all users who have created an account belong to. */
+  public Set<AccountGroup.Id> getRegisteredGroups() {
+    return registeredGroups;
+  }
+
   public boolean isIdentityTrustable(final Collection<AccountExternalId> ids) {
     switch (getLoginType()) {
       case DEVELOPMENT_BECOME_ANY_ACCOUNT:
       case HTTP:
+      case HTTP_LDAP:
         // Its safe to assume yes for an HTTP authentication type, as the
         // only way in is through some external system that the admin trusts
         //

src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -18,6 +18,7 @@ import static com.google.inject.Scopes.SINGLETON;
 import static com.google.inject.Stage.PRODUCTION;
 
 import com.google.gerrit.client.data.ApprovalTypes;
+import com.google.gerrit.client.reviewdb.LoginType;
 import com.google.gerrit.git.ChangeMergeQueue;
 import com.google.gerrit.git.MergeOp;
 import com.google.gerrit.git.MergeQueue;
@@ -35,9 +36,12 @@ import com.google.gerrit.server.MimeUtilFileTypeRegistry;
 import com.google.gerrit.server.account.AccountByEmailCache;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountInfoCacheFactory;
+import com.google.gerrit.server.account.DefaultRealm;
 import com.google.gerrit.server.account.EmailExpander;
 import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.Realm;
 import com.google.gerrit.server.cache.CachePool;
+import com.google.gerrit.server.ldap.LdapModule;
 import com.google.gerrit.server.mail.AbandonedSender;
 import com.google.gerrit.server.mail.AddReviewerSender;
 import com.google.gerrit.server.mail.CommentSender;
@@ -54,9 +58,12 @@ import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.ssh.SshKeyCache;
 import com.google.gerrit.server.workflow.FunctionState;
 import com.google.inject.Guice;
+import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.Module;
 
+import org.spearce.jgit.lib.Config;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -70,12 +77,30 @@ public class GerritGlobalModule extends FactoryModule {
   public static Injector createInjector(final Injector db) {
     final Injector cfg = db.createChildInjector(new GerritConfigModule());
     final List<Module> modules = new ArrayList<Module>();
-    modules.add(new GerritGlobalModule());
+    modules.add(cfg.getInstance(GerritGlobalModule.class));
     return cfg.createChildInjector(modules);
   }
 
+  private final LoginType loginType;
+
+  @Inject
+  GerritGlobalModule(final AuthConfig authConfig,
+      @GerritServerConfig final Config config) {
+    loginType = authConfig.getLoginType();
+  }
+
   @Override
   protected void configure() {
+    switch (loginType) {
+      case HTTP_LDAP:
+        install(new LdapModule());
+        break;
+
+      default:
+        bind(Realm.class).to(DefaultRealm.class);
+        break;
+    }
+
     bind(ApprovalTypes.class).toProvider(ApprovalTypesProvider.class).in(
         SINGLETON);
     bind(EmailExpander.class).toProvider(EmailExpanderProvider.class).in(

src/main/java/com/google/gerrit/server/http/WebModule.java

@@ -70,6 +70,7 @@ class WebModule extends FactoryModule {
         break;
 
       case HTTP:
+      case HTTP_LDAP:
         install(new HttpAuthModule());
         break;
 

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

@@ -0,0 +1,39 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.ldap;
+
+import static java.util.concurrent.TimeUnit.HOURS;
+
+import com.google.gerrit.client.reviewdb.AccountGroup;
+import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.cache.Cache;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.inject.Scopes;
+import com.google.inject.TypeLiteral;
+
+import java.util.Set;
+
+public class LdapModule extends CacheModule {
+  static final String GROUP_CACHE = "ldap_groups";
+
+  @Override
+  protected void configure() {
+    final TypeLiteral<Cache<String, Set<AccountGroup.Id>>> type =
+        new TypeLiteral<Cache<String, Set<AccountGroup.Id>>>() {};
+
+    core(type, GROUP_CACHE).timeToIdle(1, HOURS).timeToLive(1, HOURS);
+    bind(Realm.class).to(LdapRealm.class).in(Scopes.SINGLETON);
+  }
+}

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

@@ -0,0 +1,121 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.ldap;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.DirContext;
+import javax.naming.directory.SearchControls;
+import javax.naming.directory.SearchResult;
+
+/** Supports issuing parameterized queries against an LDAP data source. */
+class LdapQuery {
+  private final String base;
+  private final String pattern;
+  private final String[] patternArgs;
+  private final String[] returnAttributes;
+
+  LdapQuery(final String base, final String pattern,
+      final Set<String> returnAttributes) {
+    this.base = base;
+
+    final StringBuilder p = new StringBuilder();
+    final List<String> a = new ArrayList<String>(4);
+    int i = 0;
+    while (i < pattern.length()) {
+      final int b = pattern.indexOf("${", i);
+      if (b < 0) {
+        break;
+      }
+      final int e = pattern.indexOf("}", b + 2);
+      if (e < 0) {
+        break;
+      }
+
+      p.append(pattern.substring(i, b));
+      p.append("{" + a.size() + "}");
+      a.add(pattern.substring(b + 2, e));
+      i = e + 1;
+    }
+    if (i < pattern.length()) {
+      p.append(pattern.substring(i));
+    }
+    this.pattern = p.toString();
+    this.patternArgs = new String[a.size()];
+    a.toArray(this.patternArgs);
+
+    this.returnAttributes = new String[returnAttributes.size()];
+    returnAttributes.toArray(this.returnAttributes);
+  }
+
+  String[] getParameters() {
+    return patternArgs;
+  }
+
+  List<Result> query(final DirContext ctx, final Map<String, String> params)
+      throws NamingException {
+    final SearchControls sc = new SearchControls();
+    final NamingEnumeration<SearchResult> res;
+
+    sc.setSearchScope(SearchControls.ONELEVEL_SCOPE);
+    sc.setReturningAttributes(returnAttributes);
+    res = ctx.search(base, pattern, bind(params), sc);
+    try {
+      final List<Result> r = new ArrayList<Result>();
+      while (res.hasMore()) {
+        r.add(new Result(res.next()));
+      }
+      return r;
+    } finally {
+      res.close();
+    }
+  }
+
+  private String[] bind(final Map<String, String> params) {
+    final String[] r = new String[patternArgs.length];
+    for (int i = 0; i < r.length; i++) {
+      r[i] = params.get(patternArgs[i]);
+      if (r[i] == null) {
+        r[i] = "";
+      }
+    }
+    return r;
+  }
+
+  class Result {
+    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)));
+        }
+      }
+      atts.put("dn", sr.getNameInNamespace());
+    }
+
+    String get(final String attName) {
+      return atts.get(attName);
+    }
+  }
+}

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

@@ -0,0 +1,285 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.ldap;
+
+import com.google.gerrit.client.reviewdb.AccountExternalId;
+import com.google.gerrit.client.reviewdb.AccountGroup;
+import com.google.gerrit.server.account.AccountException;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.AuthRequest;
+import com.google.gerrit.server.account.EmailExpander;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.cache.Cache;
+import com.google.gerrit.server.cache.SelfPopulatingCache;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.google.inject.name.Named;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.spearce.jgit.lib.Config;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Properties;
+import java.util.Set;
+
+import javax.naming.Context;
+import javax.naming.NamingException;
+import javax.naming.directory.DirContext;
+import javax.naming.directory.InitialDirContext;
+
+@Singleton
+class LdapRealm implements Realm {
+  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";
+
+  private final String server;
+  private final String username;
+  private final String password;
+
+  private final EmailExpander emailExpander;
+  private final String accountDisplayName;
+  private final String accountEmailAddress;
+  private final LdapQuery accountQuery;
+
+  private final GroupCache groupCache;
+  private final String groupName;
+  private boolean groupNeedsAccount;
+  private final LdapQuery groupMemberQuery;
+  private final SelfPopulatingCache<String, Set<AccountGroup.Id>> membershipCache;
+
+  @Inject
+  LdapRealm(
+      final GroupCache groupCache,
+      final EmailExpander emailExpander,
+      @Named(LdapModule.GROUP_CACHE) final Cache<String, Set<AccountGroup.Id>> rawGroup,
+      @GerritServerConfig final Config config) {
+    this.emailExpander = emailExpander;
+    this.groupCache = groupCache;
+
+    this.server = required(config, "server");
+    this.username = optional(config, "username");
+    this.password = optional(config, "password");
+
+    // Group query
+    //
+    final Set<String> groupAtts = new HashSet<String>();
+    groupName = reqdef(config, "groupName", "cn");
+    groupAtts.add(groupName);
+    final String groupBase = required(config, "groupBase");
+    final String groupMemberPattern =
+        reqdef(config, "groupMemberPattern", "(memberUid=${username})");
+    groupMemberQuery = new LdapQuery(groupBase, groupMemberPattern, groupAtts);
+    if (groupMemberQuery.getParameters().length == 0) {
+      throw new IllegalArgumentException(
+          "No variables in ldap.groupMemberPattern");
+    }
+
+    membershipCache =
+        new SelfPopulatingCache<String, Set<AccountGroup.Id>>(rawGroup) {
+          @Override
+          public Set<AccountGroup.Id> createEntry(final String username)
+              throws Exception {
+            return queryForGroups(username);
+          }
+
+          @Override
+          protected Set<AccountGroup.Id> missing(final String key) {
+            return Collections.emptySet();
+          }
+        };
+
+    // Account query
+    //
+    final Set<String> accountAtts = new HashSet<String>();
+    accountDisplayName = optdef(config, "accountDisplayName", "displayName");
+    if (accountDisplayName != null) {
+      accountAtts.add(accountDisplayName);
+    }
+    accountEmailAddress = optdef(config, "accountEmailAddress", "mail");
+    if (accountEmailAddress != null) {
+      accountAtts.add(accountEmailAddress);
+    }
+    for (final String name : groupMemberQuery.getParameters()) {
+      if (!USERNAME.equals(name)) {
+        groupNeedsAccount = true;
+        accountAtts.add(name);
+      }
+    }
+    final String accountBase = required(config, "accountBase");
+    final String accountPattern =
+        reqdef(config, "accountPattern", "(uid=${username})");
+    accountQuery = new LdapQuery(accountBase, accountPattern, accountAtts);
+    if (accountQuery.getParameters().length == 0) {
+      throw new IllegalArgumentException("No variables in ldap.accountPattern");
+    }
+  }
+
+  private static String optional(final Config config, final String name) {
+    return config.getString("ldap", null, name);
+  }
+
+  private static String required(final Config config, final String name) {
+    final String v = optional(config, name);
+    if (v == null || "".equals(v)) {
+      throw new IllegalArgumentException("No ldap." + name + " configured");
+    }
+    return v;
+  }
+
+  private static String optdef(final Config c, final String n, final String d) {
+    final String v = c.getString("ldap", null, n);
+    if (v == null) {
+      return d;
+
+    } else if ("".equals(v)) {
+      return null;
+
+    } else {
+      return v;
+    }
+  }
+
+  private static String reqdef(final Config c, final String n, final String d) {
+    final String v = optdef(c, n, d);
+    if (v == null) {
+      throw new IllegalArgumentException("No ldap." + n + " configured");
+    }
+    return v;
+  }
+
+  public AuthRequest authenticate(final AuthRequest who)
+      throws AccountException {
+    final String username = who.getLocalUser();
+    try {
+      final DirContext ctx = open();
+      try {
+        final LdapQuery.Result m = findAccount(ctx, username);
+        who.setDisplayName(m.get(accountDisplayName));
+        if (accountEmailAddress != null) {
+          who.setEmailAddress(m.get(accountEmailAddress));
+
+        } else if (emailExpander.canExpand(username)) {
+          // If LDAP cannot give us a valid email address for this user
+          // try expanding it through the older email expander code which
+          // assumes a user name within a domain.
+          //
+          who.setEmailAddress(emailExpander.expand(username));
+        }
+        return who;
+      } finally {
+        try {
+          ctx.close();
+        } catch (NamingException e) {
+          log.warn("Cannot close LDAP query handle", e);
+        }
+      }
+    } catch (NamingException e) {
+      throw new AccountException("Cannot query LDAP for account", e);
+    }
+  }
+
+  @Override
+  public Set<AccountGroup.Id> groups(final AccountState who) {
+    final HashSet<AccountGroup.Id> r = new HashSet<AccountGroup.Id>();
+    r.addAll(membershipCache.get(findId(who.getExternalIds())));
+    r.addAll(who.getInternalGroups());
+    return r;
+  }
+
+  private Set<AccountGroup.Id> queryForGroups(final String username)
+      throws NamingException, AccountException {
+    final DirContext ctx = open();
+    try {
+      final HashMap<String, String> params = new HashMap<String, String>();
+      params.put(USERNAME, username);
+      if (groupNeedsAccount) {
+        final LdapQuery.Result m = findAccount(ctx, username);
+        for (final String name : groupMemberQuery.getParameters()) {
+          params.put(name, m.get(name));
+        }
+      }
+
+      final Set<AccountGroup.Id> actual = new HashSet<AccountGroup.Id>();
+      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());
+        }
+      }
+      if (actual.isEmpty()) {
+        return Collections.emptySet();
+      } else {
+        return Collections.unmodifiableSet(actual);
+      }
+    } finally {
+      try {
+        ctx.close();
+      } catch (NamingException e) {
+        log.warn("Cannot close LDAP query handle", e);
+      }
+    }
+  }
+
+  private boolean isLdapGroup(final AccountGroup group) {
+    return group.isAutomaticMembership();
+  }
+
+  private static String findId(final Collection<AccountExternalId> ids) {
+    for (final AccountExternalId i : ids) {
+      if (i.isScheme(AccountExternalId.SCHEME_GERRIT)) {
+        return i.getSchemeRest(AccountExternalId.SCHEME_GERRIT);
+      }
+    }
+    return null;
+  }
+
+  private DirContext open() throws NamingException {
+    final Properties env = new Properties();
+    env.put(Context.INITIAL_CONTEXT_FACTORY, LDAP);
+    env.put(Context.PROVIDER_URL, server);
+    if (username != null) {
+      env.put(Context.SECURITY_PRINCIPAL, username);
+      env.put(Context.SECURITY_CREDENTIALS, password);
+    }
+    return new InitialDirContext(env);
+  }
+
+  private LdapQuery.Result findAccount(final DirContext ctx,
+      final String username) throws NamingException, AccountException {
+    final HashMap<String, String> params = new HashMap<String, String>();
+    params.put(USERNAME, username);
+
+    final List<LdapQuery.Result> res = accountQuery.query(ctx, params);
+    switch (res.size()) {
+      case 0:
+        throw new AccountException("No such user:" + username);
+
+      case 1:
+        return res.get(0);
+
+      default:
+        throw new AccountException("Duplicate users: " + username);
+    }
+  }
+}

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

@@ -21,6 +21,7 @@ import com.google.gerrit.client.reviewdb.AccountGroup;
 import com.google.gerrit.client.reviewdb.AccountGroupMember;
 import com.google.gerrit.client.reviewdb.AccountGroupMemberAudit;
 import com.google.gerrit.client.reviewdb.ReviewDb;
+import com.google.gerrit.client.reviewdb.AccountGroup.NameKey;
 import com.google.gerrit.client.rpc.NameAlreadyUsedException;
 import com.google.gerrit.client.rpc.NoSuchAccountException;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
@@ -154,7 +155,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
         assertAmGroupOwner(db, group);
         group.setDescription(description);
         db.accountGroups().update(Collections.singleton(group));
-        groupCache.evict(groupId);
+        groupCache.evict(group);
         return VoidResult.INSTANCE;
       }
     });
@@ -175,7 +176,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
 
         group.setOwnerGroupId(owner.getId());
         db.accountGroups().update(Collections.singleton(group));
-        groupCache.evict(groupId);
+        groupCache.evict(group);
         return VoidResult.INSTANCE;
       }
     });
@@ -188,14 +189,16 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
         final AccountGroup group = db.accountGroups().get(groupId);
         assertAmGroupOwner(db, group);
 
-        final AccountGroup.NameKey nameKey = new AccountGroup.NameKey(newName);
+        final AccountGroup.NameKey oldKey = group.getNameKey();
-        if (!nameKey.equals(group.getNameKey())) {
+        final AccountGroup.NameKey newKey = new AccountGroup.NameKey(newName);
-          if (db.accountGroups().get(nameKey) != null) {
+        if (!newKey.equals(oldKey)) {
+          if (db.accountGroups().get(newKey) != null) {
             throw new Failure(new NameAlreadyUsedException());
           }
-          group.setNameKey(nameKey);
+          group.setNameKey(newKey);
           db.accountGroups().update(Collections.singleton(group));
-          groupCache.evict(groupId);
+          groupCache.evict(group);
+          groupCache.evictAfterRename(oldKey);
         }
         return VoidResult.INSTANCE;
       }

src/main/java/com/google/gerrit/server/ssh/commands/AdminCreateProject.java

@@ -24,6 +24,7 @@ import com.google.gerrit.client.reviewdb.Project.SubmitType;
 import com.google.gerrit.git.ReplicationQueue;
 import com.google.gerrit.server.GerritServer;
 import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.ssh.AdminCommand;
 import com.google.gerrit.server.ssh.BaseCommand;
 import com.google.gwtorm.client.OrmException;
@@ -66,6 +67,9 @@ final class AdminCreateProject extends BaseCommand {
   @Inject
   private GroupCache groupCache;
 
+  @Inject
+  private AuthConfig authConfig;
+
   @Inject
   private ReplicationQueue rq;
 
@@ -167,7 +171,7 @@ final class AdminCreateProject extends BaseCommand {
     }
 
     if (ownerName == null) {
-      ownerId = groupCache.getAdministrators();
+      ownerId = authConfig.getAdministratorsGroup();
     } else {
       AccountGroup ownerGroup = groupCache.lookup(ownerName);
       if (ownerGroup == null)  {

src/main/java/com/google/gerrit/server/workflow/FunctionState.java

@@ -26,7 +26,7 @@ import com.google.gerrit.client.reviewdb.PatchSetApproval;
 import com.google.gerrit.client.reviewdb.Project;
 import com.google.gerrit.client.reviewdb.ProjectRight;
 import com.google.gerrit.client.reviewdb.ApprovalCategory.Id;
-import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
@@ -50,7 +50,7 @@ public class FunctionState {
   }
 
   private final ApprovalTypes approvalTypes;
-  private final AccountCache accountCache;
+  private final IdentifiedUser.GenericFactory userFactory;
   private final ProjectCache projectCache;
 
   private final Map<ApprovalCategory.Id, Collection<PatchSetApproval>> approvals =
@@ -67,12 +67,12 @@ public class FunctionState {
 
   @Inject
   FunctionState(final ApprovalTypes approvalTypes, final ProjectCache pc,
-      final AccountCache ac, final GroupCache egc, @Assisted final Change c,
+      final IdentifiedUser.GenericFactory userFactory, final GroupCache egc,
-      @Assisted final PatchSet.Id psId,
+      @Assisted final Change c, @Assisted final PatchSet.Id psId,
       @Assisted final Collection<PatchSetApproval> all) {
     this.approvalTypes = approvalTypes;
+    this.userFactory = userFactory;
     projectCache = pc;
-    accountCache = ac;
 
     change = c;
     project = projectCache.get(change.getDest().getParentKey());
@@ -231,7 +231,7 @@ public class FunctionState {
     for (final ProjectRight r : getAllRights(a.getCategoryId())) {
       final Account.Id who = a.getAccountId();
       final AccountGroup.Id grp = r.getAccountGroupId();
-      if (accountCache.get(who).getEffectiveGroups().contains(grp)) {
+      if (userFactory.create(who).getEffectiveGroups().contains(grp)) {
         minAllowed = (short) Math.min(minAllowed, r.getMinValue());
         maxAllowed = (short) Math.max(maxAllowed, r.getMaxValue());
       }