diff --git a/Documentation/user-search-accounts.txt b/Documentation/user-search-accounts.txt index 04f40c0732..6bcd18e929 100644 --- a/Documentation/user-search-accounts.txt +++ b/Documentation/user-search-accounts.txt @@ -23,6 +23,11 @@ are added to the same query string, they further restrict the returned results. Search can also be performed by typing only a text with no operator, which will match against a variety of fields. +[[cansee]] +cansee:'CHANGE':: ++ +Matches accounts that can see the change 'CHANGE'. + [[email]] email:'EMAIL':: + diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java b/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java index a470696a86..6905cf4b13 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java +++ b/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java @@ -21,6 +21,7 @@ import com.google.gerrit.index.query.IndexPredicate; import com.google.gerrit.index.query.IntegerRangePredicate; import com.google.gerrit.index.query.NotPredicate; import com.google.gerrit.index.query.OrPredicate; +import com.google.gerrit.index.query.PostFilterPredicate; import com.google.gerrit.index.query.Predicate; import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.index.query.RegexPredicate; @@ -43,6 +44,8 @@ public class ElasticQueryBuilder { return not(p); } else if (p instanceof IndexPredicate) { return fieldQuery((IndexPredicate) p); + } else if (p instanceof PostFilterPredicate) { + return QueryBuilders.matchAllQuery(); } else { throw new QueryParseException("cannot create query for index: " + p); } diff --git a/java/com/google/gerrit/index/query/PostFilterPredicate.java b/java/com/google/gerrit/index/query/PostFilterPredicate.java new file mode 100644 index 0000000000..3e780bfa6f --- /dev/null +++ b/java/com/google/gerrit/index/query/PostFilterPredicate.java @@ -0,0 +1,21 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.index.query; + +/** + * Matches all documents in the index, with additional filtering done in the subclass's {@code + * match} method. + */ +public abstract class PostFilterPredicate extends Predicate implements Matchable {} diff --git a/java/com/google/gerrit/lucene/QueryBuilder.java b/java/com/google/gerrit/lucene/QueryBuilder.java index 2f2a1cdd42..4500942d35 100644 --- a/java/com/google/gerrit/lucene/QueryBuilder.java +++ b/java/com/google/gerrit/lucene/QueryBuilder.java @@ -28,6 +28,7 @@ import com.google.gerrit.index.query.IndexPredicate; import com.google.gerrit.index.query.IntegerRangePredicate; import com.google.gerrit.index.query.NotPredicate; import com.google.gerrit.index.query.OrPredicate; +import com.google.gerrit.index.query.PostFilterPredicate; import com.google.gerrit.index.query.Predicate; import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.index.query.RegexPredicate; @@ -76,6 +77,8 @@ public class QueryBuilder { return not(p); } else if (p instanceof IndexPredicate) { return fieldQuery((IndexPredicate) p); + } else if (p instanceof PostFilterPredicate) { + return new MatchAllDocsQuery(); } else { throw new QueryParseException("cannot create query for index: " + p); } diff --git a/java/com/google/gerrit/server/ChangeFinder.java b/java/com/google/gerrit/server/ChangeFinder.java index cb8277804e..677b091cc3 100644 --- a/java/com/google/gerrit/server/ChangeFinder.java +++ b/java/com/google/gerrit/server/ChangeFinder.java @@ -110,6 +110,14 @@ public class ChangeFinder { this.allowedIdTypes = ImmutableSet.copyOf(configuredChangeIdTypes); } + public ChangeNotes findOne(String id) throws OrmException { + List ctls = find(id); + if (ctls.size() != 1) { + return null; + } + return ctls.get(0); + } + /** * Find changes matching the given identifier. * diff --git a/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java b/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java index e8b1861b5d..644f1ebae3 100644 --- a/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java +++ b/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java @@ -14,21 +14,41 @@ package com.google.gerrit.server.index.account; +import static com.google.common.base.Preconditions.checkState; + import com.google.gerrit.index.Index; import com.google.gerrit.index.IndexedQuery; import com.google.gerrit.index.QueryOptions; import com.google.gerrit.index.query.DataSource; +import com.google.gerrit.index.query.Matchable; import com.google.gerrit.index.query.Predicate; import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.AccountState; +import com.google.gwtorm.server.OrmException; public class IndexedAccountQuery extends IndexedQuery - implements DataSource { + implements DataSource, Matchable { public IndexedAccountQuery( Index index, Predicate pred, QueryOptions opts) throws QueryParseException { super(index, pred, opts.convertForBackend()); } + + @Override + public boolean match(AccountState accountState) throws OrmException { + Predicate pred = getChild(0); + checkState( + pred.isMatchable(), + "match invoked, but child predicate %s doesn't implement %s", + pred, + Matchable.class.getName()); + return pred.asMatchable().match(accountState); + } + + @Override + public int getCost() { + return 1; + } } diff --git a/java/com/google/gerrit/server/query/account/AccountPredicates.java b/java/com/google/gerrit/server/query/account/AccountPredicates.java index e643470c79..acb963c89c 100644 --- a/java/com/google/gerrit/server/query/account/AccountPredicates.java +++ b/java/com/google/gerrit/server/query/account/AccountPredicates.java @@ -19,12 +19,15 @@ import com.google.common.primitives.Ints; import com.google.gerrit.index.FieldDef; import com.google.gerrit.index.Schema; import com.google.gerrit.index.query.IndexPredicate; +import com.google.gerrit.index.query.Matchable; import com.google.gerrit.index.query.Predicate; import com.google.gerrit.index.query.QueryBuilder; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.index.account.AccountField; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gwtorm.server.OrmException; import java.util.List; public class AccountPredicates { @@ -121,7 +124,14 @@ public class AccountPredicates { return new AccountPredicate(AccountField.WATCHED_PROJECT, project.get()); } - static class AccountPredicate extends IndexPredicate { + public static Predicate cansee( + AccountQueryBuilder.Arguments args, ChangeNotes changeNotes) { + return new CanSeeChangePredicate( + args.db, args.permissionBackend, args.userFactory, changeNotes); + } + + static class AccountPredicate extends IndexPredicate + implements Matchable { AccountPredicate(FieldDef def, String value) { super(def, value); } @@ -129,6 +139,16 @@ public class AccountPredicates { AccountPredicate(FieldDef def, String name, String value) { super(def, name, value); } + + @Override + public boolean match(AccountState object) throws OrmException { + return true; + } + + @Override + public int getCost() { + return 1; + } } private AccountPredicates() {} diff --git a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java index 8f6ec8b5a3..1fbf877cc9 100644 --- a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java +++ b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java @@ -25,14 +25,19 @@ import com.google.gerrit.index.query.Predicate; import com.google.gerrit.index.query.QueryBuilder; import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeFinder; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.index.account.AccountField; import com.google.gerrit.server.index.account.AccountIndexCollection; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.permissions.ChangePermission; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.ProvisionException; @@ -56,17 +61,27 @@ public class AccountQueryBuilder extends QueryBuilder { new QueryBuilder.Definition<>(AccountQueryBuilder.class); public static class Arguments { + final Provider db; + final ChangeFinder changeFinder; + final IdentifiedUser.GenericFactory userFactory; + final PermissionBackend permissionBackend; + private final Provider self; private final AccountIndexCollection indexes; - private final PermissionBackend permissionBackend; @Inject public Arguments( Provider self, AccountIndexCollection indexes, + Provider db, + ChangeFinder changeFinder, + IdentifiedUser.GenericFactory userFactory, PermissionBackend permissionBackend) { this.self = self; this.indexes = indexes; + this.db = db; + this.changeFinder = changeFinder; + this.userFactory = userFactory; this.permissionBackend = permissionBackend; } @@ -104,6 +119,22 @@ public class AccountQueryBuilder extends QueryBuilder { this.args = args; } + @Operator + public Predicate cansee(String change) + throws QueryParseException, OrmException, PermissionBackendException { + ChangeNotes changeNotes = args.changeFinder.findOne(change); + if (changeNotes == null + || !args.permissionBackend + .user(args.getUser()) + .database(args.db) + .change(changeNotes) + .test(ChangePermission.READ)) { + throw error(String.format("change %s not found", change)); + } + + return AccountPredicates.cansee(args, changeNotes); + } + @Operator public Predicate email(String email) throws PermissionBackendException, QueryParseException { diff --git a/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java new file mode 100644 index 0000000000..f8b8cc7d84 --- /dev/null +++ b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java @@ -0,0 +1,71 @@ +package com.google.gerrit.server.query.account; + +import com.google.gerrit.index.query.PostFilterPredicate; +import com.google.gerrit.index.query.Predicate; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.permissions.ChangePermission; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Provider; +import java.util.Collection; +import java.util.Objects; + +public class CanSeeChangePredicate extends PostFilterPredicate { + private final Provider db; + private final PermissionBackend permissionBackend; + private final IdentifiedUser.GenericFactory userFactory; + private final ChangeNotes changeNotes; + + CanSeeChangePredicate( + Provider db, + PermissionBackend permissionBackend, + IdentifiedUser.GenericFactory userFactory, + ChangeNotes changeNotes) { + this.db = db; + this.permissionBackend = permissionBackend; + this.userFactory = userFactory; + this.changeNotes = changeNotes; + } + + @Override + public boolean match(AccountState accountState) throws OrmException { + try { + return permissionBackend + .user(userFactory.create(accountState.getAccount().getId())) + .database(db) + .change(changeNotes) + .test(ChangePermission.READ); + } catch (PermissionBackendException e) { + throw new OrmException("Failed to check if account can see change", e); + } + } + + @Override + public int getCost() { + return 1; + } + + @Override + public Predicate copy(Collection> children) { + return new CanSeeChangePredicate(db, permissionBackend, userFactory, changeNotes); + } + + @Override + public int hashCode() { + return Objects.hash(changeNotes.getChange().getChangeId()); + } + + @Override + public boolean equals(Object other) { + if (other == null) { + return false; + } + return getClass() == other.getClass() + && changeNotes.getChange().getChangeId() + == ((CanSeeChangePredicate) other).changeNotes.getChange().getChangeId(); + } +} diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java index 4f14dda6a0..a1e5e2b4d4 100644 --- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java +++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java @@ -19,14 +19,24 @@ import static com.google.common.truth.Truth8.assertThat; import static java.util.stream.Collectors.toList; import static org.junit.Assert.fail; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Streams; import com.google.gerrit.extensions.api.GerritApi; +import com.google.gerrit.extensions.api.access.AccessSectionInfo; +import com.google.gerrit.extensions.api.access.PermissionInfo; +import com.google.gerrit.extensions.api.access.PermissionRuleInfo; +import com.google.gerrit.extensions.api.access.ProjectAccessInput; import com.google.gerrit.extensions.api.accounts.Accounts.QueryRequest; +import com.google.gerrit.extensions.api.groups.GroupInput; +import com.google.gerrit.extensions.api.projects.ProjectInput; import com.google.gerrit.extensions.client.ListAccountsOption; import com.google.gerrit.extensions.client.ProjectWatchInfo; import com.google.gerrit.extensions.common.AccountExternalIdInfo; import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.ChangeInput; +import com.google.gerrit.extensions.common.GroupInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; @@ -75,6 +85,7 @@ import com.google.inject.Provider; import com.google.inject.util.Providers; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Optional; @@ -392,6 +403,22 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { assertQuery("name:DOE", user1); } + @Test + public void byCansee() throws Exception { + String domain = name("test.com"); + AccountInfo user1 = newAccountWithEmail("account1", "account1@" + domain); + AccountInfo user2 = newAccountWithEmail("account2", "account2@" + domain); + AccountInfo user3 = newAccountWithEmail("account3", "account3@" + domain); + + Project.NameKey p = createProject(name("p")); + ChangeInfo c = createChange(p); + assertQuery("name:" + domain + " cansee:" + c.changeId, user1, user2, user3); + + GroupInfo group = createGroup(name("group"), user1, user2); + blockRead(p, group); + assertQuery("name:" + domain + " cansee:" + c.changeId, user3); + } + @Test public void byWatchedProject() throws Exception { Project.NameKey p = createProject(name("p")); @@ -614,10 +641,43 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { } protected Project.NameKey createProject(String name) throws RestApiException { - gApi.projects().create(name); + ProjectInput in = new ProjectInput(); + in.name = name; + in.createEmptyCommit = true; + gApi.projects().create(in); return new Project.NameKey(name); } + protected void blockRead(Project.NameKey project, GroupInfo group) throws RestApiException { + ProjectAccessInput in = new ProjectAccessInput(); + in.add = new HashMap<>(); + + AccessSectionInfo a = new AccessSectionInfo(); + PermissionInfo p = new PermissionInfo(null, null); + p.rules = + ImmutableMap.of(group.id, new PermissionRuleInfo(PermissionRuleInfo.Action.BLOCK, false)); + a.permissions = ImmutableMap.of("read", p); + in.add = ImmutableMap.of("refs/*", a); + + gApi.projects().name(project.get()).access(in); + } + + protected ChangeInfo createChange(Project.NameKey project) throws RestApiException { + ChangeInput in = new ChangeInput(); + in.subject = "A change"; + in.project = project.get(); + in.branch = "master"; + return gApi.changes().create(in).get(); + } + + protected GroupInfo createGroup(String name, AccountInfo... members) throws RestApiException { + GroupInput in = new GroupInput(); + in.name = name; + in.members = + Arrays.asList(members).stream().map(a -> String.valueOf(a._accountId)).collect(toList()); + return gApi.groups().create(in).get(); + } + protected void watch(AccountInfo account, Project.NameKey project, String filter) throws RestApiException { List projectsToWatch = new ArrayList<>(); diff --git a/javatests/com/google/gerrit/server/query/account/BUILD b/javatests/com/google/gerrit/server/query/account/BUILD index e0b59d591e..497fc2269e 100644 --- a/javatests/com/google/gerrit/server/query/account/BUILD +++ b/javatests/com/google/gerrit/server/query/account/BUILD @@ -19,6 +19,7 @@ java_library( "//lib:truth-java8-extension", "//lib/guice", "//lib/jgit/org.eclipse.jgit:jgit", + "//prolog:gerrit-prolog-common", ], )