diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 773ad44a2e..bcf62bdc6c 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -62,6 +62,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.git.ReceiveCommits; import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.project.Util; import com.google.gerrit.server.query.change.ChangeData; @@ -454,15 +455,51 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { assertThat(r.getChange().change().isWorkInProgress()).isFalse(); // Make the change work-in-progress again. - r = pushTo("refs/for/master%wip"); + r = amendChange(r.getChangeId(), "refs/for/master%wip"); r.assertOkStatus(); assertThat(r.getChange().change().isWorkInProgress()).isTrue(); // Can't use --wip and --ready together. - r = pushTo("refs/for/master%wip,ready"); + r = amendChange(r.getChangeId(), "refs/for/master%wip,ready"); r.assertErrorStatus(); } + @Test + public void pushWorkInProgressChangeWhenNotOwner() throws Exception { + TestRepository userRepo = cloneProject(project, user); + PushOneCommit.Result r = + pushFactory.create(db, user.getIdent(), userRepo).to("refs/for/master%wip"); + r.assertOkStatus(); + assertThat(r.getChange().change().getOwner()).isEqualTo(user.id); + assertThat(r.getChange().change().isWorkInProgress()).isTrue(); + + // Other user trying to move from WIP to ready should fail. + GitUtil.fetch(testRepo, r.getPatchSet().getRefName() + ":ps"); + testRepo.reset("ps"); + r = amendChange(r.getChangeId(), "refs/for/master%ready", admin, testRepo); + r.assertErrorStatus(ReceiveCommits.ONLY_OWNER_CAN_MODIFY_WIP); + + // Other user trying to move from WIP to WIP should succeed. + r = amendChange(r.getChangeId(), "refs/for/master%wip", admin, testRepo); + r.assertOkStatus(); + assertThat(r.getChange().change().isWorkInProgress()).isTrue(); + + // Push as change owner to move change from WIP to ready. + r = pushFactory.create(db, user.getIdent(), userRepo).to("refs/for/master%ready"); + r.assertOkStatus(); + assertThat(r.getChange().change().isWorkInProgress()).isFalse(); + + // Other user trying to move from ready to WIP should fail. + GitUtil.fetch(testRepo, r.getPatchSet().getRefName() + ":ps"); + testRepo.reset("ps"); + r = amendChange(r.getChangeId(), "refs/for/master%wip", admin, testRepo); + r.assertErrorStatus(ReceiveCommits.ONLY_OWNER_CAN_MODIFY_WIP); + + // Other user trying to move from ready to ready should succeed. + r = amendChange(r.getChangeId(), "refs/for/master%ready", admin, testRepo); + r.assertOkStatus(); + } + @Test public void pushForMasterAsDraft() throws Exception { // create draft by pushing to 'refs/drafts/' diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 6b3f173bfa..5cd2184325 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -202,6 +202,9 @@ public class ReceiveCommits { + "Squash the commits with the same Change-Id or " + "ensure Change-Ids are unique for each commit"; + public static final String ONLY_OWNER_CAN_MODIFY_WIP = + "only change owner can modify Work-in-Progress"; + private enum Error { CONFIG_UPDATE( "You are not allowed to perform this operation.\n" @@ -2484,6 +2487,14 @@ public class ReceiveCommits { } } + if (magicBranch != null + && (magicBranch.workInProgress || magicBranch.ready) + && magicBranch.workInProgress != change.isWorkInProgress() + && !user.getAccountId().equals(change.getOwner())) { + reject(inputCommand, ONLY_OWNER_CAN_MODIFY_WIP); + return false; + } + if (magicBranch != null && magicBranch.edit) { return newEdit(); }