Check permissions on refs/meta/config on save of ACLs for review

Users should not be able to create a change for a branch that is not
visible to them, especially since the created change would be not
visible to them.

Creation of changes for the refs/meta/config in addition requires that
the user has the permission to upload changes to this branch or is
project owner.

If a project owner tried to save access rights modifications for
review it failed with a Server Error showing a NotFoundException,
since the created change was not visible to the user. Now the creation
of the change fails and a proper error message is displayed.

Change-Id: I6ca243eb8546011427c921e6e1fdef23626f0fa6
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-12-02 09:17:29 +01:00
parent b1c401bc1a
commit 9023196e86
3 changed files with 24 additions and 13 deletions

View File

@@ -19,7 +19,6 @@ import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.ProjectAccess;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -76,13 +75,14 @@ class ChangeProjectAccess extends ProjectAccessHandler<ProjectAccess> {
}
@Override
protected ProjectAccess updateProjectConfig(CurrentUser user,
protected ProjectAccess updateProjectConfig(ProjectControl projectControl,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, NoSuchProjectException, ConfigInvalidException {
RevCommit commit = config.commit(md);
gitRefUpdated.fire(config.getProject().getNameKey(), RefNames.REFS_CONFIG,
base, commit.getId(), user.asIdentifiedUser().getAccount());
base, commit.getId(),
projectControl.getUser().asIdentifiedUser().getAccount());
projectCache.evict(config.getProject());
return projectAccessFactory.create(projectName).call();

View File

@@ -31,7 +31,6 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupBackends;
import com.google.gerrit.server.config.AllProjectsName;
@@ -163,17 +162,17 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
md.setMessage("Modify access rules\n");
}
return updateProjectConfig(projectControl.getUser(), config, md,
return updateProjectConfig(projectControl, config, md,
parentProjectUpdate);
} catch (RepositoryNotFoundException notFound) {
throw new NoSuchProjectException(projectName);
}
}
protected abstract T updateProjectConfig(CurrentUser user,
protected abstract T updateProjectConfig(ProjectControl projectControl,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, NoSuchProjectException, ConfigInvalidException,
OrmException;
OrmException, PermissionDeniedException;
private void replace(ProjectConfig config, Set<String> toDelete,
AccessSection section) throws NoSuchGroupException {

View File

@@ -19,6 +19,7 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.errors.PermissionDeniedException;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -27,7 +28,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.change.ChangeInserter;
@@ -43,6 +43,7 @@ import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.project.SetParent;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -106,9 +107,20 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
}
@Override
protected Change.Id updateProjectConfig(CurrentUser user,
protected Change.Id updateProjectConfig(ProjectControl projectControl,
ProjectConfig config, MetaDataUpdate md, boolean parentProjectUpdate)
throws IOException, OrmException {
throws IOException, OrmException, PermissionDeniedException {
RefControl refsMetaConfigControl =
projectControl.controlForRef(RefNames.REFS_CONFIG);
if (!refsMetaConfigControl.isVisible()) {
throw new PermissionDeniedException(
RefNames.REFS_CONFIG + " not visible");
}
if (!projectControl.isOwner() && !refsMetaConfigControl.canUpload()) {
throw new PermissionDeniedException(
"cannot upload to " + RefNames.REFS_CONFIG);
}
md.setInsertChangeId(true);
Change.Id changeId = new Change.Id(seq.nextChangeId());
RevCommit commit =
@@ -120,9 +132,9 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
try (RevWalk rw = new RevWalk(md.getRepository());
ObjectInserter objInserter = md.getRepository().newObjectInserter();
BatchUpdate bu = updateFactory.create(
db, config.getProject().getNameKey(), user,
TimeUtil.nowTs())) {
BatchUpdate bu =
updateFactory.create(db, config.getProject().getNameKey(),
projectControl.getUser(), TimeUtil.nowTs())) {
bu.setRepository(md.getRepository(), rw, objInserter);
bu.insertChange(
changeInserterFactory.create(changeId, commit, RefNames.REFS_CONFIG)