ListAccess: Fail for non-visible projects
ListAccess allows to specify a list of projects via request parameters
but it wasn't checked whether these projects are visible. This allowed
users to probe whether a project exists.
If a project is not visible we return the same error message as for a
non-existing project so that users cannot probe the existence of a
project.
In contrast to ListAccess GetAccess doesn't need to do the permission
check as it is already done by ProjectsCollection#parse which is
invoked before GetAccess is invoked if a REST call is done for a single
project.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I76de47b097d18a6c2f4cd42650aa05a51094b839
diff --git a/java/com/google/gerrit/server/restapi/access/ListAccess.java b/java/com/google/gerrit/server/restapi/access/ListAccess.java
index 9ce35de..dca969d 100644
--- a/java/com/google/gerrit/server/restapi/access/ListAccess.java
+++ b/java/com/google/gerrit/server/restapi/access/ListAccess.java
@@ -17,9 +17,14 @@
import com.google.common.base.Strings;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.restapi.project.GetAccess;
import com.google.inject.Inject;
import java.util.ArrayList;
@@ -42,10 +47,15 @@
usage = "projects for which the access rights should be returned")
private List<String> projects = new ArrayList<>();
+ private final PermissionBackend permissionBackend;
+ private final ProjectCache projectCache;
private final GetAccess getAccess;
@Inject
- public ListAccess(GetAccess getAccess) {
+ public ListAccess(
+ PermissionBackend permissionBackend, ProjectCache projectCache, GetAccess getAccess) {
+ this.permissionBackend = permissionBackend;
+ this.projectCache = projectCache;
this.getAccess = getAccess;
}
@@ -58,7 +68,19 @@
continue;
}
- access.put(p, getAccess.apply(Project.nameKey(p)));
+ Project.NameKey projectName = Project.nameKey(p);
+
+ if (!projectCache.get(projectName).isPresent()) {
+ throw new ResourceNotFoundException(projectName.get());
+ }
+
+ try {
+ permissionBackend.currentUser().project(projectName).check(ProjectPermission.ACCESS);
+ } catch (AuthException e) {
+ throw new ResourceNotFoundException(projectName.get(), e);
+ }
+
+ access.put(p, getAccess.apply(projectName));
}
return Response.ok(access);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 1e08c4b..b99c624 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -15,15 +15,21 @@
package com.google.gerrit.acceptance.rest.project;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.Permission;
import com.google.gerrit.extensions.api.access.ProjectAccessInfo;
import com.google.gson.reflect.TypeToken;
+import com.google.inject.Inject;
import java.util.Map;
import org.junit.Test;
public class AccessIT extends AbstractDaemonTest {
+ @Inject private ProjectOperations projectOperations;
@Test
public void listAccessWithoutSpecifyingProject() throws Exception {
@@ -53,6 +59,19 @@
}
@Test
+ public void listAccessForNonVisibleProject() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ RestResponse r = userRestSession.get("/access/?project=" + project.get());
+ r.assertNotFound();
+ assertThat(r.getEntityContent()).isEqualTo(project.get());
+ }
+
+ @Test
public void listAccess() throws Exception {
RestResponse r = adminRestSession.get("/access/?project=" + project.get());
r.assertOK();
