Add a global capability for viewing all accounts
Users with this capability can auto-complete all accounts regardless
of the global accounts.visibility setting. The use case is a core
group of committers that should be able to view all accounts, where
the rest of the contributors are all in small, non-overlapping
accounts that should not be able to view one another. For small
numbers of groups it is sufficient to add the committer groups to each
of the contributor groups, but this approach becomes a maintenance
headache as the number of contributor groups grows to the 10-100
range.
Change-Id: Ib31d642484459afa0c129524bc0bc52348dd7416
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 9b7fd4f..688f637 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -1268,6 +1268,14 @@
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 62cebe8..aeb7b5d 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1137,6 +1137,9 @@
|`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 0586fe4..dbc1dd8 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 @@
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 f52104d..7266c97 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 @@
assertEquals(reviewers.size(), 1);
}
- private List<SuggestedReviewerInfo> 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<SuggestedReviewerInfo> 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<SuggestedReviewerInfo> 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<SuggestedReviewerInfo> 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 @@
.getType());
}
- private void group(String name) throws IOException {
- adminSession.put("/groups/" + name, new Object()).consume();
+ private List<SuggestedReviewerInfo> 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 9a6728e..80a04fa 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 @@
/** 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 @@
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 1589097..825bd3b 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 @@
&& ((IdentifiedUser) currentUser).getAccountId().equals(otherUser)) {
return true;
}
+ if (currentUser.getCapabilities().canViewAllAccounts()) {
+ return true;
+ }
switch (accountVisibility) {
case ALL:
@@ -139,7 +142,7 @@
default:
throw new IllegalStateException("Bad AccountVisibility " + accountVisibility);
}
- return currentUser.getCapabilities().canAdministrateServer();
+ return false;
}
private Set<AccountGroup.UUID> 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 beef976..aad22eb 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 @@
|| 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 03ba94f..e02f5a2 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.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 @@
}
}
+ 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 92e9bf88..4510616 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 static class SuggestedReviewerInfo implements Comparable<SuggestedReviewerInfo> {
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 2e54f3e..26081ca4 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 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 a1b966f7..8d1e983 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 @@
runAs = Run As
runGC = Run Garbage Collection
streamEvents = Stream Events
+viewAllAccounts = View All Accounts
viewCaches = View Caches
viewConnections = View Connections
viewQueue = View Queue
