diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 4249b00d65..c2221df753 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2272,6 +2272,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 a6a8ec06c9..42f3129835 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 @@ -187,13 +187,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); @@ -215,7 +222,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"); @@ -236,9 +243,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"); @@ -313,6 +320,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; @@ -324,6 +332,7 @@ import javax.security.auth.login.LoginException; type = discoverLdapType(ctx); groupMemberQueryList = new ArrayList<>(); accountQueryList = new ArrayList<>(); + accountWithMemberOfQueryList = new ArrayList<>(); final Set accountAtts = new HashSet<>(); @@ -377,7 +386,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; } @@ -386,8 +394,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()) { @@ -395,6 +410,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 eb6249c3bf..1d90f0c3b9 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 0f22d2cc5e..7b79add4a3 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 @@ -69,6 +69,7 @@ public class LdapRealm implements Realm { private final EmailExpander emailExpander; private final LoadingCache> usernameCache; private final Set readOnlyAccountFields; + private final boolean fetchMemberOfEagerly; private final Config config; private final LoadingCache> membershipCache; @@ -96,6 +97,8 @@ public class LdapRealm implements Realm { 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) { @@ -216,7 +219,8 @@ public class LdapRealm implements Realm { } 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 @@ -245,7 +249,9 @@ public class LdapRealm implements Realm { // 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 {