Invalidate cache when DangerousPermissionHandlingMode changes

jenkinsci · Jun 25, 2019 · ae93e1e · ae93e1e
1 parent b84b0ec
commit ae93e1e
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 15 deletions.
diff --git a/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java b/src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleMap.java
@@ -30,20 +30,22 @@
 import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleMacroExtension;
 import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import hudson.model.Item;
 import hudson.model.Items;
 import hudson.model.User;
 import hudson.security.AccessControlled;
 import hudson.security.Permission;
 import hudson.security.SidACL;
-import hudson.model.Item;
 import jenkins.model.Jenkins;
-
 import org.acegisecurity.BadCredentialsException;
 import org.acegisecurity.GrantedAuthority;
 import org.acegisecurity.acls.sid.Sid;
 import org.acegisecurity.userdetails.UserDetails;
 import org.jenkinsci.plugins.rolestrategy.Settings;
+import org.jenkinsci.plugins.rolestrategy.permissions.DangerousPermissionHandlingMode;
 import org.jenkinsci.plugins.rolestrategy.permissions.PermissionHelper;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.springframework.dao.DataAccessException;
 
@@ -108,6 +110,14 @@ public RoleMap(@Nonnull SortedMap<Role,Set<String>> grantedRoles) {
         this.grantedRoles = new ConcurrentSkipListMap<Role, Set<String>>(grantedRoles);
     }
 
+  /**
+   * Invalidate the dangerous permissions from the cache when the {@link DangerousPermissionHandlingMode} changes.
+   */
+  @Restricted(NoExternalUse.class)
+  public static void invalidateDangerousPermissions() {
+    PermissionHelper.DANGEROUS_PERMISSIONS.forEach(implyingPermissionCache::remove);
+  }
+
   /**
    * Check if the given sid has the provided {@link Permission}.
    * @return True if the sid's granted permission
@@ -188,15 +198,17 @@ private static Set<Permission> getImplyingPermissions(Permission p) {
    * @return a set of permissions that imply this permission (including itself)
    */
   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 */
-      permission = Jenkins.ADMINISTER;
-    }
+      implyingPermissions = getImplyingPermissions(Jenkins.ADMINISTER);
+    } else {
+      implyingPermissions = new HashSet<>();
 
-    Set<Permission> implyingPermissions = new HashSet<>();
-    // Get the implying permissions
-    for (Permission p = permission; p != null; p = p.impliedBy) {
-      implyingPermissions.add(p);
+      // Get the implying permissions
+      for (Permission p = permission; p != null; p = p.impliedBy) {
+        implyingPermissions.add(p);
+      }
     }
 
     implyingPermissionCache.put(permission, implyingPermissions);

diff --git a/.../java/org/jenkinsci/plugins/rolestrategy/permissions/DangerousPermissionHandlingMode.java b/.../java/org/jenkinsci/plugins/rolestrategy/permissions/DangerousPermissionHandlingMode.java
@@ -61,9 +61,8 @@ public enum DangerousPermissionHandlingMode {
     public static final String PROPERTY_NAME = DangerousPermissionHandlingMode.class.getName() + ".enableDangerousPermissions";
 
     @Nonnull
-    @Restricted(NoExternalUse.class)
     @SuppressFBWarnings(value = "MS_SHOULD_BE_REFACTORED_TO_BE_FINAL", justification = "Groovy script console access")
-    public static /* allow script access */ DangerousPermissionHandlingMode CURRENT;
+    private static DangerousPermissionHandlingMode CURRENT;
 
     static {
         String str = System.getProperty(PROPERTY_NAME);
@@ -82,4 +81,15 @@ public enum DangerousPermissionHandlingMode {
     public static DangerousPermissionHandlingMode getCurrent() {
         return CURRENT;
     }
+
+    /**
+     * Set the {@link DangerousPermissionHandlingMode
+     *
+     * @param mode the {@code DangerousPermissionHandlingMode} to be set
+     */
+    @Restricted(NoExternalUse.class)
+    public static void setCURRENT(@Nonnull DangerousPermissionHandlingMode mode) {
+        CURRENT = mode;
+        RoleMap.invalidateDangerousPermissions();
+    }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/rolestrategy/permissions/PermissionHelper.java b/src/main/java/org/jenkinsci/plugins/rolestrategy/permissions/PermissionHelper.java
@@ -98,7 +98,7 @@ public static Set<Permission> fromStrings(@CheckForNull Collection<String> permi
      *         Always {@code false} in the {@link DangerousPermissionHandlingMode#ENABLED} mode.
      */
     public static boolean isDangerous(@Nonnull Permission p) {
-        if (DangerousPermissionHandlingMode.CURRENT == DangerousPermissionHandlingMode.ENABLED) {
+        if (DangerousPermissionHandlingMode.getCurrent() == DangerousPermissionHandlingMode.ENABLED) {
             // all permissions are fine
             return false;
         }

diff --git a/src/test/java/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest.java b/src/test/java/org/jenkinsci/plugins/rolestrategy/DangerousPermissionsTest.java
@@ -119,7 +119,7 @@ public void shouldShowDangerousWhenAssigned() throws Exception {
     @SuppressWarnings("deprecation")
     public void shouldGrantDangerousPermissionsWhenEnabled() throws Exception {
         try {
-            DangerousPermissionHandlingMode.CURRENT = DangerousPermissionHandlingMode.ENABLED;
+            DangerousPermissionHandlingMode.setCURRENT(DangerousPermissionHandlingMode.ENABLED);
             assertThat(j.jenkins.getAuthorizationStrategy(), instanceOf(RoleBasedAuthorizationStrategy.class));
             RoleBasedAuthorizationStrategy strategy = (RoleBasedAuthorizationStrategy)j.jenkins.getAuthorizationStrategy();
 
@@ -148,7 +148,7 @@ public void shouldGrantDangerousPermissionsWhenEnabled() throws Exception {
             Assert.assertFalse("DangerousPermissionAdministrativeMonitor should be disabled", m.isEnabled());
             Assert.assertFalse("DangerousPermissionAdministrativeMonitor should be deactivated", m.isActivated());
         } finally {
-            DangerousPermissionHandlingMode.CURRENT = DangerousPermissionHandlingMode.UNDEFINED;
+            DangerousPermissionHandlingMode.setCURRENT(DangerousPermissionHandlingMode.UNDEFINED);
         }
     }
 
@@ -158,7 +158,7 @@ public void shouldGrantDangerousPermissionsWhenEnabled() throws Exception {
     @LocalData
     public void shouldNotShowDangerousPermissionsWhenDisabled() throws Exception {
         try {
-            DangerousPermissionHandlingMode.CURRENT = DangerousPermissionHandlingMode.DISABLED;
+            DangerousPermissionHandlingMode.setCURRENT(DangerousPermissionHandlingMode.DISABLED);
             assertThat(j.jenkins.getAuthorizationStrategy(), instanceOf(RoleBasedAuthorizationStrategy.class));
             RoleBasedAuthorizationStrategy strategy = (RoleBasedAuthorizationStrategy)j.jenkins.getAuthorizationStrategy();
 
@@ -187,7 +187,7 @@ public void shouldNotShowDangerousPermissionsWhenDisabled() throws Exception {
             Assert.assertFalse("DangerousPermissionAdministrativeMonitor should be disabled", m.isEnabled());
             Assert.assertFalse("DangerousPermissionAdministrativeMonitor should be deactivated", m.isActivated());
         } finally {
-            DangerousPermissionHandlingMode.CURRENT = DangerousPermissionHandlingMode.UNDEFINED;
+            DangerousPermissionHandlingMode.setCURRENT(DangerousPermissionHandlingMode.UNDEFINED);
         }
     }