remove obsolete permissions

src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java

@@ -560,7 +560,7 @@ public void doRemoveTemplates(@QueryParameter(required = true) String names,
   /**
    * API method to add a role.
    *
-   * <p>Unknown and dangerous permissions are ignored.
+   * <p>Unknown permissions are ignored.
    *
    * When specifying a <code>template</code> for an item role, the given permissions are ignored. The named template must exist,
    * otherwise the request fails with status <code>400</code>.
@@ -1683,9 +1683,6 @@ public List<PermissionGroup> getGroups(@NonNull String type) {
     public boolean showPermission(String type, Permission p) {
       switch (type) {
         case GLOBAL:
-          if (PermissionHelper.isDangerous(p)) {
-            return false;
-          }
           return p.getEnabled();
         case PROJECT:
           if (!p.isContainedBy(PermissionScope.ITEM_GROUP)) {

src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java

@@ -250,21 +250,14 @@ private static Set<Permission> getImplyingPermissions(Permission p) {
    */
   private static Set<Permission> cacheImplyingPermissions(Permission permission) {
     Set<Permission> implyingPermissions;
-    if (PermissionHelper.isDangerous(permission)) {
-      /*
-       * if this is a dangerous permission, fall back to Administer unless we're in compat mode
-       */
-      implyingPermissions = getImplyingPermissions(Jenkins.ADMINISTER);
-    } else {
-      implyingPermissions = new HashSet<>();
-
-      // Get the implying permissions
-      for (Permission p = permission; p != null; p = p.impliedBy) {
-        if (!p.getEnabled()) {
-          continue;
-        }
-        implyingPermissions.add(p);
+    implyingPermissions = new HashSet<>();
+
+    // Get the implying permissions
+    for (Permission p = permission; p != null; p = p.impliedBy) {
+      if (!p.getEnabled()) {
+        continue;
       }
+      implyingPermissions.add(p);
     }
 
     return implyingPermissions;

src/main/java/org/jenkinsci/plugins/rolestrategy/permissions/PermissionHelper.java

@@ -26,10 +26,8 @@
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
-import hudson.PluginManager;
 import hudson.security.Permission;
 import hudson.security.PermissionGroup;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -39,12 +37,11 @@
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import jenkins.model.Jenkins;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
- * Helper methods for dangerous permission handling.
+ * Helper methods for permission handling.
  *
  * @author Oleg Nenashev
  */
@@ -55,21 +52,13 @@ public class PermissionHelper {
 
   private static final Pattern PERMISSION_PATTERN = Pattern.compile("^([^\\/]+)\\/(.+)$");
 
-  /**
-   * List of the dangerous permissions, which need to be suppressed by the plugin.
-   */
-  @SuppressWarnings("deprecation")
-  @Restricted(NoExternalUse.class)
-  public static final Set<Permission> DANGEROUS_PERMISSIONS = Collections.unmodifiableSet(
-      new HashSet<>(Arrays.asList(Jenkins.RUN_SCRIPTS, PluginManager.CONFIGURE_UPDATECENTER, PluginManager.UPLOAD_PLUGINS)));
-
   private PermissionHelper() {
     // Cannot be constructed
   }
 
   /**
    * Convert a set of string to a collection of permissions.
-   * Dangerous and non-solvable permissions are ignored
+   * Non-solvable permissions are ignored
    *
    * @param permissionStrings A list of Permission IDs or UI names.
    * @param allowPermissionId Allow to resolve the permission from the ID.
@@ -94,16 +83,6 @@ public static Set<Permission> fromStrings(@CheckForNull Collection<String> permi
     return res;
   }
 
-  /**
-   * Check if the permissions is dangerous.
-   *
-   * @param p Permission
-   * @return {@code true} if the permission is considered as dangerous.
-   */
-  public static boolean isDangerous(Permission p) {
-    return DANGEROUS_PERMISSIONS.contains(p);
-  }
-
   /**
    * Attempt to match a given permission to what is defined in the UI.
    *
@@ -113,7 +92,7 @@ public static boolean isDangerous(Permission p) {
   @CheckForNull
   public static Permission findPermission(String id) {
     final String resolvedId = findPermissionId(id);
-    return resolvedId != null ? getSafePermission(resolvedId) : null;
+    return resolvedId != null ? Permission.fromId(resolvedId) : null;
   }
 
   /**
@@ -145,23 +124,18 @@ public static String findPermissionId(String id) {
   }
 
   private static @CheckForNull Permission getSafePermission(String id) {
-    Permission permission = Permission.fromId(id);
-    if (permission != null && isDangerous(permission)) {
-      LOGGER.log(Level.WARNING, "The permission: '"  + permission + "' is dangerous and will be ignored.");
-      return null;
-    }
-    return permission;
+    return Permission.fromId(id);
   }
 
   /**
    * Attempt to match a given permission to what is defined in the UI or from the ID representation used in the config.xml.
    *
    * @param id String of the form "Title/Permission" (Look in the UI) for a particular permission or in the form used in the config.xml
-   * @return a matched permission, null if permission couldn't be resolved or is dangerous
+   * @return a matched permission, null if permission couldn't be resolved
    */
   @CheckForNull
   public static Permission resolvePermissionFromString(String id) {
-    Permission permission = getSafePermission(id);
+    Permission permission = Permission.fromId(id);
     if (permission == null) {
       permission = PermissionHelper.findPermission(id);
     }

src/test/java/com/michelin/cio/hudson/plugins/rolestrategy/ApiTest.java

@@ -533,28 +533,6 @@ void testUnassignGroupRole() throws IOException {
     assertFalse(folder.hasPermission2(alice, Item.CONFIGURE));
   }
 
-  @Test
-  void ignoreDangerousPermissionInAddRole() throws IOException {
-    String roleName = "new-role";
-    // Adding role via web request
-    URL apiUrl = new URL(jenkinsRule.jenkins.getRootUrl() + "role-strategy/strategy/addRole");
-    WebRequest request = new WebRequest(apiUrl, HttpMethod.POST);
-    request.setRequestParameters(
-            Arrays.asList(new NameValuePair("type", RoleType.Global.getStringType()), new NameValuePair("roleName", roleName),
-                    new NameValuePair("permissionIds",
-                            "hudson.model.Hudson.RunScripts,hudson.model.Hudson.ConfigureUpdateCenter,"
-                                    + "hudson.model.Hudson.UploadPlugins,hudson.model.Item.Read"),
-                    new NameValuePair("overwrite", "false")));
-    Page page = webClient.getPage(request);
-    assertEquals(HttpURLConnection.HTTP_OK, page.getWebResponse().getStatusCode(), "Testing if request is successful");
-
-    // Verifying that the role is in
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(PluginManager.CONFIGURE_UPDATECENTER), is(false));
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(PluginManager.UPLOAD_PLUGINS), is(false));
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(Jenkins.RUN_SCRIPTS), is(false));
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole(roleName).hasPermission(Item.READ), is(true));
-  }
-
   @Test
   void testRemoveRolesAs() throws Exception {
 

src/test/java/org/jenkinsci/plugins/rolestrategy/ConfigurationAsCodeTest.java

@@ -139,18 +139,6 @@ void shouldHandleNullItemsAndAgentsCorrectly(JenkinsConfiguredWithCodeRule jcwcR
     assertThat(globalRoles.size(), equalTo(2));
   }
 
-  @Test
-  @ConfiguredWithCode("Configuration-as-Code3.yml")
-  void dangerousPermissionsAreIgnored(JenkinsConfiguredWithCodeRule jcwcRule) {
-    AuthorizationStrategy s = jcwcRule.jenkins.getAuthorizationStrategy();
-    assertThat("Authorization Strategy has been read incorrectly", s, instanceOf(RoleBasedAuthorizationStrategy.class));
-    RoleBasedAuthorizationStrategy rbas = (RoleBasedAuthorizationStrategy) s;
-
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole("dangerous").hasPermission(PluginManager.CONFIGURE_UPDATECENTER), is(false));
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole("dangerous").hasPermission(PluginManager.UPLOAD_PLUGINS), is(false));
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole("dangerous").hasPermission(Jenkins.RUN_SCRIPTS), is(false));
-  }
-
   @Test
   @ConfiguredWithCode("Configuration-as-Code-no-permissions.yml")
   void exportWithEmptyRole(JenkinsConfiguredWithCodeRule jcwcRule) throws Exception {

src/test/java/org/jenkinsci/plugins/rolestrategy/RoleStrategyTest.java

@@ -1,19 +1,13 @@
 package org.jenkinsci.plugins.rolestrategy;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import com.michelin.cio.hudson.plugins.rolestrategy.RoleBasedAuthorizationStrategy;
 import com.michelin.cio.hudson.plugins.rolestrategy.RoleMap;
-import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType;
-import hudson.PluginManager;
 import hudson.model.User;
 import hudson.security.ACL;
 import hudson.security.ACLContext;
 import hudson.security.Permission;
-import jenkins.model.Jenkins;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.jvnet.hudson.test.JenkinsRule;
@@ -67,13 +61,4 @@ void testRoleAssignmentCaseSensitiveNoMatchFails() {
     }
   }
 
-  @LocalData
-  @Test
-  void dangerousPermissionsAreIgnored() {
-    RoleBasedAuthorizationStrategy rbas = (RoleBasedAuthorizationStrategy) jenkinsRule.jenkins.getAuthorizationStrategy();
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole("POWERUSERS").hasPermission(PluginManager.CONFIGURE_UPDATECENTER), is(false));
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole("POWERUSERS").hasPermission(PluginManager.UPLOAD_PLUGINS), is(false));
-    assertThat(rbas.getRoleMap(RoleType.Global).getRole("POWERUSERS").hasPermission(Jenkins.RUN_SCRIPTS), is(false));
-  }
-
 }

src/test/resources/org/jenkinsci/plugins/rolestrategy/Configuration-as-Code3.yml

@@ -12,9 +12,6 @@ jenkins:
           - name: "dangerous"
             description: "Dangerous"
             permissions:
-              - "Overall/RunScripts"
-              - "Overall/ConfigureUpdateCenter"
-              - "Overall/UploadPlugins"
               - "Job/Read"
             entries:
               - user: "test"