Favour Secret tokens over String tokens

[mezpahlan]

Deprecates the use of String tokens 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.

This is an alternative to #59 . I'll be happy to accept one or the other. I don't think we need both.

[mezpahlan]

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.

This version uses your suggestion of simply changing over to use Secrets from String. Much simpler! I wish I had done this sooner.

[mezpahlan]

Hi @daniel-beck any final thoughts on this before I merge it in? What would you recommend to put in the changelog on the wiki page? Just something to say users should consider their credentials compromised and to change them? Or something else? Many thanks.

[daniel-beck]

Most of the content of this patch is unnecessary. You can change the field type, and the next time this gets saved, the secret will be encrypted on disk. Change the form field type and a few getter/setter types and you should be done.

[daniel-beck]

Just something to say users should consider their credentials compromised and to change them? Or something else?

Depends on the setup. If there are other users with local disk access, or backups that exclude secrets/ have been compromised, then perhaps. But in general, would just summarize as in the advisory and let users decide what it means for their setup. I would expect it to not actually matter for most of them.

[mezpahlan]

Thanks, this plugin has some odd legacy use cases with default tokens that I really struggled with. If there is unnecessary stuff in the PR, at this stage, I don't mind. I want to deprecate this plugin because the service is being shutdown at the end of the year.

If anyone else wants to spend time wrestling with this code base then I'd welcome a hand but for now I'm done.

Anyways, thanks for the reviews. Appreciate it.

[njtman]

What does the new Jenkins pipeline API look like? We upgraded our plugin and the build is now failing. It appears apiToken changed to apiTokenSecret, but what is the appropriate format? Can you provide a sample demonstrating how one would pull a secret from the Jenkins credentials plugin and then pass it to this plugin?

[mezpahlan]

@njtman If you can roll back then give that a go. Clearly I don't understand Jenkins as I thought. I thought changing the type to Secret was transparent to using a String (which does seem to be the case for Free style jobs) but not for pipelines.

I'll look into a fix over the weekend.

Apologies.

[njtman]

@mezpahlan Thanks for the response. Yes, we did rollback and everything is running normally.

Before rolling back, we did get this to work by changing apiToken to apiTokenSecret and changing the value passed from type string to type hudson.util.Secret. This is not ideal however because we needed to globally whitelist the staticMethod hudson.util.Secret fromString java.lang.String signature and change our pipeline code to conform to the new API.

Ideally, the interface should remain the same and only the internal implementation should change. I discovered the interface change after my pipeline failed and I tried regenerating the pipeline script from the Snippet Generator -> hockeyApp: Upload to HockeyApp page on my Jenkins instance.