Convert parsing projects to use PermissionBackend
When parsing a project name from a command line argument or in the
REST API, check the caller has ACCESS permission using
PermissionBackend, failing if they don't.
In UploadArchive check READ permission to determine if the
reachability check can be skipped.
Change-Id: I8b9155834a4ab36b964e339f5d9e1d110f771158
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java
index d80c40b..38887f6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java
@@ -80,7 +80,8 @@
}
private Map<ProjectWatchKey, Set<NotifyType>> asMap(List<ProjectWatchInfo> input)
- throws BadRequestException, UnprocessableEntityException, IOException {
+ throws BadRequestException, UnprocessableEntityException, IOException,
+ PermissionBackendException {
Map<ProjectWatchKey, Set<NotifyType>> m = new HashMap<>();
for (ProjectWatchInfo info : input) {
if (info.project == null) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
index c77f86f..9a89d48 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
@@ -32,6 +32,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.CreateChange;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gerrit.server.update.UpdateException;
@@ -87,7 +88,11 @@
try {
ChangeInfo out = createChange.apply(TopLevelResource.INSTANCE, in).value();
return api.create(changes.parse(new Change.Id(out._number)));
- } catch (OrmException | IOException | InvalidChangeOperationException | UpdateException e) {
+ } catch (OrmException
+ | IOException
+ | InvalidChangeOperationException
+ | UpdateException
+ | PermissionBackendException e) {
throw new RestApiException("Cannot create change", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/groups/GroupsImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/groups/GroupsImpl.java
index 6eef5e1..ecbde59 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/groups/GroupsImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/groups/GroupsImpl.java
@@ -122,7 +122,7 @@
for (String project : req.getProjects()) {
try {
list.addProject(projects.parse(tlr, IdString.fromDecoded(project)).getControl());
- } catch (IOException e) {
+ } catch (IOException | PermissionBackendException e) {
throw new RestApiException("Error looking up project " + project, e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
index 440b997..1aa203c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
@@ -389,7 +389,7 @@
public ChildProjectApi child(String name) throws RestApiException {
try {
return childApi.create(children.parse(checkExists(), IdString.fromDecoded(name)));
- } catch (IOException e) {
+ } catch (IOException | PermissionBackendException e) {
throw new RestApiException("Cannot parse child project", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectsImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectsImpl.java
index 9483508..b843328 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectsImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/projects/ProjectsImpl.java
@@ -21,6 +21,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ListProjects;
import com.google.gerrit.server.project.ListProjects.FilterType;
import com.google.gerrit.server.project.ProjectsCollection;
@@ -52,8 +53,8 @@
return api.create(projects.parse(name));
} catch (UnprocessableEntityException e) {
return api.create(name);
- } catch (IOException e) {
- throw new RestApiException("Cannot retrieve project");
+ } catch (IOException | PermissionBackendException e) {
+ throw new RestApiException("Cannot retrieve project", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ProjectControlHandler.java b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ProjectControlHandler.java
index 02e907f..bd0cdcd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ProjectControlHandler.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ProjectControlHandler.java
@@ -15,8 +15,12 @@
package com.google.gerrit.server.args4j;
import com.google.gerrit.common.ProjectUtil;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.inject.Inject;
@@ -34,18 +38,22 @@
public class ProjectControlHandler extends OptionHandler<ProjectControl> {
private static final Logger log = LoggerFactory.getLogger(ProjectControlHandler.class);
+
private final ProjectControl.GenericFactory projectControlFactory;
+ private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
@Inject
public ProjectControlHandler(
- final ProjectControl.GenericFactory projectControlFactory,
+ ProjectControl.GenericFactory projectControlFactory,
+ PermissionBackend permissionBackend,
Provider<CurrentUser> user,
@Assisted final CmdLineParser parser,
@Assisted final OptionDef option,
@Assisted final Setter<ProjectControl> setter) {
super(parser, option, setter);
this.projectControlFactory = projectControlFactory;
+ this.permissionBackend = permissionBackend;
this.user = user;
}
@@ -69,14 +77,15 @@
String nameWithoutSuffix = ProjectUtil.stripGitSuffix(projectName);
Project.NameKey nameKey = new Project.NameKey(nameWithoutSuffix);
- final ProjectControl control;
+ ProjectControl control;
try {
- control =
- projectControlFactory.validateFor(
- nameKey, ProjectControl.OWNER | ProjectControl.VISIBLE, user.get());
+ control = projectControlFactory.controlFor(nameKey, user.get());
+ permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
+ } catch (AuthException e) {
+ throw new CmdLineException(owner, new NoSuchProjectException(nameKey).getMessage());
} catch (NoSuchProjectException e) {
throw new CmdLineException(owner, e.getMessage());
- } catch (IOException e) {
+ } catch (PermissionBackendException | IOException e) {
log.warn("Cannot load project " + nameWithoutSuffix, e);
throw new CmdLineException(owner, new NoSuchProjectException(nameKey).getMessage());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
index 374a8cc..e0cb2e1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
@@ -53,6 +53,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.ProjectControl;
@@ -145,7 +146,7 @@
@Override
public Response<ChangeInfo> apply(TopLevelResource parent, ChangeInput input)
throws OrmException, IOException, InvalidChangeOperationException, RestApiException,
- UpdateException {
+ UpdateException, PermissionBackendException {
if (Strings.isNullOrEmpty(input.project)) {
throw new BadRequestException("project must be non-empty");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChildProjectsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChildProjectsCollection.java
index 7aa5f68..2110034 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChildProjectsCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChildProjectsCollection.java
@@ -21,6 +21,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -50,7 +51,7 @@
@Override
public ChildProjectResource parse(ProjectResource parent, IdString id)
- throws ResourceNotFoundException, IOException {
+ throws ResourceNotFoundException, IOException, PermissionBackendException {
ProjectResource p = projectsCollection.parse(TopLevelResource.INSTANCE, id);
for (ProjectState pp : p.getControl().getProjectState().parents()) {
if (parent.getNameKey().equals(pp.getProject().getNameKey())) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
index ff7e31e..199f5c0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
@@ -55,6 +55,7 @@
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.RepositoryCaseMismatchException;
import com.google.gerrit.server.group.GroupsCollection;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
@@ -148,7 +149,8 @@
@Override
public Response<ProjectInfo> apply(TopLevelResource resource, ProjectInput input)
throws BadRequestException, UnprocessableEntityException, ResourceConflictException,
- ResourceNotFoundException, IOException, ConfigInvalidException {
+ ResourceNotFoundException, IOException, ConfigInvalidException,
+ PermissionBackendException {
if (input == null) {
input = new ProjectInput();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java
index dcb3404..d461a7d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java
@@ -14,8 +14,10 @@
package com.google.gerrit.server.project;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsCreate;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestCollection;
@@ -25,6 +27,9 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.OutputFormat;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -37,6 +42,7 @@
private final DynamicMap<RestView<ProjectResource>> views;
private final Provider<ListProjects> list;
private final ProjectControl.GenericFactory controlFactory;
+ private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
private final CreateProject.Factory createProjectFactory;
@@ -45,11 +51,13 @@
DynamicMap<RestView<ProjectResource>> views,
Provider<ListProjects> list,
ProjectControl.GenericFactory controlFactory,
+ PermissionBackend permissionBackend,
CreateProject.Factory factory,
Provider<CurrentUser> user) {
this.views = views;
this.list = list;
this.controlFactory = controlFactory;
+ this.permissionBackend = permissionBackend;
this.user = user;
this.createProjectFactory = factory;
}
@@ -61,7 +69,7 @@
@Override
public ProjectResource parse(TopLevelResource parent, IdString id)
- throws ResourceNotFoundException, IOException {
+ throws ResourceNotFoundException, IOException, PermissionBackendException {
ProjectResource rsrc = _parse(id.get(), true);
if (rsrc == null) {
throw new ResourceNotFoundException(id);
@@ -77,8 +85,10 @@
* @throws UnprocessableEntityException thrown if the project ID cannot be resolved or if the
* project is not visible to the calling user
* @throws IOException thrown when there is an error.
+ * @throws PermissionBackendException
*/
- public ProjectResource parse(String id) throws UnprocessableEntityException, IOException {
+ public ProjectResource parse(String id)
+ throws UnprocessableEntityException, IOException, PermissionBackendException {
return parse(id, true);
}
@@ -86,33 +96,43 @@
* Parses a project ID from a request body and returns the project.
*
* @param id ID of the project, can be a project name
- * @param checkVisibility Whether to check or not that project is visible to the calling user
+ * @param checkAccess if true, check the project is accessible by the current user
* @return the project
* @throws UnprocessableEntityException thrown if the project ID cannot be resolved or if the
* project is not visible to the calling user and checkVisibility is true.
* @throws IOException thrown when there is an error.
+ * @throws PermissionBackendException
*/
- public ProjectResource parse(String id, boolean checkVisibility)
- throws UnprocessableEntityException, IOException {
- ProjectResource rsrc = _parse(id, checkVisibility);
+ public ProjectResource parse(String id, boolean checkAccess)
+ throws UnprocessableEntityException, IOException, PermissionBackendException {
+ ProjectResource rsrc = _parse(id, checkAccess);
if (rsrc == null) {
throw new UnprocessableEntityException(String.format("Project Not Found: %s", id));
}
return rsrc;
}
- private ProjectResource _parse(String id, boolean checkVisibility) throws IOException {
+ @Nullable
+ private ProjectResource _parse(String id, boolean checkAccess)
+ throws IOException, PermissionBackendException {
if (id.endsWith(Constants.DOT_GIT_EXT)) {
id = id.substring(0, id.length() - Constants.DOT_GIT_EXT.length());
}
+
+ Project.NameKey nameKey = new Project.NameKey(id);
ProjectControl ctl;
try {
- ctl = controlFactory.controlFor(new Project.NameKey(id), user.get());
+ ctl = controlFactory.controlFor(nameKey, user.get());
} catch (NoSuchProjectException e) {
return null;
}
- if (checkVisibility && !ctl.isVisible() && !ctl.isOwner()) {
- return null;
+
+ if (checkAccess) {
+ try {
+ permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
+ } catch (AuthException e) {
+ return null; // Pretend like not found on access denied.
+ }
}
return new ProjectResource(ctl);
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
index 4144ed2..dc96e18 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
@@ -18,6 +18,7 @@
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.sshd.SshScope.Context;
import com.google.inject.Inject;
@@ -84,7 +85,7 @@
return n;
}
- private void service() throws IOException, Failure {
+ private void service() throws IOException, PermissionBackendException, Failure {
project = projectControl.getProjectState().getProject();
try {
@@ -100,5 +101,5 @@
}
}
- protected abstract void runImpl() throws IOException, Failure;
+ protected abstract void runImpl() throws IOException, PermissionBackendException, Failure;
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java
index 4b8771a..093808c 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java
@@ -17,9 +17,14 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.AllowedFormats;
import com.google.gerrit.server.change.ArchiveFormat;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.sshd.AbstractGitCommand;
import com.google.inject.Inject;
import java.io.IOException;
@@ -115,6 +120,8 @@
private List<String> path;
}
+ @Inject private PermissionBackend permissionBackend;
+ @Inject private IdentifiedUser user;
@Inject private AllowedFormats allowedFormats;
@Inject private ReviewDb db;
private Options options = new Options();
@@ -156,7 +163,7 @@
}
@Override
- protected void runImpl() throws IOException, Failure {
+ protected void runImpl() throws IOException, PermissionBackendException, Failure {
PacketLineOut packetOut = new PacketLineOut(out);
packetOut.setFlushOnEnd(true);
packetOut.writeString("ACK");
@@ -177,8 +184,8 @@
throw new Failure(4, "fatal: reference not found");
}
- // Verify the user has permissions to read the specified reference
- if (!projectControl.allRefsAreVisible() && !canRead(treeId)) {
+ // Verify the user has permissions to read the specified tree.
+ if (!canRead(treeId)) {
throw new Failure(5, "fatal: cannot perform upload-archive operation");
}
@@ -235,10 +242,16 @@
return Collections.emptyMap();
}
- private boolean canRead(ObjectId revId) throws IOException {
- try (RevWalk rw = new RevWalk(repo)) {
- RevCommit commit = rw.parseCommit(revId);
- return projectControl.canReadCommit(db, repo, commit);
+ private boolean canRead(ObjectId revId) throws IOException, PermissionBackendException {
+ try {
+ permissionBackend.user(user).project(project.getNameKey()).check(ProjectPermission.READ);
+ return true;
+ } catch (AuthException e) {
+ // Check reachability of the specific revision.
+ try (RevWalk rw = new RevWalk(repo)) {
+ RevCommit commit = rw.parseCommit(revId);
+ return projectControl.canReadCommit(db, repo, commit);
+ }
}
}
}
