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>
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
index 3f471bf..b39e2a2 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java
@@ -19,7 +19,6 @@
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 @@
}
@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();
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
index 111dfc9..4c7d257 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
@@ -31,7 +31,6 @@
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 @@
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 {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
index 9260e01..966cd88 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
@@ -19,6 +19,7 @@
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.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.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 @@
}
@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 @@
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)
