Check canEditTopicName with PermissionBackend

Change-Id: Iaa731018ea2a0aaf2a44e684d9b232769117ef1a
This commit is contained in:
Shawn Pearce
2017-02-18 15:35:05 -08:00
committed by David Pursehouse
parent 3a9bd79051
commit 03c48e308c
6 changed files with 23 additions and 15 deletions

View File

@@ -1737,7 +1737,7 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(gApi.changes().id(r.getChangeId()).topic()).isEqualTo(""); assertThat(gApi.changes().id(r.getChangeId()).topic()).isEqualTo("");
setApiUser(user); setApiUser(user);
exception.expect(AuthException.class); exception.expect(AuthException.class);
exception.expectMessage("changing topic not permitted"); exception.expectMessage("edit topic name not permitted");
gApi.changes().id(r.getChangeId()).topic("mytopic"); gApi.changes().id(r.getChangeId()).topic("mytopic");
} }

View File

@@ -74,6 +74,7 @@ import com.google.gerrit.server.change.Reviewers;
import com.google.gerrit.server.change.Revisions; import com.google.gerrit.server.change.Revisions;
import com.google.gerrit.server.change.SubmittedTogether; import com.google.gerrit.server.change.SubmittedTogether;
import com.google.gerrit.server.change.SuggestChangeReviewers; import com.google.gerrit.server.change.SuggestChangeReviewers;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -386,7 +387,7 @@ class ChangeApiImpl implements ChangeApi {
in.topic = topic; in.topic = topic;
try { try {
putTopic.apply(change, in); putTopic.apply(change, in);
} catch (UpdateException e) { } catch (UpdateException | PermissionBackendException e) {
throw new RestApiException("Cannot set topic", e); throw new RestApiException("Cannot set topic", e);
} }
} }

View File

@@ -29,6 +29,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -53,15 +54,24 @@ public class ChangeResource implements RestResource, HasETag {
ChangeResource create(ChangeControl ctl); ChangeResource create(ChangeControl ctl);
} }
private final PermissionBackend permissionBackend;
private final StarredChangesUtil starredChangesUtil; private final StarredChangesUtil starredChangesUtil;
private final ChangeControl control; private final ChangeControl control;
@Inject @Inject
ChangeResource(StarredChangesUtil starredChangesUtil, @Assisted ChangeControl control) { ChangeResource(
PermissionBackend permissionBackend,
StarredChangesUtil starredChangesUtil,
@Assisted ChangeControl control) {
this.permissionBackend = permissionBackend;
this.starredChangesUtil = starredChangesUtil; this.starredChangesUtil = starredChangesUtil;
this.control = control; this.control = control;
} }
public PermissionBackend.ForChange permissions() {
return permissionBackend.user(getControl().getUser()).change(getNotes());
}
public ChangeControl getControl() { public ChangeControl getControl() {
return control; return control;
} }

View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.change;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
@@ -29,7 +28,8 @@ import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.change.PutTopic.Input; import com.google.gerrit.server.change.PutTopic.Input;
import com.google.gerrit.server.extensions.events.TopicEdited; import com.google.gerrit.server.extensions.events.TopicEdited;
import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.server.update.ChangeContext;
@@ -65,16 +65,13 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>, UiAction
@Override @Override
public Response<String> apply(ChangeResource req, Input input) public Response<String> apply(ChangeResource req, Input input)
throws UpdateException, RestApiException { throws UpdateException, RestApiException, PermissionBackendException {
ChangeControl ctl = req.getControl(); req.permissions().check(ChangePermission.EDIT_TOPIC_NAME);
if (!ctl.canEditTopicName()) {
throw new AuthException("changing topic not permitted");
}
Op op = new Op(input != null ? input : new Input()); Op op = new Op(input != null ? input : new Input());
try (BatchUpdate u = try (BatchUpdate u =
batchUpdateFactory.create( batchUpdateFactory.create(
dbProvider.get(), req.getChange().getProject(), ctl.getUser(), TimeUtil.nowTs())) { dbProvider.get(), req.getChange().getProject(), req.getUser(), TimeUtil.nowTs())) {
u.addOp(req.getId(), op); u.addOp(req.getId(), op);
u.execute(); u.execute();
} }
@@ -129,9 +126,9 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>, UiAction
} }
@Override @Override
public UiAction.Description getDescription(ChangeResource resource) { public UiAction.Description getDescription(ChangeResource rsrc) {
return new UiAction.Description() return new UiAction.Description()
.setLabel("Edit Topic") .setLabel("Edit Topic")
.setVisible(resource.getControl().canEditTopicName()); .setVisible(rsrc.permissions().testOrFalse(ChangePermission.EDIT_TOPIC_NAME));
} }
} }

View File

@@ -432,7 +432,7 @@ public class ChangeControl {
} }
/** Can this user edit the topic name? */ /** Can this user edit the topic name? */
public boolean canEditTopicName() { private boolean canEditTopicName() {
if (getChange().getStatus().isOpen()) { if (getChange().getStatus().isOpen()) {
return isOwner() // owner (aka creator) of the change can edit topic return isOwner() // owner (aka creator) of the change can edit topic
|| getRefControl().isOwner() // branch owner can edit topic || getRefControl().isOwner() // branch owner can edit topic

View File

@@ -442,7 +442,7 @@ public class RefControl {
} }
/** @return true if this user can edit topic names. */ /** @return true if this user can edit topic names. */
public boolean canEditTopicName() { boolean canEditTopicName() {
return canPerform(Permission.EDIT_TOPIC_NAME); return canPerform(Permission.EDIT_TOPIC_NAME);
} }