Skip to content

Add support for Credentials plugin#59

Closed
mezpahlan wants to merge 38 commits intojenkinsci:masterfrom
mezpahlan:bugfix/JENKINS-57005
Closed

Add support for Credentials plugin#59
mezpahlan wants to merge 38 commits intojenkinsci:masterfrom
mezpahlan:bugfix/JENKINS-57005

Conversation

@mezpahlan
Copy link

Attempt to add support for Credentials handling as the old method stored credentials in plain text.

Note: This plugin is to be deprecated soon as the HockeyApp service is going to be closed at the end of the year. So I don't particularly mind if the implementation could be better. The whole plugin is going to be re-written anyway.

I have attempted to provide a migration mechanism for free style jobs so am interested if that implementation works. I haven't found a way to do that for Pipeline jobs so will list some instructions on the release page after merging this.

Based on a similar class from the HipChat plugin. Helps with migrations
and credential look ups.
This was we can use it in Global and Job migration modes.
createHostUrl does the same thing as getBaseUrl but with a variable
expansion. Rather than confuse the class with different ways of doing
things we can refactor like this and simplify.
@mezpahlan
Copy link
Author

Hi @daniel-beck this is the patch for SECURITY-839 that I didn't get time to do in March. Would you mind giving it a quick review? Also @jenkinsci/code-reviewers if anyone has time.

Just some notes on this, I am looking to deprecate this plugin as the backend service (HockeyApp) is coming to its end of life so I am primarily concerned about keeping existing users happy and not breaking anything for them (or breaking as little as possible) rather than a bigger code clean up.

@daniel-beck
Copy link
Member

@mezpahlan If you just want to fix the security issue we published as easily as possible, I recommend switching the type of the access token field from String to Secret (and adapt everything that touches it), see https://javadoc.jenkins.io/hudson/util/Secret.html

Nothing wrong with migrating to credentials, of course. If you still want to proceed with credentials integration, please respond and I'll give this PR a closer look.

@mezpahlan
Copy link
Author

Now I feel stupid 😭.

@daniel-beck Let me see how it would look like to simply migrate the String token to Secret and I'll then update if I want this PR to continue.

@mezpahlan
Copy link
Author

Closed in favour of #60

@mezpahlan mezpahlan closed this Apr 30, 2019
@mezpahlan mezpahlan deleted the bugfix/JENKINS-57005 branch May 3, 2019 21:46
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.

2 participants