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
parent fb42262089
commit c89f32404c
9 changed files with 49 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -46,31 +46,6 @@ import java.util.Optional;
* </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 a 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;

3
java/com/google/gerrit/server/account/AccountResolver.java
View File

@@ -20,6 +20,7 @@ import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -115,7 +116,7 @@ public class AccountResolver {
}
}
if (nameOrEmail.matches(Account.USER_NAME_PATTERN)) {
if (nameOrEmail.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
Optional<AccountState> who = byId.getByUsername(nameOrEmail);
if (who.isPresent()) {
return ImmutableSet.of(who.map(a -> a.getAccount().getId()).get());

7
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.externalids.ExternalId#USER_NAME_PATTERN_REGEX} pattern.
*/
public class InvalidUserNameException extends Exception {
private static final long serialVersionUID = 1L;

30
java/com/google/gerrit/server/account/externalids/ExternalId.java
View File

@@ -43,7 +43,35 @@ import org.eclipse.jgit.lib.ObjectId;
@AutoValue
public abstract class ExternalId implements Serializable {
private static final Pattern USER_NAME_PATTERN = Pattern.compile(Account.USER_NAME_PATTERN);
// 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();

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

@@ -20,6 +20,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.externalids.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");
}

2
java/com/google/gerrit/server/restapi/account/CreateAccount.java
View File

@@ -119,7 +119,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 .");
}

3
java/com/google/gerrit/server/restapi/group/AddMembers.java
View File

@@ -39,6 +39,7 @@ import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.group.GroupResource;
import com.google.gerrit.server.group.MemberResource;
@@ -189,7 +190,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
}
private Optional<Account> createAccountByLdap(String user) throws IOException {
if (!user.matches(Account.USER_NAME_PATTERN)) {
if (!user.matches(ExternalId.USER_NAME_PATTERN_REGEX)) {
return Optional.empty();
}