opendev/gerrit
Code Issues Proposed changes

Move regular expressions for user name from Account to ExternalId

Browse Source 
User names are stored as external IDs, hence the regular expressions to
validate the format of a user name should be part of the ExternalId
class.

There is one place in the GWT UI where user names are validated. Since
the GWT UI doesn't have access to the ExternalId class it needs to copy
two of the regular expressions. This should be OK since the format of
user names is not expected to change and the GWT UI will be gone soon.
It's actually a goal to remove all dependencies from the GWT UI on the
Account class so that the Account class can be moved out of the
reviewdb.client package and be turned into an immutable AutoValue type.

Change-Id: I6ea7139019eb0e7f0e06efaf77ec4bd89d514006
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2018-02-02 15:46:45 +01:00 committed by David Pursehouse
parent 1ed9d84cee
commit 45bf80651a
10 changed files with 51 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
View File

@ -23,7 +23,6 @@ import com.google.gerrit.client.rpc.NativeString;
import com.google.gerrit.client.rpc.RestApi;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.extensions.client.AccountFieldName;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.event.dom.client.KeyCodes;
@ -38,6 +37,12 @@ import com.google.gwtexpui.globalkey.client.NpTextBox;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
class UsernameField extends Composite {
// If these regular expressions are modified the same modifications should be done to the
// corresponding regular expressions in the
// com.google.gerrit.server.account.ExternalId class.
private static final String USER_NAME_PATTERN_FIRST_REGEX = "[a-zA-Z0-9]";
private static final String USER_NAME_PATTERN_REST_REGEX = "[a-zA-Z0-9._@-]";
private CopyableLabel userNameLbl;
private NpTextBox userNameTxt;
private Button setUserName;
@ -185,9 +190,9 @@ class UsernameField extends Composite {
final TextBox box = (TextBox) event.getSource();
final String re;
if (box.getCursorPos() == 0) {
re = Account.USER_NAME_PATTERN_FIRST;
re = USER_NAME_PATTERN_FIRST_REGEX;
} else {
re = Account.USER_NAME_PATTERN_REST;
re = USER_NAME_PATTERN_REST_REGEX;
}
if (!String.valueOf(code).matches("^" + re + "$")) {
event.preventDefault();

25
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Account.java
View File

@ -44,31 +44,6 @@ import java.sql.Timestamp;
* </ul>
*/
public final class Account {
public static final String USER_NAME_PATTERN_FIRST = "[a-zA-Z0-9]";
public static final String USER_NAME_PATTERN_REST = "[a-zA-Z0-9._@-]";
public static final String USER_NAME_PATTERN_LAST = "[a-zA-Z0-9]";
/** Regular expression that {@link #userName} must match. */
public static final String USER_NAME_PATTERN =
"^"
+ //
"("
+ //
USER_NAME_PATTERN_FIRST
+ //
USER_NAME_PATTERN_REST
+ "*"
+ //
USER_NAME_PATTERN_LAST
+ //
"|"
+ //
USER_NAME_PATTERN_FIRST
+ //
")"
+ //
"$";
/** Key local to Gerrit to identify a user. */
public static class Id extends IntKey<com.google.gwtorm.client.Key<?>> {
private static final long serialVersionUID = 1L;

2
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
View File

@ -105,7 +105,7 @@ public class AccountResolver {
return Collections.emptySet();
}
if (nameOrEmail.matches(Account.USER_NAME_PATTERN)) {
if (nameOrEmail.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
AccountState who = byId.getByUsername(nameOrEmail);
if (who != null) {
return Collections.singleton(who.getAccount().getId());

5
gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
View File

@ -19,7 +19,6 @@ import static java.util.stream.Collectors.toSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.errors.NameAlreadyUsedException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ssh.SshKeyCache;
@ -31,7 +30,6 @@ import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.Collection;
import java.util.concurrent.Callable;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -39,7 +37,6 @@ import org.slf4j.LoggerFactory;
/** Operation to change the username of an account. */
public class ChangeUserName implements Callable<VoidResult> {
private static final Logger log = LoggerFactory.getLogger(ChangeUserName.class);
private static final Pattern USER_NAME_PATTERN = Pattern.compile(Account.USER_NAME_PATTERN);
public static final String USERNAME_CANNOT_BE_CHANGED = "Username cannot be changed.";
@ -90,7 +87,7 @@ public class ChangeUserName implements Callable<VoidResult> {
ExternalIdsUpdate externalIdsUpdate = externalIdsUpdateFactory.create();
if (newUsername != null && !newUsername.isEmpty()) {
if (!USER_NAME_PATTERN.matcher(newUsername).matches()) {
if (!newUsername.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
throw new InvalidUserNameException();
}

2
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
View File

@ -111,7 +111,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
throw new BadRequestException("username must match URL");
}
if (!username.matches(Account.USER_NAME_PATTERN)) {
if (!username.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
throw new BadRequestException(
"Username '" + username + "' must contain only letters, numbers, _, - or .");
}

31
gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalId.java
View File

@ -31,12 +31,43 @@ import com.google.gerrit.reviewdb.client.AccountExternalId;
import java.io.Serializable;
import java.util.Collection;
import java.util.Set;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@AutoValue
public abstract class ExternalId implements Serializable {
// If these regular expressions are modified the same modifications should be done to the
// corresponding regular expressions in the
// com.google.gerrit.client.account.UsernameField class.
private static final String USER_NAME_PATTERN_FIRST_REGEX = "[a-zA-Z0-9]";
private static final String USER_NAME_PATTERN_REST_REGEX = "[a-zA-Z0-9._@-]";
private static final String USER_NAME_PATTERN_LAST_REGEX = "[a-zA-Z0-9]";
/** Regular expression that a username must match. */
public static final String USER_NAME_PATTERN_REGEX =
"^("
+ //
USER_NAME_PATTERN_FIRST_REGEX
+ //
USER_NAME_PATTERN_REST_REGEX
+ "*"
+ //
USER_NAME_PATTERN_LAST_REGEX
+ //
"|"
+ //
USER_NAME_PATTERN_FIRST_REGEX
+ //
")$";
private static final Pattern USER_NAME_PATTERN = Pattern.compile(USER_NAME_PATTERN_REGEX);
public static boolean isValidUsername(String username) {
return USER_NAME_PATTERN.matcher(username).matches();
}
private static final long serialVersionUID = 1L;
private static final String EXTERNAL_ID_SECTION = "externalId";

7
gerrit-server/src/main/java/com/google/gerrit/server/account/InvalidUserNameException.java
View File

@ -14,9 +14,10 @@
package com.google.gerrit.server.account;
import com.google.gerrit.reviewdb.client.Account;
/** Error indicating the SSH user name does not match {@link Account#USER_NAME_PATTERN} pattern. */
/**
* Error indicating the SSH user name does not match {@link
* com.google.gerrit.server.account.ExternalId#USER_NAME_PATTERN_REGEX} pattern.
*/
public class InvalidUserNameException extends Exception {
private static final long serialVersionUID = 1L;

3
gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountIdHandler.java
View File

@ -21,6 +21,7 @@ import com.google.gerrit.server.account.AccountException;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.ExternalId;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -90,7 +91,7 @@ public class AccountIdHandler extends OptionHandler<Account.Id> {
}
private Account.Id createAccountByLdap(String user) throws CmdLineException, IOException {
if (!user.matches(Account.USER_NAME_PATTERN)) {
if (!user.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
throw new CmdLineException(owner, "user \"" + user + "\" not found");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java
View File

@ -37,6 +37,7 @@ import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.ExternalId;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.group.AddMembers.Input;
@ -198,7 +199,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
}
private Account createAccountByLdap(String user) throws IOException {
if (!user.matches(Account.USER_NAME_PATTERN)) {
if (!user.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
return null;
}

2
plugins/singleusergroup

@ -1 +1 @@
Subproject commit 570b6e287a74750d37d2a94e2cf66297c004dce4
Subproject commit 0d7b78fc86360730df807bd3c459c1a3a095ea63