Add new "Delete Changes" permission
Changes can only be deleted by Administrators, or by the change owner
when they are granted the "Delete Own Changes" permission either on the
"Change Owners" virtual group or an internal group they are a member of.
This means that it is not possible to allow users to delete other users'
changes other than by granting the "Administrate Server" permission,
which also gives them all the other administrative capabilities. This is
also inconsistent with how the "Delete Drafts" permission works.
An example of where it would be better to have more flexible permissions
is allowing changes to be deleted by Project Owners on a per-project
basis, or by a group of users who are trusted (but not enough to grant
them Administrator capabilities).
Add a new permission "Delete Changes" which allows these use cases, thus
a change can now also be deleted by users who are:
- Member of a group that is explicitly granted "Delete Changes" on the
change's destination branch.
- Member of a group that is given the "Owner" permission on the project,
when the virtual group "Project Owners" is granted "Delete Changes" on
the change's destination branch.
Bug: Issue 9354
Change-Id: I9d5d779eb0a6ce18faca02b9fc90904ec5da91d9
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 9a849dc..e55378f 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -887,6 +887,17 @@
This category permits users to delete their own changes if they are not merged
yet. This means only own changes that are open or abandoned can be deleted.
+[[category_delete_changes]]
+=== Delete Changes
+
+This category permits users to delete other users' changes if they are not merged
+yet. This means only changes that are open or abandoned can be deleted.
+
+Having this permission implies having the link:#category_delete_own_changes[
+Delete Own Changes] permission.
+
+Administrators may always delete changes without having this permission.
+
[[category_edit_topic_name]]
=== Edit Topic Name
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 434a7a4..821ec4b 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -276,7 +276,9 @@
For open or abandoned changes, the `Delete Change` button will be available
and if the user is the change owner and is granted the
link:access-control.html#category_delete_own_changes[Delete Own Changes]
-permission or if they are an administrator. For draft changes,
+permission, if they are granted the
+link:access-control.html#category_delete_changes[Delete Changes] permisson,
+or if they are an administrator. For draft changes,
the `Delete Change` / `Delete Revision` buttons will be available if the user is
the change owner or has the
link:access-control.html#category_delete_drafts[Delete Drafts] access right assigned.
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 95bde73..1bde868 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -26,6 +26,7 @@
import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
+import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.value;
@@ -64,6 +65,8 @@
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.groups.GroupApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.api.projects.ProjectApi;
+import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.Comment.Range;
@@ -460,6 +463,33 @@
}
@Test
+ public void deleteNewChangeAsUserWithDeleteChangesPermissionForGroup() throws Exception {
+ allow(Permission.DELETE_CHANGES, REGISTERED_USERS, "refs/*");
+ deleteChangeAsUser(admin, user);
+ }
+
+ @Test
+ public void deleteNewChangeAsUserWithDeleteChangesPermissionForProjectOwners() throws Exception {
+ GroupApi groupApi = gApi.groups().create(name("delete-change"));
+ groupApi.addMembers("user");
+
+ ProjectInput in = new ProjectInput();
+ in.name = name("delete-change");
+ in.owners = Lists.newArrayListWithCapacity(1);
+ in.owners.add(groupApi.name());
+ in.createEmptyCommit = true;
+ ProjectApi api = gApi.projects().create(in);
+
+ Project.NameKey nameKey = new Project.NameKey(api.get().name);
+
+ ProjectConfig cfg = projectCache.checkedGet(nameKey).getConfig();
+ Util.allow(cfg, Permission.DELETE_CHANGES, PROJECT_OWNERS, "refs/*");
+ saveProjectConfig(nameKey, cfg);
+
+ deleteChangeAsUser(nameKey, admin, user);
+ }
+
+ @Test
public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
allow(Permission.DELETE_OWN_CHANGES, REGISTERED_USERS, "refs/*");
deleteChangeAsUser(user, user);
@@ -472,10 +502,15 @@
}
private void deleteChangeAsUser(TestAccount owner, TestAccount deleteAs) throws Exception {
+ deleteChangeAsUser(project, owner, deleteAs);
+ }
+
+ private void deleteChangeAsUser(
+ Project.NameKey projectName, TestAccount owner, TestAccount deleteAs) throws Exception {
try {
setApiUser(owner);
ChangeInput in = new ChangeInput();
- in.project = project.get();
+ in.project = projectName.get();
in.branch = "refs/heads/master";
in.subject = "test";
String changeId = gApi.changes().create(in).get().changeId;
@@ -488,6 +523,7 @@
assertThat(query(changeId)).isEmpty();
} finally {
removePermission(Permission.DELETE_OWN_CHANGES, project, "refs/*");
+ removePermission(Permission.DELETE_CHANGES, project, "refs/*");
}
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
index 47c5224..30bd089 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
@@ -27,6 +27,7 @@
public static final String DELETE = "delete";
public static final String CREATE_TAG = "createTag";
public static final String CREATE_SIGNED_TAG = "createSignedTag";
+ public static final String DELETE_CHANGES = "deleteChanges";
public static final String DELETE_DRAFTS = "deleteDrafts";
public static final String DELETE_OWN_CHANGES = "deleteOwnChanges";
public static final String EDIT_HASHTAGS = "editHashtags";
@@ -79,6 +80,7 @@
NAMES_LC.add(EDIT_ASSIGNEE.toLowerCase());
NAMES_LC.add(DELETE_DRAFTS.toLowerCase());
NAMES_LC.add(DELETE_OWN_CHANGES.toLowerCase());
+ NAMES_LC.add(DELETE_CHANGES.toLowerCase());
NAMES_LC.add(PUBLISH_DRAFTS.toLowerCase());
LABEL_INDEX = NAMES_LC.indexOf(Permission.LABEL);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 9e244a5..40be5f5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -261,7 +261,7 @@
return (isOwner() || getRefControl().canDeleteDrafts());
case NEW:
case ABANDONED:
- return (isAdmin() || (isOwner() && getRefControl().canDeleteOwnChanges(isOwner())));
+ return (isAdmin() || getRefControl().canDeleteChanges(isOwner()));
case MERGED:
default:
return false;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 5c50001..ada1855 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -417,9 +417,10 @@
return canPerform(Permission.DELETE_DRAFTS);
}
- /** @return true if this user can delete their own changes. */
- public boolean canDeleteOwnChanges(boolean isChangeOwner) {
- return canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner);
+ /** @return true if this user can delete changes. */
+ public boolean canDeleteChanges(boolean isChangeOwner) {
+ return canPerform(Permission.DELETE_CHANGES)
+ || (isChangeOwner && canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner));
}
/** @return true if this user can edit topic names. */
