Add separate 'Delete Reference' permission

This new permission allows users to delete branches and tags, without
allowing them to push branches and tags for new commits and thus
bypass code review, as it would be possible when force push would be
granted.

Bug: Issue 2179
Change-Id: I678650570ed2ce14fc1784fc7766b8ddbe7f9729
Signed-off-by: Edwin Kempin <ekempin@google.com>

Documentation/access-control.txt

@@ -483,6 +483,19 @@ to create new branches. This is done by using the special
 you grant the users the push force permission to be able to clean up
 stale branches.
 
+[[category_delete]]
+=== Delete Reference
+
+The delete reference category controls whether it is possible to delete
+references, branches or tags. It doesn't allow any other update of
+references.
+
+Deletion of references is also possible if `Push` with the force option
+is granted, however that includes the permission to fast-forward and
+force-update references to exiting and new commits. Being able to push
+references for new commits is bad if bypassing of code review must be
+prevented.
+
 
 [[category_forge_author]]
 === Forge Author

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java

@@ -81,6 +81,13 @@ public class DeleteBranchIT extends AbstractDaemonTest {
     assertDeleteSucceeds();
   }
 
+  @Test
+  public void deleteBranchByUserWithDeletePermission() throws Exception {
+    grantDelete();
+    setApiUser(user);
+    assertDeleteSucceeds();
+  }
+
   private void blockForcePush() throws Exception {
     block(Permission.PUSH, ANONYMOUS_USERS, "refs/heads/*").setForce(true);
   }
@@ -89,6 +96,10 @@ public class DeleteBranchIT extends AbstractDaemonTest {
     grant(Permission.PUSH, project, "refs/heads/*", true, ANONYMOUS_USERS);
   }
 
+  private void grantDelete() throws Exception {
+    allow(Permission.DELETE, ANONYMOUS_USERS, "refs/*");
+  }
+
   private void grantOwner() throws Exception {
     allow(Permission.OWNER, REGISTERED_USERS, "refs/*");
   }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/PushTagIT.java

@@ -101,6 +101,11 @@ public class PushTagIT extends AbstractDaemonTest {
           Status.REJECTED_OTHER_REASON);
       fastForwardTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON);
 
+      allowTagDeletion();
+      fastForwardTagToExistingCommit(tagType, tagName,
+          Status.REJECTED_OTHER_REASON);
+      fastForwardTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON);
+
       allowPushOnRefsTags();
       Status expectedStatus =
           tagType == ANNOTATED ? Status.REJECTED_OTHER_REASON : Status.OK;
@@ -130,6 +135,11 @@ public class PushTagIT extends AbstractDaemonTest {
           Status.REJECTED_OTHER_REASON);
       forceUpdateTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON);
 
+      allowTagDeletion();
+      forceUpdateTagToExistingCommit(tagType, tagName,
+          Status.REJECTED_OTHER_REASON);
+      forceUpdateTagToNewCommit(tagType, tagName, Status.REJECTED_OTHER_REASON);
+
       allowForcePushOnRefsTags();
       forceUpdateTagToExistingCommit(tagType, tagName, Status.OK);
       forceUpdateTagToNewCommit(tagType, tagName, Status.OK);
@@ -155,6 +165,13 @@ public class PushTagIT extends AbstractDaemonTest {
       String tagName = pushTagForExistingCommit(tagType, Status.OK);
       pushTagDeletion(tagType, tagName, Status.OK);
     }
+
+    removePushFromRefsTags();
+    allowTagDeletion();
+    for (TagType tagType : TagType.values()) {
+      String tagName = pushTagForExistingCommit(tagType, Status.OK);
+      pushTagDeletion(tagType, tagName, Status.OK);
+    }
   }
 
   private String pushTagForExistingCommit(TagType tagType,
@@ -251,6 +268,11 @@ public class PushTagIT extends AbstractDaemonTest {
     grant(Permission.PUSH, project, "refs/tags/*", true, REGISTERED_USERS);
   }
 
+  private void allowTagDeletion() throws Exception {
+    removePushFromRefsTags();
+    grant(Permission.DELETE, project, "refs/tags/*", true, REGISTERED_USERS);
+  }
+
   private void removePushFromRefsTags() throws Exception {
     removePermission(Permission.PUSH, project, "refs/tags/*");
   }

gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java

@@ -24,6 +24,7 @@ public class Permission implements Comparable<Permission> {
   public static final String ABANDON = "abandon";
   public static final String ADD_PATCH_SET = "addPatchSet";
   public static final String CREATE = "create";
+  public static final String DELETE = "delete";
   public static final String DELETE_DRAFTS = "deleteDrafts";
   public static final String EDIT_HASHTAGS = "editHashtags";
   public static final String EDIT_TOPIC_NAME = "editTopicName";
@@ -56,6 +57,7 @@ public class Permission implements Comparable<Permission> {
     NAMES_LC.add(ABANDON.toLowerCase());
     NAMES_LC.add(ADD_PATCH_SET.toLowerCase());
     NAMES_LC.add(CREATE.toLowerCase());
+    NAMES_LC.add(DELETE.toLowerCase());
     NAMES_LC.add(FORGE_AUTHOR.toLowerCase());
     NAMES_LC.add(FORGE_COMMITTER.toLowerCase());
     NAMES_LC.add(FORGE_SERVER.toLowerCase());

gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties

@@ -123,6 +123,7 @@ permissionNames = \
 	abandon, \
 	addPatchSet, \
 	create, \
+	delete, \
 	deleteDrafts, \
 	editHashtags, \
 	editTopicName, \
@@ -145,6 +146,7 @@ permissionNames = \
 abandon = Abandon
 addPatchSet = Add Patch Set
 create = Create Reference
+delete = Delete Reference
 deleteDrafts = Delete Drafts
 editHashtags = Edit Hashtags
 editTopicName = Edit Topic Name

gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java

@@ -213,7 +213,27 @@ public class RefControl {
 
   /** @return true if the user can rewind (force push) the reference. */
   public boolean canForceUpdate() {
-    return (canPushWithForce() || canDelete()) && canWrite();
+    if (!canWrite()) {
+      return false;
+    }
+
+    if (canPushWithForce()) {
+      return true;
+    }
+
+    switch (getUser().getAccessPath()) {
+      case GIT:
+        return false;
+
+      case JSON_RPC:
+      case REST_API:
+      case SSH_COMMAND:
+      case UNKNOWN:
+      case WEB_BROWSER:
+      default:
+        return getUser().getCapabilities().canAdministrateServer()
+            || (isOwner() && !isForceBlocked(Permission.PUSH));
+    }
   }
 
   public boolean canWrite() {
@@ -356,7 +376,7 @@ public class RefControl {
 
     switch (getUser().getAccessPath()) {
       case GIT:
-        return canPushWithForce();
+        return canPushWithForce() || canPerform(Permission.DELETE);
 
       case JSON_RPC:
       case REST_API:
@@ -366,7 +386,8 @@ public class RefControl {
       default:
         return getUser().getCapabilities().canAdministrateServer()
             || (isOwner() && !isForceBlocked(Permission.PUSH))
-            || canPushWithForce();
+            || canPushWithForce()
+            || canPerform(Permission.DELETE);
     }
   }