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

Git plugin credential lookup uses too low level API

    • Icon: Improvement Improvement
    • Resolution: Unresolved
    • Icon: Minor Minor
    • git-plugin
    • None

      In GitSCMFileSystem.build(), this bypasses several features provided natively by the credentials plugin including user-scoped credentials, authorize project scoped credentials, other sources, and the credential parameter shadowing feature. This should instead use CredentialsProvider.findCredentialsById() with the listed domain requirements and the associated run.

          [JENKINS-61824] Git plugin credential lookup uses too low level API

          Matt Sicker added a comment -

          I'm also open to updating the credentials plugin API if there's a limitation here I missed back when I first updated that.

          Matt Sicker added a comment - I'm also open to updating the credentials plugin API if there's a limitation here I missed back when I first updated that.

          Mark Waite added a comment -

          Thanks for the report jvz. Since GitSCMFileSystem.build() was implemented by stephenconnolly in 2016, I assume that the credentials API support for user-scoped credentials, authorize project scoped credentials, other credentials, and the credential shadowing feature were added to the credentials API after 2016. Can you confirm my assumption?

          If my assumption is correct, then I assume all uses of credentials in the git plugin and the git client plugin should probably be visited to assure they are using the correct and current credential APIs.

          If those features were not added to the credentials API after 2016, then I assume there was a compelling reason that they were not used in that implementation.

          Mark Waite added a comment - Thanks for the report jvz . Since GitSCMFileSystem.build() was implemented by stephenconnolly in 2016, I assume that the credentials API support for user-scoped credentials, authorize project scoped credentials, other credentials, and the credential shadowing feature were added to the credentials API after 2016. Can you confirm my assumption? If my assumption is correct, then I assume all uses of credentials in the git plugin and the git client plugin should probably be visited to assure they are using the correct and current credential APIs. If those features were not added to the credentials API after 2016, then I assume there was a compelling reason that they were not used in that implementation.

          Matt Sicker added a comment -

          Yes, the shadowing is from 2018 or 2019, and the user scoped credentials are just poorly supported in general. Seems like this is less a bug and more so an improvement. I'm able to work around this by using git credentialsId: params.foo instead of git credentialsId: 'foo', but that will only work for global scoped credentials. If I wanted to use credentials from my account to, say, tag a sensitive git repo interactively, it wouldn't work without a lookup attempt using findCredentialsById.

          Matt Sicker added a comment - Yes, the shadowing is from 2018 or 2019, and the user scoped credentials are just poorly supported in general. Seems like this is less a bug and more so an improvement. I'm able to work around this by using git credentialsId: params.foo instead of git credentialsId: 'foo' , but that will only work for global scoped credentials. If I wanted to use credentials from my account to, say, tag a sensitive git repo interactively, it wouldn't work without a lookup attempt using findCredentialsById .

          Matt Sicker added a comment - - edited

          Matt Sicker added a comment - - edited One other note: the low level API for interacting with credential parameters on builds is via https://github.com/jenkinsci/credentials-plugin/blob/master/docs/consumer.adoc#binding-user-supplied-credentials-parameters-to-builds Edit: more relevant would be https://github.com/jenkinsci/credentials-plugin/blob/master/docs/consumer.adoc#retrieve-a-previously-selected-credentials-instance

          Mark Waite added a comment -

          jvz pull requests are welcomed to adapt the plugin to the new APIs.

          May be related to JENKINS-58902 and JENKINS-44773

          Mark Waite added a comment - jvz pull requests are welcomed to adapt the plugin to the new APIs. May be related to JENKINS-58902 and JENKINS-44773

          Matt Sicker added a comment -

          Likely related! I started looking at patching this yesterday, though since the affected code seemed to be a little fancier than expected, I think I may need to take a step back to examine credentials-plugin and see if there's any more APIs that needed to be updated in it to properly integrate the fancier credential resolution. This API has clearly evolved over a long period of time, so not every plugin seems to be using the most effective APIs.

          Matt Sicker added a comment - Likely related! I started looking at patching this yesterday, though since the affected code seemed to be a little fancier than expected, I think I may need to take a step back to examine credentials-plugin and see if there's any more APIs that needed to be updated in it to properly integrate the fancier credential resolution. This API has clearly evolved over a long period of time, so not every plugin seems to be using the most effective APIs.

          Matt Sicker added a comment -

          I came across a similar issue today in the slack plugin which uses even lower level APIs: https://github.com/jenkinsci/slack-plugin/blob/master/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java

          Seems like there's too much choice!

          Matt Sicker added a comment - I came across a similar issue today in the slack plugin which uses even lower level APIs: https://github.com/jenkinsci/slack-plugin/blob/master/src/main/java/jenkins/plugins/slack/CredentialsObtainer.java Seems like there's too much choice!

            Unassigned Unassigned
            jvz Matt Sicker
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: