Skip to content

Commit

Permalink
[JENKINS-16956] Make BuildTrigger.execute pay attention to build perm…
Browse files Browse the repository at this point in the history 
…issions, rather than checking the configuring user.
  • Loading branch information
@jglick
jglick committed Apr 2, 2014
1 parent 44a8ec1 commit 48e92ef
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 39 deletions.
14 changes: 0 additions & 14 deletions core/src/main/java/hudson/model/AbstractProject.java
View file
Expand Up @@ -1853,20 +1853,6 @@ protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOExceptio
triggers.replaceBy(buildDescribable(req, Trigger.for_(this)));
for (Trigger t : triggers())
t.start(this,true);

for (Publisher _t : Descriptor.newInstancesFromHeteroList(req, json, "publisher", Jenkins.getInstance().getExtensionList(BuildTrigger.DescriptorImpl.class))) {
BuildTrigger t = (BuildTrigger) _t;
List<AbstractProject> childProjects;
SecurityContext orig = ACL.impersonate(ACL.SYSTEM);
try {
childProjects = t.getChildProjects(this);
} finally {
SecurityContextHolder.setContext(orig);
}
for (AbstractProject downstream : childProjects) {
downstream.checkPermission(BUILD);
}
}
}

/**
Expand Down
37 changes: 25 additions & 12 deletions core/src/main/java/hudson/tasks/BuildTrigger.java
View file
Expand Up @@ -33,10 +33,8 @@
import hudson.model.AutoCompletionCandidates;
import hudson.model.BuildListener;
import hudson.model.Cause.UpstreamCause;
import jenkins.model.DependencyDeclarer;
import hudson.model.DependencyGraph;
import hudson.model.DependencyGraph.Dependency;
import jenkins.model.Jenkins;
import hudson.model.Item;
import hudson.model.ItemGroup;
import hudson.model.Items;
Expand All @@ -46,14 +44,8 @@
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.model.listeners.ItemListener;
import hudson.security.ACL;
import hudson.util.FormValidation;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;

import java.io.IOException;
import java.io.PrintStream;
import java.util.ArrayList;
Expand All @@ -65,6 +57,15 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import jenkins.model.DependencyDeclarer;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.acegisecurity.Authentication;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;

/**
* Triggers builds of other projects.
Expand Down Expand Up @@ -203,8 +204,22 @@ public int compare(Dependency lhs, Dependency rhs) {
}
});

Authentication auth = Jenkins.getAuthentication(); // from build
if (auth.equals(ACL.SYSTEM)) { // i.e., unspecified
auth = Jenkins.ANONYMOUS;
}

for (Dependency dep : downstreamProjects) {
AbstractProject p = dep.getDownstreamProject();
// TODO do we need to separately check READ on all parents?
// For example, by impersonating auth (if ANONYMOUS) and then checking Jenkins.instance.getItemByFullName(p.fullName) == p?
if (!p.getACL().hasPermission(auth, Item.READ)) {
continue; // do not even issue a warning (could do so if have DISCOVER)
}
if (!p.getACL().hasPermission(auth, Item.BUILD)) {
logger.println(Messages.BuildTrigger_you_have_no_permission_to_build_(ModelHyperlinkNote.encodeTo(p)));
continue;
}
if (p.isDisabled()) {
logger.println(Messages.BuildTrigger_Disabled(ModelHyperlinkNote.encodeTo(p)));
continue;
Expand Down Expand Up @@ -331,9 +346,7 @@ public FormValidation doCheck(@AncestorInPath Item project, @QueryParameter Stri
AbstractProject.findNearest(projectName,project.getParent()).getRelativeNameFrom(project)));
if(!(item instanceof AbstractProject))
return FormValidation.error(Messages.BuildTrigger_NotBuildable(projectName));
if (!upstream && !item.hasPermission(Item.BUILD)) {
return FormValidation.error(Messages.BuildTrigger_you_have_no_permission_to_build_(projectName));
}
// Will require Item.BUILD on project (if upstream) or item (if !upstream) but we cannot predict what QueueItemAuthenticator will produce
hasProjects = true;
}
}
Expand Down
76 changes: 63 additions & 13 deletions test/src/test/java/hudson/tasks/BuildTriggerTest.java
View file
Expand Up @@ -23,17 +23,20 @@
*/
package hudson.tasks;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
import hudson.maven.MavenModuleSet;
import hudson.maven.MavenModuleSetBuild;
import hudson.model.Cause;
import hudson.model.Computer;
import hudson.model.FreeStyleBuild;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.Queue;
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.User;
import hudson.security.AuthorizationMatrixProperty;
import hudson.security.LegacySecurityRealm;
import hudson.security.Permission;
Expand All @@ -43,6 +46,9 @@
import java.util.Map;
import java.util.Set;
import jenkins.model.Jenkins;
import jenkins.security.QueueItemAuthenticator;
import jenkins.security.QueueItemAuthenticatorConfiguration;
import org.acegisecurity.Authentication;
import org.jvnet.hudson.test.ExtractResourceSCM;
import org.jvnet.hudson.test.HudsonTestCase;
import org.jvnet.hudson.test.MockBuilder;
Expand Down Expand Up @@ -144,20 +150,25 @@ public void testMavenTriggerEvenWhenUnstable() throws Exception {
doMavenTriggerTest(true);
}

public void testConfigureDownstreamProjectSecurity() throws Exception {
public void testDownstreamProjectSecurity() throws Exception {
jenkins.setSecurityRealm(new LegacySecurityRealm());
ProjectMatrixAuthorizationStrategy auth = new ProjectMatrixAuthorizationStrategy();
auth.add(Jenkins.READ, "alice");
auth.add(Computer.BUILD, "alice");
auth.add(Computer.BUILD, "anonymous");
jenkins.setAuthorizationStrategy(auth);
FreeStyleProject upstream = createFreeStyleProject("upstream");
final FreeStyleProject upstream = createFreeStyleProject("upstream");
QueueItemAuthenticatorConfiguration.get().getAuthenticators().add(new QueueItemAuthenticator() {
@Override public Authentication authenticate(Queue.Item item) {
return item.task == upstream ? User.get("alice").impersonate() : null;
}
private Object writeReplace() {return "";}
});
Map<Permission,Set<String>> perms = new HashMap<Permission,Set<String>>();
perms.put(Item.READ, Collections.singleton("alice"));
perms.put(Item.CONFIGURE, Collections.singleton("alice"));
upstream.addProperty(new AuthorizationMatrixProperty(perms));
FreeStyleProject downstream = createFreeStyleProject("downstream");
/* Original SECURITY-55 test case:
downstream.addProperty(new AuthorizationMatrixProperty(Collections.singletonMap(Item.READ, Collections.singleton("alice"))));
*/
WebClient wc = createWebClient();
wc.login("alice");
HtmlPage page = wc.getPage(upstream, "configure");
Expand All @@ -166,13 +177,52 @@ public void testConfigureDownstreamProjectSecurity() throws Exception {
page.getAnchorByText("Build other projects").click();
HtmlTextInput childProjects = config.getInputByName("buildTrigger.childProjects");
childProjects.setValueAttribute("downstream");
try {
submit(config);
fail();
} catch (FailingHttpStatusCodeException x) {
assertEquals(403, x.getStatusCode());
}
assertEquals(Collections.emptyList(), upstream.getDownstreamProjects());
submit(config);
// DependencyGraph is rebuilt as SYSTEM so is always complete even if configuring user does not know it:
assertEquals(Collections.singletonList(downstream), upstream.getDownstreamProjects());
// Downstream projects whose existence we are not aware of will silently not be triggered:
FreeStyleBuild b = buildAndAssertSuccess(upstream);
assertLogNotContains("downstream", b);
waitUntilNoActivity();
assertNull(downstream.getLastBuild());
Map<Permission,Set<String>> grantedPermissions = new HashMap<Permission,Set<String>>();
grantedPermissions.put(Item.READ, Collections.singleton("alice"));
AuthorizationMatrixProperty amp = new AuthorizationMatrixProperty(grantedPermissions);
// If we can see them, but not build them, that is a warning (but this is in cleanUp so the build is still considered a success):
downstream.addProperty(amp);
b = buildAndAssertSuccess(upstream);
assertLogContains("downstream", b);
waitUntilNoActivity();
assertNull(downstream.getLastBuild());
// If we can build them, then great:
grantedPermissions.put(Item.BUILD, Collections.singleton("alice"));
downstream.removeProperty(amp);
amp = new AuthorizationMatrixProperty(grantedPermissions);
downstream.addProperty(amp);
b = buildAndAssertSuccess(upstream);
assertLogContains("downstream", b);
waitUntilNoActivity();
FreeStyleBuild b2 = downstream.getLastBuild();
assertNotNull(b2);
Cause.UpstreamCause cause = b2.getCause(Cause.UpstreamCause.class);
assertNotNull(cause);
assertEquals(b, cause.getUpstreamRun());
// Now for legacy behavior: we should run as anonymous. Which would normally have no permissions:
QueueItemAuthenticatorConfiguration.get().getAuthenticators().clear();
b = buildAndAssertSuccess(upstream);
assertLogNotContains("downstream", b);
waitUntilNoActivity();
assertEquals(1, downstream.getLastBuild().number);
// Unless we explicitly granted them:
grantedPermissions.put(Item.READ, Collections.singleton("anonymous"));
grantedPermissions.put(Item.BUILD, Collections.singleton("anonymous"));
downstream.removeProperty(amp);
amp = new AuthorizationMatrixProperty(grantedPermissions);
downstream.addProperty(amp);
b = buildAndAssertSuccess(upstream);
assertLogContains("downstream", b);
waitUntilNoActivity();
assertEquals(2, downstream.getLastBuild().number);
}

}

0 comments on commit 48e92ef

Please sign in to comment.