Merge pull request #96 from oleg-nenashev/JENKINS-31727

[FIXED JENKINS-31727] - ConfigTrigger should be tolerant against Item.DISCOVER without Item.READ

src/main/java/hudson/plugins/parameterizedtrigger/BuildTriggerConfig.java

@@ -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> {
 
@@ -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;
     }
 
@@ -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());
@@ -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
@@ -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();
@@ -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));
         }
     }
 

src/test/java/hudson/plugins/parameterizedtrigger/test/BuildTriggerConfigTest.java

@@ -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;
@@ -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;
@@ -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 {
 
@@ -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
      *