Check if project state is readable in ProjectsCollection

This commit adds a dedicated check for project state to
ProjectsCollection that is only triggered when the user is not a project
owner. This is intended as safety net for the case where child views
forget to check for the project state explicity.

We can't do a general check in ProjectsCollection that is not gated on
being a project owner since the API has to allow setting back a project
state from HIDDEN to READ_ONLY or ACTIVE. Therfore child views should
continue with explicit checks on the project state.

Change-Id: I4bf6d50569ac41678e8a75711ed604ab22154894

java/com/google/gerrit/server/project/ProjectState.java

@@ -263,6 +263,13 @@ public class ProjectState {
     return getProject().getState().permitsRead();
   }
 
+  public void checkStatePermitsRead() throws ResourceConflictException {
+    if (!statePermitsRead()) {
+      throw new ResourceConflictException(
+          "project state " + getProject().getState().name() + " does not permit read");
+    }
+  }
+
   public boolean statePermitsWrite() {
     return getProject().getState().permitsWrite();
   }

java/com/google/gerrit/server/restapi/account/PostWatchedProjects.java

@@ -18,7 +18,6 @@ import com.google.gerrit.extensions.client.ProjectWatchInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResource;
 import com.google.gerrit.server.account.AccountsUpdate;
@@ -83,8 +82,7 @@ public class PostWatchedProjects
   }
 
   private Map<ProjectWatchKey, Set<NotifyType>> asMap(List<ProjectWatchInfo> input)
-      throws BadRequestException, UnprocessableEntityException, IOException,
-          PermissionBackendException {
+      throws RestApiException, IOException, PermissionBackendException {
     Map<ProjectWatchKey, Set<NotifyType>> m = new HashMap<>();
     for (ProjectWatchInfo info : input) {
       if (info.project == null) {

java/com/google/gerrit/server/restapi/project/ChildProjectsCollection.java

@@ -19,6 +19,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ChildCollection;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -54,7 +55,7 @@ public class ChildProjectsCollection
 
   @Override
   public ChildProjectResource parse(ProjectResource parent, IdString id)
-      throws ResourceNotFoundException, IOException, PermissionBackendException {
+      throws RestApiException, IOException, PermissionBackendException {
     ProjectResource p = projectsCollection.parse(TopLevelResource.INSTANCE, id);
     for (ProjectState pp : p.getProjectState().parents()) {
       if (parent.getNameKey().equals(pp.getProject().getNameKey())) {

java/com/google/gerrit/server/restapi/project/CreateProject.java

@@ -35,11 +35,10 @@ import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
 import com.google.gerrit.reviewdb.client.Project;
@@ -156,9 +155,7 @@ public class CreateProject implements RestModifyView<TopLevelResource, ProjectIn
 
   @Override
   public Response<ProjectInfo> apply(TopLevelResource resource, ProjectInput input)
-      throws BadRequestException, UnprocessableEntityException, ResourceConflictException,
-          ResourceNotFoundException, IOException, ConfigInvalidException,
-          PermissionBackendException {
+      throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
     if (input == null) {
       input = new ProjectInput();
     }

java/com/google/gerrit/server/restapi/project/ProjectsCollection.java

@@ -22,7 +22,9 @@ import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.NeedsParams;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestCollection;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -91,7 +93,7 @@ public class ProjectsCollection
 
   @Override
   public ProjectResource parse(TopLevelResource parent, IdString id)
-      throws ResourceNotFoundException, IOException, PermissionBackendException {
+      throws RestApiException, IOException, PermissionBackendException {
     ProjectResource rsrc = _parse(id.get(), true);
     if (rsrc == null) {
       throw new ResourceNotFoundException(id);
@@ -104,13 +106,13 @@ public class ProjectsCollection
    *
    * @param id ID of the project, can be a project name
    * @return the project
-   * @throws UnprocessableEntityException thrown if the project ID cannot be resolved or if the
-   *     project is not visible to the calling user
+   * @throws RestApiException 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, PermissionBackendException {
+      throws RestApiException, IOException, PermissionBackendException {
     return parse(id, true);
   }
 
@@ -120,13 +122,13 @@ public class ProjectsCollection
    * @param id ID of the project, can be a project name
    * @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 RestApiException 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 checkAccess)
-      throws UnprocessableEntityException, IOException, PermissionBackendException {
+      throws RestApiException, IOException, PermissionBackendException {
     ProjectResource rsrc = _parse(id, checkAccess);
     if (rsrc == null) {
       throw new UnprocessableEntityException(String.format("Project Not Found: %s", id));
@@ -136,7 +138,7 @@ public class ProjectsCollection
 
   @Nullable
   private ProjectResource _parse(String id, boolean checkAccess)
-      throws IOException, PermissionBackendException {
+      throws IOException, PermissionBackendException, ResourceConflictException {
     if (id.endsWith(Constants.DOT_GIT_EXT)) {
       id = id.substring(0, id.length() - Constants.DOT_GIT_EXT.length());
     }
@@ -153,6 +155,16 @@ public class ProjectsCollection
       } catch (AuthException e) {
         return null; // Pretend like not found on access denied.
       }
+      // If the project's state does not permit reading, we want to hide it from all callers. The
+      // only exception to that are users who are allowed to mutate the project's configuration.
+      // This enables these users to still mutate the project's state (e.g. set a HIDDEN project to
+      // ACTIVE). Individual views should still check for checkStatePermitsRead() and this should
+      // just serve as a safety net in case the individual check is forgotten.
+      try {
+        permissionBackend.user(user).project(nameKey).check(ProjectPermission.WRITE_CONFIG);
+      } catch (AuthException e) {
+        state.checkStatePermitsRead();
+      }
     }
     return new ProjectResource(state, user.get());
   }