Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #96 from oleg-nenashev/JENKINS-31727
[FIXED JENKINS-31727] - ConfigTrigger should be tolerant against Item.DISCOVER without Item.READ
  • Loading branch information
svanoort committed Dec 14, 2015
2 parents 9741736 + 9ebaae3 commit 5cc4a44
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 6 deletions.
Expand Up @@ -55,6 +55,8 @@
import java.util.Set;
import java.util.StringTokenizer;
import java.util.concurrent.Future;
import javax.annotation.Nonnull;
import org.acegisecurity.AccessDeniedException;

public class BuildTriggerConfig implements Describable<BuildTriggerConfig> {

Expand Down Expand Up @@ -141,10 +143,11 @@ public List<AbstractProject> getProjectList(ItemGroup context, EnvVars env) {
* @param env Environment variables from which to expand project names; Might be {@code null}.
* @param context
* The container with which to resolve relative project names.
* If the user has no {@link Item#READ} permission, the job won't be added to the list.
*/
public List<Job> getJobs(ItemGroup context, EnvVars env) {
List<Job> projectList = new ArrayList<Job>();
projectList.addAll(Items.fromNameList(context, getProjects(env), Job.class));
projectList.addAll(readableItemsFromNameList(context, getProjects(env), Job.class));
return projectList;
}

Expand Down Expand Up @@ -207,7 +210,8 @@ private static void iterateBuilds(AbstractProject context, String projects, SubP
// If we don't have any build there's no point to trying to resolved dynamic projects
if (currentBuild == null) {
// But we can still get statically defined project
subProjectData.getFixed().addAll(Items.fromNameList(context.getParent(), projects, AbstractProject.class));
subProjectData.getFixed().addAll(readableItemsFromNameList(context.getParent(), projects, AbstractProject.class));

// Remove them from unsolved
for (AbstractProject staticProject : subProjectData.getFixed()) {
subProjectData.getUnresolved().remove(staticProject.getFullName());
Expand All @@ -233,6 +237,34 @@ private static void iterateBuilds(AbstractProject context, String projects, SubP
}
}
}

/**
* Retrieves readable items from the list.
* @param <T> Type of the item
* @param context Current item
* @param list String list of items
* @param type Type of items to be retrieved
* @return List of readable items, others will be skipped if {@link AccessDeniedException} happens
*/
private static <T extends Item> List<T> readableItemsFromNameList(
ItemGroup context, @Nonnull String list, @Nonnull Class<T> type) {
Jenkins hudson = Jenkins.getInstance();

List<T> r = new ArrayList<T>();
StringTokenizer tokens = new StringTokenizer(list,",");
while(tokens.hasMoreTokens()) {
String fullName = tokens.nextToken().trim();
T item = null;
try {
item = hudson.getItem(fullName, context, type);
} catch (AccessDeniedException ex) {
// Ignore, item won't be added to the resulting list
}
if(item!=null)
r.add(item);
}
return r;
}

/**
* Retrieves the environment variable from a build and tries to resolves the remaining unresolved projects. If
Expand Down Expand Up @@ -265,7 +297,14 @@ private static void resolveProject(AbstractBuild build, SubProjectData subProjec
destinationSet = subProjectData.getDynamic();
}

AbstractProject resolvedProject = Jenkins.getInstance().getItem(unresolvedProjectName, build.getProject().getParent(), AbstractProject.class);
final Jenkins jenkins = Jenkins.getInstance();
AbstractProject resolvedProject = null;
try {
resolvedProject = jenkins == null ? null :
jenkins.getItem(unresolvedProjectName, build.getProject().getParent(), AbstractProject.class);
} catch (AccessDeniedException ex) {
// Permission check failure (DISCOVER w/o READ) => we leave the job unresolved
}
if (resolvedProject != null) {
destinationSet.add(resolvedProject);
unsolvedProjectIterator.remove();
Expand All @@ -274,7 +313,7 @@ private static void resolveProject(AbstractBuild build, SubProjectData subProjec

if (build != null && build.getAction(BuildInfoExporterAction.class) != null) {
String triggeredProjects = build.getAction(BuildInfoExporterAction.class).getProjectListString(",");
subProjectData.getTriggered().addAll(Items.fromNameList(build.getParent().getParent(), triggeredProjects, AbstractProject.class));
subProjectData.getTriggered().addAll(readableItemsFromNameList(build.getParent().getParent(), triggeredProjects, AbstractProject.class));
}
}

Expand Down
Expand Up @@ -26,8 +26,11 @@

import hudson.model.AbstractProject;
import hudson.model.Cause;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.Project;
import hudson.model.User;
import hudson.plugins.parameterizedtrigger.AbstractBuildParameters;
import hudson.plugins.parameterizedtrigger.BlockableBuildTriggerConfig;
import hudson.plugins.parameterizedtrigger.BlockingBehaviour;
Expand All @@ -36,6 +39,10 @@
import hudson.plugins.parameterizedtrigger.PredefinedBuildParameters;
import hudson.plugins.parameterizedtrigger.SubProjectData;
import hudson.plugins.parameterizedtrigger.TriggerBuilder;
import hudson.security.ACL;
import hudson.security.AuthorizationMatrixProperty;
import hudson.security.Permission;
import hudson.security.ProjectMatrixAuthorizationStrategy;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
Expand All @@ -44,13 +51,19 @@
import org.jvnet.hudson.test.HudsonTestCase;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.collection.IsMapContaining.hasEntry;
import org.jvnet.hudson.test.Bug;

public class BuildTriggerConfigTest extends HudsonTestCase {

Expand Down Expand Up @@ -182,8 +195,52 @@ public void testBuildWithWorkflowProjects() throws Exception {
assertBuildStatusSuccess(workflowRun);
assertLogContains("GOOBER", workflowRun);
}



@Bug(31727)
public void testShouldNotFailOnDiscoverWithoutReadPermission() throws Exception {
// Setup global security
jenkins.setSecurityRealm(createDummySecurityRealm());
User user = User.get("testUser");
ProjectMatrixAuthorizationStrategy strategy = new ProjectMatrixAuthorizationStrategy();
strategy.add(Item.DISCOVER, "anonymous");
strategy.add(Jenkins.READ, "anonymous");
jenkins.setAuthorizationStrategy(strategy);

// Create project with downstream trigger
final FreeStyleProject downstreamProject = createFreeStyleProject("downstreamProject");
final FreeStyleProject upstreamProject = createFreeStyleProject("upstreamProject");
final BlockableBuildTriggerConfig triggerConfg = createConfig("downstreamProject");
addParameterizedTrigger(upstreamProject, triggerConfg);

// Setup upstream project security
Map<Permission,Set<String>> permissions = new HashMap<Permission,Set<String>>();
Set<String> userIds = new HashSet<String>(Arrays.asList("testUser"));
permissions.put(Item.READ, userIds);
AuthorizationMatrixProperty projectPermissions = new AuthorizationMatrixProperty(permissions);
upstreamProject.addProperty(projectPermissions);

// Ensure that we can get the info about the downstream project, but it is unresolved
ACL.impersonate(user.impersonate(), new Runnable() {
@Override
public void run() {
SubProjectData projectInfo = triggerConfg.getProjectInfo(upstreamProject);
assertTrue("Downstream project should be unresolved, because testUser has no READ permission",
projectInfo.getUnresolved().contains(downstreamProject.getName()));
}
});

// Now invoke the build and check again (other logic handlers)
buildAndAssertSuccess(upstreamProject);
ACL.impersonate(user.impersonate(), new Runnable() {
@Override
public void run() {
SubProjectData projectInfo = triggerConfg.getProjectInfo(upstreamProject);
assertTrue("Downstream project should be unresolved, because testUser has no READ permission",
projectInfo.getUnresolved().contains(downstreamProject.getName()));
}
});
}

/**
* Testing statically and dynamically defined projects
*
Expand Down

0 comments on commit 5cc4a44

Please sign in to comment.