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

Support treating usernames as non-secret #322

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

Conversation

xRokco
Copy link

@xRokco xRokco commented May 24, 2024

Adds support to treat usernames as non-secret for Username + Password creds. Uses a tag called "maskUsername" to control this option. Defaults to existing behaviour of masking usernames but if maskUsername tag is set to "false" the credential's username will be treated as non-secret, which will allow it to be printed in Jenkins logs, etc.

Gives this plugin the ability to take advantage of this fix: https://issues.jenkins.io/browse/JENKINS-44860

Testing done

Added some integration tests which creates an unmasked username credential and confirms that the username appears in logs, etc.

[INFO] Results:
[INFO] 
[INFO] Tests run: 97, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] 
[INFO] >>> spotbugs:4.8.3.1:check (spotbugs) > :spotbugs @ aws-secrets-manager-credentials-provider >>>
[INFO] 
[INFO] --- spotbugs:4.8.3.1:spotbugs (spotbugs) @ aws-secrets-manager-credentials-provider ---
[INFO] Fork Value is true
[INFO] Done SpotBugs Analysis....
[INFO] 
[INFO] <<< spotbugs:4.8.3.1:check (spotbugs) < :spotbugs @ aws-secrets-manager-credentials-provider <<<
[INFO] 
[INFO] 
[INFO] --- spotbugs:4.8.3.1:check (spotbugs) @ aws-secrets-manager-credentials-provider ---
[INFO] BugInstance size is 0
[INFO] Error size is 0
[INFO] No errors/warnings found
[INFO] 
[INFO] --- spotless:2.43.0:check (default) @ aws-secrets-manager-credentials-provider ---
[INFO] Spotless check skipped
[INFO] 
[INFO] --- failsafe:3.2.5:verify (default) @ aws-secrets-manager-credentials-provider ---
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  08:07 min
[INFO] Finished at: 2024-05-24T14:00:09+02:00
[INFO] ------------------------------------------------------------------------

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@xRokco xRokco changed the title Support treating usernames as non-secret with an AWS tag to allow Jenkins logs to print the username Support treating usernames as non-secret with an AWS tag May 24, 2024
@xRokco xRokco changed the title Support treating usernames as non-secret with an AWS tag Support treating usernames as non-secret May 24, 2024
@chriskilding
Copy link
Contributor

chriskilding commented May 29, 2024

Hi, thanks for your contribution :) I like the look of it, however I would favour implementing a new maskUsernames boolean option on the PluginConfiguration, which allows you to toggle username masking on/off at the plugin level, instead of using an AWS tag.

The option should default to off to match the existing default behaviour; this should be simple enough, because boolean options default to false if they are unset.

Would you be able to refactor your PR to accomplish this?

@xRokco
Copy link
Author

xRokco commented May 31, 2024

Hey,

Thanks for the feedback, I can look into that yeah. One question though:

I would favour implementing a new maskUsernames boolean option on the PluginConfiguration, which allows you to toggle username masking on/off at the plugin level, instead of using an AWS tag.

Does this mean having a setting that treats all usernames as secret or non-secret? i.e. not configurable per credential.

I like having the ability personally to configure this on a per credential basis, but maybe I'm not fully understanding the proposed refactor.

Or maybe you mean both mechanisms could be kept - add a PluginConfiguration option that sets the default maskUsernames behaviour, but allow some per credential setting (like an AWS tag) to override it.

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