Make SetAccessUtil#validateChanges respect plugin capabilities

Bug: Issue 8465
Change-Id: Ic601d542a89310ef1df0360aed962a146e44bcc5

gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionEditor.java

@@ -273,7 +273,7 @@ public class PermissionEditor extends Composite
                 lt.getMin().getValue(),
                 lt.getMax().getValue());
       }
-    } else if (GlobalCapability.isCapability(value.getName())) {
+    } else if (GlobalCapability.isGlobalCapability(value.getName())) {
       validRange = GlobalCapability.getRange(value.getName());
 
     } else {

java/com/google/gerrit/common/data/GlobalCapability.java

@@ -156,7 +156,7 @@ public class GlobalCapability {
   }
 
   /** @return true if the name is recognized as a capability name. */
-  public static boolean isCapability(String varName) {
+  public static boolean isGlobalCapability(String varName) {
     return NAMES_LC.contains(varName.toLowerCase());
   }
 

java/com/google/gerrit/server/restapi/config/ListCapabilities.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.restapi.config;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.extensions.config.CapabilityDefinition;
 import com.google.gerrit.extensions.registration.DynamicMap;
@@ -23,8 +24,8 @@ import com.google.gerrit.server.config.ConfigResource;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
+import java.util.HashMap;
 import java.util.Map;
-import java.util.TreeMap;
 import java.util.regex.Pattern;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -45,23 +46,14 @@ public class ListCapabilities implements RestReadView<ConfigResource> {
   @Override
   public Map<String, CapabilityInfo> apply(ConfigResource resource)
       throws IllegalAccessException, NoSuchFieldException {
-    Map<String, CapabilityInfo> output = new TreeMap<>();
-    collectCoreCapabilities(output);
-    collectPluginCapabilities(output);
-    return output;
+    return ImmutableMap.<String, CapabilityInfo>builder()
+        .putAll(collectCoreCapabilities())
+        .putAll(collectPluginCapabilities())
+        .build();
   }
 
-  private void collectCoreCapabilities(Map<String, CapabilityInfo> output)
-      throws IllegalAccessException, NoSuchFieldException {
-    Class<? extends CapabilityConstants> bundleClass = CapabilityConstants.get().getClass();
-    CapabilityConstants c = CapabilityConstants.get();
-    for (String id : GlobalCapability.getAllNames()) {
-      String name = (String) bundleClass.getField(id).get(c);
-      output.put(id, new CapabilityInfo(id, name));
-    }
-  }
-
-  private void collectPluginCapabilities(Map<String, CapabilityInfo> output) {
+  public Map<String, CapabilityInfo> collectPluginCapabilities() {
+    Map<String, CapabilityInfo> output = new HashMap<>();
     for (String pluginName : pluginCapabilities.plugins()) {
       if (!PLUGIN_NAME_PATTERN.matcher(pluginName).matches()) {
         log.warn(
@@ -76,6 +68,19 @@ public class ListCapabilities implements RestReadView<ConfigResource> {
         output.put(id, new CapabilityInfo(id, entry.getValue().get().getDescription()));
       }
     }
+    return output;
+  }
+
+  private Map<String, CapabilityInfo> collectCoreCapabilities()
+      throws IllegalAccessException, NoSuchFieldException {
+    Map<String, CapabilityInfo> output = new HashMap<>();
+    Class<? extends CapabilityConstants> bundleClass = CapabilityConstants.get().getClass();
+    CapabilityConstants c = CapabilityConstants.get();
+    for (String id : GlobalCapability.getAllNames()) {
+      String name = (String) bundleClass.getField(id).get(c);
+      output.put(id, new CapabilityInfo(id, name));
+    }
+    return output;
   }
 
   public static class CapabilityInfo {

java/com/google/gerrit/server/restapi/project/SetAccessUtil.java

@@ -35,6 +35,7 @@ import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.RefPattern;
+import com.google.gerrit.server.restapi.config.ListCapabilities;
 import com.google.gerrit.server.restapi.group.GroupsCollection;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -43,21 +44,25 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 @Singleton
 public class SetAccessUtil {
   private final GroupsCollection groupsCollection;
   private final AllProjectsName allProjects;
   private final Provider<SetParent> setParent;
+  private final ListCapabilities listCapabilities;
 
   @Inject
   private SetAccessUtil(
       GroupsCollection groupsCollection,
       AllProjectsName allProjects,
-      Provider<SetParent> setParent) {
+      Provider<SetParent> setParent,
+      ListCapabilities listCapabilities) {
     this.groupsCollection = groupsCollection;
     this.allProjects = allProjects;
     this.setParent = setParent;
+    this.listCapabilities = listCapabilities;
   }
 
   List<AccessSection> getAccessSections(Map<String, AccessSectionInfo> sectionInfos)
@@ -148,7 +153,7 @@ public class SetAccessUtil {
       } else {
         // Check all permissions for soundness
         for (Permission p : section.getPermissions()) {
-          if (!GlobalCapability.isCapability(p.getName())) {
+          if (!isCapability(p.getName())) {
             throw new BadRequestException(
                 "Cannot add non-global capability " + p.getName() + " to global capabilities");
           }
@@ -234,4 +239,12 @@ public class SetAccessUtil {
       config.getProject().setParentName(newParentProjectName);
     }
   }
+
+  private boolean isCapability(String name) {
+    if (GlobalCapability.isGlobalCapability(name)) {
+      return true;
+    }
+    Set<String> pluginCapabilities = listCapabilities.collectPluginCapabilities().keySet();
+    return pluginCapabilities.contains(name);
+  }
 }

javatests/com/google/gerrit/acceptance/rest/project/PluginAccessIT.java

@@ -0,0 +1,78 @@
+// Copyright (C) 2018 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.acceptance.rest.project;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.extensions.annotations.Exports;
+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.api.access.ProjectAccessInput;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.inject.AbstractModule;
+import com.google.inject.Module;
+import org.junit.Test;
+
+public class PluginAccessIT extends AbstractDaemonTest {
+
+  private static final String CORE_PLUGIN_PREFIX = "gerrit-";
+  private static final String PLUGIN_CAPABILITY = "printHello";
+
+  @Override
+  public Module createModule() {
+    return new AbstractModule() {
+      @Override
+      protected void configure() {
+        bind(CapabilityDefinition.class)
+            .annotatedWith(Exports.named(PLUGIN_CAPABILITY))
+            .toInstance(
+                new CapabilityDefinition() {
+                  @Override
+                  public String getDescription() {
+                    return "Print Hello";
+                  }
+                });
+      }
+    };
+  }
+
+  @Test
+  public void addPluginCapability() throws Exception {
+    ProjectAccessInput accessInput = new ProjectAccessInput();
+    AccessSectionInfo accessSectionInfo = new AccessSectionInfo();
+    PermissionInfo email = new PermissionInfo(null, null);
+    PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+
+    email.rules = ImmutableMap.of(SystemGroupBackend.REGISTERED_USERS.get(), pri);
+    accessSectionInfo.permissions = ImmutableMap.of(CORE_PLUGIN_PREFIX + PLUGIN_CAPABILITY, email);
+    accessInput.add = ImmutableMap.of(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo);
+
+    ProjectAccessInfo updatedAccessSectionInfo =
+        gApi.projects().name(allProjects.get()).access(accessInput);
+    assertThat(
+            updatedAccessSectionInfo
+                .local
+                .get(AccessSection.GLOBAL_CAPABILITIES)
+                .permissions
+                .keySet())
+        .containsAllIn(accessSectionInfo.permissions.keySet());
+  }
+}