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

Feature/token #187

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Feature/token #187

wants to merge 9 commits into from

Conversation

reftel
Copy link

@reftel reftel commented Dec 21, 2023

Description

See JENKINS-66449.

This change lets users pick StringCredentials in addition to UsernamePasswordCredentials in the credentials drop-down in the settings. This makes the SigningInterceptor send the secret of the credential as a bearer token, instead of using username and password for Basic Authentication. See https://confluence.atlassian.com/enterprise/using-personal-access-tokens-1026032365.html for the documentation on the Jira side.

There are automated tests in SingningInterceptorTest.

This can be tested manually by adding a Personal Access Token on a Jira instance, and add that as a String Credential in Jenkins. Then add a new site in the settings of jira-steps-plugin for that Jira instance, and choose Credentials as login type, and pick the new credential in the drop-down. Verify that a pipeline can run Jira steps using the new site.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description.
  • Appropriate unit or acceptance tests or explanation to why this change has no tests.
  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • Run the changes and verified the change matches the issue description.
  • Reviewed the code.
  • Verified that the appropriate tests have been written or valid explanation given.
  • If applicable, tested by installing this plugin on the Jenkins instance.

The code was already correct, but Spotbugs did not know that the methods
called were pure, and therefore thoought they might return null when
called a second time. This change enables Spotbugs to reason correctly
about the code.
Earlier, the settings page on Jenkins would crash if there were no other
plugins that pulled in okhttp3.
Makes changes to add new types of credentials less invasive, thus easier
to review.
This change lets users pick `StringCredentials` in addition to
`UsernamePasswordCredentials` in the credentials drop-down in the
settings. This makes the `SigningInterceptor` send the secret of the
credential as a bearer token, instead of using username and password for
Basic Authentication. See
https://confluence.atlassian.com/enterprise/using-personal-access-tokens-1026032365.html
for the documentation on the Jira side.
@reftel
Copy link
Author

reftel commented Jan 4, 2024

Ping @nrayapati

1 similar comment
@reftel
Copy link
Author

reftel commented Jan 22, 2024

Ping @nrayapati

@jiricermak-sap
Copy link

@nrayapati can you please review?

@KylanZhang
Copy link

@nrayapati @nfalco79 Can you review this PR? This is a very useful feature.

@faminepq
Copy link

i'm already using code from this PR for some time, and eveything works fine for me - is there any chance to see it merged into master?

@nfalco79
Copy link
Member

nfalco79 commented May 23, 2024

I'm not the mantainer, so my code review is useless to proceed with merge

@akulakhan
Copy link

This is critically needed, my org has forced a move to disabling basic auth, and oauth is not supported, so this would be the only way to continue using this plugin.

@faminepq
Copy link

This is critically needed, my org has forced a move to disabling basic auth, and oauth is not supported, so this would be the only way to continue using this plugin.

I'm in the same situation, had to locally build code from the fork and upload it manually... Anyway seems like this plugin is abandoned :-( @nrayapati

@faminepq
Copy link

@reftel After upgrading Jenkins to latest LTS - i've started getting Parameter specified as non-null is null: method okhttp3.Credentials.basic, parameter username (also for Test Connection button click). Have not updated jenkins for cca 2 months, previously everything worked fine. Any tip what might be the problem? Everything is set as before (i'm restoring configs&secrets from backup).

@reftel
Copy link
Author

reftel commented Jun 20, 2024

Looks to me like the only place where that method is called is when setting up proxy authentication. You should be able to get that exception if you have proxyConfiguration being non-null, but proxyConfiguration.getUserName() returning null. Are you using a proxy that requires authentication? If not, could you please check if you have a partially filled in proxy authentication configuration ("HTTP Proxy Configuration" in the global Jenkins configuration, not specific to Jira Steps) that you could delete?

@faminepq
Copy link

faminepq commented Jun 20, 2024

Thank you for quick response. We use/need proxy + port + no_proxy settings, but we're not using username & password. Alas, i cannot delete all proxy settings, since we need it.

In Jenkins changelog i've found this change, which might be related to the problem: https://github.com/jenkinsci/jenkins/pull/8990/files

Anyway once i've removed on our test server all proxy settings, it started working... but proxy settings are needed for other urls.

@faminepq
Copy link

Screenshot 2024-06-21 at 10 08 10 So the problem is not in your PR, it's for another PR... Change in screenshot above fixed the issue for me. So it was really caused by this jenkins change: https://github.com/jenkinsci/jenkins/pull/8990

@reftel
Copy link
Author

reftel commented Jun 21, 2024

Fixed in 8c4a487 . Thank you!

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

Successfully merging this pull request may close these issues.

6 participants