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

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

    Details

    • Similar Issues:

      Description

      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.

        Attachments

          Issue Links

            Activity

            Hide
            jglick 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.

            Show
            jglick 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.
            Hide
            jglick 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.

            Show
            jglick 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.
            Hide
            teilo James Nord added a comment - - edited

            Jesse Glick 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.

            Show
            teilo James Nord added a comment - - edited Jesse Glick 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.
            Hide
            jglick 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.

            Show
            jglick 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.
            Hide
            jglick 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).

            Show
            jglick 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).
            Hide
            jglick 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.

            Show
            jglick 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.
            Hide
            jglick 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.

            Show
            jglick 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.

              People

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

                Dates

                • Created:
                  Updated: