Merge pull request #24 from daniel-beck/prevent-mistake

[JENKINS-46832] Prevent initial misconfiguration by making sure there's an admin
jenkinsci · Sep 14, 2017 · b53384b · b53384b
2 parents 4e62ab9 + 2533a68
commit b53384b
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 0 deletions.
diff --git a/src/main/java/hudson/security/GlobalMatrixAuthorizationStrategy.java b/src/main/java/hudson/security/GlobalMatrixAuthorizationStrategy.java
@@ -384,6 +384,9 @@ public String getDisplayName() {
         public AuthorizationStrategy newInstance(StaplerRequest req, JSONObject formData) throws FormException {
             GlobalMatrixAuthorizationStrategy gmas = create();
             Map<String,Object> data = formData.getJSONObject("data");
+
+            boolean adminAdded = false;
+
             for(Map.Entry<String,Object> r : data.entrySet()) {
                 String sid = r.getKey();
                 if (!(r.getValue() instanceof JSONObject)) {
@@ -399,11 +402,26 @@ public AuthorizationStrategy newInstance(StaplerRequest req, JSONObject formData
                         if (p == null) {
                             LOGGER.log(Level.FINE, "Silently skip unknown permission \"{0}\" for sid:\"{1}\"", new Object[]{e.getKey(), sid});
                         } else {
+                            if (p == Jenkins.ADMINISTER) {
+                                adminAdded = true;
+                            }
                             gmas.add(p, sid);
                         }
                     }
                 }
             }
+
+            if (!adminAdded) {
+                User current = User.current();
+                String id;
+                if (current == null) {
+                    id = "anonymous";
+                } else {
+                    id = current.getId();
+                }
+                gmas.add(Jenkins.ADMINISTER, id);
+            }
+
             return gmas;
         }
 

diff --git a/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java b/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java
@@ -0,0 +1,75 @@
+package hudson.security;
+
+import com.gargoylesoftware.htmlunit.html.HtmlElement;
+import com.gargoylesoftware.htmlunit.html.HtmlForm;
+import com.gargoylesoftware.htmlunit.html.HtmlLabel;
+import hudson.model.User;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public class ProjectMatrixAuthorizationStrategyTest {
+
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Test
+    public void submitEmptyPropertyEnsuresPermissionsForSubmitter() throws Exception {
+        HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false);
+        realm.createAccount("alice","alice");
+        realm.createAccount("bob","bob");
+        r.jenkins.setSecurityRealm(realm);
+
+        r.jenkins.setAuthorizationStrategy(new FullControlOnceLoggedInAuthorizationStrategy());
+
+        // ensure logged in users are admins, but anon is not
+        try (ACLContext _ = ACL.as(User.get("alice"))) {
+            Assert.assertTrue("alice is admin", r.jenkins.hasPermission(Jenkins.ADMINISTER));
+        }
+        try (ACLContext _ = ACL.as(User.get("bob"))) {
+            Assert.assertTrue("bob is admin", r.jenkins.hasPermission(Jenkins.ADMINISTER));
+        }
+        Assert.assertFalse("anon is not admin", r.jenkins.getACL().hasPermission(Jenkins.ANONYMOUS, Jenkins.ADMINISTER));
+
+        JenkinsRule.WebClient wc = r.createWebClient().login("alice");
+        HtmlForm form = wc.goTo("configureSecurity").getFormByName("config");
+
+        // TODO this must be possible in a nicer way
+        HtmlElement label = form.getElementsByTagName("label").stream().filter(
+                lbl -> lbl.asText().contains(GlobalMatrixAuthorizationStrategy.DESCRIPTOR.getDisplayName())).findAny().get();
+        ((HtmlLabel)label).click();
+        r.submit(form);
+
+        try (ACLContext _ = ACL.as(User.get("alice"))) {
+            // ensure that the user submitting the empty matrix will be admin
+            Assert.assertTrue("alice is admin", r.jenkins.hasPermission(Jenkins.ADMINISTER));
+        }
+        try (ACLContext _ = ACL.as(User.get("bob"))) {
+            Assert.assertFalse("bob is not admin", r.jenkins.hasPermission(Jenkins.ADMINISTER));
+        }
+        Assert.assertFalse("anon is not admin", r.jenkins.getACL().hasPermission(Jenkins.ANONYMOUS, Jenkins.ADMINISTER));
+    }
+
+    @Test
+    public void submitEmptyPropertyEnsuresPermissionsForAnonymousSubmitter() throws Exception {
+        // prepare form to have options visible
+        r.jenkins.setSecurityRealm(new HudsonPrivateSecurityRealm(true));
+        r.jenkins.setAuthorizationStrategy(new AuthorizationStrategy.Unsecured());
+
+        Assert.assertTrue("anon is admin", r.jenkins.getACL().hasPermission(Jenkins.ANONYMOUS, Jenkins.ADMINISTER));
+
+        JenkinsRule.WebClient wc = r.createWebClient();
+        HtmlForm form = wc.goTo("configureSecurity").getFormByName("config");
+
+        // TODO this must be possible in a nicer way
+        HtmlElement label = form.getElementsByTagName("label").stream().filter(
+                lbl -> lbl.asText().contains(GlobalMatrixAuthorizationStrategy.DESCRIPTOR.getDisplayName())).findAny().get();
+        ((HtmlLabel)label).click();
+        r.submit(form);
+
+        Assert.assertTrue("anon is admin", r.jenkins.getACL().hasPermission(Jenkins.ANONYMOUS, Jenkins.ADMINISTER));
+        Assert.assertTrue(r.jenkins.getAuthorizationStrategy() instanceof GlobalMatrixAuthorizationStrategy);
+    }
+}