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

Password in clear text #44

Closed
jetersen opened this issue Jan 24, 2018 · 6 comments
Closed

Password in clear text #44

jetersen opened this issue Jan 24, 2018 · 6 comments

Comments

@jetersen
Copy link
Member

The way you use credentials plugin is bad, you should never put the password in clear text, which you do several times through logging and in config xml 🙈 🙉 🙊
Working on a PR that fixes a lot of your jelly hacking and going to deprecate the use of username and password.

@jetersen
Copy link
Member Author

Takes the credentials and paste it into clear text

private static void setCredentialsIfExists(
final TaskListener listener, final ViolationsToBitbucketServerConfig configExpanded) {
if (configExpanded.isUseUsernamePasswordCredentials()) {
if (!isNullOrEmpty(configExpanded.getUsernamePasswordCredentialsId())) {
final Optional<StandardUsernamePasswordCredentials> credentials =
findCredentials(configExpanded.getUsernamePasswordCredentialsId());
if (credentials.isPresent()) {
final String username =
checkNotNull(
emptyToNull(credentials.get().getUsername()),
"Credentials username selected but not set!");
final String password =
checkNotNull(
emptyToNull(credentials.get().getPassword().getPlainText()),
"Credentials password selected but not set!");
configExpanded.setUsername(username);
configExpanded.setPassword(password);
listener.getLogger().println("Using username and password from credentials");
} else {
listener.getLogger().println("Credentials not found!");
return;
}
} else {
listener.getLogger().println("Credentials checked but not selected!");
return;
}
}
}

if I then go and save any configuration afterwards the password is now in clear text on the Jenkins instance

😭

logger.println(FIELD_PASSWORD + ": " + !isNullOrEmpty(config.getPassword()));

@tomasbjerre
Copy link
Contributor

tomasbjerre commented Jan 24, 2018

Nice! It all sounds very good!

I am generally a bit frustrated about the terrible API:s in Jenkins. And credentials is not an exception. Undocumented, lots of deprications and just very unclear how it should be used. Often feels like things are unnecessary complex... So your refactorings are more than welcome =)

Your findings probably needs fixing in these plugins as well:

@jetersen
Copy link
Member Author

jetersen commented Jan 24, 2018

@tomasbjerre after a little tweaking. I got this.
image

Current progress on simplified generator:

ViolationsToBitbucketServer([violationConfigs: [[parser: 'ANDROIDLINT', pattern: 'b', reporter: 'a'], [parser: 'CHECKSTYLE', pattern: 'b', reporter: 'a']]])

In the process of deprecating username, password 👍

@jetersen
Copy link
Member Author

whats the default behaviour between create one comment per violation and create one big comment with all violations

@tomasbjerre
Copy link
Contributor

I think the first one is most used. I would vote for that being default.

@tomasbjerre
Copy link
Contributor

This is now released in 1.65. Thanks again for 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

2 participants