diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 9b7fd4f3a2..688f63793a 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -1268,6 +1268,14 @@ allows the granted group to link:cmd-stream-events.html[stream Gerrit events via ssh]. +[[capability_viewAllAccounts]] +=== View All Accounts + +Allow viewing all accounts for purposes of auto-completion, regardless +of link:config-gerrit.html#accounts.visibility[accounts.visibility] +setting. + + [[capability_viewCaches]] === View Caches diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt index 62cebe8943..aeb7b5d80d 100644 --- a/Documentation/rest-api-accounts.txt +++ b/Documentation/rest-api-accounts.txt @@ -1137,6 +1137,9 @@ capability. |`streamEvents` |not set if `false`|Whether the user has the link:access-control.html#capability_streamEvents[Stream Events] capability. +|`viewAllAccounts` |not set if `false`|Whether the user has the +link:access-control.html#capability_viewAllAccounts[View All Accounts] +capability. |`viewCaches` |not set if `false`|Whether the user has the link:access-control.html#capability_viewCaches[View Caches] capability. |`viewConnections` |not set if `false`|Whether the user has the diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CapabilityInfo.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CapabilityInfo.java index 0586fe47b6..dbc1dd8dc7 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CapabilityInfo.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/CapabilityInfo.java @@ -29,6 +29,7 @@ class CapabilityInfo { public boolean runAs; public boolean runGC; public boolean streamEvents; + public boolean viewAllAccounts; public boolean viewCaches; public boolean viewConnections; public boolean viewQueue; diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java index f52104db0d..7266c9750a 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java @@ -15,31 +15,65 @@ package com.google.gerrit.acceptance.rest.change; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.GerritConfigs; +import com.google.gerrit.acceptance.RestSession; +import com.google.gerrit.acceptance.TestAccount; +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GlobalCapability; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.server.account.CreateGroupArgs; +import com.google.gerrit.server.account.PerformCreateGroup; import com.google.gerrit.server.change.SuggestReviewers.SuggestedReviewerInfo; +import com.google.gerrit.server.config.AllProjectsName; +import com.google.gerrit.server.git.MetaDataUpdate; +import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.project.ProjectCache; import com.google.gson.reflect.TypeToken; +import com.google.inject.Inject; import org.eclipse.jgit.api.errors.GitAPIException; import org.junit.Before; import org.junit.Test; import java.io.IOException; +import java.util.Collections; import java.util.List; public class SuggestReviewersIT extends AbstractDaemonTest { + @Inject + private PerformCreateGroup.Factory createGroupFactory; + + @Inject + private MetaDataUpdate.Server metaDataUpdateFactory; + + @Inject + private AllProjectsName allProjects; + + @Inject + private ProjectCache projectCache; + + private AccountGroup group1; + private TestAccount user1; + private TestAccount user2; + private TestAccount user3; @Before public void setUp() throws Exception { - group("users1"); + group1 = group("users1"); group("users2"); group("users3"); - accounts.create("user1", "user1@example.com", "User1", "users1"); - accounts.create("user2", "user2@example.com", "User2", "users2"); - accounts.create("user3", "user3@example.com", "User3", "users1", "users2"); + user1 = accounts.create("user1", "user1@example.com", "User1", "users1"); + user2 = accounts.create("user2", "user2@example.com", "User2", "users2"); + user3 = accounts.create("user3", "user3@example.com", "User3", + "users1", "users2"); } @Test @@ -85,11 +119,52 @@ public class SuggestReviewersIT extends AbstractDaemonTest { assertEquals(reviewers.size(), 1); } - private List suggestReviewers(String changeId, - String query, int n) - throws IOException { + @Test + @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP") + public void suggestReviewersSameGroupVisibility() throws Exception { + String changeId = createChange().getChangeId(); + List reviewers; + + reviewers = suggestReviewers(changeId, "user2", 2); + assertEquals(1, reviewers.size()); + assertEquals("User2", Iterables.getOnlyElement(reviewers).account.name); + + reviewers = suggestReviewers(new RestSession(server, user1), + changeId, "user2", 2); + assertTrue(reviewers.isEmpty()); + + reviewers = suggestReviewers(new RestSession(server, user2), + changeId, "user2", 2); + assertEquals(1, reviewers.size()); + assertEquals("User2", Iterables.getOnlyElement(reviewers).account.name); + + reviewers = suggestReviewers(new RestSession(server, user3), + changeId, "user2", 2); + assertEquals(1, reviewers.size()); + assertEquals("User2", Iterables.getOnlyElement(reviewers).account.name); + } + + @Test + @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP") + public void suggestReviewersViewAllAccounts() throws Exception { + String changeId = createChange().getChangeId(); + List reviewers; + + reviewers = suggestReviewers(new RestSession(server, user1), + changeId, "user2", 2); + assertTrue(reviewers.isEmpty()); + + grantCapability(GlobalCapability.VIEW_ALL_ACCOUNTS, group1); + reviewers = suggestReviewers(new RestSession(server, user1), + changeId, "user2", 2); + assertEquals(1, reviewers.size()); + assertEquals("User2", Iterables.getOnlyElement(reviewers).account.name); + } + + private List suggestReviewers(RestSession session, + String changeId, String query, int n) throws IOException { return newGson().fromJson( - adminSession.get("/changes/" + session.get("/changes/" + changeId + "/suggest_reviewers?q=" + query @@ -100,7 +175,27 @@ public class SuggestReviewersIT extends AbstractDaemonTest { .getType()); } - private void group(String name) throws IOException { - adminSession.put("/groups/" + name, new Object()).consume(); + private List suggestReviewers(String changeId, + String query, int n) throws IOException { + return suggestReviewers(adminSession, changeId, query, n); + } + + private AccountGroup group(String name) throws Exception { + CreateGroupArgs args = new CreateGroupArgs(); + args.setGroupName(name); + args.initialMembers = Collections.singleton(admin.getId()); + return createGroupFactory.create(args).createGroup(); + } + + private void grantCapability(String name, AccountGroup group) + throws Exception { + MetaDataUpdate md = metaDataUpdateFactory.create(allProjects); + ProjectConfig config = ProjectConfig.read(md); + AccessSection s = config.getAccessSection( + AccessSection.GLOBAL_CAPABILITIES); + Permission p = s.getPermission(name, true); + p.add(new PermissionRule(config.resolve(group))); + config.commit(md); + projectCache.evict(config.getProject()); } } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GlobalCapability.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GlobalCapability.java index 9a6728e43c..80a04fa2fc 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GlobalCapability.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GlobalCapability.java @@ -79,6 +79,9 @@ public class GlobalCapability { /** Can perform streaming of Gerrit events. */ public static final String STREAM_EVENTS = "streamEvents"; + /** Can view all accounts, regardless of {@code accounts.visibility}. */ + public static final String VIEW_ALL_ACCOUNTS = "viewAllAccounts"; + /** Can view the server's current cache states. */ public static final String VIEW_CACHES = "viewCaches"; @@ -106,6 +109,7 @@ public class GlobalCapability { NAMES_ALL.add(RUN_AS); NAMES_ALL.add(RUN_GC); NAMES_ALL.add(STREAM_EVENTS); + NAMES_ALL.add(VIEW_ALL_ACCOUNTS); NAMES_ALL.add(VIEW_CACHES); NAMES_ALL.add(VIEW_CONNECTIONS); NAMES_ALL.add(VIEW_QUEUE); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java index 1589097cee..825bd3b1f2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountControl.java @@ -104,6 +104,9 @@ public class AccountControl { && ((IdentifiedUser) currentUser).getAccountId().equals(otherUser)) { return true; } + if (currentUser.getCapabilities().canViewAllAccounts()) { + return true; + } switch (accountVisibility) { case ALL: @@ -139,7 +142,7 @@ public class AccountControl { default: throw new IllegalStateException("Bad AccountVisibility " + accountVisibility); } - return currentUser.getCapabilities().canAdministrateServer(); + return false; } private Set groupsOf(Account.Id account) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java index beef9766e5..aad22eb0b7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java @@ -110,6 +110,12 @@ public class CapabilityControl { || canAdministrateServer(); } + /** @return true if the user can view all accounts. */ + public boolean canViewAllAccounts() { + return canPerform(GlobalCapability.VIEW_ALL_ACCOUNTS) + || canAdministrateServer(); + } + /** @return true if the user can view the server caches. */ public boolean canViewCaches() { return canPerform(GlobalCapability.VIEW_CACHES) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetCapabilities.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetCapabilities.java index 03ba94fec5..e02f5a2730 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetCapabilities.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetCapabilities.java @@ -24,6 +24,7 @@ import static com.google.gerrit.common.data.GlobalCapability.KILL_TASK; import static com.google.gerrit.common.data.GlobalCapability.PRIORITY; import static com.google.gerrit.common.data.GlobalCapability.RUN_GC; import static com.google.gerrit.common.data.GlobalCapability.STREAM_EVENTS; +import static com.google.gerrit.common.data.GlobalCapability.VIEW_ALL_ACCOUNTS; import static com.google.gerrit.common.data.GlobalCapability.VIEW_CACHES; import static com.google.gerrit.common.data.GlobalCapability.VIEW_CONNECTIONS; import static com.google.gerrit.common.data.GlobalCapability.VIEW_QUEUE; @@ -104,18 +105,19 @@ class GetCapabilities implements RestReadView { } } + have.put(ACCESS_DATABASE, cc.canAccessDatabase()); have.put(CREATE_ACCOUNT, cc.canCreateAccount()); have.put(CREATE_GROUP, cc.canCreateGroup()); have.put(CREATE_PROJECT, cc.canCreateProject()); have.put(EMAIL_REVIEWERS, cc.canEmailReviewers()); - have.put(KILL_TASK, cc.canKillTask()); - have.put(VIEW_CACHES, cc.canViewCaches()); have.put(FLUSH_CACHES, cc.canFlushCaches()); - have.put(VIEW_CONNECTIONS, cc.canViewConnections()); - have.put(VIEW_QUEUE, cc.canViewQueue()); + have.put(KILL_TASK, cc.canKillTask()); have.put(RUN_GC, cc.canRunGC()); have.put(STREAM_EVENTS, cc.canStreamEvents()); - have.put(ACCESS_DATABASE, cc.canAccessDatabase()); + have.put(VIEW_ALL_ACCOUNTS, cc.canViewAllAccounts()); + have.put(VIEW_CACHES, cc.canViewCaches()); + have.put(VIEW_CONNECTIONS, cc.canViewConnections()); + have.put(VIEW_QUEUE, cc.canViewQueue()); QueueProvider.QueueType queue = cc.getQueueType(); if (queue != QueueProvider.QueueType.INTERACTIVE diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java index 92e9bf8840..45106169b2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestReviewers.java @@ -270,8 +270,8 @@ public class SuggestReviewers implements RestReadView { public static class SuggestedReviewerInfo implements Comparable { String kind = "gerritcodereview#suggestedreviewer"; - AccountInfo account; - GroupBaseInfo group; + public AccountInfo account; + public GroupBaseInfo group; SuggestedReviewerInfo(AccountInfo a) { this.account = a; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/CapabilityConstants.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/CapabilityConstants.java index 2e54f3ec75..26081ca42f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/CapabilityConstants.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/CapabilityConstants.java @@ -35,6 +35,7 @@ public class CapabilityConstants extends TranslationBundle { public String runAs; public String runGC; public String streamEvents; + public String viewAllAccounts; public String viewCaches; public String viewConnections; public String viewQueue; diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/config/CapabilityConstants.properties b/gerrit-server/src/main/resources/com/google/gerrit/server/config/CapabilityConstants.properties index a1b966f739..8d1e983dd8 100644 --- a/gerrit-server/src/main/resources/com/google/gerrit/server/config/CapabilityConstants.properties +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/config/CapabilityConstants.properties @@ -11,6 +11,7 @@ queryLimit = Query Limit runAs = Run As runGC = Run Garbage Collection streamEvents = Stream Events +viewAllAccounts = View All Accounts viewCaches = View Caches viewConnections = View Connections viewQueue = View Queue