Reject %wip/%ready from non-owners of change

When a user who is not the owner of a change pushes a new patch set to
that change, prevent them from modifying the WIP state of the change.

Bug: Issue 6596
Change-Id: I4931d9cea634eedec09ba75f458e8383756d3560

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/'

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