Merge pull request #86 from jenkinsci-cert/SECURITY-380

[FIX SECURITY-380] Don't optimize permission check in getItems()
jenkinsci · Dec 20, 2016 · 09cfbc9 · 09cfbc9
2 parents 8ebf09f + 8de0056
commit 09cfbc9
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 6 deletions.
diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -136,7 +136,6 @@
 import hudson.security.AuthorizationStrategy;
 import hudson.security.BasicAuthenticationFilter;
 import hudson.security.FederatedLoginService;
-import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
 import hudson.security.HudsonFilter;
 import hudson.security.LegacyAuthorizationStrategy;
 import hudson.security.LegacySecurityRealm;
@@ -1479,11 +1478,6 @@ public List<Action> getActions() {
      */
     @Exported(name="jobs")
     public List<TopLevelItem> getItems() {
-        if (authorizationStrategy instanceof AuthorizationStrategy.Unsecured ||
-            authorizationStrategy instanceof FullControlOnceLoggedInAuthorizationStrategy) {
-            return new ArrayList(items.values());
-        }
-
         List<TopLevelItem> viewableItems = new ArrayList<TopLevelItem>();
         for (TopLevelItem item : items.values()) {
             if (item.hasPermission(Item.READ))

diff --git a/test/src/test/java/jenkins/security/Security380Test.java b/test/src/test/java/jenkins/security/Security380Test.java
@@ -0,0 +1,95 @@
+package jenkins.security;
+
+import com.gargoylesoftware.htmlunit.Page;
+import hudson.model.UnprotectedRootAction;
+import hudson.security.ACL;
+import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
+import hudson.util.HttpResponses;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.HttpResponse;
+
+public class Security380Test {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Issue("SECURITY-380")
+    @Test
+    public void testGetItemsWithoutAnonRead() throws Exception {
+        FullControlOnceLoggedInAuthorizationStrategy strategy = new FullControlOnceLoggedInAuthorizationStrategy();
+        strategy.setAllowAnonymousRead(false);
+        Jenkins.getInstance().setAuthorizationStrategy(strategy);
+
+        Jenkins.getInstance().setSecurityRealm(j.createDummySecurityRealm());
+
+        j.createFreeStyleProject();
+        ACL.impersonate(Jenkins.ANONYMOUS, new Runnable() {
+            @Override
+            public void run() {
+                Assert.assertEquals("no items", 0, Jenkins.getInstance().getItems().size());
+            }
+        });
+    }
+
+    @Issue("SECURITY-380")
+    @Test
+    public void testGetItems() throws Exception {
+        FullControlOnceLoggedInAuthorizationStrategy strategy = new FullControlOnceLoggedInAuthorizationStrategy();
+        strategy.setAllowAnonymousRead(true);
+        Jenkins.getInstance().setAuthorizationStrategy(strategy);
+
+        Jenkins.getInstance().setSecurityRealm(j.createDummySecurityRealm());
+
+        j.createFreeStyleProject();
+        ACL.impersonate(Jenkins.ANONYMOUS, new Runnable() {
+            @Override
+            public void run() {
+                Assert.assertEquals("one item", 1, Jenkins.getInstance().getItems().size());
+            }
+        });
+    }
+
+    @Issue("SECURITY-380")
+    @Test
+    public void testWithUnprotectedRootAction() throws Exception {
+        FullControlOnceLoggedInAuthorizationStrategy strategy = new FullControlOnceLoggedInAuthorizationStrategy();
+        strategy.setAllowAnonymousRead(false);
+        Jenkins.getInstance().setAuthorizationStrategy(strategy);
+
+        Jenkins.getInstance().setSecurityRealm(j.createDummySecurityRealm());
+        j.createFreeStyleProject();
+
+        JenkinsRule.WebClient wc = j.createWebClient();
+        Page page = wc.goTo("listJobs", "text/plain");
+        Assert.assertEquals("expect 0 items", "0", page.getWebResponse().getContentAsString().trim());
+    }
+
+    @TestExtension
+    public static class JobListingUnprotectedRootAction implements UnprotectedRootAction {
+
+        @Override
+        public String getIconFileName() {
+            return null;
+        }
+
+        @Override
+        public String getDisplayName() {
+            return null;
+        }
+
+        @Override
+        public String getUrlName() {
+            return "listJobs";
+        }
+
+        public HttpResponse doIndex() throws Exception {
+            return HttpResponses.plainText(Integer.toString(Jenkins.getInstance().getItems().size()));
+        }
+    }
+}