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>

3 files changed, 98 insertions, 3 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
index 87903511fc..6b53121b1a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
@@ -51,7 +51,26 @@ enum CommitMergeStatus {
   /** */
   NOT_FAST_FORWARD("Project policy requires all submissions to be a fast-forward.\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;
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index cec284173f..2e8f183f0d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -36,6 +36,7 @@ import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 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.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.mail.MergeFailSender;
@@ -168,6 +169,7 @@ public class MergeOp {
   private final SubmoduleOp.Factory subOpFactory;
   private final WorkQueue workQueue;
   private final RequestScopePropagator requestScopePropagator;
+  private final AllProjectsName allProjectsName;
 
   @Inject
   MergeOp(final GitRepositoryManager grm, final SchemaFactory<ReviewDb> sf,
@@ -184,7 +186,8 @@ public class MergeOp {
       final TagCache tagCache, final CreateCodeReviewNotes.Factory crnf,
       final SubmoduleOp.Factory subOpFactory,
       final WorkQueue workQueue,
-      final RequestScopePropagator requestScopePropagator) {
+      final RequestScopePropagator requestScopePropagator,
+      final AllProjectsName allProjectsName) {
     repoManager = grm;
     schemaFactory = sf;
     functionState = fs;
@@ -205,6 +208,7 @@ public class MergeOp {
     this.subOpFactory = subOpFactory;
     this.workQueue = workQueue;
     this.requestScopePropagator = requestScopePropagator;
+    this.allProjectsName = allProjectsName;
     this.myIdent = myIdent;
     destBranch = branch;
     toMerge = new ArrayList<CodeReviewCommit>();
@@ -451,6 +455,50 @@ public class MergeOp {
         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.patchsetId = ps.getId();
       commit.originalOrder = commitOrder++;
@@ -1148,7 +1196,11 @@ public class MergeOp {
         case PATH_CONFLICT:
         case CRISS_CROSS_MERGE:
         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));
           break;
         }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 981f0d80aa..57996a6aa8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -51,6 +51,7 @@ import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 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.TrackingFooters;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -231,6 +232,7 @@ public class ReceiveCommits {
   private final TagCache tagCache;
   private final WorkQueue workQueue;
   private final RequestScopePropagator requestScopePropagator;
+  private final AllProjectsName allProjectsName;
 
   private final ProjectControl projectControl;
   private final Project project;
@@ -281,6 +283,7 @@ public class ReceiveCommits {
       final TrackingFooters trackingFooters,
       final WorkQueue workQueue,
       final RequestScopePropagator requestScopePropagator,
+      final AllProjectsName allProjectsName,
 
       @Assisted final ProjectControl projectControl,
       @Assisted final Repository repo,
@@ -303,6 +306,7 @@ public class ReceiveCommits {
     this.tagCache = tagCache;
     this.workQueue = workQueue;
     this.requestScopePropagator = requestScopePropagator;
+    this.allProjectsName = allProjectsName;
 
     this.projectControl = projectControl;
     this.project = projectControl.getProject();
@@ -768,6 +772,26 @@ public class ReceiveCommits {
                     + cmd.getNewId().name() + " for " + project.getName());
                 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) {
               reject(cmd, "invalid project configuration");
               log.error("User " + currentUser.getUserName()