Fix 'visibleto' predicate for group

Issue:
When one uses a valid group name in 'visibleto' predicate no change
gets returned even if it should be from the access perspective.

Solution:
It boils down to the problem in ChangeIsVisibleToPredicate for
SingleGroupUser user - it is not an IdentifiedUser hence it gets
swapped for AnonymousUser and visibility check fails unless changes
in question are granted to Anonymous Users group. Fix it to let it
use the user it has in case it is SingleGroupUser or follow existing
user resolution (with fallback to anonymous) as it was before.

Bug: Issue 12606
Change-Id: Ieea8abf4b52d528d4cb2503270c7eff68476fc6c

java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java

@@ -31,6 +31,7 @@ import com.google.gerrit.server.project.ProjectState;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Provider;
 import java.io.IOException;
+import java.util.Optional;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 
 public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData> {
@@ -88,7 +89,10 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
     PermissionBackend.WithUser withUser =
         user.isIdentifiedUser()
             ? permissionBackend.absentUser(user.getAccountId())
-            : permissionBackend.user(anonymousUserProvider.get());
+            : permissionBackend.user(
+                Optional.of(user)
+                    .filter(u -> u instanceof SingleGroupUser)
+                    .orElseGet(anonymousUserProvider::get));
     try {
       withUser.indexedChange(cd, notes).database(db).check(ChangePermission.READ);
     } catch (PermissionBackendException e) {

javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -40,8 +40,10 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Streams;
 import com.google.common.truth.ThrowableSubject;
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.extensions.api.GerritApi;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AssigneeInput;
@@ -74,6 +76,7 @@ import com.google.gerrit.index.Schema;
 import com.google.gerrit.lifecycle.LifecycleManager;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Account.Id;
+import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
@@ -109,6 +112,7 @@ import com.google.gerrit.server.notedb.NoteDbChangeState;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.testing.Util;
 import com.google.gerrit.server.schema.SchemaCreator;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.util.ManualRequestContext;
@@ -127,6 +131,7 @@ import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.Provider;
 import com.google.inject.util.Providers;
+import java.io.IOException;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -139,6 +144,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
@@ -1716,6 +1723,18 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 
     String g1 = createGroup("group1", "Administrators");
     gApi.groups().id(g1).addMembers("user2");
+
+    // By default when a group is created without any permission granted,
+    // nothing is visible to it; having members or not has nothing to do with it
+    assertQuery(q + " visibleto:" + g1);
+
+    // change is visible to group ONLY when access is granted
+    grant(
+        new Project.NameKey("repo"),
+        "refs/*",
+        Permission.READ,
+        false,
+        new AccountGroup.UUID(gApi.groups().id(g1).get().id));
     assertQuery(q + " visibleto:" + g1, change1);
 
     requestContext.setContext(newRequestContext(user2));
@@ -1749,6 +1768,26 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
         q + " visibleto:\"Another User\"", "\"Another User\" resolves to multiple accounts");
   }
 
+  protected void grant(
+      Project.NameKey project,
+      String ref,
+      String permission,
+      boolean force,
+      AccountGroup.UUID groupUUID)
+      throws RepositoryNotFoundException, IOException, ConfigInvalidException {
+    try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
+      md.setMessage(String.format("Grant %s on %s", permission, ref));
+      ProjectConfig config = ProjectConfig.read(md);
+      AccessSection s = config.getAccessSection(ref, true);
+      Permission p = s.getPermission(permission, true);
+      PermissionRule rule = Util.newRule(config, groupUUID);
+      rule.setForce(force);
+      p.add(rule);
+      config.commit(md);
+      projectCache.evict(config.getProject());
+    }
+  }
+
   @Test
   public void byCommentBy() throws Exception {
     TestRepository<Repo> repo = createProject("repo");