Add project owner checks for refs/meta/config

Update the refControl rules for refs/meta/config to check
for project ownership when submitting or pushing.

Do not allow deleting the magic refs/meta/config branch,
ever, as it would remove magic Gerrit control data.

Bug: issue 960
Change-Id: Idfa41d512060ad7085bbe9894b27f043c8f58d48

1 files changed, 36 insertions, 0 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 36674b1156..ff1d09fd53 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -23,6 +23,7 @@ import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.reviewdb.AccountGroup;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
@@ -146,11 +147,28 @@ public class RefControl {
 
   /** @return true if this user can submit patch sets to this ref */
   public boolean canSubmit() {
+    if (GitRepositoryManager.REF_CONFIG.equals(refName)) {
+      // Always allow project owners to submit configuration changes.
+      // Submitting configuration changes modifies the access control
+      // rules. Allowing this to be done by a non-project-owner opens
+      // a security hole enabling editing of access rules, and thus
+      // granting of powers beyond submitting to the configuration.
+      return getProjectControl().isOwner();
+    }
     return canPerform(Permission.SUBMIT);
   }
 
   /** @return true if the user can update the reference as a fast-forward. */
   public boolean canUpdate() {
+    if (GitRepositoryManager.REF_CONFIG.equals(refName)
+        && !getProjectControl().isOwner()) {
+      // Pushing requires being at least project owner, in addition to push.
+      // Pushing configuration changes modifies the access control
+      // rules. Allowing this to be done by a non-project-owner opens
+      // a security hole enabling editing of access rules, and thus
+      // granting of powers beyond pushing to the configuration.
+      return false;
+    }
     return canPerform(Permission.PUSH);
   }
 
@@ -160,6 +178,15 @@ public class RefControl {
   }
 
   private boolean canPushWithForce() {
+    if (GitRepositoryManager.REF_CONFIG.equals(refName)
+        && !getProjectControl().isOwner()) {
+      // Pushing requires being at least project owner, in addition to push.
+      // Pushing configuration changes modifies the access control
+      // rules. Allowing this to be done by a non-project-owner opens
+      // a security hole enabling editing of access rules, and thus
+      // granting of powers beyond pushing to the configuration.
+      return false;
+    }
     for (PermissionRule rule : access(Permission.PUSH)) {
       if (rule.getForce()) {
         return true;
@@ -235,6 +262,15 @@ public class RefControl {
    * @return {@code true} if the user specified can delete a Git ref.
    */
   public boolean canDelete() {
+    if (GitRepositoryManager.REF_CONFIG.equals(refName)) {
+      // Never allow removal of the refs/meta/config branch.
+      // Deleting the branch would destroy all Gerrit specific
+      // metadata about the project, including its access rules.
+      // If a project is to be removed from Gerrit, its repository
+      // should be removed first.
+      return false;
+    }
+
     switch (getCurrentUser().getAccessPath()) {
       case WEB_UI:
         return isOwner() || canPushWithForce();