Don't allow project owners to delete branches if force push is blocked

For deleting a branch by pushing to Gerrit it is required to have
force push permissions. For convenience project owners are allowed to
delete branches through WebUI/REST/SSH without force push permission
(because they anyway can assign this permission themselves). However
force push can be blocked on a parent project to prevent project
owners from deleting branches. In this case assigning force push on a
project has no effect. The problem is that project owners can still
delete the branches through WebUI/REST/SSH although force push is
actually blocked for them.

With this change project owners are only allowed to delete branches if
force push is not blocked for them. Administrators can still delete
branches even if force push is blocked.

Change-Id: I9ec712907e6f6f5a3cf33648d59340eaa72548f6
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>

2 files changed, 115 insertions, 2 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
index 5b9c6e7e77..d23ad6da29 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
@@ -24,11 +24,21 @@ import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.RestSession;
 import com.google.gerrit.acceptance.SshSession;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.config.AllProjectsNameProvider;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCache;
 import com.google.inject.Inject;
 
 import org.apache.http.HttpStatus;
+import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -39,6 +49,18 @@ public class DeleteBranchIT extends AbstractDaemonTest {
   @Inject
   private AccountCreator accounts;
 
+  @Inject
+  private MetaDataUpdate.Server metaDataUpdateFactory;
+
+  @Inject
+  private ProjectCache projectCache;
+
+  @Inject
+  private GroupCache groupCache;
+
+  @Inject
+  private AllProjectsNameProvider allProjects;
+
   private RestSession adminSession;
   private RestSession userSession;
 
@@ -78,7 +100,40 @@ public class DeleteBranchIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void deleteBranch() throws IOException {
+  public void deleteBranchByAdmin() throws IOException {
+    RestResponse r =
+        adminSession.delete("/projects/" + project.get()
+            + "/branches/" + branch.getShortName());
+    assertEquals(HttpStatus.SC_NO_CONTENT, r.getStatusCode());
+    r.consume();
+
+    r = adminSession.get("/projects/" + project.get()
+        + "/branches/" + branch.getShortName());
+    assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
+    r.consume();
+  }
+
+  @Test
+  public void deleteBranchByProjectOwner() throws IOException,
+      ConfigInvalidException {
+    grantOwner();
+
+    RestResponse r =
+        userSession.delete("/projects/" + project.get()
+            + "/branches/" + branch.getShortName());
+    assertEquals(HttpStatus.SC_NO_CONTENT, r.getStatusCode());
+    r.consume();
+
+    r = userSession.get("/projects/" + project.get()
+        + "/branches/" + branch.getShortName());
+    assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
+    r.consume();
+  }
+
+  @Test
+  public void deleteBranchByAdminForcePushBlocked() throws IOException,
+      ConfigInvalidException {
+    blockForcePush();
     RestResponse r =
         adminSession.delete("/projects/" + project.get()
             + "/branches/" + branch.getShortName());
@@ -90,4 +145,44 @@ public class DeleteBranchIT extends AbstractDaemonTest {
     assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
     r.consume();
   }
+
+  @Test
+  public void deleteBranchByProjectOwnerForcePushBlocked_Forbidden()
+      throws IOException, ConfigInvalidException {
+    grantOwner();
+    blockForcePush();
+    RestResponse r =
+        userSession.delete("/projects/" + project.get()
+            + "/branches/" + branch.getShortName());
+    assertEquals(HttpStatus.SC_FORBIDDEN, r.getStatusCode());
+    r.consume();
+  }
+
+  private void blockForcePush() throws IOException, ConfigInvalidException {
+    MetaDataUpdate md = metaDataUpdateFactory.create(allProjects.get());
+    md.setMessage(String.format("Block force %s", Permission.PUSH));
+    ProjectConfig config = ProjectConfig.read(md);
+    AccessSection s = config.getAccessSection("refs/heads/*", true);
+    Permission p = s.getPermission(Permission.PUSH, true);
+    AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Anonymous Users"));
+    PermissionRule rule = new PermissionRule(config.resolve(adminGroup));
+    rule.setForce(true);
+    rule.setBlock();
+    p.add(rule);
+    config.commit(md);
+    projectCache.evict(config.getProject());
+  }
+
+  private void grantOwner() throws IOException, ConfigInvalidException {
+    MetaDataUpdate md = metaDataUpdateFactory.create(project);
+    md.setMessage(String.format("Grant %s", Permission.OWNER));
+    ProjectConfig config = ProjectConfig.read(md);
+    AccessSection s = config.getAccessSection("refs/*", true);
+    Permission p = s.getPermission(Permission.OWNER, true);
+    AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Registered Users"));
+    PermissionRule rule = new PermissionRule(config.resolve(adminGroup));
+    p.add(rule);
+    config.commit(md);
+    projectCache.evict(config.getProject());
+  }
 }
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 e292a76e7c..849d6f2e3e 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
@@ -308,7 +308,9 @@ public class RefControl {
       case REST_API:
       case JSON_RPC:
       case SSH_COMMAND:
-        return isOwner() || canPushWithForce();
+        return getCurrentUser().getCapabilities().canAdministrateServer()
+            || (isOwner() && !isForceBlocked(Permission.PUSH))
+            || canPushWithForce();
 
       case GIT:
         return canPushWithForce();
@@ -496,6 +498,22 @@ public class RefControl {
     return blocks.isEmpty() && !allows.isEmpty();
   }
 
+  /** True if for this permission force is blocked for the user. Works only for non labels. */
+  private boolean isForceBlocked(String permissionName) {
+    List<PermissionRule> access = access(permissionName);
+    Set<ProjectRef> allows = Sets.newHashSet();
+    Set<ProjectRef> blocks = Sets.newHashSet();
+    for (PermissionRule rule : access) {
+      if (rule.isBlock()) {
+        blocks.add(relevant.getRuleProps(rule));
+      } else if (rule.getForce()) {
+        allows.add(relevant.getRuleProps(rule));
+      }
+    }
+    blocks.removeAll(allows);
+    return !blocks.isEmpty();
+  }
+
   /** Rules for the given permission, or the empty list. */
   private List<PermissionRule> access(String permissionName) {
     List<PermissionRule> rules = effective.get(permissionName);