Skip to content

Commit

Permalink
[FIXED JENKINS-22262] CopyJobCommand and CreateJobCommand were doing …
Browse files Browse the repository at this point in the history
…incorrect permission checks.
  • Loading branch information
jglick committed Mar 19, 2014
1 parent 8078894 commit 8861563
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 9 deletions.
3 changes: 3 additions & 0 deletions changelog.html
Expand Up @@ -55,6 +55,9 @@
<!-- 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>)
Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/hudson/cli/CopyJobCommand.java
Expand Up @@ -51,7 +51,6 @@ public String getShortDescription() {

protected int run() throws Exception {
Jenkins jenkins = Jenkins.getInstance();
jenkins.checkPermission(Item.CREATE);

if (jenkins.getItemByFullName(dst)!=null) {
stderr.println("Job '"+dst+"' already exists");
Expand Down
3 changes: 0 additions & 3 deletions core/src/main/java/hudson/cli/CreateJobCommand.java
Expand Up @@ -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;
Expand All @@ -48,7 +46,6 @@ public String getShortDescription() {

protected int run() throws Exception {
Jenkins h = Jenkins.getInstance();
h.checkPermission(Item.CREATE);

if (h.getItemByFullName(name)!=null) {
stderr.println("Job '"+name+"' already exists");
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/hudson/model/ItemGroupMixIn.java
Expand Up @@ -206,6 +206,7 @@ protected String redirectAfterCreateItem(StaplerRequest req, TopLevelItem result
@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);

Expand Down Expand Up @@ -235,6 +236,7 @@ public synchronized TopLevelItem createProjectFromXML(String name, InputStream x
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();
Expand Down Expand Up @@ -269,6 +271,7 @@ public synchronized TopLevelItem createProject( TopLevelItemDescriptor type, Str
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 {
Expand Down
84 changes: 79 additions & 5 deletions test/src/test/java/hudson/cli/CopyJobCommandTest.java
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
87 changes: 87 additions & 0 deletions test/src/test/java/hudson/cli/CreateJobCommandTest.java
@@ -0,0 +1,87 @@
/*
* 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"));
}

}

0 comments on commit 8861563

Please sign in to comment.