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

disable "build by token" by default using a system property in Jenkins

      Jenkins has ACLs and you need to have read access in order to find the build to trigger so this should be removed as it creates confusion and can be handled by granting the user Item.BUILD

      For post commit hooks they should post to global hook (e.g. svn hook or git hook url),
      for other systems if special anonymous access to trigger a build is required a new plugin should be introduced.

      The backend code was marked as deprecated with the early versions of Hudson (> 10 years ago) yet there is nothing in the UI to show this is the case.

          [JENKINS-50249] disable "build by token" by default using a system property in Jenkins

          Jesse Glick added a comment -

          if special anonymous access to trigger a build is required a new plugin should be introduced

          It already exists: build-token-root

          The backend code was marked as deprecated

          Yet there is no satisfactory replacement. Saving your personal API token in a post-commit hook allows anyone on the team who can view SCM settings to impersonate you in Jenkins, which is undesirable.

          Jesse Glick added a comment - if special anonymous access to trigger a build is required a new plugin should be introduced It already exists: build-token-root The backend code was marked as deprecated Yet there is no satisfactory replacement. Saving your personal API token in a post-commit hook allows anyone on the team who can view SCM settings to impersonate you in Jenkins, which is undesirable.

          Jesse Glick added a comment -

          See JENKINS-38257 for example. Unless and until a general system of limited-scope revokable tokens is added to Jenkins, BuildAuthorizationToken is an irreplaceable feature. I do not think it should have been deprecated in code to begin with, since disabling the user feature would lead to a critical loss of functionality.

          Jesse Glick added a comment - See JENKINS-38257 for example. Unless and until a general system of limited-scope revokable tokens is added to Jenkins, BuildAuthorizationToken is an irreplaceable feature. I do not think it should have been deprecated in code to begin with, since disabling the user feature would lead to a critical loss of functionality.

          James Nord added a comment - - edited

          jglick I am suggesting completely disabling https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/ParameterizedJobMixIn.java#L208 (replacing with an ACL check), but that there is no point in using this when you have authorised users - so the only use case is really using the build-token-root plugin (where you want an anonymous user to be able to trigger the build but not see it) Thus for core the behaviour would be (if the property was not set) use the projects ACL rather than check the BuildToken, it the property was set then it would behave as of today.

          Now we could probably completely yank it out of core and move it to a lib and introduce a couple of new endooints job/buildWithToken job/buildWIthTokenAndParmaters job/pollingWithToken but I still do not see the use of this token unless you have anonymous read access, or you share a global read only user around. (and in that case I am left wondering if the build-token-root plugin would be better extended to require an list of authorised users (including anonymous) and then there is questionable need for any of this to be in core at all.

          James Nord added a comment - - edited jglick I am suggesting completely disabling https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/ParameterizedJobMixIn.java#L208  (replacing with an ACL check), but that there is no point in using this when you have authorised users - so the only use case is really using the build-token-root plugin (where you want an anonymous user to be able to trigger the build but not see it) Thus for core the behaviour would be (if the property was not set) use the projects ACL rather than check the BuildToken, it the property was set then it would behave as of today. Now we could probably completely yank it out of core and move it to a lib and introduce a couple of new endooints job/buildWithToken job/buildWIthTokenAndParmaters job/pollingWithToken but I still do not see the use of this token unless you have anonymous read access, or you share a global read only user around. (and in that case I am left wondering if the build-token-root plugin would be better extended to require an list of authorised users (including anonymous) and then there is questionable need for any of this to be in core at all.

          Jesse Glick added a comment -

          the only use case is really using the build-token-root plugin

          No, that plugin is needed only when you deny even read access to anonymous users.

          Jesse Glick added a comment - the only use case is really using the  build-token-root  plugin No, that plugin is needed only when you deny even read access to anonymous users.

          Jesse Glick added a comment -

          introduce a couple of new endpoints

          That does not solve anything as you are still breaking existing usage. If you are going to force people to update scripts, you might as well have them use the build-token-root URL patterns.

          there is no point in using this when you have authorised users

          As previously noted, that is not true.

          for core the behaviour would be (if the property was not set) use the projects ACL rather than check the BuildToken, it the property was set then it would behave as of today

          That is already the behavior of core. If there is no token, Item.BUILD is checked (and only POST requests or GET requests with API token are allowed, as a CSRF defense).

          Jesse Glick added a comment - introduce a couple of new endpoints That does not solve anything as you are still breaking existing usage. If you are going to force people to update scripts, you might as well have them use the build-token-root URL patterns. there is no point in using this when you have authorised users As previously noted, that is not true. for core the behaviour would be (if the property was not set) use the projects ACL rather than check the BuildToken, it the property was set then it would behave as of today That is already the behavior of core. If there is no token, Item.BUILD is checked (and only POST requests or GET requests with API token are allowed, as a CSRF defense).

          Jesse Glick added a comment -

          To be clear, I have no problem with moving the token functionality to the build-token-root plugin except for the fact that existing installations making requests to job/stuff/build?token=s3cr3t would be suddenly broken. Now we could try to make the migration easier in various ways:

          • continue to detect the token query parameter, and return an error response whose text mentions the plugin
          • delay the move for a while, in the meantime printing a warning to the system log when a token build is initiated
          • make the token field read-only for jobs already using it, and hidden for new jobs

          Disabling via a system property does not seem like a good approach. This is just adding one more underdocumented runtime flag.

          Jesse Glick added a comment - To be clear, I have no problem with moving the token functionality to the build-token-root plugin except for the fact that existing installations making requests to job/stuff/build?token=s3cr3t would be suddenly broken. Now we could try to make the migration easier in various ways: continue to detect the token query parameter, and return an error response whose text mentions the plugin delay the move for a while, in the meantime printing a warning to the system log when a token build is initiated make the token field read-only for jobs already using it, and hidden for new jobs Disabling via a system property does not seem like a good approach. This is just adding one more underdocumented runtime flag.

          Jesse Glick added a comment -

          I think BuildAuthorizationToken itself could be split out into the build-token-root plugin, with a PluginServletFilter for URL compatibility.

          Jesse Glick added a comment - I think BuildAuthorizationToken itself could be split out into the build-token-root plugin, with a PluginServletFilter for URL compatibility.

            Unassigned Unassigned
            teilo James Nord
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: