JENKINS-61208 Allow system read to view more admin monitors (#4685)

jenkinsci · May 1, 2020 · 1837293 · 1837293
1 parent 05dd409
commit 1837293
Show file tree

Hide file tree

Showing 10 changed files with 135 additions and 17 deletions.
diff --git a/core/src/main/java/hudson/diagnosis/ReverseProxySetupMonitor.java b/core/src/main/java/hudson/diagnosis/ReverseProxySetupMonitor.java
@@ -27,6 +27,7 @@
 import hudson.RestrictedSince;
 import hudson.Util;
 import hudson.model.AdministrativeMonitor;
+import hudson.security.Permission;
 import jenkins.security.stapler.StaplerDispatchable;
 import org.jenkinsci.Symbol;
 import org.kohsuke.accmod.Restricted;
@@ -103,6 +104,11 @@ public void getTestForReverseProxySetup(String rest) {
         }
     }
 
+    @Override
+    public Permission getRequiredPermission() {
+        return Jenkins.SYSTEM_READ;
+    }
+
     /**
      * Depending on whether the user said "yes" or "no", send him to the right place.
      */
@@ -111,6 +117,7 @@ public void getTestForReverseProxySetup(String rest) {
     @RequirePOST
     public HttpResponse doAct(@QueryParameter String no) throws IOException {
         if(no!=null) { // dismiss
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
             disable(true);
             // of course the irony is that this redirect won't work
             return HttpResponses.redirectViaContextPath("/manage");

diff --git a/core/src/main/java/hudson/diagnosis/TooManyJobsButNoView.java b/core/src/main/java/hudson/diagnosis/TooManyJobsButNoView.java
@@ -24,6 +24,7 @@
 package hudson.diagnosis;
 
 import hudson.model.AdministrativeMonitor;
+import hudson.security.Permission;
 import jenkins.model.Jenkins;
 import hudson.Extension;
 import org.jenkinsci.Symbol;
@@ -50,15 +51,20 @@ public String getDisplayName() {
     }
 
     public boolean isActivated() {
-        Jenkins h = Jenkins.get();
-        return h.getViews().size()==1 && h.getItemMap().size()> THRESHOLD;
+        Jenkins j = Jenkins.get();
+        if (j.hasPermission(Jenkins.ADMINISTER)) {
+            return j.getViews().size() == 1 && j.getItemMap().size() > THRESHOLD;
+        }
+        // SystemRead
+        return j.getViews().size() == 1 && j.getItems().size() > THRESHOLD;
     }
 
     /**
      * Depending on whether the user said "yes" or "no", send him to the right place.
      */
     @RequirePOST
     public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException {
+        Jenkins.get().checkPermission(Jenkins.ADMINISTER);
         if(req.hasParameter("no")) {
             disable(true);
             rsp.sendRedirect(req.getContextPath()+"/manage");
@@ -67,5 +73,10 @@ public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException {
         }
     }
 
+    @Override
+    public Permission getRequiredPermission() {
+        return Jenkins.SYSTEM_READ;
+    }
+
     public static final int THRESHOLD = 16;
 }
diff --git a/core/src/main/java/hudson/model/UpdateCenter.java b/core/src/main/java/hudson/model/UpdateCenter.java
@@ -32,6 +32,7 @@
 import hudson.PluginWrapper;
 import hudson.ProxyConfiguration;
 import hudson.security.ACLContext;
+import hudson.security.Permission;
 import hudson.util.VersionNumber;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
@@ -1111,6 +1112,11 @@ public Data getData() {
             if (cs!=null)   return cs.getData();
             return null;
         }
+
+        @Override
+        public Permission getRequiredPermission() {
+            return Jenkins.SYSTEM_READ;
+        }
     }
 
 

diff --git a/core/src/main/java/jenkins/security/ResourceDomainRecommendation.java b/core/src/main/java/jenkins/security/ResourceDomainRecommendation.java
@@ -26,7 +26,9 @@
 import hudson.Extension;
 import hudson.model.AdministrativeMonitor;
 import hudson.model.DirectoryBrowserSupport;
+import hudson.security.Permission;
 import hudson.util.HttpResponses;
+import jenkins.model.Jenkins;
 import jenkins.util.SystemProperties;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -63,6 +65,7 @@ public boolean isActivated() {
 
     @RequirePOST
     public HttpResponse doAct(@QueryParameter String redirect, @QueryParameter String dismiss) throws IOException {
+        Jenkins.get().checkPermission(Jenkins.ADMINISTER);
         if (dismiss != null) {
             disable(true);
             return HttpResponses.redirectViaContextPath("manage");
@@ -72,4 +75,9 @@ public HttpResponse doAct(@QueryParameter String redirect, @QueryParameter Strin
         }
         return HttpResponses.forwardToPreviousPage();
     }
+
+    @Override
+    public Permission getRequiredPermission() {
+        return Jenkins.SYSTEM_READ;
+    }
 }
diff --git a/core/src/main/java/jenkins/security/UpdateSiteWarningsMonitor.java b/core/src/main/java/jenkins/security/UpdateSiteWarningsMonitor.java
@@ -29,6 +29,7 @@
 import hudson.PluginWrapper;
 import hudson.model.AdministrativeMonitor;
 import hudson.model.UpdateSite;
+import hudson.security.Permission;
 import hudson.util.HttpResponses;
 import jenkins.model.Jenkins;
 import org.kohsuke.accmod.Restricted;
@@ -141,6 +142,7 @@ private Set<UpdateSite.Warning> getActiveWarnings() {
      */
     @RequirePOST
     public HttpResponse doForward(@QueryParameter String fix, @QueryParameter String configure) {
+        Jenkins.get().checkPermission(Jenkins.ADMINISTER);
         if (fix != null) {
             return HttpResponses.redirectViaContextPath("pluginManager");
         }
@@ -162,6 +164,11 @@ public boolean hasApplicableHiddenWarnings() {
         return getActiveWarnings().size() < configuration.getApplicableWarnings().size();
     }
 
+    @Override
+    public Permission getRequiredPermission() {
+        return Jenkins.SYSTEM_READ;
+    }
+
     @Override
     public String getDisplayName() {
         return Messages.UpdateSiteWarningsMonitor_DisplayName();

diff --git a/core/src/main/resources/hudson/diagnosis/ReverseProxySetupMonitor/message.jelly b/core/src/main/resources/hudson/diagnosis/ReverseProxySetupMonitor/message.jelly
@@ -22,12 +22,14 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 THE SOFTWARE.
 -->
 <?jelly escape-by-default='true'?>
-<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form">
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form" xmlns:l="/lib/layout">
   <div id="redirect-error" class="alert alert-danger reverse-proxy__hidden"
        data-url="${rootURL}/${it.url}/test" data-context="${rootURL}">
     <form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
       <f:submit name="yes" value="${%More Info}"/>
-      <f:submit name="no" value="${%Dismiss}"/>
+      <l:isAdmin>
+        <f:submit name="no" value="${%Dismiss}"/>
+      </l:isAdmin>
     </form>
     <div>${%blurb}</div>
     <div class="js-context-message reverse-proxy__hidden">${%missingContextMessage(rootURL)}</div>

diff --git a/core/src/main/resources/hudson/diagnosis/TooManyJobsButNoView/message.jelly b/core/src/main/resources/hudson/diagnosis/TooManyJobsButNoView/message.jelly
@@ -24,11 +24,13 @@ THE SOFTWARE.
 
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
-  <div class="alert alert-warning">
-    <form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
-      <f:submit name="yes" value="${%Create a view now}"/>
-      <f:submit name="no" value="${%Dismiss}"/>
-    </form>
+  <div id="tooManyJobsButNoView" class="alert alert-warning">
+    <l:isAdmin>
+      <form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
+        <f:submit name="yes" value="${%Create a view now}"/>
+        <f:submit name="no" value="${%Dismiss}"/>
+      </form>
+    </l:isAdmin>
     ${%blurb}
   </div>
 </j:jelly>
diff --git a/core/src/main/resources/jenkins/security/ResourceDomainRecommendation/message.groovy b/core/src/main/resources/jenkins/security/ResourceDomainRecommendation/message.groovy
@@ -24,13 +24,17 @@
 package jenkins.security.ResourceDomainRecommendation
 
 def f = namespace(lib.FormTagLib)
+def l = namespace(lib.LayoutTagLib)
 
 dl {
     div(class: "alert alert-info") {
         a(name: "resource-root-url")
-        form(method: "post", action: "${rootURL}/${my.url}/act") {
-            f.submit(name: 'redirect', value: _("Go to resource root URL configuration"))
-            f.submit(name: 'dismiss', value: _("Dismiss"))
+
+        l.isAdmin() {
+            form(method: "post", action: "${rootURL}/${my.url}/act") {
+                f.submit(name: 'redirect', value: _("Go to resource root URL configuration"))
+                f.submit(name: 'dismiss', value: _("Dismiss"))
+            }
         }
 
         raw(_("blurb"))

diff --git a/core/src/main/resources/jenkins/security/UpdateSiteWarningsMonitor/message.groovy b/core/src/main/resources/jenkins/security/UpdateSiteWarningsMonitor/message.groovy
@@ -25,6 +25,7 @@
 package jenkins.security.UpdateSiteWarningsMonitor
 
 def f = namespace(lib.FormTagLib)
+def l = namespace(lib.LayoutTagLib)
 
 def listWarnings(warnings) {
     warnings.each { warning ->
@@ -39,11 +40,13 @@ def pluginWarnings = my.activePluginWarningsByPlugin
 
 div(class: "alert alert-danger", role: "alert") {
 
-    form(method: "post", action: "${rootURL}/${my.url}/forward") {
-        if (!pluginWarnings.isEmpty()) {
-            f.submit(name: 'fix', value: _("pluginManager.link"))
+    l.isAdmin() {
+        form(method: "post", action: "${rootURL}/${my.url}/forward") {
+            if (!pluginWarnings.isEmpty()) {
+                f.submit(name: 'fix', value: _("pluginManager.link"))
+            }
+            f.submit(name: 'configure', value: _("configureSecurity.link"))
         }
-        f.submit(name: 'configure', value: _("configureSecurity.link"))
     }
 
     text(_("blurb"))

diff --git a/test/src/test/java/hudson/diagnosis/TooManyJobsButNoViewTest.java b/test/src/test/java/hudson/diagnosis/TooManyJobsButNoViewTest.java
@@ -1,17 +1,31 @@
 package hudson.diagnosis;
 
 import com.gargoylesoftware.htmlunit.ElementNotFoundException;
+import com.gargoylesoftware.htmlunit.html.DomElement;
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import hudson.model.AdministrativeMonitor;
+import hudson.model.Item;
 import hudson.model.ListView;
+import hudson.model.View;
 import java.io.IOException;
 import java.net.URL;
-import static org.junit.Assert.*;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import jenkins.model.Jenkins;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
 import org.xml.sax.SAXException;
 
 /**
@@ -67,4 +81,58 @@ private void verifyNoForm() throws IOException, SAXException {
 
         verifyNoForm();
     }
+
+    @Test
+    public void systemReadNoViewAccessVerifyNoForm() throws Exception {
+        final String READONLY = "readonly";
+
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
+                .grant(Jenkins.READ).everywhere().to(READONLY)
+                .grant(Jenkins.SYSTEM_READ).everywhere().to(READONLY)
+        );
+
+        for (int i = 0; i <= TooManyJobsButNoView.THRESHOLD; i++)
+            r.createFreeStyleProject();
+
+        JenkinsRule.WebClient wc = r.createWebClient();
+        wc.login(READONLY);
+
+        verifyNoMonitor(wc);
+    }
+
+    private void verifyNoMonitor(JenkinsRule.WebClient wc) throws IOException, SAXException {
+        HtmlPage p = wc.goTo("manage");
+        DomElement adminMonitorDiv = p.getElementById("tooManyJobsButNoView");
+        assertThat(adminMonitorDiv, is(nullValue()));
+    }
+
+    @Test
+    public void systemReadVerifyForm() throws Exception {
+        final String READONLY = "readonly";
+
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
+                .grant(Jenkins.READ).everywhere().to(READONLY)
+                .grant(Jenkins.SYSTEM_READ).everywhere().to(READONLY)
+                .grant(Item.READ).everywhere().to(READONLY)
+                .grant(View.READ).everywhere().to(READONLY)
+        );
+
+        for (int i = 0; i <= TooManyJobsButNoView.THRESHOLD; i++)
+            r.createFreeStyleProject();
+
+        JenkinsRule.WebClient wc = r.createWebClient();
+        wc.login(READONLY);
+
+        verifyMonitor(wc);
+    }
+
+    private void verifyMonitor(JenkinsRule.WebClient wc) throws IOException, SAXException {
+        HtmlPage p = wc.goTo("manage");
+        DomElement adminMonitorDiv = p.getElementById("tooManyJobsButNoView");
+        assertThat(adminMonitorDiv, is(notNullValue()));
+        assertThat(adminMonitorDiv.getTextContent(), is(notNullValue()));
+    }
+
 }