Merge branch 'stable-3.1' into stable-3.2

* stable-3.1:
  Introduce LDAP metrics
  Respect auth.userNameToLowerCase when creating accounts via REST or SSH

Also rename the import of c.g.g.acceptance.GerritConfig
to c.g.g.acceptance.config.GerritConfig.

Change-Id: I1766105ce83e1a63bbdcc5afc9c422f212e16365

7 files changed, 121 insertions, 13 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index e9d5e55a08..9fcebb061d 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -634,7 +634,9 @@ set to `${sAMAccountName.toLowerCase}`). It is important that for all
 existing accounts this username is already in lower case. It is not
 possible to convert the usernames of the existing accounts to lower
 case because this would break the access to existing per-user
-branches and Gerrit provides no tool to do such a conversion.
+branches and Gerrit provides no tool to do such a conversion. Accounts
+created using the REST API or the `create-account` SSH command will
+be created with all lowercase characters, when this option is set.
 +
 Setting this parameter to `true` will prevent all users from login that
 have a non-lower-case username.
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 47244452b2..5eadc74cb6 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -96,6 +96,13 @@ setting.
 * `http/server/jetty/threadpool/pool_size`: Current thread pool size
 * `http/server/jetty/threadpool/queue_size`: Queued requests waiting for a thread
 
+==== LDAP
+
+* `ldap/login_latency`: Latency of logins.
+* `ldap/user_search_latency`: Latency for searching the user account.
+* `ldap/group_search_latency`: Latency for querying the group memberships of an account.
+* `ldap/group_expansion_latency`: Latency for expanding nested groups.
+
 ==== REST API
 
 * `http/server/error_count`: Rate of REST API error responses.
diff --git a/java/com/google/gerrit/server/auth/ldap/Helper.java b/java/com/google/gerrit/server/auth/ldap/Helper.java
index 5c6b391369..b0f011a3a6 100644
--- a/java/com/google/gerrit/server/auth/ldap/Helper.java
+++ b/java/com/google/gerrit/server/auth/ldap/Helper.java
@@ -20,6 +20,10 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.data.ParameterizedString;
 import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
 import com.google.gerrit.server.account.AccountException;
 import com.google.gerrit.server.account.AuthenticationFailedException;
 import com.google.gerrit.server.auth.NoSuchUserException;
@@ -81,11 +85,16 @@ class Helper {
   private final String connectTimeoutMillis;
   private final boolean useConnectionPooling;
   private final boolean groupsVisibleToAll;
+  private final Timer0 loginLatencyTimer;
+  private final Timer0 userSearchLatencyTimer;
+  private final Timer0 groupSearchLatencyTimer;
+  private final Timer0 groupExpansionLatencyTimer;
 
   @Inject
   Helper(
       @GerritServerConfig Config config,
-      @Named(LdapModule.PARENT_GROUPS_CACHE) Cache<String, ImmutableSet<String>> parentGroups) {
+      @Named(LdapModule.PARENT_GROUPS_CACHE) Cache<String, ImmutableSet<String>> parentGroups,
+      MetricMaker metricMaker) {
     this.config = config;
     this.server = LdapRealm.optional(config, "server");
     this.username = LdapRealm.optional(config, "username");
@@ -112,6 +121,33 @@ class Helper {
     }
     this.parentGroups = parentGroups;
     this.useConnectionPooling = LdapRealm.optional(config, "useConnectionPooling", false);
+
+    this.loginLatencyTimer =
+        metricMaker.newTimer(
+            "ldap/login_latency",
+            new Description("Latency of logins").setCumulative().setUnit(Units.NANOSECONDS));
+    this.userSearchLatencyTimer =
+        metricMaker.newTimer(
+            "ldap/user_search_latency",
+            new Description("Latency for searching the user account")
+                .setCumulative()
+                .setUnit(Units.NANOSECONDS));
+    this.groupSearchLatencyTimer =
+        metricMaker.newTimer(
+            "ldap/group_search_latency",
+            new Description("Latency for querying the groups membership of an account")
+                .setCumulative()
+                .setUnit(Units.NANOSECONDS));
+    this.groupExpansionLatencyTimer =
+        metricMaker.newTimer(
+            "ldap/group_expansion_latency",
+            new Description("Latency for expanding nested groups")
+                .setCumulative()
+                .setUnit(Units.NANOSECONDS));
+  }
+
+  Timer0 getGroupSearchLatencyTimer() {
+    return groupSearchLatencyTimer;
   }
 
   private Properties createContextProperties() {
@@ -191,7 +227,9 @@ class Helper {
   private DirContext kerberosOpen(Properties env)
       throws IOException, LoginException, NamingException {
     LoginContext ctx = new LoginContext("KerberosLogin");
-    ctx.login();
+    try (Timer0.Context ignored = loginLatencyTimer.start()) {
+      ctx.login();
+    }
     Subject subject = ctx.getSubject();
     try {
       return Subject.doAs(
@@ -209,7 +247,7 @@ class Helper {
 
   DirContext authenticate(String dn, String password) throws AccountException {
     final Properties env = createContextProperties();
-    try {
+    try (Timer0.Context ignored = loginLatencyTimer.start()) {
       env.put(Context.REFERRAL, referral);
 
       if (!supportAnonymous) {
@@ -258,7 +296,7 @@ class Helper {
     }
 
     for (LdapQuery accountQuery : accountQueryList) {
-      List<LdapQuery.Result> res = accountQuery.query(ctx, params);
+      List<LdapQuery.Result> res = accountQuery.query(ctx, params, userSearchLatencyTimer);
       if (res.size() == 1) {
         return res.get(0);
       } else if (res.size() > 1) {
@@ -290,8 +328,10 @@ class Helper {
       params.put(LdapRealm.USERNAME, username);
 
       for (LdapQuery groupMemberQuery : schema.groupMemberQueryList) {
-        for (LdapQuery.Result r : groupMemberQuery.query(ctx, params)) {
-          recursivelyExpandGroups(groupDNs, schema, ctx, r.getDN());
+        for (LdapQuery.Result r : groupMemberQuery.query(ctx, params, groupSearchLatencyTimer)) {
+          try (Timer0.Context ignored = groupExpansionLatencyTimer.start()) {
+            recursivelyExpandGroups(groupDNs, schema, ctx, r.getDN());
+          }
         }
       }
     }
diff --git a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
index 1d85a5eb7a..0d8f3f8a95 100644
--- a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
+++ b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
@@ -213,7 +213,8 @@ public class LdapGroupBackend implements GroupBackend {
         Map<String, String> params = Collections.emptyMap();
         for (String groupBase : schema.groupBases) {
           LdapQuery query = new LdapQuery(groupBase, schema.groupScope, filter, returnAttrs);
-          for (LdapQuery.Result res : query.query(ctx, params)) {
+          for (LdapQuery.Result res :
+              query.query(ctx, params, helper.getGroupSearchLatencyTimer())) {
             out.add(groupReference(schema.groupName, res));
           }
         }
diff --git a/java/com/google/gerrit/server/auth/ldap/LdapQuery.java b/java/com/google/gerrit/server/auth/ldap/LdapQuery.java
index 3d25e864b5..3e549f6fdd 100644
--- a/java/com/google/gerrit/server/auth/ldap/LdapQuery.java
+++ b/java/com/google/gerrit/server/auth/ldap/LdapQuery.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.server.auth.ldap;
 
 import com.google.gerrit.common.data.ParameterizedString;
+import com.google.gerrit.metrics.Timer0;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -61,13 +62,16 @@ class LdapQuery {
     return pattern.getParameterNames();
   }
 
-  List<Result> query(DirContext ctx, Map<String, String> params) throws NamingException {
+  List<Result> query(DirContext ctx, Map<String, String> params, Timer0 queryTimer)
+      throws NamingException {
     final SearchControls sc = new SearchControls();
     final NamingEnumeration<SearchResult> res;
 
     sc.setSearchScope(searchScope.scope());
     sc.setReturningAttributes(returnAttributes);
-    res = ctx.search(base, pattern.getRawPattern(), pattern.bind(params), sc);
+    try (Timer0.Context ignored = queryTimer.start()) {
+      res = ctx.search(base, pattern.getRawPattern(), pattern.bind(params), sc);
+    }
     try {
       final List<Result> r = new ArrayList<>();
       try {
diff --git a/java/com/google/gerrit/server/restapi/account/CreateAccount.java b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
index 907dd18f75..26333b40f8 100644
--- a/java/com/google/gerrit/server/restapi/account/CreateAccount.java
+++ b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
@@ -44,6 +44,7 @@ import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.VersionedAuthorizedKeys;
 import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
 import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.group.GroupResolver;
 import com.google.gerrit.server.group.db.GroupsUpdate;
 import com.google.gerrit.server.group.db.InternalGroupUpdate;
@@ -59,6 +60,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 
@@ -82,6 +84,7 @@ public class CreateAccount
   private final PluginSetContext<AccountExternalIdCreator> externalIdCreators;
   private final Provider<GroupsUpdate> groupsUpdate;
   private final OutgoingEmailValidator validator;
+  private final AuthConfig authConfig;
 
   @Inject
   CreateAccount(
@@ -93,7 +96,8 @@ public class CreateAccount
       AccountLoader.Factory infoLoader,
       PluginSetContext<AccountExternalIdCreator> externalIdCreators,
       @UserInitiated Provider<GroupsUpdate> groupsUpdate,
-      OutgoingEmailValidator validator) {
+      OutgoingEmailValidator validator,
+      AuthConfig authConfig) {
     this.seq = seq;
     this.groupResolver = groupResolver;
     this.authorizedKeys = authorizedKeys;
@@ -103,6 +107,7 @@ public class CreateAccount
     this.externalIdCreators = externalIdCreators;
     this.groupsUpdate = groupsUpdate;
     this.validator = validator;
+    this.authConfig = authConfig;
   }
 
   @Override
@@ -116,14 +121,18 @@ public class CreateAccount
   public Response<AccountInfo> apply(IdString id, AccountInput input)
       throws BadRequestException, ResourceConflictException, UnprocessableEntityException,
           IOException, ConfigInvalidException, PermissionBackendException {
-    String username = id.get();
-    if (input.username != null && !username.equals(input.username)) {
+    String username = applyCaseOfUsername(id.get());
+    if (input.username != null && !username.equals(applyCaseOfUsername(input.username))) {
       throw new BadRequestException("username must match URL");
     }
     if (!ExternalId.isValidUsername(username)) {
       throw new BadRequestException("Invalid username '" + username + "'");
     }
 
+    if (input.name == null) {
+      input.name = input.username;
+    }
+
     Set<AccountGroup.UUID> groups = parseGroups(input.groups);
 
     Account.Id accountId = Account.id(seq.nextAccountId());
@@ -182,6 +191,10 @@ public class CreateAccount
     return Response.created(info);
   }
 
+  private String applyCaseOfUsername(String username) {
+    return authConfig.isUserNameToLowerCase() ? username.toLowerCase(Locale.US) : username;
+  }
+
   private Set<AccountGroup.UUID> parseGroups(List<String> groups)
       throws UnprocessableEntityException {
     Set<AccountGroup.UUID> groupUuids = new HashSet<>();
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java b/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java
index aca6c4cbac..8d801f17da 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java
@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth8.assertThat;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.config.GerritConfig;
 import com.google.gerrit.extensions.api.accounts.AccountInput;
 import org.junit.Test;
 
@@ -31,4 +32,44 @@ public class CreateAccountIT extends AbstractDaemonTest {
     r.assertCreated();
     assertThat(accountCache.getByUsername(input.username)).isPresent();
   }
+
+  @Test
+  @GerritConfig(name = "auth.userNameToLowerCase", value = "false")
+  public void createAccountRestApiUserNameToLowerCaseFalse() throws Exception {
+    AccountInput input = new AccountInput();
+    input.username = "JohnDoe";
+    assertThat(accountCache.getByUsername(input.username)).isEmpty();
+    RestResponse r = adminRestSession.put("/accounts/" + input.username, input);
+    r.assertCreated();
+    assertThat(accountCache.getByUsername(input.username)).isPresent();
+  }
+
+  @Test
+  @GerritConfig(name = "auth.userNameToLowerCase", value = "true")
+  public void createAccountRestApiUserNameToLowerCaseTrue() throws Exception {
+    testUserNameToLowerCase("John1", "John1", "john1");
+    assertThat(accountCache.getByUsername("John1")).isEmpty();
+
+    testUserNameToLowerCase("john2", "John2", "john2");
+    assertThat(accountCache.getByUsername("John2")).isEmpty();
+
+    testUserNameToLowerCase("John3", "john3", "john3");
+    assertThat(accountCache.getByUsername("John3")).isEmpty();
+
+    testUserNameToLowerCase("John4", "johN4", "john4");
+    assertThat(accountCache.getByUsername("John4")).isEmpty();
+    assertThat(accountCache.getByUsername("johN4")).isEmpty();
+
+    testUserNameToLowerCase("john5", "john5", "john5");
+  }
+
+  private void testUserNameToLowerCase(String usernameUrl, String usernameInput, String usernameDb)
+      throws Exception {
+    AccountInput input = new AccountInput();
+    input.username = usernameInput;
+    assertThat(accountCache.getByUsername(usernameDb)).isEmpty();
+    RestResponse r = adminRestSession.put("/accounts/" + usernameUrl, input);
+    r.assertCreated();
+    assertThat(accountCache.getByUsername(usernameDb)).isPresent();
+  }
 }