diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index ca567d15fc..20298fa0cc 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2390,6 +2390,16 @@ Directory servers. Default is unset for RFC 2307 servers (disabled) and `memberOf` for Active Directory. +[[ldap.fetchMemberOfEagerly]]ldap.fetchMemberOfEagerly:: ++ +_(Optional)_ Whether to fetch the `memberOf` account attribute on +login. Setups which use LDAP for user authentication but don't make +use of the LDAP groups may benefit from setting this option to `false` +as this will result in a much faster LDAP login. ++ +Default is unset for RFC 2307 servers (disabled) and `true` for +Active Directory. + [[ldap.groupBase]]ldap.groupBase:: + Root of the tree containing all group objects. This is typically diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java index 730a86fb6e..8d8ab38071 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java +++ b/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 params = new HashMap<>(); params.put(LdapRealm.USERNAME, username); - for (LdapQuery accountQuery : schema.accountQueryList) { + List accountQueryList; + if (fetchMemberOf) { + accountQueryList = schema.accountWithMemberOfQueryList; + } else { + accountQueryList = schema.accountQueryList; + } + + for (LdapQuery accountQuery : accountQueryList) { List 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 accountQueryList; + final List accountWithMemberOfQueryList; final List 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 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 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); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java index d87a209641..8dc71776ff 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java +++ b/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 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java index 2e216d4a57..90601080df 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java +++ b/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> usernameCache; private final Set readOnlyAccountFields; + private final boolean fetchMemberOfEagerly; private final Config config; private final LoadingCache> 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 {