Add query option allowing administrators to skip visibility filtering

If an administrator wants to reindex changes which were created or
updated in a given period based on a query for that period the
query results are subject to visibility filtering. This can have the
effect that e.g. private changes are missed.

Add a query option "skip-visibility" to allow administrators
to skip visibility filtering.

Change-Id: I66c13659587b9459eb7cc585697c1655926ceac3

3 files changed, 117 insertions, 1 deletions

diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 3cb40f6a0d..f67670b893 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -138,6 +138,12 @@ limit or a supplied `n` query parameter, the last change object has a
 The `S` or `start` query parameter can be supplied to skip a number
 of changes from the list.
 
+Administrators can use the `skip-visibility` query parameter to skip visibility filtering.
+This can be used to ensure that no changes are missed e.g. when querying for changes which
+need to be reindexed. Without this parameter query results the user has no permission to read
+are filtered out. REST requests with the skip-visibility option are rejected when the current
+user doesn't have the ADMINISTRATE_SERVER capability.
+
 Clients are allowed to specify more than one query by setting the `q`
 parameter multiple times. In this case the result is an array of
 arrays, one per query in the same order the queries were given in.
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index 6e5f554304..bf4d197147 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -27,13 +27,17 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.index.query.QueryRequiresAuthException;
 import com.google.gerrit.index.query.QueryResult;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.DynamicOptions;
 import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.permissions.GlobalPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeQueryBuilder;
 import com.google.gerrit.server.query.change.ChangeQueryProcessor;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -46,6 +50,8 @@ public class QueryChanges implements RestReadView<TopLevelResource>, DynamicOpti
   private final ChangeJson.Factory json;
   private final ChangeQueryBuilder qb;
   private final ChangeQueryProcessor imp;
+  private final Provider<CurrentUser> userProvider;
+  private final PermissionBackend permissionBackend;
   private EnumSet<ListChangesOption> options;
 
   @Option(
@@ -88,16 +94,32 @@ public class QueryChanges implements RestReadView<TopLevelResource>, DynamicOpti
     imp.setNoLimit(on);
   }
 
+  @Option(name = "--skip-visibility", usage = "Skip visibility check, only for administrators")
+  public void skipVisibility(boolean on) throws AuthException, PermissionBackendException {
+    if (on) {
+      CurrentUser user = userProvider.get();
+      permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
+      imp.enforceVisibility(false);
+    }
+  }
+
   @Override
   public void setDynamicBean(String plugin, DynamicOptions.DynamicBean dynamicBean) {
     imp.setDynamicBean(plugin, dynamicBean);
   }
 
   @Inject
-  QueryChanges(ChangeJson.Factory json, ChangeQueryBuilder qb, ChangeQueryProcessor qp) {
+  QueryChanges(
+      ChangeJson.Factory json,
+      ChangeQueryBuilder qb,
+      ChangeQueryProcessor qp,
+      Provider<CurrentUser> userProvider,
+      PermissionBackend permissionBackend) {
     this.json = json;
     this.qb = qb;
     this.imp = qp;
+    this.userProvider = userProvider;
+    this.permissionBackend = permissionBackend;
 
     options = EnumSet.noneOf(ListChangesOption.class);
   }
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
index 78354d653b..6839b967fc 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java
@@ -15,22 +15,32 @@
 package com.google.gerrit.acceptance.api.change;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.server.project.ProjectConfig;
 import com.google.gerrit.server.restapi.change.QueryChanges;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
+import java.util.Arrays;
 import java.util.List;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
 import org.junit.Test;
 
 @NoHttpd
 public class QueryChangesIT extends AbstractDaemonTest {
 
   @Inject private Provider<QueryChanges> queryChangesProvider;
+  @Inject private RequestScopeOperations requestScopeOperations;
 
   @Test
   @SuppressWarnings("unchecked")
@@ -97,9 +107,87 @@ public class QueryChangesIT extends AbstractDaemonTest {
     assertThat(result2.get(1).get(0)._moreChanges).isTrue();
   }
 
+  @Test
+  public void skipVisibility_rejectedForNonAdmin() throws Exception {
+    requestScopeOperations.setApiUser(user.id());
+    final QueryChanges queryChanges = queryChangesProvider.get();
+    String query = "is:open repo:" + project.get();
+    queryChanges.addQuery(query);
+    AuthException thrown =
+        assertThrows(AuthException.class, () -> queryChanges.skipVisibility(true));
+    assertThat(thrown).hasMessageThat().isEqualTo("administrate server not permitted");
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  public void skipVisibility_noReadPermission() throws Exception {
+    createChange().getChangeId();
+    requestScopeOperations.setApiUser(admin.id());
+    QueryChanges queryChanges = queryChangesProvider.get();
+
+    queryChanges.addQuery("is:open repo:" + project.get());
+    List<List<ChangeInfo>> result =
+        (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+    assertThat(result).hasSize(1);
+
+    try (ProjectConfigUpdate u = updateProject(allProjects)) {
+      ProjectConfig cfg = u.getConfig();
+      removeAllBranchPermissions(cfg, Permission.READ);
+      u.save();
+    }
+
+    queryChanges = queryChangesProvider.get();
+    queryChanges.addQuery("is:open repo:" + project.get());
+    List<List<ChangeInfo>> result2 =
+        (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+    assertThat(result2).hasSize(0);
+
+    queryChanges = queryChangesProvider.get();
+    queryChanges.addQuery("is:open repo:" + project.get());
+    queryChanges.skipVisibility(true);
+    List<List<ChangeInfo>> result3 =
+        (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+    assertThat(result3).hasSize(1);
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  public void skipVisibility_privateChange() throws Exception {
+    TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
+    PushOneCommit.Result result =
+        pushFactory.create(user.newIdent(), userRepo).to("refs/for/master");
+    requestScopeOperations.setApiUser(user.id());
+    gApi.changes().id(result.getChangeId()).setPrivate(true);
+
+    requestScopeOperations.setApiUser(admin.id());
+    QueryChanges queryChanges = queryChangesProvider.get();
+
+    queryChanges.addQuery("is:open repo:" + project.get());
+    List<List<ChangeInfo>> result2 =
+        (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+    assertThat(result2).hasSize(0);
+
+    queryChanges = queryChangesProvider.get();
+    queryChanges.addQuery("is:open repo:" + project.get());
+    queryChanges.skipVisibility(true);
+    List<List<ChangeInfo>> result3 =
+        (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+    assertThat(result3).hasSize(1);
+  }
+
   private static void assertNoChangeHasMoreChangesSet(List<ChangeInfo> results) {
     for (ChangeInfo info : results) {
       assertThat(info._moreChanges).isNull();
     }
   }
+
+  private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) {
+    cfg.getAccessSections().stream()
+        .filter(
+            s ->
+                s.getName().startsWith("refs/heads/")
+                    || s.getName().startsWith("refs/for/")
+                    || s.getName().equals("refs/*"))
+        .forEach(s -> Arrays.stream(permissions).forEach(s::removePermission));
+  }
 }