[SECURITY-3053]

pom.xml

@@ -10,7 +10,7 @@
     </parent>
 
    <artifactId>role-strategy</artifactId>
-   <version>${changelist}</version>
+   <version>587.${changelist}</version>
    <packaging>hpi</packaging>
 
    <name>Role-based Authorization Strategy</name>

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

@@ -86,7 +86,8 @@ public class RoleMap {
 
   private static final Logger LOGGER = Logger.getLogger(RoleMap.class.getName());
 
-  private static final ConcurrentMap<Permission, Set<Permission>> implyingPermissionCache = new ConcurrentHashMap<>();
+  private static final Cache<Permission, Set<Permission>> implyingPermissionCache = Caffeine.newBuilder().maximumSize(100)
+      .expireAfterWrite(20, TimeUnit.SECONDS).build();
 
   static {
     Permission.getAll().forEach(RoleMap::cacheImplyingPermissions);
@@ -185,12 +186,7 @@ public void perform(Role current) {
    * @return set of permissions which imply {@code p}
    */
   private static Set<Permission> getImplyingPermissions(Permission p) {
-    Set<Permission> implyingPermissions = implyingPermissionCache.get(p);
-    if (implyingPermissions != null) {
-      return implyingPermissions;
-    } else {
-      return cacheImplyingPermissions(p);
-    }
+    return implyingPermissionCache.get(p, RoleMap::cacheImplyingPermissions);
   }
 
   /**
@@ -211,11 +207,13 @@ private static Set<Permission> cacheImplyingPermissions(Permission permission) {
 
       // Get the implying permissions
       for (Permission p = permission; p != null; p = p.impliedBy) {
+        if (!p.getEnabled()) {
+          continue;
+        }
         implyingPermissions.add(p);
       }
     }
 
-    implyingPermissionCache.put(permission, implyingPermissions);
     return implyingPermissions;
   }
 

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

@@ -0,0 +1,50 @@
+package com.michelin.cio.hudson.plugins.rolestrategy;
+
+import static org.junit.Assert.assertFalse;
+
+import hudson.model.Item;
+import hudson.model.Job;
+import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.ACLContext;
+import hudson.security.HudsonPrivateSecurityRealm;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import jenkins.model.Jenkins;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public class GrantingDisabledPermissionTest {
+
+  @Rule
+  public JenkinsRule r = new JenkinsRule();
+
+  @Test
+  public void grantDisabledPermissionTest() throws Exception {
+    HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(true, false, null);
+    realm.createAccount("admin", "admin");
+    realm.createAccount("alice", "alice");
+    r.jenkins.setSecurityRealm(realm);
+
+    RoleMap roleMap = new RoleMap();
+    Role adminRole = new Role("admin-role", new HashSet<>(Collections.singletonList(Jenkins.ADMINISTER)));
+    roleMap.addRole(adminRole);
+    Role manage = new Role("manage-role", new HashSet<>(Collections.singletonList(Jenkins.MANAGE)));
+    roleMap.addRole(manage);
+    roleMap.assignRole(adminRole, "admin");
+    roleMap.assignRole(manage, "alice");
+
+    Map<String, RoleMap> constructorArg = new HashMap<>();
+    constructorArg.put("globalRoles", roleMap);
+
+    r.jenkins.setAuthorizationStrategy(new RoleBasedAuthorizationStrategy(constructorArg));
+
+    try (ACLContext ctx = ACL.as2(User.get("alice").impersonate2())) {
+      assertFalse(Jenkins.get().hasPermission(Jenkins.MANAGE));
+    }
+  }
+}