From fe1a9138fab2c9ef666c3f2c31367c20a28d8252 Mon Sep 17 00:00:00 2001 From: Simon Hwang Date: Thu, 5 Nov 2015 14:29:27 -0500 Subject: [PATCH] Fix Incorrect owner group matching behaviour for creating projects This change fixes a critical bug that might add a wrong group as an owner of a project being created by ssh commands. Before this fix, when the --owner argument does not match any group, the commands tried to find a group whose name starts with the argument instead of throwing an error to notify the user. This behaviour seemed to be insecure and this fix prevents the commands to create the project and throws an error if the --owner argument is not valid, that is, if there is no exact match for the group name. In total, the ssh commands that are affected by this change are: CreateGroupCommand, CreateProjectCommand, ListGroupsCommand, and SetMembersCommand. Moreover, it makes more sense to use exactSuggestion instead of bestSuggestion for those commands as well. Bug: Issue 3655 Change-Id: Icec00651b4268a2b70799f1cba739db5248b04f5 --- .../acceptance/ssh/CreateProjectIT.java | 56 +++++++++++++++++++ .../args4j/AccountGroupUUIDHandler.java | 2 +- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/CreateProjectIT.java diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/CreateProjectIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/CreateProjectIT.java new file mode 100644 index 0000000000..a7791368ff --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/CreateProjectIT.java @@ -0,0 +1,56 @@ +// Copyright (C) 2015 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.acceptance.ssh; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assert_; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.project.ProjectState; + +import org.junit.Test; + +public class CreateProjectIT extends AbstractDaemonTest { + + @Test + public void withValidGroupName() throws Exception { + String newGroupName = "newGroup"; + adminSession.put("/groups/" + newGroupName); + String newProjectName = "newProject"; + sshSession.exec("gerrit create-project --branch master --owner " + + newGroupName + " " + newProjectName); + assert_().withFailureMessage(sshSession.getError()) + .that(sshSession.hasError()).isFalse(); + ProjectState projectState = + projectCache.get(new Project.NameKey(newProjectName)); + assertThat(projectState).isNotNull(); + } + + @Test + public void withInvalidGroupName() throws Exception { + String newGroupName = "newGroup"; + adminSession.put("/groups/" + newGroupName); + String wrongGroupName = "newG"; + String newProjectName = "newProject"; + sshSession.exec("gerrit create-project --branch master --owner " + + wrongGroupName + " " + newProjectName); + assert_().withFailureMessage(sshSession.getError()) + .that(sshSession.hasError()).isTrue(); + ProjectState projectState = + projectCache.get(new Project.NameKey(newProjectName)); + assertThat(projectState).isNull(); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountGroupUUIDHandler.java b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountGroupUUIDHandler.java index 406ca58b6d..674fe086c4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountGroupUUIDHandler.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/AccountGroupUUIDHandler.java @@ -43,7 +43,7 @@ public class AccountGroupUUIDHandler extends OptionHandler { public final int parseArguments(final Parameters params) throws CmdLineException { final String n = params.getParameter(0); - final GroupReference group = GroupBackends.findBestSuggestion(groupBackend, n); + GroupReference group = GroupBackends.findExactSuggestion(groupBackend, n); if (group == null) { throw new CmdLineException(owner, "Group \"" + n + "\" does not exist"); }