Teach PermissionBackend to accept plugin defined project permission

This commit allows plugins to define project permissions
and updates the PermissionBackend interface to accept
plugin defined project permissions.

This commit doesn't implement the strategies for handling
plugin project permissions and leave it to follow-up
commits.

Change-Id: Ib9d8d80045e50d51237da6d420aa9bfe001ca207

java/com/google/gerrit/extensions/api/access/CoreOrPluginProjectPermission.java

@@ -0,0 +1,18 @@
+// Copyright (C) 2019 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.extensions.api.access;
+
+/** A repository permission either defined in Gerrit core or a plugin. */
+public interface CoreOrPluginProjectPermission extends GerritPermission {}

java/com/google/gerrit/extensions/api/access/PluginProjectPermission.java

@@ -0,0 +1,87 @@
+// Copyright (C) 2019 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.extensions.api.access;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.base.MoreObjects;
+import java.util.Objects;
+import java.util.regex.Pattern;
+
+/** Repository permissions defined by plugins. */
+public final class PluginProjectPermission implements CoreOrPluginProjectPermission {
+  public static final String PLUGIN_PERMISSION_NAME_PATTERN_STRING = "[a-zA-Z]+";
+  private static final Pattern PLUGIN_PERMISSION_PATTERN =
+      Pattern.compile("^" + PLUGIN_PERMISSION_NAME_PATTERN_STRING + "$");
+
+  private final String pluginName;
+  private final String permission;
+
+  public PluginProjectPermission(String pluginName, String permission) {
+    requireNonNull(pluginName, "pluginName");
+    requireNonNull(permission, "permission");
+    checkArgument(
+        isValidPluginPermissionName(permission), "invalid plugin permission name: ", permission);
+
+    this.pluginName = pluginName;
+    this.permission = permission;
+  }
+
+  public String pluginName() {
+    return pluginName;
+  }
+
+  public String permission() {
+    return permission;
+  }
+
+  @Override
+  public String describeForException() {
+    return permission + " for plugin " + pluginName;
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(pluginName, permission);
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    if (other instanceof PluginProjectPermission) {
+      PluginProjectPermission b = (PluginProjectPermission) other;
+      return pluginName.equals(b.pluginName) && permission.equals(b.permission);
+    }
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("pluginName", pluginName)
+        .add("permission", permission)
+        .toString();
+  }
+
+  /**
+   * Checks if a given name is valid to be used for plugin permissions.
+   *
+   * @param name a name string.
+   * @return whether the name is valid as a plugin permission.
+   */
+  private static boolean isValidPluginPermissionName(String name) {
+    return PLUGIN_PERMISSION_PATTERN.matcher(name).matches();
+  }
+}

java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java

@@ -40,7 +40,6 @@ import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.util.Collection;
-import java.util.EnumSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -128,7 +127,7 @@ public class DefaultPermissionBackend extends PermissionBackend {
     @Override
     public <T extends GlobalOrPluginPermission> Set<T> test(Collection<T> permSet)
         throws PermissionBackendException {
-      Set<T> ok = newSet(permSet);
+      Set<T> ok = Sets.newHashSetWithExpectedSize(permSet.size());
       for (T perm : permSet) {
         if (can(perm)) {
           ok.add(perm);
@@ -147,7 +146,7 @@ public class DefaultPermissionBackend extends PermissionBackend {
         return can((GlobalPermission) perm);
       } else if (perm instanceof PluginPermission) {
         PluginPermission pluginPermission = (PluginPermission) perm;
-        return has(DefaultPermissionMappings.pluginPermissionName(pluginPermission))
+        return has(DefaultPermissionMappings.pluginCapabilityName(pluginPermission))
             || (pluginPermission.fallBackToAdmin() && isAdmin());
       }
       throw new PermissionBackendException(perm + " unsupported");
@@ -269,14 +268,4 @@ public class DefaultPermissionBackend extends PermissionBackend {
       return denied.isEmpty() || !user.getEffectiveGroups().containsAnyOf(denied);
     }
   }
-
-  private static <T extends GlobalOrPluginPermission> Set<T> newSet(Collection<T> permSet) {
-    if (permSet instanceof EnumSet) {
-      @SuppressWarnings({"unchecked", "rawtypes"})
-      Set<T> s = ((EnumSet) permSet).clone();
-      s.clear();
-      return s;
-    }
-    return Sets.newHashSetWithExpectedSize(permSet.size());
-  }
 }

java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java

@@ -23,6 +23,7 @@ import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
 import com.google.gerrit.extensions.api.access.PluginPermission;
+import com.google.gerrit.extensions.api.access.PluginProjectPermission;
 import com.google.gerrit.server.permissions.LabelPermission.ForUser;
 import java.util.EnumSet;
 import java.util.Optional;
@@ -117,14 +118,18 @@ public class DefaultPermissionMappings {
     return Optional.ofNullable(CAPABILITIES.inverse().get(capabilityName));
   }
 
-  public static String pluginPermissionName(PluginPermission pluginPermission) {
+  public static String pluginCapabilityName(PluginPermission pluginPermission) {
     return pluginPermission.pluginName() + '-' + pluginPermission.capability();
   }
 
+  public static String pluginProjectPermissionName(PluginProjectPermission pluginPermission) {
+    return "plugin-" + pluginPermission.pluginName() + '-' + pluginPermission.permission();
+  }
+
   public static String globalOrPluginPermissionName(GlobalOrPluginPermission permission) {
     return permission instanceof GlobalPermission
         ? globalPermissionName((GlobalPermission) permission)
-        : pluginPermissionName((PluginPermission) permission);
+        : pluginCapabilityName((PluginPermission) permission);
   }
 
   public static Optional<String> projectPermissionName(ProjectPermission projectPermission) {

java/com/google/gerrit/server/permissions/FailedPermissionBackend.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.permissions;
 
+import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.reviewdb.client.Project;
@@ -124,18 +125,18 @@ public class FailedPermissionBackend {
     }
 
     @Override
-    public void check(ProjectPermission perm) throws PermissionBackendException {
+    public void check(CoreOrPluginProjectPermission perm) throws PermissionBackendException {
       throw new PermissionBackendException(message, cause);
     }
 
     @Override
-    public Set<ProjectPermission> test(Collection<ProjectPermission> permSet)
+    public <T extends CoreOrPluginProjectPermission> Set<T> test(Collection<T> permSet)
         throws PermissionBackendException {
       throw new PermissionBackendException(message, cause);
     }
 
     @Override
-    public BooleanCondition testCond(ProjectPermission perm) {
+    public BooleanCondition testCond(CoreOrPluginProjectPermission perm) {
       throw new UnsupportedOperationException(
           "FailedPermissionBackend does not support conditions");
     }

java/com/google/gerrit/server/permissions/PermissionBackend.java

@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -300,18 +301,23 @@ public abstract class PermissionBackend {
     }
 
     /** Verify scoped user can {@code perm}, throwing if denied. */
-    public abstract void check(ProjectPermission perm)
+    public abstract void check(CoreOrPluginProjectPermission perm)
         throws AuthException, PermissionBackendException;
 
     /** Filter {@code permSet} to permissions scoped user might be able to perform. */
-    public abstract Set<ProjectPermission> test(Collection<ProjectPermission> permSet)
+    public abstract <T extends CoreOrPluginProjectPermission> Set<T> test(Collection<T> permSet)
         throws PermissionBackendException;
 
-    public boolean test(ProjectPermission perm) throws PermissionBackendException {
+    public boolean test(CoreOrPluginProjectPermission perm) throws PermissionBackendException {
-      return test(EnumSet.of(perm)).contains(perm);
+      if (perm instanceof ProjectPermission) {
+        return test(EnumSet.of((ProjectPermission) perm)).contains(perm);
+      }
+
+      // TODO(xchangcheng): implement for plugin defined project permissions.
+      return false;
     }
 
-    public boolean testOrFalse(ProjectPermission perm) {
+    public boolean testOrFalse(CoreOrPluginProjectPermission perm) {
       try {
         return test(perm);
       } catch (PermissionBackendException e) {
@@ -320,7 +326,7 @@ public abstract class PermissionBackend {
       }
     }
 
-    public abstract BooleanCondition testCond(ProjectPermission perm);
+    public abstract BooleanCondition testCond(CoreOrPluginProjectPermission perm);
 
     /**
      * Filter a map of references by visibility.

java/com/google/gerrit/server/permissions/PermissionBackendCondition.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.permissions;
 
+import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.conditions.PrivateInternals_BooleanCondition;
@@ -100,10 +101,11 @@ public abstract class PermissionBackendCondition
 
   public static class ForProject extends PermissionBackendCondition {
     private final PermissionBackend.ForProject impl;
-    private final ProjectPermission perm;
+    private final CoreOrPluginProjectPermission perm;
     private final CurrentUser user;
 
-    public ForProject(PermissionBackend.ForProject impl, ProjectPermission perm, CurrentUser user) {
+    public ForProject(
+        PermissionBackend.ForProject impl, CoreOrPluginProjectPermission perm, CurrentUser user) {
       this.impl = impl;
       this.perm = perm;
       this.user = user;
@@ -113,7 +115,7 @@ public abstract class PermissionBackendCondition
       return impl;
     }
 
-    public ProjectPermission permission() {
+    public CoreOrPluginProjectPermission permission() {
       return perm;
     }
 

java/com/google/gerrit/server/permissions/PluginPermissionsUtil.java

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server.permissions;
 
+import static com.google.gerrit.extensions.api.access.PluginProjectPermission.PLUGIN_PERMISSION_NAME_PATTERN_STRING;
+
 import com.google.common.collect.ImmutableMap;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.extensions.config.CapabilityDefinition;
@@ -41,7 +43,12 @@ public final class PluginPermissionsUtil {
    * enough for this purpose since some core permissions, e.g. "label-", also contain "-".
    */
   private static final Pattern PLUGIN_PERMISSION_NAME_IN_CONFIG_PATTERN =
-      Pattern.compile("^plugin-" + PLUGIN_NAME_PATTERN_STRING + "-[a-zA-Z]+$");
+      Pattern.compile(
+          "^plugin-"
+              + PLUGIN_NAME_PATTERN_STRING
+              + "-"
+              + PLUGIN_PERMISSION_NAME_PATTERN_STRING
+              + "$");
 
   /** Name pattern for a Gerrit plugin. */
   private static final Pattern PLUGIN_NAME_PATTERN =
@@ -104,7 +111,7 @@ public final class PluginPermissionsUtil {
    * @param name a config name which may stand for a plugin permission.
    * @return whether the name matches the plugin permission name pattern for configs.
    */
-  public static boolean isPluginPermission(String name) {
+  public static boolean isValidPluginPermission(String name) {
     return PLUGIN_PERMISSION_NAME_IN_CONFIG_PATTERN.matcher(name).matches();
   }
 }

java/com/google/gerrit/server/permissions/ProjectControl.java

@@ -17,9 +17,12 @@ package com.google.gerrit.server.permissions;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.gerrit.reviewdb.client.RefNames.REFS_TAGS;
 
+import com.google.common.collect.Sets;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
+import com.google.gerrit.extensions.api.access.PluginProjectPermission;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -45,7 +48,6 @@ import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -372,17 +374,18 @@ class ProjectControl {
     }
 
     @Override
-    public void check(ProjectPermission perm) throws AuthException, PermissionBackendException {
+    public void check(CoreOrPluginProjectPermission perm)
+        throws AuthException, PermissionBackendException {
       if (!can(perm)) {
         throw new AuthException(perm.describeForException() + " not permitted");
       }
     }
 
     @Override
-    public Set<ProjectPermission> test(Collection<ProjectPermission> permSet)
+    public <T extends CoreOrPluginProjectPermission> Set<T> test(Collection<T> permSet)
         throws PermissionBackendException {
-      EnumSet<ProjectPermission> ok = EnumSet.noneOf(ProjectPermission.class);
+      Set<T> ok = Sets.newHashSetWithExpectedSize(permSet.size());
-      for (ProjectPermission perm : permSet) {
+      for (T perm : permSet) {
         if (can(perm)) {
           ok.add(perm);
         }
@@ -391,7 +394,7 @@ class ProjectControl {
     }
 
     @Override
-    public BooleanCondition testCond(ProjectPermission perm) {
+    public BooleanCondition testCond(CoreOrPluginProjectPermission perm) {
       return new PermissionBackendCondition.ForProject(this, perm, getUser());
     }
 
@@ -404,6 +407,17 @@ class ProjectControl {
       return refFilter.filter(refs, repo, opts);
     }
 
+    private boolean can(CoreOrPluginProjectPermission perm) throws PermissionBackendException {
+      if (perm instanceof ProjectPermission) {
+        return can((ProjectPermission) perm);
+      } else if (perm instanceof PluginProjectPermission) {
+        // TODO(xchangcheng): implement for plugin defined project permissions.
+        return false;
+      }
+
+      throw new PermissionBackendException(perm.describeForException() + " unsupported");
+    }
+
     private boolean can(ProjectPermission perm) throws PermissionBackendException {
       switch (perm) {
         case ACCESS:

java/com/google/gerrit/server/permissions/ProjectPermission.java

@@ -16,10 +16,11 @@ package com.google.gerrit.server.permissions;
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
 import com.google.gerrit.extensions.api.access.GerritPermission;
 import com.google.gerrit.reviewdb.client.RefNames;
 
-public enum ProjectPermission implements GerritPermission {
+public enum ProjectPermission implements CoreOrPluginProjectPermission {
   /**
    * Can access at least one reference or change within the repository.
    *

java/com/google/gerrit/server/project/ProjectConfig.java

@@ -18,7 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.gerrit.common.data.Permission.isPermission;
 import static com.google.gerrit.reviewdb.client.Project.DEFAULT_SUBMIT_TYPE;
-import static com.google.gerrit.server.permissions.PluginPermissionsUtil.isPluginPermission;
+import static com.google.gerrit.server.permissions.PluginPermissionsUtil.isValidPluginPermission;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.base.CharMatcher;
@@ -788,7 +788,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
   private boolean isCoreOrPluginPermission(String permission) {
     // Since plugins are loaded dynamically, here we can't load all plugin permissions and verify
     // their existence.
-    return isPermission(permission) || isPluginPermission(permission);
+    return isPermission(permission) || isValidPluginPermission(permission);
   }
 
   private boolean isValidRegex(String refPattern) {

java/com/google/gerrit/server/restapi/account/GetCapabilities.java

@@ -17,7 +17,7 @@ package com.google.gerrit.server.restapi.account;
 import static com.google.gerrit.common.data.GlobalCapability.PRIORITY;
 import static com.google.gerrit.server.permissions.DefaultPermissionMappings.globalOrPluginPermissionName;
 import static com.google.gerrit.server.permissions.DefaultPermissionMappings.globalPermissionName;
-import static com.google.gerrit.server.permissions.DefaultPermissionMappings.pluginPermissionName;
+import static com.google.gerrit.server.permissions.DefaultPermissionMappings.pluginCapabilityName;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.common.data.GlobalCapability;
@@ -113,7 +113,7 @@ public class GetCapabilities implements RestReadView<AccountResource> {
     for (String pluginName : pluginCapabilities.plugins()) {
       for (String capability : pluginCapabilities.byPlugin(pluginName).keySet()) {
         PluginPermission p = new PluginPermission(pluginName, capability);
-        if (want(pluginPermissionName(p))) {
+        if (want(pluginCapabilityName(p))) {
           toTest.add(p);
         }
       }

javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java

@@ -18,6 +18,8 @@ import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.google.gerrit.extensions.api.access.CoreOrPluginProjectPermission;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.Account;
@@ -55,19 +57,29 @@ public class UiActionsTest extends GerritBaseTests {
     }
 
     @Override
-    public void check(ProjectPermission perm) throws AuthException, PermissionBackendException {
+    public void check(CoreOrPluginProjectPermission perm)
+        throws AuthException, PermissionBackendException {
       throw new UnsupportedOperationException("not implemented");
     }
 
     @Override
-    public Set<ProjectPermission> test(Collection<ProjectPermission> permSet)
+    public <T extends CoreOrPluginProjectPermission> Set<T> test(Collection<T> permSet)
         throws PermissionBackendException {
       assertThat(allowValueQueries).isTrue();
-      return ImmutableSet.of(ProjectPermission.READ);
+      Set<T> ok = Sets.newHashSetWithExpectedSize(permSet.size());
+      for (T perm : permSet) {
+        // Allow ProjectPermission.READ, if it was requested in the input permSet. This implies
+        // that permSet has type Collection<ProjectPermission>, otherwise no permission would
+        // compare equal to READ.
+        if (perm.equals(ProjectPermission.READ)) {
+          ok.add(perm);
+        }
+      }
+      return ok;
     }
 
     @Override
-    public BooleanCondition testCond(ProjectPermission perm) {
+    public BooleanCondition testCond(CoreOrPluginProjectPermission perm) {
       return new PermissionBackendCondition.ForProject(this, perm, fakeUser());
     }
 

javatests/com/google/gerrit/server/permissions/PluginPermissionsUtilTest.java

@@ -15,7 +15,7 @@
 package com.google.gerrit.server.permissions;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.server.permissions.PluginPermissionsUtil.isPluginPermission;
+import static com.google.gerrit.server.permissions.PluginPermissionsUtil.isValidPluginPermission;
 
 import com.google.common.collect.ImmutableList;
 import org.junit.Test;
@@ -30,7 +30,7 @@ public final class PluginPermissionsUtilTest {
         ImmutableList.of("plugin-foo-a", "plugin-foo-a-b");
 
     for (String permission : validPluginPermissions) {
-      assertThat(isPluginPermission(permission))
+      assertThat(isValidPluginPermission(permission))
           .named("valid plugin permission: %s", permission)
           .isTrue();
     }
@@ -38,7 +38,7 @@ public final class PluginPermissionsUtilTest {
 
   @Test
   public void isPluginPermissionReturnsFalseForInvalidName() {
-    ImmutableList<String> inValidPluginPermissions =
+    ImmutableList<String> invalidPluginPermissions =
         ImmutableList.of(
             "create",
             "label-Code-Review",
@@ -47,8 +47,8 @@ public final class PluginPermissionsUtilTest {
             "plugin-foo-a-",
             "plugin-foo-a1");
 
-    for (String permission : inValidPluginPermissions) {
+    for (String permission : invalidPluginPermissions) {
-      assertThat(isPluginPermission(permission))
+      assertThat(isValidPluginPermission(permission))
           .named("invalid plugin permission: %s", permission)
           .isFalse();
     }