opendev/gerrit
Code Issues Proposed changes

SSH: set-project-parent is no longer admin exclusive

Browse Source 
Config option "recieve.allowProjectOwnersToChangeParent"
may allow project owners to reparent a project.

We fix this by delegating the actual project re-parenting
to the SetParent REST Api endpoint, which already have the
neccessary permission checks in place. This also allows
us to remove a lot of reduntant input sanity checks from the
ssh command.

Change-Id: I2da2eabd1f3bbaebcdfa5ba86524297181867282
This commit is contained in:
Gustaf Lundh
2018-08-20 15:02:15 +02:00
parent fd5fcf4058
commit 7ed07a74ef
4 changed files with 37 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
Documentation/cmd-index.txt
View File

@@ -85,6 +85,9 @@ link:cmd-set-head.html[gerrit set-head]::
link:cmd-set-project.html[gerrit set-project]::
Change a project's settings.
link:cmd-set-project-parent.html[gerrit set-project-parent]::
Change the project permissions are inherited from.
link:cmd-set-reviewers.html[gerrit set-reviewers]::
Add or remove reviewers on a change.
@@ -178,9 +181,6 @@ link:cmd-set-account.html[gerrit set-account]::
link:cmd-set-members.html[gerrit set-members]::
Set group members.
link:cmd-set-project-parent.html[gerrit set-project-parent]::
Change the project permissions are inherited from.
link:cmd-show-caches.html[gerrit show-caches]::
Display current cache statistics.

6
Documentation/cmd-set-project-parent.txt
View File

@@ -20,7 +20,11 @@ default this is `All-Projects`. This command sets
the project to inherit through another one.
== ACCESS
Caller must be a member of the privileged 'Administrators' group.
Caller must be a member of the privileged 'Administrators' group
or, if
link:config-gerrit.html#receive.allowProjectOwnersToChangeParent[receive.allowProjectOwnersToChangeParent]
is enabled, be a project owner of the projects that is getting their
parent updated.
== SCRIPTING
This command is intended to be used in scripts.

2
java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
View File

@@ -114,7 +114,7 @@ public class DefaultCommandModule extends CommandModule {
command(gerrit, SetMembersCommand.class);
command(gerrit, CreateBranchCommand.class);
command(gerrit, SetAccountCommand.class);
command(gerrit, AdminSetParent.class);
command(gerrit, SetParentCommand.class);
command(testSubmit, TestSubmitRuleCommand.class);
command(testSubmit, TestSubmitTypeCommand.class);

101
java/com/google/gerrit/sshd/commands/AdminSetParent.java → java/com/google/gerrit/sshd/commands/SetParentCommand.java
View File

@@ -16,41 +16,36 @@ package com.google.gerrit.sshd.commands;
import static java.util.stream.Collectors.toList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.projects.ParentInput;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.restapi.project.ListChildProjects;
import com.google.gerrit.server.restapi.project.SetParent;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@CommandMetaData(
name = "set-project-parent",
description = "Change the project permissions are inherited from")
final class AdminSetParent extends SshCommand {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
final class SetParentCommand extends SshCommand {
@Option(
name = "--parent",
aliases = {"-p"},
@@ -80,14 +75,18 @@ final class AdminSetParent extends SshCommand {
@Inject private ProjectCache projectCache;
@Inject private MetaDataUpdate.User metaDataUpdateFactory;
@Inject private AllProjectsName allProjectsName;
@Inject private ListChildProjects listChildProjects;
@Inject private SetParent setParent;
private Project.NameKey newParentKey;
private static ParentInput parentInput(String parent) {
ParentInput input = new ParentInput();
input.parent = parent;
return input;
}
@Override
protected void run() throws Failure {
if (oldParent == null && children.isEmpty()) {
@@ -100,25 +99,9 @@ final class AdminSetParent extends SshCommand {
}
final StringBuilder err = new StringBuilder();
final Set<Project.NameKey> grandParents = new HashSet<>();
grandParents.add(allProjectsName);
if (newParent != null) {
newParentKey = newParent.getProject().getNameKey();
// Catalog all grandparents of the "parent", we want to
// catch a cycle in the parent pointers before it occurs.
//
Project.NameKey gp = newParent.getProject().getParent();
while (gp != null && grandParents.add(gp)) {
final ProjectState s = projectCache.get(gp);
if (s != null) {
gp = s.getProject().getParent();
} else {
break;
}
}
}
final List<Project.NameKey> childProjects =
@@ -135,47 +118,19 @@ final class AdminSetParent extends SshCommand {
for (Project.NameKey nameKey : childProjects) {
final String name = nameKey.get();
if (allProjectsName.equals(nameKey)) {
// Don't allow the wild card project to have a parent.
//
err.append("error: Cannot set parent of '").append(name).append("'\n");
continue;
}
if (grandParents.contains(nameKey) || nameKey.equals(newParentKey)) {
// Try to avoid creating a cycle in the parent pointers.
//
err.append("error: Cycle exists between '")
.append(name)
.append("' and '")
.append(newParentKey != null ? newParentKey.get() : allProjectsName.get())
.append("'\n");
continue;
}
try (MetaDataUpdate md = metaDataUpdateFactory.create(nameKey)) {
ProjectConfig config = ProjectConfig.read(md);
config.getProject().setParentName(newParentKey);
md.setMessage(
"Inherit access from "
+ (newParentKey != null ? newParentKey.get() : allProjectsName.get())
+ "\n");
config.commit(md);
} catch (RepositoryNotFoundException notFound) {
err.append("error: Project ").append(name).append(" not found\n");
} catch (IOException | ConfigInvalidException e) {
final String msg = "Cannot update project " + name;
logger.atSevere().withCause(e).log(msg);
err.append("error: ").append(msg).append("\n");
}
ProjectState project = projectCache.get(nameKey);
try {
projectCache.evict(nameKey);
} catch (IOException e) {
final String msg = "Cannot reindex project: " + name;
logger.atSevere().withCause(e).log(msg);
err.append("error: ").append(msg).append("\n");
setParent.apply(new ProjectResource(project, user), parentInput(newParentKey.get()));
} catch (AuthException e) {
err.append("error: insuffient access rights to change parent of '")
.append(name)
.append("'\n");
} catch (ResourceConflictException | ResourceNotFoundException | BadRequestException e) {
err.append("error: ").append(e.getMessage()).append("'\n");
} catch (UnprocessableEntityException | IOException e) {
throw new Failure(1, "failure in request", e);
} catch (PermissionBackendException e) {
throw new Failure(1, "permissions unavailable", e);
}
}