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
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index f81ea15..b5f3a4f 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -31,6 +31,7 @@
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 @@
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) {
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index bd2b359..5311b0f 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -40,8 +40,10 @@
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.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.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.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.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 @@
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 @@
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");
