diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index fe44ce9979..bf9cb6de65 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -577,7 +577,7 @@ safest mode as commits cannot be discarded. * Force option + -Allows an existing branch to be deleted. Since a force push is +Implies <>. Since a force push is effectively a delete immediately followed by a create, but performed atomically on the server and logged, this option also permits forced push updates to branches. Enabling this option allows existing commits diff --git a/Documentation/project-configuration.txt b/Documentation/project-configuration.txt index c95cf2ccaa..079ff49180 100644 --- a/Documentation/project-configuration.txt +++ b/Documentation/project-configuration.txt @@ -273,13 +273,20 @@ There are several ways to delete a branch: - in the Web UI under 'Projects' > 'List' > > 'Branches' - via the link:rest-api-projects.html#delete-branch[Delete Branch] REST endpoint -- by using a git client to force push nothing to an existing branch +- by using a git client ++ +---- + $ git push origin --delete refs/heads/ +---- ++ +another method, by force pushing nothing to an existing branch: + ---- $ git push --force origin :refs/heads/ ---- To be able to delete branches, the user must have the +link:access-control.html#category_delete[Delete Reference] or the link:access-control.html#category_push[Push] access right with the `force` option. In addition, project owners and Gerrit administrators can delete branches from the Web UI or via REST even without having the diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java index d462b4344f..241b050c7a 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java @@ -464,6 +464,7 @@ public class PushOneCommit { public void assertErrorStatus() { RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref); + assertThat(refUpdate).isNotNull(); assertThat(refUpdate.getStatus()) .named(message(refUpdate)) .isEqualTo(Status.REJECTED_OTHER_REASON); @@ -471,12 +472,14 @@ public class PushOneCommit { private void assertStatus(Status expectedStatus, String expectedMessage) { RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref); + assertThat(refUpdate).isNotNull(); assertThat(refUpdate.getStatus()).named(message(refUpdate)).isEqualTo(expectedStatus); assertThat(refUpdate.getMessage()).isEqualTo(expectedMessage); } public void assertMessage(String expectedMessage) { RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref); + assertThat(refUpdate).isNotNull(); assertThat(message(refUpdate).toLowerCase()).contains(expectedMessage.toLowerCase()); } @@ -487,6 +490,7 @@ public class PushOneCommit { public String getMessage() { RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref); + assertThat(refUpdate).isNotNull(); return message(refUpdate); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/ForcePushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/ForcePushIT.java index 17fb08abd0..752da69816 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/ForcePushIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/ForcePushIT.java @@ -15,15 +15,23 @@ package com.google.gerrit.acceptance.git; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.acceptance.GitUtil.deleteRef; import static org.eclipse.jgit.lib.Constants.HEAD; +import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.OK; +import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.common.data.Permission; +import com.google.gerrit.extensions.api.projects.BranchInput; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.transport.PushResult; +import org.eclipse.jgit.transport.RemoteRefUpdate; import org.junit.Test; +@NoHttpd public class ForcePushIT extends AbstractDaemonTest { @Test @@ -66,4 +74,36 @@ public class ForcePushIT extends AbstractDaemonTest { PushOneCommit.Result r2 = push2.to("refs/heads/master"); r2.assertOkStatus(); } + + @Test + public void deleteNotAllowed() throws Exception { + assertDeleteRef(REJECTED_OTHER_REASON); + } + + @Test + public void deleteNotAllowedWithOnlyPushPermission() throws Exception { + grant(project, "refs/*", Permission.PUSH, false); + assertDeleteRef(REJECTED_OTHER_REASON); + } + + @Test + public void deleteAllowedWithForcePushPermission() throws Exception { + grant(project, "refs/*", Permission.PUSH, true); + assertDeleteRef(OK); + } + + @Test + public void deleteAllowedWithDeletePermission() throws Exception { + grant(project, "refs/*", Permission.PUSH, true); + assertDeleteRef(OK); + } + + private void assertDeleteRef(RemoteRefUpdate.Status expectedStatus) throws Exception { + BranchInput in = new BranchInput(); + in.ref = "refs/heads/test"; + gApi.projects().name(project.get()).branch(in.ref).create(in); + PushResult result = deleteRef(testRepo, in.ref); + RemoteRefUpdate refUpdate = result.getRemoteUpdate(in.ref); + assertThat(refUpdate.getStatus()).isEqualTo(expectedStatus); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 3b27c06835..f88dc8113c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -209,7 +209,8 @@ class ReceiveCommits { UPDATE( "You are not allowed to perform this operation.\n" + "To push into this reference you need 'Push' rights."), - DELETE("You need 'Push' rights with the 'Force Push'\nflag set to delete references."), + DELETE("You need 'Delete Reference' rights or 'Push' rights with the \n" + + "'Force Push' flag set to delete references."), DELETE_CHANGES("Cannot delete from '" + REFS_CHANGES + "'"), CODE_REVIEW( "You need 'Push' rights to upload code review requests.\n"