Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-19922

"Build after other projects are built" job trigger causes permission check errors in case of enabled security

      Use-case:

      2 projects: P1, P2
      2 Users: U1 and U2
      Permissions:
      U1: Full access to P1, Read-only to P2
      U2: Read-only access to P1, Full access to P2

      Reproduction steps
      1) U1 adds a "Build after other projects are built" trigger to P1. Then he sets completion of P2 as a trigger. Although U1 has no configuration access rights to P1, P1 is reconfigured to trigger P2 <<< ERROR?
      2) After that, U2 configures some stuff in his job. After clicking on "Save button" he receives error "You have no permissions to build P1"
      3) User U2 won't be able to save his job till he removes triggering of P1 from his job. Jenkins does not check configuration permissions, so U2 is able to remove trigger from P1 <<< ERROR?

      => Configuration wars and complains from users

      Issue can be even more complex if many users have access to these jobs => configuration saving will work differently for users.

      Proposal 1:

      • Require "build" access rights for adding of new items;
      • Print warning for projects w/o build permissions, but allow saving of configs

      Proposal 2:

      • Completely decouple fictive triggers (replace them to BuildListeners) and post-build actions in jobs

          [JENKINS-19922] "Build after other projects are built" job trigger causes permission check errors in case of enabled security

          Jesse Glick added a comment -

          Pseudo-triggers are rather misleading. They always add a post-build step BuildTrigger (which is not a Trigger! rather a Notifier; and really should be a Recorder) to the upstream project. You do not need permission to configure the upstream project to do this. The downstream project is not changed, despite the appearance in the configuration screen.

          SECURITY-55 (and SECURITY-109) force anyone configuring a job to have build permission on any specified downstream jobs (whether added by a pseudotrigger or not). JENKINS-16956 reverts that, doing the permission check at runtime instead.

          Note that BuildTrigger is even weirder than the above description would imply because its perform method does nothing! It exists solely to be a DependencyDeclarer, thus adding an edge to the project dependency graph. And then in AbstractBuildExecution.cleanUp, BuildTrigger.execute is called, which triggers downstream builds for all dependencies from this upstream project—whether added by BuildTrigger or not—in topological order, regardless of what order those project configuration elements were added in (for example in the publishers section). So for example if you use the Parameterized Build plugin, and add a non-blocking trigger to your publishers section, running it does nothing per se, except insofar as that is marked as a dependency in the graph. As compared to adding a blocking trigger as a build step, which does not affect the dependency graph, and does exactly what it looks like it does. Confused yet?

          JENKINS-22397, recently implemented, allows the Build Result trigger to display a node in the dependency graph—but for now it is purely informational: not used during BuildTrigger.execute. Currently that trigger (which is a true trigger) runs on a polling schedule, meaning that there is a delay before the downstream build is scheduled. Probably this should be changed to immediately schedule the downstream build, though the rather complex implementation there is tightly tied to polling.

          Perhaps there should be a fresh, simple plugin added implementing a Trigger based on an upstream project’s completion, and a Notifier/Builder which schedules a downstream project, both implementing an informational edge in the dependency graph and performing intuitive access control checks. (If you really want the automated topological sort functionality that Jenkins core offers, and understand the implications, you can continue to use BuildTrigger.)

          Jesse Glick added a comment - Pseudo-triggers are rather misleading. They always add a post-build step BuildTrigger (which is not a Trigger ! rather a Notifier ; and really should be a Recorder ) to the upstream project. You do not need permission to configure the upstream project to do this. The downstream project is not changed, despite the appearance in the configuration screen. SECURITY-55 (and SECURITY-109) force anyone configuring a job to have build permission on any specified downstream jobs (whether added by a pseudotrigger or not). JENKINS-16956 reverts that, doing the permission check at runtime instead. Note that BuildTrigger is even weirder than the above description would imply because its perform method does nothing! It exists solely to be a DependencyDeclarer , thus adding an edge to the project dependency graph. And then in AbstractBuildExecution.cleanUp , BuildTrigger.execute is called, which triggers downstream builds for all dependencies from this upstream project—whether added by BuildTrigger or not—in topological order, regardless of what order those project configuration elements were added in (for example in the publishers section). So for example if you use the Parameterized Build plugin, and add a non-blocking trigger to your publishers section, running it does nothing per se, except insofar as that is marked as a dependency in the graph. As compared to adding a blocking trigger as a build step, which does not affect the dependency graph, and does exactly what it looks like it does. Confused yet? JENKINS-22397 , recently implemented, allows the Build Result trigger to display a node in the dependency graph—but for now it is purely informational: not used during BuildTrigger.execute . Currently that trigger (which is a true trigger) runs on a polling schedule, meaning that there is a delay before the downstream build is scheduled. Probably this should be changed to immediately schedule the downstream build, though the rather complex implementation there is tightly tied to polling. Perhaps there should be a fresh, simple plugin added implementing a Trigger based on an upstream project’s completion, and a Notifier / Builder which schedules a downstream project, both implementing an informational edge in the dependency graph and performing intuitive access control checks. (If you really want the automated topological sort functionality that Jenkins core offers, and understand the implications, you can continue to use BuildTrigger .)

          Oleg Nenashev added a comment - - edited

          On our installations "Build Trigger" is slightly patched and prohibited by policies. Unfortunately, we cannot completely remove it due to the migration of legacy Jenkins instances.

          It makes sense to slightly refactor your clarification and move it to the built-in help as a first step. Since there are well-known workarounds like Parameterized Trigger, the complete fix does not have a big priority.

          P.S: It would be very useful to mark descriptors as "deprecated" and somehow display warnings icon (with expandable text box) near the help icon.

          Oleg Nenashev added a comment - - edited On our installations "Build Trigger" is slightly patched and prohibited by policies. Unfortunately, we cannot completely remove it due to the migration of legacy Jenkins instances. It makes sense to slightly refactor your clarification and move it to the built-in help as a first step. Since there are well-known workarounds like Parameterized Trigger, the complete fix does not have a big priority. P.S: It would be very useful to mark descriptors as "deprecated" and somehow display warnings icon (with expandable text box) near the help icon.

          Jesse Glick added a comment -

          I am just trying to cover this is one place, JENKINS-16956.

          Jesse Glick added a comment - I am just trying to cover this is one place, JENKINS-16956 .

            Unassigned Unassigned
            oleg_nenashev Oleg Nenashev
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: