Merge changes I67865057,Ic45e51bd

* changes:
  Add separate 'Delete Reference' permission
  Add more tests for branch deletion

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

@@ -36,6 +36,7 @@ public class DeleteBranchIT extends AbstractDaemonTest {
 
   @Before
   public void setUp() throws Exception {
+    project = createProject(name("p"));
     branch = new Branch.NameKey(project, "test");
     branch().create(new BranchInput());
   }
@@ -73,10 +74,32 @@ public class DeleteBranchIT extends AbstractDaemonTest {
     assertDeleteForbidden();
   }
 
+  @Test
+  public void deleteBranchByUserWithForcePushPermission() throws Exception {
+    grantForcePush();
+    setApiUser(user);
+    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);
   }
 
+  private void grantForcePush() throws Exception {
+    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/*");
   }
@@ -99,6 +122,7 @@ public class DeleteBranchIT extends AbstractDaemonTest {
 
   private void assertDeleteForbidden() throws Exception {
     exception.expect(AuthException.class);
+    exception.expectMessage("Cannot delete branch");
     branch().delete();
   }
 }

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);
     }
   }