diff --git a/core/src/main/java/hudson/model/AdministrativeMonitor.java b/core/src/main/java/hudson/model/AdministrativeMonitor.java
index 7a9a0dde970c..e2f69d654ca8 100644
--- a/core/src/main/java/hudson/model/AdministrativeMonitor.java
+++ b/core/src/main/java/hudson/model/AdministrativeMonitor.java
@@ -183,7 +183,7 @@ 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.
*
* 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
@@ -191,6 +191,10 @@ public void doDisable(StaplerRequest req, StaplerResponse rsp) throws IOExceptio
* {@link #doDisable(StaplerRequest, StaplerResponse)} will still always require Administer permission.
*
*
+ * 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.
+ *
+ *
* 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.
+ *
+ * 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}.
+ *
+ * @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.
+ *
+ * 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}.
+ *
+ * @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;
}
diff --git a/core/src/main/java/jenkins/management/AdministrativeMonitorsDecorator.java b/core/src/main/java/jenkins/management/AdministrativeMonitorsDecorator.java
index 21cc0ba1df0c..5b94090f4e3b 100644
--- a/core/src/main/java/jenkins/management/AdministrativeMonitorsDecorator.java
+++ b/core/src/main/java/jenkins/management/AdministrativeMonitorsDecorator.java
@@ -139,7 +139,7 @@ private Collection getAllActiveAdministrativeMonitors() {
* @return the list of active monitors if we should display them, otherwise null.
*/
public Collection getMonitorsToDisplay() {
- if (!Jenkins.get().hasPermission(Jenkins.SYSTEM_READ)) {
+ if (!(AdministrativeMonitor.hasPermissionToDisplay())) {
return null;
}
diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
index 4782e4b321f2..a3376f99d75e 100644
--- a/core/src/main/java/jenkins/model/Jenkins.java
+++ b/core/src/main/java/jenkins/model/Jenkins.java
@@ -2355,12 +2355,12 @@ public AdministrativeMonitor getAdministrativeMonitor(String id) {
* @since 2.64
*/
public List 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;
diff --git a/test/src/test/java/jenkins/management/AdministrativeMonitorsDecoratorTest.java b/test/src/test/java/jenkins/management/AdministrativeMonitorsDecoratorTest.java
index 37d28638e65b..691b9d2c8868 100644
--- a/test/src/test/java/jenkins/management/AdministrativeMonitorsDecoratorTest.java
+++ b/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 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;
+ }
+ }
}