[FIXED JENKINS-22262] CopyJobCommand and CreateJobCommand were doing incorrect permission checks.

changelog.html

@@ -55,6 +55,9 @@ Upcoming changes</a>
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
+  <li class=bug>
+    Corrected permission checks for <code>copy-job</code> and <code>create-job</code> CLI commands.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-22262">issue 22262</a>)
   <li class=rfe>
     Ability for custom view types to disable automatic refresh.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-21190">issue 21190</a>)

core/src/main/java/hudson/cli/CopyJobCommand.java

@@ -51,7 +51,6 @@ public class CopyJobCommand extends CLICommand {
 
     protected int run() throws Exception {
         Jenkins jenkins = Jenkins.getInstance();
-        jenkins.checkPermission(Item.CREATE);
 
         if (jenkins.getItemByFullName(dst)!=null) {
             stderr.println("Job '"+dst+"' already exists");

core/src/main/java/hudson/cli/CreateJobCommand.java

@@ -23,8 +23,6 @@
  */
 package hudson.cli;
 
-import hudson.model.ModifiableItemGroup;
-import hudson.model.TopLevelItem;
 import jenkins.model.Jenkins;
 import hudson.Extension;
 import hudson.model.Item;
@@ -48,7 +46,6 @@ public class CreateJobCommand extends CLICommand {
 
     protected int run() throws Exception {
         Jenkins h = Jenkins.getInstance();
-        h.checkPermission(Item.CREATE);
 
         if (h.getItemByFullName(name)!=null) {
             stderr.println("Job '"+name+"' already exists");

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

@@ -206,6 +206,7 @@ public abstract class ItemGroupMixIn {
     @SuppressWarnings({"unchecked"})
     public synchronized <T extends TopLevelItem> T copy(T src, String name) throws IOException {
         acl.checkPermission(Item.CREATE);
+        src.checkPermission(Item.EXTENDED_READ);
 
         T result = (T)createProject(src.getDescriptor(),name,false);
 
@@ -235,6 +236,7 @@ public abstract class ItemGroupMixIn {
         if (parent.getItem(name) != null) {
             throw new IllegalArgumentException(parent.getDisplayName() + " already contains an item '" + name + "'");
         }
+        // TODO what if we have no DISCOVER permission on the existing job?
 
         // place it as config.xml
         File configXml = Items.getConfigFile(getRootDirFor(name)).getFile();
@@ -269,6 +271,7 @@ public abstract class ItemGroupMixIn {
         Jenkins.getInstance().getProjectNamingStrategy().checkName(name);
         if(parent.getItem(name)!=null)
             throw new IllegalArgumentException("Project of the name "+name+" already exists");
+        // TODO problem with DISCOVER as noted above
 
         TopLevelItem item = type.newInstance(parent, name);
         try {

test/src/test/java/hudson/cli/CopyJobCommandTest.java

@@ -25,26 +25,38 @@
 package hudson.cli;
 
 import edu.umd.cs.findbugs.annotations.SuppressWarnings;
+import static hudson.cli.CLICommandInvoker.Matcher.*;
+import hudson.model.AbstractItem;
 import hudson.model.FreeStyleProject;
+import hudson.model.Item;
+import hudson.model.Job;
+import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.AuthorizationStrategy;
+import hudson.security.SparseACL;
+import java.util.Collection;
+import java.util.Collections;
+import jenkins.model.Jenkins;
+import org.acegisecurity.acls.sid.PrincipalSid;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.MockFolder;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-
-import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently;
-
 @SuppressWarnings("DM_DEFAULT_ENCODING")
 public class CopyJobCommandTest {
 
     @Rule public JenkinsRule j = new JenkinsRule();
+    private CLICommand copyJobCommand;
     private CLICommandInvoker command;
 
     @Before public void setUp() {
-        command = new CLICommandInvoker(j, new CopyJobCommand());
+        copyJobCommand = new CopyJobCommand();
+        command = new CLICommandInvoker(j, copyJobCommand);
     }
 
     @Test public void copyBetweenFolders() throws Exception {
@@ -60,6 +72,68 @@ public class CopyJobCommandTest {
         // TODO test copying from/to root, or into nonexistent folder
     }
 
+    @Bug(22262)
+    @Test public void folderPermissions() throws Exception {
+        final MockFolder d1 = j.createFolder("d1");
+        final FreeStyleProject p = d1.createProject(FreeStyleProject.class, "p");
+        final MockFolder d2 = j.createFolder("d2");
+        // alice has no real permissions. bob has READ on everything but no more. charlie has CREATE on d2 but not EXTENDED_READ on p. debbie has both.
+        final SparseACL rootACL = new SparseACL(null);
+        rootACL.add(new PrincipalSid("alice"), Jenkins.READ, true);
+        rootACL.add(new PrincipalSid("bob"), Jenkins.READ, true);
+        rootACL.add(new PrincipalSid("charlie"), Jenkins.READ, true);
+        rootACL.add(new PrincipalSid("debbie"), Jenkins.READ, true);
+        final SparseACL d1ACL = new SparseACL(null);
+        d1ACL.add(new PrincipalSid("bob"), Item.READ, true);
+        d1ACL.add(new PrincipalSid("charlie"), Item.READ, true);
+        d1ACL.add(new PrincipalSid("debbie"), Item.READ, true);
+        final SparseACL pACL = new SparseACL(null);
+        pACL.add(new PrincipalSid("bob"), Item.READ, true);
+        pACL.add(new PrincipalSid("charlie"), Item.READ, true);
+        pACL.add(new PrincipalSid("debbie"), Item.READ, true);
+        pACL.add(new PrincipalSid("debbie"), Item.EXTENDED_READ, true);
+        final SparseACL d2ACL = new SparseACL(null);
+        d2ACL.add(new PrincipalSid("bob"), Item.READ, true);
+        d2ACL.add(new PrincipalSid("charlie"), Item.READ, true);
+        d2ACL.add(new PrincipalSid("charlie"), Item.CREATE, true);
+        d2ACL.add(new PrincipalSid("debbie"), Item.READ, true);
+        d2ACL.add(new PrincipalSid("debbie"), Item.CREATE, true);
+        j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+        j.jenkins.setAuthorizationStrategy(new AuthorizationStrategy() {
+            @Override public ACL getRootACL() {
+                return rootACL;
+            }
+            @Override public ACL getACL(Job<?, ?> project) {
+                if (project == p) {
+                    return pACL;
+                } else {
+                    throw new AssertionError(project);
+                }
+            }
+            @Override public ACL getACL(AbstractItem item) {
+                if (item == d1) {
+                    return d1ACL;
+                } else if (item == d2) {
+                    return d2ACL;
+                } else {
+                    throw new AssertionError(item);
+                }
+            }
+            @Override public Collection<String> getGroups() {
+                return Collections.emptySet();
+            }
+        });
+        copyJobCommand.setTransportAuth(User.get("alice").impersonate());
+        assertThat(command.invokeWithArgs("d1/p", "d2/p"), failedWith(-1));
+        copyJobCommand.setTransportAuth(User.get("bob").impersonate());
+        assertThat(command.invokeWithArgs("d1/p", "d2/p"), failedWith(-1));
+        copyJobCommand.setTransportAuth(User.get("charlie").impersonate());
+        assertThat(command.invokeWithArgs("d1/p", "d2/p"), failedWith(-1));
+        copyJobCommand.setTransportAuth(User.get("debbie").impersonate());
+        assertThat(command.invokeWithArgs("d1/p", "d2/p"), succeededSilently());
+        assertNotNull(d2.getItem("p"));
+    }
+
     // hold off build until saved only makes sense on the UI with config screen shown after copying;
     // expect the CLI copy command to leave the job buildable
     @Test public void copiedJobIsBuildable() throws Exception {

test/src/test/java/hudson/cli/CreateJobCommandTest.java

+/*
+ * The MIT License
+ *
+ * Copyright 2014 Jesse Glick.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package hudson.cli;
+
+import static hudson.cli.CLICommandInvoker.Matcher.failedWith;
+import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently;
+import hudson.model.AbstractItem;
+import hudson.model.Item;
+import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.AuthorizationStrategy;
+import hudson.security.SparseACL;
+import java.io.ByteArrayInputStream;
+import java.util.Collection;
+import java.util.Collections;
+import jenkins.model.Jenkins;
+import org.acegisecurity.acls.sid.PrincipalSid;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.*;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockFolder;
+
+public class CreateJobCommandTest {
+
+    @Rule public JenkinsRule r = new JenkinsRule();
+
+    @Bug(22262)
+    @Test public void folderPermissions() throws Exception {
+        CLICommand cmd = new CreateJobCommand();
+        CLICommandInvoker invoker = new CLICommandInvoker(r, cmd);
+        final MockFolder d = r.createFolder("d");
+        final SparseACL rootACL = new SparseACL(null);
+        rootACL.add(new PrincipalSid("alice"), Jenkins.READ, true);
+        rootACL.add(new PrincipalSid("bob"), Jenkins.READ, true);
+        final SparseACL dACL = new SparseACL(null);
+        dACL.add(new PrincipalSid("alice"), Item.READ, true);
+        dACL.add(new PrincipalSid("bob"), Item.READ, true);
+        dACL.add(new PrincipalSid("bob"), Item.CREATE, true);
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new AuthorizationStrategy() {
+            @Override public ACL getRootACL() {
+                return rootACL;
+            }
+            @Override public ACL getACL(AbstractItem item) {
+                if (item == d) {
+                    return dACL;
+                } else {
+                    throw new AssertionError(item);
+                }
+            }
+            @Override public Collection<String> getGroups() {
+                return Collections.emptySet();
+            }
+        });
+        cmd.setTransportAuth(User.get("alice").impersonate());
+        assertThat(invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes("US-ASCII"))).invokeWithArgs("d/p"), failedWith(-1));
+        cmd.setTransportAuth(User.get("bob").impersonate());
+        assertThat(invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes("US-ASCII"))).invokeWithArgs("d/p"), succeededSilently());
+        assertNotNull(d.getItem("p"));
+    }
+
+}