Merge pull request #9437 from Vlatombe/administrative-monitors-for-ma…

…nagers

core/src/main/java/hudson/model/AdministrativeMonitor.java

@@ -183,14 +183,18 @@ public void doDisable(StaplerRequest req, StaplerResponse rsp) throws IOExceptio
 
     /**
      * Required permission to view this admin monitor.
-     * By default {@link Jenkins#ADMINISTER}, but {@link Jenkins#SYSTEM_READ} is also supported.
+     * By default {@link Jenkins#ADMINISTER}, but {@link Jenkins#SYSTEM_READ} or {@link Jenkins#MANAGE} are also supported.
      * <p>
      *     Changing this permission check to return {@link Jenkins#SYSTEM_READ} will make the active
      *     administrative monitor appear on {@code manage.jelly} and on the globally visible
      *     {@link jenkins.management.AdministrativeMonitorsDecorator} to users without Administer permission.
      *     {@link #doDisable(StaplerRequest, StaplerResponse)} will still always require Administer permission.
      * </p>
      * <p>
+     *     This method only allows for a single permission to be returned. If more complex permission checks are required,
+     *     override {@link #checkRequiredPermission()} and {@link #hasRequiredPermission()} instead.
+     * </p>
+     * <p>
      *     Implementers need to ensure that {@code doAct} and other web methods perform necessary permission checks:
      *     Users with System Read permissions are expected to be limited to read-only access.
      *     Form UI elements that change system state, e.g. toggling a feature on or off, need to be hidden from users
@@ -201,13 +205,50 @@ public Permission getRequiredPermission() {
         return Jenkins.ADMINISTER;
     }
 
+    /**
+     * Checks if the current user has the minimum required permission to view this administrative monitor.
+     * <p>
+     * Subclasses may override this method and {@link #hasRequiredPermission()} instead of {@link #getRequiredPermission()} to perform more complex permission checks,
+     * for example, checking either {@link Jenkins#MANAGE} or {@link Jenkins#SYSTEM_READ}.
+     * </p>
+     * @see #getRequiredPermission()
+     * @see #hasRequiredPermission()
+     */
+    public void checkRequiredPermission() {
+        Jenkins.get().checkPermission(getRequiredPermission());
+    }
+
+    /**
+     * Checks if the current user has the minimum required permission to view this administrative monitor.
+     * <p>
+     * Subclasses may override this method and {@link #checkRequiredPermission} instead of {@link #getRequiredPermission()} to perform more complex permission checks,
+     * for example, checking either {@link Jenkins#MANAGE} or {@link Jenkins#SYSTEM_READ}.
+     * </p>
+     * @see #getRequiredPermission()
+     * @see #checkRequiredPermission()
+     */
+    public boolean hasRequiredPermission() {
+        return Jenkins.get().hasPermission(getRequiredPermission());
+    }
+
+    /**
+     * Checks if the current user has the minimum required permission to view any administrative monitor.
+     *
+     * @return true if the current user has the minimum required permission to view any administrative monitor.
+     *
+     * @since TODO
+     */
+    public static boolean hasPermissionToDisplay() {
+        return Jenkins.get().hasAnyPermission(Jenkins.SYSTEM_READ, Jenkins.MANAGE);
+    }
+
     /**
      * Ensure that URLs in this administrative monitor are only accessible to users with {@link #getRequiredPermission()}.
      */
     @Override
     @Restricted(NoExternalUse.class)
     public Object getTarget() {
-        Jenkins.get().checkPermission(getRequiredPermission());
+        checkRequiredPermission();
         return this;
     }
 

core/src/main/java/jenkins/management/AdministrativeMonitorsDecorator.java

@@ -139,7 +139,7 @@ private Collection<AdministrativeMonitor> getAllActiveAdministrativeMonitors() {
      * @return the list of active monitors if we should display them, otherwise null.
      */
     public Collection<AdministrativeMonitor> getMonitorsToDisplay() {
-        if (!Jenkins.get().hasPermission(Jenkins.SYSTEM_READ)) {
+        if (!(AdministrativeMonitor.hasPermissionToDisplay())) {
             return null;
         }
 

core/src/main/java/jenkins/model/Jenkins.java

@@ -2355,12 +2355,12 @@ public AdministrativeMonitor getAdministrativeMonitor(String id) {
      * @since 2.64
      */
     public List<AdministrativeMonitor> getActiveAdministrativeMonitors() {
-        if (!Jenkins.get().hasPermission(SYSTEM_READ)) {
+        if (!AdministrativeMonitor.hasPermissionToDisplay()) {
             return Collections.emptyList();
         }
         return administrativeMonitors.stream().filter(m -> {
             try {
-                return Jenkins.get().hasPermission(m.getRequiredPermission()) && m.isEnabled() && m.isActivated();
+                return m.hasRequiredPermission() && m.isEnabled() && m.isActivated();
             } catch (Throwable x) {
                 LOGGER.log(Level.WARNING, null, x);
                 return false;

test/src/test/java/jenkins/management/AdministrativeMonitorsDecoratorTest.java

@@ -24,15 +24,22 @@
 
 package jenkins.management;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 
 import hudson.ExtensionList;
 import hudson.model.AdministrativeMonitor;
 import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.Permission;
 import jenkins.model.Jenkins;
 import org.jenkinsci.Symbol;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.FlagRule;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.MockAuthorizationStrategy;
@@ -43,6 +50,9 @@ public class AdministrativeMonitorsDecoratorTest {
     @Rule
     public JenkinsRule j = new JenkinsRule();
 
+    @Rule
+    public final FlagRule<String> managePermissionRule = FlagRule.systemProperty("jenkins.security.ManagePermission", "true");
+
     @Test
     public void ensureAdminMonitorsAreNotRunPerNonAdminPage() throws Exception {
         j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
@@ -131,4 +141,80 @@ public boolean isSecurity() {
             return true;
         }
     }
+
+    @Test
+    public void ensureAdminMonitorsCanBeSeenByManagers() {
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        var managerLogin = "manager";
+        var systemReadLogin = "system-reader";
+        var managerUser = User.getById(managerLogin, true);
+        var systemReadUser = User.getById(systemReadLogin, true);
+
+        j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
+                .grant(Jenkins.MANAGE, Jenkins.READ).everywhere().to(managerLogin)
+                .grant(Jenkins.READ, Jenkins.SYSTEM_READ).everywhere().to(systemReadLogin)
+        );
+
+        try (var ignored = ACL.as2(managerUser.impersonate2())) {
+            assertThat(Jenkins.get().getActiveAdministrativeMonitors(), hasItem(instanceOf(ManagerAdministrativeMonitor.class)));
+        }
+        try (var ignored = ACL.as2(systemReadUser.impersonate2())) {
+            assertThat(Jenkins.get().getActiveAdministrativeMonitors(), not(hasItem(instanceOf(ManagerAdministrativeMonitor.class))));
+        }
+    }
+
+    @TestExtension("ensureAdminMonitorsCanBeSeenByManagers")
+    public static class ManagerAdministrativeMonitor extends AdministrativeMonitor {
+        @Override
+        public Permission getRequiredPermission() {
+            return Jenkins.MANAGE;
+        }
+
+        @Override
+        public boolean isActivated() {
+            return true;
+        }
+    }
+
+    @Test
+    public void ensureAdminMonitorsCanBeSeenByManagersOrSystemReaders() {
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        var managerLogin = "manager";
+        var systemReadLogin = "system-reader";
+        var managerUser = User.getById(managerLogin, true);
+        var systemReadUser = User.getById(systemReadLogin, true);
+
+        j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
+                .grant(Jenkins.MANAGE, Jenkins.READ).everywhere().to(managerLogin)
+                .grant(Jenkins.READ, Jenkins.SYSTEM_READ).everywhere().to(systemReadLogin)
+        );
+
+        try (var ignored = ACL.as2(managerUser.impersonate2())) {
+            assertThat(Jenkins.get().getActiveAdministrativeMonitors(), hasItem(instanceOf(ManagerOrSystemReaderAdministrativeMonitor.class)));
+        }
+        try (var ignored = ACL.as2(systemReadUser.impersonate2())) {
+            assertThat(Jenkins.get().getActiveAdministrativeMonitors(), hasItem(instanceOf(ManagerOrSystemReaderAdministrativeMonitor.class)));
+        }
+    }
+
+    @TestExtension("ensureAdminMonitorsCanBeSeenByManagersOrSystemReaders")
+    public static class ManagerOrSystemReaderAdministrativeMonitor extends AdministrativeMonitor {
+
+        private static final Permission[] REQUIRED_ANY_PERMISSIONS = {Jenkins.MANAGE, Jenkins.SYSTEM_READ};
+
+        @Override
+        public void checkRequiredPermission() {
+            Jenkins.get().checkAnyPermission(REQUIRED_ANY_PERMISSIONS);
+        }
+
+        @Override
+        public boolean hasRequiredPermission() {
+            return Jenkins.get().hasAnyPermission(REQUIRED_ANY_PERMISSIONS);
+        }
+
+        @Override
+        public boolean isActivated() {
+            return true;
+        }
+    }
 }