Configurable ldap.fetchMemberOfEagerly to optimize LDAP login

Only query for all groups where the user belongs to if the
fetchMemberOfEagerly=true. Since querying for LDAP group membership
also performs recursive group lookup, this may save a lot of LDAP
requests and traffic on user login. Gerrit instances which use LDAP
for user authentication but otherwise rely on local Gerrit groups
will want to set the fetchMemberOfEagerly=false.

NOTE: Even if we avoid fetching LDAP group membership eagerly on user
login, we just postpone querying of the LDAP group membership until the
UI asks for all capabilities for this user. This happens immediately
after the login. This issue is addressed in the follow up change.

Change-Id: I4c33361976d814788dceb58a67c2027b9fd3e8d1

gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java

@@ -185,13 +185,20 @@ import javax.security.auth.login.LoginException;
     return ldapSchema;
   }
 
-  LdapQuery.Result findAccount(final Helper.LdapSchema schema,
-      final DirContext ctx, final String username) throws NamingException,
-      AccountException {
+  LdapQuery.Result findAccount(Helper.LdapSchema schema,
+      DirContext ctx, String username, boolean fetchMemberOf)
+      throws NamingException, AccountException {
     final HashMap<String, String> params = new HashMap<>();
     params.put(LdapRealm.USERNAME, username);
 
-    for (LdapQuery accountQuery : schema.accountQueryList) {
+    List<LdapQuery> accountQueryList;
+    if (fetchMemberOf) {
+      accountQueryList = schema.accountWithMemberOfQueryList;
+    } else {
+      accountQueryList = schema.accountQueryList;
+    }
+
+    for (LdapQuery accountQuery : accountQueryList) {
       List<LdapQuery.Result> res = accountQuery.query(ctx, params);
       if (res.size() == 1) {
         return res.get(0);
@@ -213,7 +220,7 @@ import javax.security.auth.login.LoginException;
 
       if (account == null) {
         try {
-          account = findAccount(schema, ctx, username);
+          account = findAccount(schema, ctx, username, false);
         } catch (AccountException e) {
           LdapRealm.log.warn("Account " + username +
               " not found, assuming empty group membership");
@@ -234,9 +241,9 @@ import javax.security.auth.login.LoginException;
     }
 
     if (schema.accountMemberField != null) {
-      if (account == null) {
+      if (account == null || account.getAll(schema.accountMemberField) == null) {
         try {
-          account = findAccount(schema, ctx, username);
+          account = findAccount(schema, ctx, username, true);
         } catch (AccountException e) {
           LdapRealm.log.warn("Account " + username +
               " not found, assuming empty group membership");
@@ -311,6 +318,7 @@ import javax.security.auth.login.LoginException;
     final String accountMemberField;
     final String[] accountMemberFieldArray;
     final List<LdapQuery> accountQueryList;
+    final List<LdapQuery> accountWithMemberOfQueryList;
 
     final List<String> groupBases;
     final SearchScope groupScope;
@@ -322,6 +330,7 @@ import javax.security.auth.login.LoginException;
       type = discoverLdapType(ctx);
       groupMemberQueryList = new ArrayList<>();
       accountQueryList = new ArrayList<>();
+      accountWithMemberOfQueryList = new ArrayList<>();
 
       final Set<String> accountAtts = new HashSet<>();
 
@@ -375,7 +384,6 @@ import javax.security.auth.login.LoginException;
           LdapRealm.optdef(config, "accountMemberField", type.accountMemberField());
       if (accountMemberField != null) {
         accountMemberFieldArray = new String[] {accountMemberField};
-        accountAtts.add(accountMemberField);
       } else {
         accountMemberFieldArray = null;
       }
@@ -384,8 +392,15 @@ import javax.security.auth.login.LoginException;
       final String accountPattern =
           LdapRealm.reqdef(config, "accountPattern", type.accountPattern());
 
+      Set<String> accountWithMemberOfAtts;
+      if (accountMemberField != null) {
+        accountWithMemberOfAtts = new HashSet<>(accountAtts);
+        accountWithMemberOfAtts.add(accountMemberField);
+      } else {
+        accountWithMemberOfAtts = null;
+      }
       for (String accountBase : LdapRealm.requiredList(config, "accountBase")) {
-        final LdapQuery accountQuery =
+        LdapQuery accountQuery =
             new LdapQuery(accountBase, accountScope, new ParameterizedString(
                 accountPattern), accountAtts);
         if (accountQuery.getParameters().isEmpty()) {
@@ -393,6 +408,13 @@ import javax.security.auth.login.LoginException;
               "No variables in ldap.accountPattern");
         }
         accountQueryList.add(accountQuery);
+
+        if (accountWithMemberOfAtts != null) {
+          LdapQuery accountWithMemberOfQuery =
+              new LdapQuery(accountBase, accountScope, new ParameterizedString(
+                  accountPattern), accountWithMemberOfAtts);
+          accountWithMemberOfQueryList.add(accountWithMemberOfQuery);
+        }
       }
     }
 

gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java

@@ -83,7 +83,7 @@ public class LdapAuthBackend implements AuthBackend {
       }
       try {
         final Helper.LdapSchema schema = helper.getSchema(ctx);
-        final LdapQuery.Result m = helper.findAccount(schema, ctx, username);
+        final LdapQuery.Result m = helper.findAccount(schema, ctx, username, false);
 
         if (authConfig.getAuthType() == AuthType.LDAP) {
           // We found the user account, but we need to verify

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

@@ -68,6 +68,7 @@ public class LdapRealm extends AbstractRealm {
   private final EmailExpander emailExpander;
   private final LoadingCache<String, Optional<Account.Id>> usernameCache;
   private final Set<Account.FieldName> readOnlyAccountFields;
+  private final boolean fetchMemberOfEagerly;
   private final Config config;
 
   private final LoadingCache<String, Set<AccountGroup.UUID>> membershipCache;
@@ -95,6 +96,8 @@ public class LdapRealm extends AbstractRealm {
     if (optdef(config, "accountSshUserName", "DEFAULT") != null) {
       readOnlyAccountFields.add(Account.FieldName.USER_NAME);
     }
+
+    fetchMemberOfEagerly = optional(config, "fetchMemberOfEagerly", true);
   }
 
   static SearchScope scope(final Config c, final String setting) {
@@ -215,7 +218,8 @@ public class LdapRealm extends AbstractRealm {
       }
       try {
         final Helper.LdapSchema schema = helper.getSchema(ctx);
-        final LdapQuery.Result m = helper.findAccount(schema, ctx, username);
+        final LdapQuery.Result m = helper.findAccount(schema, ctx, username,
+            fetchMemberOfEagerly);
 
         if (authConfig.getAuthType() == AuthType.LDAP && !who.isSkipAuthentication()) {
           // We found the user account, but we need to verify
@@ -244,7 +248,9 @@ public class LdapRealm extends AbstractRealm {
         // in the middle of authenticating the user, its likely we will
         // need to know what access rights they have soon.
         //
-        membershipCache.put(username, helper.queryForGroups(ctx, username, m));
+        if (fetchMemberOfEagerly) {
+          membershipCache.put(username, helper.queryForGroups(ctx, username, m));
+        }
         return who;
       } finally {
         try {