Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

withCredentials Returns the values as the name of the variables instead of the credential #532

Open
Acejames87 opened this issue Jun 1, 2022 · 5 comments

Comments

@Acejames87
Copy link

Acejames87 commented Jun 1, 2022

Jenkins and plugins versions report

Environment
Paste the output here

What Operating System are you using (both controller, and any agents involved in the problem)?

jenkins 2303.2

Reproduction steps

 @Test
    void CleanRepoCall() {
        super.setUp()
        addCredential('ARTIFACTORY_USER','USERNAME')
        addCredential('ARTIFACTORY_PASS', 'PASSWORD')
       def script=loadScript('jobs/withCredential.groovy')
       script()
}

withCredential.groovy content

def call(Map args)
{
           withCredentials([usernamePassword(
                credentialsId: 'artifactory',
                usernameVariable: 'ARTIFACTORY_USER',
                passwordVariable: 'ARTIFACTORY_PASS')]) {
                   echo ${ARTIFACTORY_USER}
                   echo ${ARTIFACTORY_PASS}
                 }

}

Expected Results

echo: USERNAME
echo: PASSWORD

Actual Results

echo: ARTIFACTORY_USER
echo: ARTIFACTORY_PASSWORD

Anything else?

To fix it I have to override the registry of withCredentials to actually read the credentials

i.e.

 helper.registerAllowedMethod("withCredentials", [List, Closure], { list, closure ->
            def previousValues = [:]
            list.forEach { creds ->
                // stringInterceptor returns a String value where the
                // usernamePasswordInterceptor returns a list of strings
                if (creds instanceof String) {
                    try {
                        previousValues[creds] = binding.getVariable(creds)
                    } catch (MissingPropertyException e) {
                        previousValues[creds] = null
                    }
                    binding.setVariable(creds, binding.credentials[creds])
                } else if (creds instanceof Map && creds.get("\$class") == "UsernamePasswordMultiBinding") {
                    def username = creds.get("usernameVariable")
                    def password = creds.get("passwordVariable")
                    try {
                        previousValues[username] = binding.getVariable(username)
                    } catch (MissingPropertyException e) {
                        previousValues[username] = null
                    }
                    binding.setVariable(username, binding.credentials[username])
                    try {
                        previousValues[password] = binding.getVariable(password)
                    } catch (MissingPropertyException e) {
                        previousValues[password] = null
                    }
                    binding.setVariable(password, binding.credentials[password])
                } else {
                    creds.each { var ->
                        try {
                            previousValues[var] = binding.getVariable(var)
                        } catch (MissingPropertyException e) {
                            previousValues[var] = null
                        }
                        binding.setVariable(var, binding.credentials[var])
                    }
                }
            }

            closure.delegate = delegate
            def res = helper.callClosure(closure)

            // If previous value was not set it will unset by using null
            // otherwise it will restore previous value.
            previousValues.each { key, value ->
                binding.setVariable(key, value)
            }
            return res
        })
@nre-ableton
Copy link
Contributor

Sorry, for the delay, I haven't had time to work on this project recently, but my team now has a hack sprint so I'll be examining this issue soon.

@nre-ableton
Copy link
Contributor

So I had a look at this yesterday, and the fix isn't that difficult:

--- a/src/main/groovy/com/lesfurets/jenkins/unit/BasePipelineTest.groovy
+++ b/src/main/groovy/com/lesfurets/jenkins/unit/BasePipelineTest.groovy
@@ -33,7 +33,7 @@ abstract class BasePipelineTest {
             }
             // we are in context of withCredentials([string()..]) { }
             if(desc.variable) {
-                return desc.variable
+                return [name: desc.credentialsId, value: desc.variable]
             }
         }
     }
@@ -68,13 +68,16 @@ abstract class BasePipelineTest {
                 }
                 binding.setVariable(password, password)
             } else {
-                creds.each { var ->
+                creds.each { var, value ->
+                    previousValues[var] = binding.getVariable(var)
                     try {
-                        previousValues[var] = binding.getVariable(var)
-                    } catch (MissingPropertyException e) {
+                        Map credentials = binding.getVariable('credentials')
+                        if (credentials.containsKey(creds.name)) {
+                            binding.setVariable(creds.value, credentials[creds.name])
+                        }
+                    } catch (MissingPropertyException ignored) {
                         previousValues[var] = null
                     }
-                    binding.setVariable(var, var)

...the problem is that this breaks other stuff. In general, I'm not a huge fan of the current implementation of withCredentialsInterceptor, but rewriting it is likely to be very tricky.

I'll keep hacking on this next week and see if I can come up with a good solution.

@nre-ableton
Copy link
Contributor

Update: I did indeed spend some time hacking on this, but I couldn't come up with a good solution. I'll have to spend some more time in the future rewriting withCredentialsInterceptor in a safe and sane way.

@meelistolk
Copy link

Any progress on this?
I stumbled upon the same issue that addCredential values are not used in withCredentials, instead the variable names are returned

@nre-ableton
Copy link
Contributor

Sorry, no, I've been too busy with other stuff to work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants