Disallow projects to inherit from All-Users

There are exactly zero cases where a project inheriting from All-Users
is a meaningful configuration. It's better to deny attempts to make such
a setting with a clear error message.

Change-Id: Id5a94a26c3816e6ddde8e2b2bd3362977aea5911
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-03-14 11:29:32 +01:00
parent bfe742f1d8
commit d3f2d43ce5
4 changed files with 30 additions and 0 deletions

View File

@@ -47,6 +47,7 @@ import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.ProjectOwnerGroupsProvider; import com.google.gerrit.server.config.ProjectOwnerGroupsProvider;
import com.google.gerrit.server.config.RepositoryConfig; import com.google.gerrit.server.config.RepositoryConfig;
import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent; import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent;
@@ -110,6 +111,7 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
private final Provider<IdentifiedUser> identifiedUser; private final Provider<IdentifiedUser> identifiedUser;
private final Provider<PutConfig> putConfig; private final Provider<PutConfig> putConfig;
private final AllProjectsName allProjects; private final AllProjectsName allProjects;
private final AllUsersName allUsers;
private final DynamicItem<ProjectNameLockManager> lockManager; private final DynamicItem<ProjectNameLockManager> lockManager;
private final String name; private final String name;
@@ -131,6 +133,7 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
Provider<IdentifiedUser> identifiedUser, Provider<IdentifiedUser> identifiedUser,
Provider<PutConfig> putConfig, Provider<PutConfig> putConfig,
AllProjectsName allProjects, AllProjectsName allProjects,
AllUsersName allUsers,
DynamicItem<ProjectNameLockManager> lockManager, DynamicItem<ProjectNameLockManager> lockManager,
@Assisted String name) { @Assisted String name) {
this.projectsCollection = projectsCollection; this.projectsCollection = projectsCollection;
@@ -149,6 +152,7 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
this.identifiedUser = identifiedUser; this.identifiedUser = identifiedUser;
this.putConfig = putConfig; this.putConfig = putConfig;
this.allProjects = allProjects; this.allProjects = allProjects;
this.allUsers = allUsers;
this.lockManager = lockManager; this.lockManager = lockManager;
this.name = name; this.name = name;
} }
@@ -169,6 +173,10 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
String parentName = String parentName =
MoreObjects.firstNonNull(Strings.emptyToNull(input.parent), allProjects.get()); MoreObjects.firstNonNull(Strings.emptyToNull(input.parent), allProjects.get());
args.newParent = projectsCollection.get().parse(parentName, false).getNameKey(); args.newParent = projectsCollection.get().parse(parentName, false).getNameKey();
if (args.newParent.equals(allUsers)) {
throw new ResourceConflictException(
String.format("Cannot inherit from '%s' project", allUsers.get()));
}
args.createEmptyCommit = input.createEmptyCommit; args.createEmptyCommit = input.createEmptyCommit;
args.permissionsOnly = input.permissionsOnly; args.permissionsOnly = input.permissionsOnly;
args.projectDescription = Strings.emptyToNull(input.description); args.projectDescription = Strings.emptyToNull(input.description);

View File

@@ -126,6 +126,11 @@ public class SetParent implements RestModifyView<ProjectResource, ParentInput> {
throw new ResourceConflictException("cannot set parent of " + allProjects.get()); throw new ResourceConflictException("cannot set parent of " + allProjects.get());
} }
if (allUsers.get().equals(newParent)) {
throw new ResourceConflictException(
String.format("Cannot inherit from '%s' project", allUsers.get()));
}
newParent = Strings.emptyToNull(newParent); newParent = Strings.emptyToNull(newParent);
if (newParent != null) { if (newParent != null) {
ProjectState parent = cache.get(new Project.NameKey(newParent)); ProjectState parent = cache.get(new Project.NameKey(newParent));

View File

@@ -179,6 +179,16 @@ public class ProjectIT extends AbstractDaemonTest {
gApi.projects().create(in); gApi.projects().create(in);
} }
@Test
public void createProjectUnderAllUsersNotAllowed() throws Exception {
ProjectInput in = new ProjectInput();
in.name = name("foo");
in.parent = allUsers.get();
exception.expect(ResourceConflictException.class);
exception.expectMessage(String.format("Cannot inherit from '%s' project", allUsers.get()));
gApi.projects().create(in);
}
@Test @Test
public void createAndDeleteBranch() throws Exception { public void createAndDeleteBranch() throws Exception {
assertThat(getRemoteHead(project.get(), "foo")).isNull(); assertThat(getRemoteHead(project.get(), "foo")).isNull();

View File

@@ -89,6 +89,13 @@ public class SetParentIT extends AbstractDaemonTest {
gApi.projects().name(project.get()).parent("non-existing"); gApi.projects().name(project.get()).parent("non-existing");
} }
@Test
public void setParentToAllUsersNotAllowed() throws Exception {
exception.expect(ResourceConflictException.class);
exception.expectMessage(String.format("Cannot inherit from '%s' project", allUsers.get()));
gApi.projects().name(project.get()).parent(allUsers.get());
}
@Test @Test
public void setParentForAllUsersMustBeAllProjects() throws Exception { public void setParentForAllUsersMustBeAllProjects() throws Exception {
gApi.projects().name(allUsers.get()).parent(allProjects.get()); gApi.projects().name(allUsers.get()).parent(allProjects.get());