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
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 773ad44..bcf62bd 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.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,16 +455,52 @@
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/'
PushOneCommit.Result r = pushTo("refs/drafts/master");
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 6b3f173..5cd2184 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 @@
+ "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 @@
}
}
+ 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();
}
