diff --git a/Documentation/rest-api-access.txt b/Documentation/rest-api-access.txt index 07a3d7848a..38f04ed4d5 100644 --- a/Documentation/rest-api-access.txt +++ b/Documentation/rest-api-access.txt @@ -263,7 +263,41 @@ The entries in the map are sorted by project name. ], "can_upload": true, "can_add": true, - "config_visible": true + "config_visible": true, + "groups": { + "53a4f647a89ea57992571187d8025f830625192a": { + "url": "#/admin/groups/uuid-53a4f647a89ea57992571187d8025f830625192a", + "options": {}, + "description": "Gerrit Site Administrators", + "group_id": 1, + "owner": "Administrators", + "owner_id": "53a4f647a89ea57992571187d8025f830625192a", + "created_on": "2009-06-08 23:31:00.000000000", + "name": "Administrators" + }, + "global:Registered-Users": { + "options": {}, + "name": "Registered Users" + }, + "global:Project-Owners": { + "options": {}, + "name": "Project Owners" + }, + "15bfcd8a6de1a69c50b30cedcdcc951c15703152": { + "url": "#/admin/groups/uuid-15bfcd8a6de1a69c50b30cedcdcc951c15703152", + "options": {}, + "description": "Users who perform batch actions on Gerrit", + "group_id": 2, + "owner": "Administrators", + "owner_id": "53a4f647a89ea57992571187d8025f830625192a", + "created_on": "2009-06-08 23:31:00.000000000", + "name": "Non-Interactive Users" + }, + "global:Anonymous-Users": { + "options": {}, + "name": "Anonymous Users" + } + } }, "MyProject": { "revision": "61157ed63e14d261b6dca40650472a9b0bd88474", @@ -368,6 +402,10 @@ Whether the calling user can add any ref. |`config_visible` |not set if `false`| Whether the calling user can see the `refs/meta/config` branch of the project. +|`groups` |A map of group UUID to +link:rest-api-groups.html#group-info[GroupInfo] objects, describing +the group UUIDs used in the `local` map. Groups that are not visible +are omitted from the `groups` map. |================================== diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 7ee7336ab3..77eb9af0b8 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -1005,7 +1005,23 @@ As result a link:#project-access-info[ProjectAccessInfo] entity is returned. ], "can_upload": true, "can_add": true, - "config_visible": true + "config_visible": true, + "groups": { + "c2ce4749a32ceb82cd6adcce65b8216e12afb41c": { + "url": "#/admin/groups/uuid-c2ce4749a32ceb82cd6adcce65b8216e12afb41c", + "options": {}, + "description": "Users who perform batch actions on Gerrit", + "group_id": 2, + "owner": "Administrators", + "owner_id": "d5b7124af4de52924ed397913e2c3b37bf186948", + "created_on": "2009-06-08 23:31:00.000000000", + "name": "Non-Interactive Users" + }, + "global:Anonymous-Users": { + "options": {}, + "name": "Anonymous Users" + } + } } ---- @@ -1086,7 +1102,13 @@ As result a link:#project-access-info[ProjectAccessInfo] entity is returned. ], "can_upload": true, "can_add": true, - "config_visible": true + "config_visible": true, + "groups": { + "global:Anonymous-Users": { + "options": {}, + "name": "Anonymous Users" + } + } } ---- diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java index 8695498270..ff2b2e1e88 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AccessIT.java @@ -207,6 +207,51 @@ public class AccessIT extends AbstractDaemonTest { gApi.projects().name(newProjectName).access(); } + @Test + public void permissionsGroupMap() throws Exception { + // Add initial permission set + ProjectAccessInput accessInput = newProjectAccessInput(); + AccessSectionInfo accessSection = newAccessSectionInfo(); + + PermissionInfo push = newPermissionInfo(); + PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false); + push.rules.put(SystemGroupBackend.PROJECT_OWNERS.get(), pri); + accessSection.permissions.put(Permission.PUSH, push); + + PermissionInfo read = newPermissionInfo(); + pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false); + read.rules.put(SystemGroupBackend.ANONYMOUS_USERS.get(), pri); + accessSection.permissions.put(Permission.READ, read); + + accessInput.add.put(REFS_ALL, accessSection); + ProjectAccessInfo result = pApi.access(accessInput); + assertThat(result.groups.keySet()) + .containsExactly( + SystemGroupBackend.PROJECT_OWNERS.get(), SystemGroupBackend.ANONYMOUS_USERS.get()); + + // Check the name, which is what the UI cares about; exhaustive + // coverage of GroupInfo should be in groups REST API tests. + assertThat(result.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).name) + .isEqualTo("Project Owners"); + // Strip the ID, since it is in the key. + assertThat(result.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).id).isNull(); + + // Get call returns groups too. + ProjectAccessInfo loggedInResult = pApi.access(); + assertThat(loggedInResult.groups.keySet()) + .containsExactly( + SystemGroupBackend.PROJECT_OWNERS.get(), SystemGroupBackend.ANONYMOUS_USERS.get()); + assertThat(loggedInResult.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).name) + .isEqualTo("Project Owners"); + assertThat(loggedInResult.groups.get(SystemGroupBackend.PROJECT_OWNERS.get()).id).isNull(); + + // PROJECT_OWNERS is invisible to anonymous user, so we strip it. + setApiUserAnonymous(); + ProjectAccessInfo anonResult = pApi.access(); + assertThat(anonResult.groups.keySet()) + .containsExactly(SystemGroupBackend.ANONYMOUS_USERS.get()); + } + @Test public void updateParentAsUser() throws Exception { // Create child diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java index 0922d955d3..9678253333 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/ProjectAccessInfo.java @@ -14,6 +14,7 @@ package com.google.gerrit.extensions.api.access; +import com.google.gerrit.extensions.common.GroupInfo; import com.google.gerrit.extensions.common.ProjectInfo; import java.util.Map; import java.util.Set; @@ -27,4 +28,5 @@ public class ProjectAccessInfo { public Boolean canUpload; public Boolean canAdd; public Boolean configVisible; + public Map groups; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java index 492d0e8c4a..99e6a9facd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java @@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.project.GetAccess; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import java.io.IOException; import java.util.ArrayList; @@ -50,7 +51,7 @@ public class ListAccess implements RestReadView { @Override public Map apply(TopLevelResource resource) throws ResourceNotFoundException, ResourceConflictException, IOException, - PermissionBackendException { + PermissionBackendException, OrmException { Map access = new TreeMap<>(); for (String p : projects) { access.put(p, getAccess.apply(new Project.NameKey(p))); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java index 7c0795ee71..00190bb3a5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetAccess.java @@ -18,6 +18,7 @@ import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE import static com.google.gerrit.server.permissions.ProjectPermission.CREATE_REF; import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE; import static com.google.gerrit.server.permissions.RefPermission.READ; +import static java.util.stream.Collectors.toMap; import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.Iterables; @@ -30,6 +31,7 @@ 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.ProjectAccessInfo; +import com.google.gerrit.extensions.common.GroupInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; @@ -43,9 +45,11 @@ import com.google.gerrit.server.account.GroupControl; 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.group.GroupJson; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.RefPermission; +import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -55,9 +59,15 @@ import java.util.HashSet; import java.util.Map; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @Singleton public class GetAccess implements RestReadView { + private static final Logger LOG = LoggerFactory.getLogger(GetAccess.class); + + /** Marker value used in {@code Map} for groups not visible to current user. */ + private static final GroupInfo INVISIBLE_SENTINEL = new GroupInfo(); public static final ImmutableBiMap ACTION_TYPE = ImmutableBiMap.of( @@ -81,6 +91,7 @@ public class GetAccess implements RestReadView { private final MetaDataUpdate.Server metaDataUpdateFactory; private final ProjectControl.GenericFactory projectControlFactory; private final GroupBackend groupBackend; + private final GroupJson groupJson; @Inject public GetAccess( @@ -92,7 +103,8 @@ public class GetAccess implements RestReadView { MetaDataUpdate.Server metaDataUpdateFactory, ProjectJson projectJson, ProjectControl.GenericFactory projectControlFactory, - GroupBackend groupBackend) { + GroupBackend groupBackend, + GroupJson groupJson) { this.user = self; this.permissionBackend = permissionBackend; this.groupControlFactory = groupControlFactory; @@ -102,11 +114,12 @@ public class GetAccess implements RestReadView { this.projectControlFactory = projectControlFactory; this.metaDataUpdateFactory = metaDataUpdateFactory; this.groupBackend = groupBackend; + this.groupJson = groupJson; } public ProjectAccessInfo apply(Project.NameKey nameKey) throws ResourceNotFoundException, ResourceConflictException, IOException, - PermissionBackendException { + PermissionBackendException, OrmException { try { return apply(new ProjectResource(projectControlFactory.controlFor(nameKey, user.get()))); } catch (NoSuchProjectException e) { @@ -117,7 +130,7 @@ public class GetAccess implements RestReadView { @Override public ProjectAccessInfo apply(ProjectResource rsrc) throws ResourceNotFoundException, ResourceConflictException, IOException, - PermissionBackendException { + PermissionBackendException, OrmException { // Load the current configuration from the repository, ensuring it's the most // recent version available. If it differs from what was in the project // state, force a cache flush now. @@ -151,27 +164,27 @@ public class GetAccess implements RestReadView { info.local = new HashMap<>(); info.ownerOf = new HashSet<>(); - Map visibleGroups = new HashMap<>(); + Map visibleGroups = new HashMap<>(); boolean checkReadConfig = check(perm, RefNames.REFS_CONFIG, READ); for (AccessSection section : config.getAccessSections()) { String name = section.getName(); if (AccessSection.GLOBAL_CAPABILITIES.equals(name)) { if (pc.isOwner()) { - info.local.put(name, createAccessSection(section)); + info.local.put(name, createAccessSection(visibleGroups, section)); info.ownerOf.add(name); } else if (checkReadConfig) { - info.local.put(section.getName(), createAccessSection(section)); + info.local.put(section.getName(), createAccessSection(visibleGroups, section)); } } else if (RefConfigSection.isValid(name)) { if (pc.controlForRef(name).isOwner()) { - info.local.put(name, createAccessSection(section)); + info.local.put(name, createAccessSection(visibleGroups, section)); info.ownerOf.add(name); } else if (checkReadConfig) { - info.local.put(name, createAccessSection(section)); + info.local.put(name, createAccessSection(visibleGroups, section)); } else if (check(perm, name, READ)) { // Filter the section to only add rules describing groups that @@ -184,26 +197,18 @@ public class GetAccess implements RestReadView { Permission dstPerm = null; for (PermissionRule srcRule : srcPerm.getRules()) { - AccountGroup.UUID group = srcRule.getGroup().getUUID(); - if (group == null) { + AccountGroup.UUID groupId = srcRule.getGroup().getUUID(); + if (groupId == null) { continue; } - Boolean canSeeGroup = visibleGroups.get(group); - if (canSeeGroup == null) { - try { - canSeeGroup = groupControlFactory.controlFor(group).isVisible(); - } catch (NoSuchGroupException e) { - canSeeGroup = Boolean.FALSE; - } - visibleGroups.put(group, canSeeGroup); - } + GroupInfo group = loadGroup(visibleGroups, groupId); - if (canSeeGroup) { + if (group != INVISIBLE_SENTINEL) { if (dstPerm == null) { if (dst == null) { dst = new AccessSection(name); - info.local.put(name, createAccessSection(dst)); + info.local.put(name, createAccessSection(visibleGroups, dst)); } dstPerm = dst.getPermission(srcPerm.getName(), true); } @@ -244,9 +249,37 @@ public class GetAccess implements RestReadView { info.canAdd = toBoolean(perm.testOrFalse(CREATE_REF)); info.configVisible = checkReadConfig || pc.isOwner(); + info.groups = + visibleGroups + .entrySet() + .stream() + .filter(e -> e.getValue() != INVISIBLE_SENTINEL) + .collect(toMap(e -> e.getKey().get(), e -> e.getValue())); + return info; } + private GroupInfo loadGroup(Map visibleGroups, AccountGroup.UUID id) + throws OrmException { + GroupInfo group = visibleGroups.get(id); + if (group == null) { + try { + GroupControl control = groupControlFactory.controlFor(id); + group = INVISIBLE_SENTINEL; + if (control.isVisible()) { + group = groupJson.format(control.getGroup()); + group.id = null; + } + } catch (NoSuchGroupException e) { + LOG.warn("NoSuchGroupException; ignoring group " + id, e); + group = INVISIBLE_SENTINEL; + } + visibleGroups.put(id, group); + } + + return group; + } + private static boolean check(PermissionBackend.ForProject ctx, String ref, RefPermission perm) throws PermissionBackendException { try { @@ -257,7 +290,8 @@ public class GetAccess implements RestReadView { } } - private AccessSectionInfo createAccessSection(AccessSection section) { + private AccessSectionInfo createAccessSection( + Map groups, AccessSection section) throws OrmException { AccessSectionInfo accessSectionInfo = new AccessSectionInfo(); accessSectionInfo.permissions = new HashMap<>(); for (Permission p : section.getPermissions()) { @@ -273,6 +307,7 @@ public class GetAccess implements RestReadView { AccountGroup.UUID group = r.getGroup().getUUID(); if (group != null) { pInfo.rules.put(group.get(), info); + loadGroup(groups, group); } } accessSectionInfo.permissions.put(p.getName(), pInfo); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java index ce0f979c40..9cba53b800 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java @@ -44,6 +44,7 @@ import com.google.gerrit.server.group.GroupsCollection; 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.Singleton; @@ -91,7 +92,8 @@ public class SetAccess implements RestModifyView removals = getAccessSections(input.remove); List additions = getAccessSections(input.add); MetaDataUpdate.User metaDataUpdateUser = metaDataUpdateFactory.get();