Make sure only Gerrit admins can change the parent of a project

Only Gerrit administrators should be able to change the parent of a
project because by changing the parent project access rights and BLOCK
rules which are configured on a parent project can be avoided.

The set-project-parent SSH command already verifies that the caller is
a Gerrit administrator, however project owners can change the parent
project by modifying the project.config file and pushing to the
refs/meta/config branch.

This change ensures that changes to the project.config file that change
the parent project can only be pushed/submitted by Gerrit
administrators.

In addition it is now not possible anymore to
- set a non-existing project as parent (as this would make the project
  be orphaned)
- set a parent project for the All-Projects root project (the root
  project by definition has no parent)
by pushing changes of the project.config file to refs/meta/config.

Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2012-11-23 14:34:10 +01:00
parent ed633c03d6
commit f7222cbed9
3 changed files with 98 additions and 3 deletions

View File

@@ -51,7 +51,26 @@ enum CommitMergeStatus {
/** */ /** */
NOT_FAST_FORWARD("Project policy requires all submissions to be a fast-forward.\n" NOT_FAST_FORWARD("Project policy requires all submissions to be a fast-forward.\n"
+ "\n" + "\n"
+ "Please rebase the change locally and upload again for review."); + "Please rebase the change locally and upload again for review."),
/** */
INVALID_PROJECT_CONFIGURATION("Change contains an invalid project configuration."),
/** */
INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND(
"Change contains an invalid project configuration:\n"
+ "Parent project does not exist."),
/** */
INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT(
"Change contains an invalid project configuration:\n"
+ "The root project cannot have a parent."),
/** */
SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN(
"Change contains a project configuration that changes the parent project.\n"
+ "The change must be submitted by a Gerrit administrator.");
private String message; private String message;

View File

@@ -36,6 +36,7 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.mail.MergeFailSender; import com.google.gerrit.server.mail.MergeFailSender;
@@ -168,6 +169,7 @@ public class MergeOp {
private final SubmoduleOp.Factory subOpFactory; private final SubmoduleOp.Factory subOpFactory;
private final WorkQueue workQueue; private final WorkQueue workQueue;
private final RequestScopePropagator requestScopePropagator; private final RequestScopePropagator requestScopePropagator;
private final AllProjectsName allProjectsName;
@Inject @Inject
MergeOp(final GitRepositoryManager grm, final SchemaFactory<ReviewDb> sf, MergeOp(final GitRepositoryManager grm, final SchemaFactory<ReviewDb> sf,
@@ -184,7 +186,8 @@ public class MergeOp {
final TagCache tagCache, final CreateCodeReviewNotes.Factory crnf, final TagCache tagCache, final CreateCodeReviewNotes.Factory crnf,
final SubmoduleOp.Factory subOpFactory, final SubmoduleOp.Factory subOpFactory,
final WorkQueue workQueue, final WorkQueue workQueue,
final RequestScopePropagator requestScopePropagator) { final RequestScopePropagator requestScopePropagator,
final AllProjectsName allProjectsName) {
repoManager = grm; repoManager = grm;
schemaFactory = sf; schemaFactory = sf;
functionState = fs; functionState = fs;
@@ -205,6 +208,7 @@ public class MergeOp {
this.subOpFactory = subOpFactory; this.subOpFactory = subOpFactory;
this.workQueue = workQueue; this.workQueue = workQueue;
this.requestScopePropagator = requestScopePropagator; this.requestScopePropagator = requestScopePropagator;
this.allProjectsName = allProjectsName;
this.myIdent = myIdent; this.myIdent = myIdent;
destBranch = branch; destBranch = branch;
toMerge = new ArrayList<CodeReviewCommit>(); toMerge = new ArrayList<CodeReviewCommit>();
@@ -451,6 +455,50 @@ public class MergeOp {
continue; continue;
} }
if (GitRepositoryManager.REF_CONFIG.equals(branchUpdate.getName())) {
final Project.NameKey newParent;
try {
ProjectConfig cfg = new ProjectConfig(destProject.getNameKey());
cfg.load(repo, commit);
newParent = cfg.getProject().getParent(allProjectsName);
} catch (Exception e) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION));
continue;
}
final Project.NameKey oldParent = destProject.getParent(allProjectsName);
if (oldParent == null) {
// update of the 'All-Projects' project
if (newParent != null) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT));
continue;
}
} else {
if (!oldParent.equals(newParent)) {
final PatchSetApproval psa = getSubmitter(db, ps.getId());
if (psa == null) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
continue;
}
final IdentifiedUser submitter =
identifiedUserFactory.create(psa.getAccountId());
if (!submitter.getCapabilities().canAdministrateServer()) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
continue;
}
if (projectCache.get(newParent) == null) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND));
continue;
}
}
}
}
commit.change = chg; commit.change = chg;
commit.patchsetId = ps.getId(); commit.patchsetId = ps.getId();
commit.originalOrder = commitOrder++; commit.originalOrder = commitOrder++;
@@ -1148,7 +1196,11 @@ public class MergeOp {
case PATH_CONFLICT: case PATH_CONFLICT:
case CRISS_CROSS_MERGE: case CRISS_CROSS_MERGE:
case CANNOT_CHERRY_PICK_ROOT: case CANNOT_CHERRY_PICK_ROOT:
case NOT_FAST_FORWARD: { case NOT_FAST_FORWARD:
case INVALID_PROJECT_CONFIGURATION:
case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND:
case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT:
case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN: {
setNew(c, message(c, txt)); setNew(c, message(c, txt));
break; break;
} }

View File

@@ -51,6 +51,7 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -231,6 +232,7 @@ public class ReceiveCommits {
private final TagCache tagCache; private final TagCache tagCache;
private final WorkQueue workQueue; private final WorkQueue workQueue;
private final RequestScopePropagator requestScopePropagator; private final RequestScopePropagator requestScopePropagator;
private final AllProjectsName allProjectsName;
private final ProjectControl projectControl; private final ProjectControl projectControl;
private final Project project; private final Project project;
@@ -281,6 +283,7 @@ public class ReceiveCommits {
final TrackingFooters trackingFooters, final TrackingFooters trackingFooters,
final WorkQueue workQueue, final WorkQueue workQueue,
final RequestScopePropagator requestScopePropagator, final RequestScopePropagator requestScopePropagator,
final AllProjectsName allProjectsName,
@Assisted final ProjectControl projectControl, @Assisted final ProjectControl projectControl,
@Assisted final Repository repo, @Assisted final Repository repo,
@@ -303,6 +306,7 @@ public class ReceiveCommits {
this.tagCache = tagCache; this.tagCache = tagCache;
this.workQueue = workQueue; this.workQueue = workQueue;
this.requestScopePropagator = requestScopePropagator; this.requestScopePropagator = requestScopePropagator;
this.allProjectsName = allProjectsName;
this.projectControl = projectControl; this.projectControl = projectControl;
this.project = projectControl.getProject(); this.project = projectControl.getProject();
@@ -768,6 +772,26 @@ public class ReceiveCommits {
+ cmd.getNewId().name() + " for " + project.getName()); + cmd.getNewId().name() + " for " + project.getName());
continue; continue;
} }
Project.NameKey newParent = cfg.getProject().getParent(allProjectsName);
Project.NameKey oldParent = project.getParent(allProjectsName);
if (oldParent == null) {
// update of the 'All-Projects' project
if (newParent != null) {
reject(cmd, "invalid project configuration: root project cannot have parent");
continue;
}
} else {
if (!oldParent.equals(newParent)
&& !currentUser.getCapabilities().canAdministrateServer()) {
reject(cmd, "invalid project configuration: only Gerrit admin can set parent");
continue;
}
if (projectCache.get(newParent) == null) {
reject(cmd, "invalid project configuration: parent does not exist");
continue;
}
}
} catch (Exception e) { } catch (Exception e) {
reject(cmd, "invalid project configuration"); reject(cmd, "invalid project configuration");
log.error("User " + currentUser.getUserName() log.error("User " + currentUser.getUserName()