opendev/gerrit
Code Issues Proposed changes

Make the ability to edit topic names a grantable permission

Browse Source 
Currently any user who can upload patchsets on a change is able to
edit the change's topic name.

This is a bit too permissive.  Many companies allow all developers to
upload changes on any project, but they may not necessarily want them
all to be able to edit topic names.

Add a new permission that can be granted to allow users to edit topic
names.  Make the edit dialog only available to users who explicitly
have the permission, the project owners, the branch owner, and site
administrators.

Change-Id: Ife02370c664b2d206d731a495988a06d3123c54d
This commit is contained in:
David Pursehouse
2012-11-15 17:25:17 +09:00
parent 6bb2a555d1
commit 59aee36cc4
10 changed files with 70 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
Documentation/access-control.txt
View File

@@ -771,6 +771,18 @@ draft changes (even without having the `View Drafts` access right
assigned).
[[category_edit_topic_name]]
Edit Topic Name
~~~~~~~~~~~~~~~
This category permits users to edit the topic name of a change that
is uploaded for review.
The change owner, branch owners, project owners, and site administrators
can always edit the topic name (even without having the `Edit Topic Name`
access right assigned).
[[category_makeoneup]]
Your Category Here
~~~~~~~~~~~~~~~~~~

9
gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java
View File

@@ -48,6 +48,7 @@ public class ChangeDetail {
protected PatchSet.Id currentPatchSetId;
protected PatchSetDetail currentDetail;
protected boolean canEdit;
protected boolean canEditTopicName;
public ChangeDetail() {
}
@@ -124,6 +125,14 @@ public class ChangeDetail {
canDeleteDraft = a;
}
public boolean canEditTopicName() {
return canEditTopicName;
}
public void setCanEditTopicName(boolean a) {
canEditTopicName = a;
}
public Change getChange() {
return change;
}

2
gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java
View File

@@ -23,6 +23,7 @@ import java.util.List;
public class Permission implements Comparable<Permission> {
public static final String ABANDON = "abandon";
public static final String CREATE = "create";
public static final String EDIT_TOPIC_NAME = "editTopicName";
public static final String FORGE_AUTHOR = "forgeAuthor";
public static final String FORGE_COMMITTER = "forgeCommitter";
public static final String FORGE_SERVER = "forgeServerAsCommitter";
@@ -57,6 +58,7 @@ public class Permission implements Comparable<Permission> {
NAMES_LC.add(REMOVE_REVIEWER.toLowerCase());
NAMES_LC.add(SUBMIT.toLowerCase());
NAMES_LC.add(VIEW_DRAFTS.toLowerCase());
NAMES_LC.add(EDIT_TOPIC_NAME.toLowerCase());
labelIndex = NAMES_LC.indexOf(Permission.LABEL);
}

5
gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
View File

@@ -82,7 +82,10 @@ public class ReviewResult {
GIT_ERROR,
/** The destination branch does not exist */
DEST_BRANCH_NOT_FOUND
DEST_BRANCH_NOT_FOUND,
/** Not permitted to edit the topic name */
EDIT_TOPIC_NAME_NOT_PERMITTED
}
protected Type type;

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
View File

@@ -105,6 +105,7 @@ addPermission = Add Permission ...
permissionNames = \
abandon, \
create, \
editTopicName, \
forgeAuthor, \
forgeCommitter, \
forgeServerAsCommitter, \
@@ -119,6 +120,7 @@ permissionNames = \
viewDrafts
abandon = Abandon
create = Create Reference
editTopicName = Edit Topic Name
forgeAuthor = Forge Author Identity
forgeCommitter = Forge Committer Identity
forgeServerAsCommitter = Forge Server Identity

23
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
View File

@@ -143,15 +143,20 @@ public class ChangeInfoBlock extends Composite {
fp.add(new BranchLink(chg.getTopic(), chg.getProject(), chg.getStatus(),
dst.get(), chg.getTopic()));
final Image edit = new Image(Gerrit.RESOURCES.edit());
edit.addClickHandler(new ClickHandler() {
@Override
public void onClick(final ClickEvent event) {
new AlterTopicDialog(chg).center();
}
});
edit.addStyleName(Gerrit.RESOURCES.css().changeInfoBlockEdit());
fp.add(edit);
ChangeDetailCache detailCache = ChangeCache.get(chg.getId()).getChangeDetailCache();
ChangeDetail changeDetail = detailCache.get();
if (changeDetail.canEditTopicName()) {
final Image edit = new Image(Gerrit.RESOURCES.edit());
edit.addClickHandler(new ClickHandler() {
@Override
public void onClick(final ClickEvent event) {
new AlterTopicDialog(chg).center();
}
});
edit.addStyleName(Gerrit.RESOURCES.css().changeInfoBlockEdit());
fp.add(edit);
}
return fp;
}

1
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
View File

@@ -154,6 +154,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
detail.setCanRevert(change.getStatus() == Change.Status.MERGED && control.canAddPatchSet());
detail.setCanEdit(control.getRefControl().canWrite());
detail.setCanEditTopicName(control.canEditTopicName());
List<SubmitRecord> submitRecords = control.getSubmitRecords(db, patch);
for (SubmitRecord rec : submitRecords) {

16
gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.changedetail;
import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -24,7 +25,6 @@ import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtjsonrpc.common.VoidResult;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -35,7 +35,7 @@ import java.util.concurrent.Callable;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
public class AlterTopic implements Callable<VoidResult> {
public class AlterTopic implements Callable<ReviewResult> {
private final ChangeControl.Factory changeControlFactory;
private final ReviewDb db;
@@ -77,12 +77,20 @@ public class AlterTopic implements Callable<VoidResult> {
}
@Override
public VoidResult call() throws EmailException,
public ReviewResult call() throws EmailException,
InvalidChangeOperationException, NoSuchChangeException, OrmException {
final ChangeControl control = changeControlFactory.validateFor(changeId);
final ReviewResult result = new ReviewResult();
result.setChangeId(changeId);
if (!control.canAddPatchSet()) {
throw new NoSuchChangeException(changeId);
}
if (!control.canEditTopicName()) {
result.addError(new ReviewResult.Error(
ReviewResult.Error.Type.EDIT_TOPIC_NAME_NOT_PERMITTED));
return result;
}
final Change change = db.changes().get(changeId);
if (!change.getTopic().equals(topic)) {
@@ -114,6 +122,6 @@ public class AlterTopic implements Callable<VoidResult> {
db.changeMessages().insert(Collections.singleton(cmsg));
}
return VoidResult.INSTANCE;
return result;
}
}

10
gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
View File

@@ -289,6 +289,16 @@ public class ChangeControl {
return false;
}
/** Can this user edit the topic name? */
public boolean canEditTopicName() {
return isOwner() // owner (aka creator) of the change can edit topic
|| getRefControl().isOwner() // branch owner can edit topic
|| getProjectControl().isOwner() // project owner can edit topic
|| getCurrentUser().getCapabilities().canAdministrateServer() // site administers are god
|| getRefControl().canEditTopicName() // user can edit topic on a specific ref
;
}
public List<SubmitRecord> getSubmitRecords(ReviewDb db, PatchSet patchSet) {
return canSubmit(db, patchSet, null, false, true);
}

4
gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
View File

@@ -332,6 +332,10 @@ public class RefControl {
return canPerform(Permission.VIEW_DRAFTS);
}
public boolean canEditTopicName() {
return canPerform(Permission.EDIT_TOPIC_NAME);
}
/** All value ranges of any allowed label permission. */
public List<PermissionRange> getLabelRanges() {
List<PermissionRange> r = new ArrayList<PermissionRange>();