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
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index e9d5e55..9fcebb0 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -634,7 +634,9 @@
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 4724445..5eadc74 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -96,6 +96,13 @@
* `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 5c6b391..b0f011a 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.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 @@
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 @@
}
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 @@
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 @@
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 @@
}
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 @@
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 1d85a5e..0d8f3f8 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 @@
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 3d25e86..3e549f6 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 @@
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 907dd18..26333b4 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.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.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 @@
private final PluginSetContext<AccountExternalIdCreator> externalIdCreators;
private final Provider<GroupsUpdate> groupsUpdate;
private final OutgoingEmailValidator validator;
+ private final AuthConfig authConfig;
@Inject
CreateAccount(
@@ -93,7 +96,8 @@
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 @@
this.externalIdCreators = externalIdCreators;
this.groupsUpdate = groupsUpdate;
this.validator = validator;
+ this.authConfig = authConfig;
}
@Override
@@ -116,14 +121,18 @@
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 @@
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 aca6c4c..8d801f1 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 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 @@
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();
+ }
}
