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

CredentialsMatcher.CQL.describe() implementations return invalid CQL

XMLWordPrintable

    • 933.0.1

      The com.cloudbees.jenkins.plugins.bitbucket.credentials.BitbucketUsernamePasswordCredentialMatcher class implements the CredentialsMatcher.CQL interface by making public String describe() return the constant string "username and password are not empty". This is not valid Credentials Query Language and cannot be parsed by the public static CredentialsMatcher parse(final String cql) method of com.cloudbees.plugins.credentials.CredentialsMatchers.

      Likewise, BitbucketOAuthCredentialMatcher implements CredentialsMatcher.CQL and returns a CQL string that is invalid in several ways:

      • Unbalanced parentheses
      • Misspelled lenght doesn't match String.length()
      • Uses a CONTAINS operator that is not defined in CQL

          [JENKINS-74972] CredentialsMatcher.CQL.describe() implementations return invalid CQL

          Kalle Niemitalo added a comment -

          Ideas for how to fix this:

          • Remove the CredentialsMatcher.CQL implementation; AFAIK it is optional anyway. As a result, a credential provider would have to load each credential into the JVM before it can check whether the credential matches BitbucketUsernamePasswordCredentialMatcher.
          • Replace the CQL expression with just "instanceof com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials", even though BitbucketUsernamePasswordCredentialMatcher also checks StringUtils.isNotBlank(username) && StringUtils.isNotBlank(password). This would let a credential provider avoid loading credentials that have the wrong type. But I am not sure it is valid to omit the isNotBlank checks from the CQL expression like this.
          • Devise a more complex CQL expression that does the equivalent of StringUtils.isNotBlank as well. This does not look feasible. (In contrast, StringUtils.isEmpty(username) could be expressed as username == null || username == "".)

          I don't see where CredentialsMatchers.parse is called at all, except in CredentialsMatchersTest. Perhaps public credential-provider plugins do not actually use the CQL feature and only some proprietary plugin does. For that reason, I think removing the CredentialsMatcher.CQL implementation is the best option.

          Kalle Niemitalo added a comment - Ideas for how to fix this: Remove the CredentialsMatcher.CQL implementation; AFAIK it is optional anyway. As a result, a credential provider would have to load each credential into the JVM before it can check whether the credential matches BitbucketUsernamePasswordCredentialMatcher. Replace the CQL expression with just "instanceof com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials", even though BitbucketUsernamePasswordCredentialMatcher also checks StringUtils.isNotBlank(username) && StringUtils.isNotBlank(password) . This would let a credential provider avoid loading credentials that have the wrong type. But I am not sure it is valid to omit the isNotBlank checks from the CQL expression like this. Devise a more complex CQL expression that does the equivalent of StringUtils.isNotBlank as well. This does not look feasible. (In contrast, StringUtils.isEmpty(username) could be expressed as username == null || username == "" .) I don't see where CredentialsMatchers.parse is called at all, except in CredentialsMatchersTest. Perhaps public credential-provider plugins do not actually use the CQL feature and only some proprietary plugin does. For that reason, I think removing the CredentialsMatcher.CQL implementation is the best option.

            kon Kalle Niemitalo
            kon Kalle Niemitalo
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: