[JENKINS-44860] `UsernameCredentials.isUsernameSecret` #205

jglick · 2021-05-06T17:38:24Z

jenkinsci/credentials-binding-plugin#131 (comment)

…need not be touched

into show-username-JENKINS-44860

jglick · 2021-05-06T22:40:58Z

src/main/java/com/cloudbees/plugins/credentials/CredentialsNameProvider.java

@@ -90,7 +90,11 @@ Result name(@NonNull Credentials credentials, @NonNull Class<?> clazz) {
        if (nameWith != null) {
            try {
                CredentialsNameProvider nameProvider = nameWith.value().newInstance();
-                return new Result(nameProvider.getName(credentials), nameWith.priority());
+                String name = nameProvider.getName(credentials);
+                if (!name.isEmpty()) {


Make it possible to just return credentials.getDescription() and if that happens to be the empty string, still behave reasonably (choose a different provider).

jglick · 2021-05-06T22:41:35Z

src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java

@@ -53,6 +55,9 @@
    @NonNull
    private final Secret password;

+    @Nullable
+    private Boolean usernameSecret = false;


Default false in the Java constructor which would be used only from tests or scripts.

jglick · 2021-05-06T22:43:37Z

src/main/java/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl.java

@@ -72,6 +77,13 @@ public UsernamePasswordCredentialsImpl(@CheckForNull CredentialsScope scope,
        this.password = Secret.fromString(password);
    }

+    private Object readResolve() {
+        if (usernameSecret == null) {
+            usernameSecret = true;


Default true for deserialized credentials, either from settings predating the upgrade, or REST uploads as XML.

jglick · 2021-05-06T22:43:55Z

...ces/com/cloudbees/plugins/credentials/impl/UsernamePasswordCredentialsImpl/credentials.jelly

@@ -28,6 +28,9 @@
  <f:entry title="${%Username}" field="username">
    <f:textbox/>
  </f:entry>
+  <f:entry field="usernameSecret">
+    <f:checkbox title="${%Treat username as secret}"/>


Default false for new credentials created from the GUI.

Wadeck

Manually tested:

✔️ Existing credentials are not changing behavior
✔️ New credentials let the checkbox uncheck (but this could be changed)

Of course, no masking until credentials-masking is merged as well.

Will merge this for next release

bitwiseman · 2021-05-12T16:39:20Z

@jglick @Wadeck
Why not default to usernameSecret=true in all cases?
We want to allow users to show user names, but what the reasoning behind making show the user name the default for UI and script?

jglick · 2021-05-12T16:47:29Z

@bitwiseman #205 (comment) for the GUI.

car-roll

I'm fine with making username not secret as default going forward since the behavior for legacy credentials is to keep them hidden. Either way works for me.

bitwiseman · 2021-05-13T01:31:40Z

@bitwiseman #205 (comment) for the GUI.

I don't understand. Yes, I know it defaults to false in GUI. Why does it default to false? The GUI couldn't default to true/checked?

jglick · 2021-05-13T14:13:06Z

I know it defaults to false in GUI. Why does it default to false?

Sorry, did not get your question. We expect the username to be nonsecret in most cases, and until now it was displayed in credentials dropdowns to anyone with view configuration permissions. In fact the originally submitted PRs just unconditionally began showing usernames in logs, merely by virtue of updating the credentials-binding plugin, which I was uncomfortable with. So here I am trying to walk a compromise path that poses no risk of unwanted surprise for an existing installation, while guiding new configurations to display the username.

into show-username-JENKINS-44860

jglick · 2021-05-13T14:18:09Z

pom.xml

@@ -65,7 +65,7 @@
  </scm>

  <properties>
-    <revision>2.4.2</revision>
+    <revision>2.5</revision>


Just proposing a version bump. Do we really need to keep the micro version? Can revert if desired.

bitwiseman · 2021-05-13T16:03:11Z

I know it defaults to false in GUI. Why does it default to false?

Sorry, did not get your question. We expect the username to be nonsecret in most cases, and until now it was displayed in credentials dropdowns to anyone with view configuration permissions. In fact the originally submitted PRs just unconditionally began showing usernames in logs, merely by virtue of updating the credentials-binding plugin, which I was uncomfortable with. So here I am trying to walk a compromise path that poses no risk of unwanted surprise for an existing installation, while guiding new configurations to display the username.

Understood. Sounds good, thanks for the explanation.

jglick · 2021-06-02T20:20:15Z

@bitwiseman @car-roll @Wadeck any updates on this & jenkinsci/credentials-binding-plugin#132 & jenkinsci/ssh-credentials-plugin#70?

jglick added 7 commits May 6, 2021 13:37

[JENKINS-44860] UsernameCredentials.isUsernameSecret

09a8622

Fixing some tests using DummyIdCredentials

6c8d6b6

usernameSecret = false in the Java constructor, so that most tests …

a2b5c5f

…need not be touched

CredentialsStoreActionTest.restCRUDSmokes

5c62332

Some fixes to CredentialsNameProvider behavior

f2df059

Inline help for UsernamePasswordCredentialsImpl.usernameSecret

4f2aa3d

Merge branch 'master' of https://github.com/jenkinsci/credentials-plugin

ba505d7

into show-username-JENKINS-44860

This was referenced May 6, 2021

[JENKINS-44860] BasicSSHUserPrivateKey.usernameSecret jenkinsci/ssh-credentials-plugin#70

Merged

[JENKINS-44860] Honor secret setting for username jenkinsci/credentials-binding-plugin#132

Merged

jglick added the enhancement label May 6, 2021

Switch to bom-managed version of configuration-as-code

3fce07b

jglick requested review from car-roll, daniel-beck and bitwiseman May 6, 2021 22:39

jglick commented May 6, 2021

View reviewed changes

Wadeck self-requested a review May 12, 2021 08:15

Wadeck approved these changes May 12, 2021

View reviewed changes

car-roll approved these changes May 12, 2021

View reviewed changes

Merge branch 'master' of https://github.com/jenkinsci/credentials-plugin

cc3d3b7

into show-username-JENKINS-44860

jglick commented May 13, 2021

View reviewed changes

bitwiseman approved these changes May 13, 2021

View reviewed changes

bitwiseman approved these changes Jun 2, 2021

View reviewed changes

jglick merged commit a1d8809 into jenkinsci:master Jun 2, 2021

jglick deleted the show-username-JENKINS-44860 branch June 2, 2021 22:19

jglick mentioned this pull request Jun 4, 2021

Initialize new LTS line (2.289) jenkinsci/bom#511

Merged

6 tasks

amuniz mentioned this pull request Jun 11, 2021

[JENKINS-44860] Fixes for credentials:2.5 jenkinsci/acceptance-test-harness#671

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-44860] `UsernameCredentials.isUsernameSecret` #205

[JENKINS-44860] `UsernameCredentials.isUsernameSecret` #205

jglick commented May 6, 2021

jglick May 6, 2021

jglick May 6, 2021

jglick May 6, 2021

jglick May 6, 2021

Wadeck left a comment

bitwiseman commented May 12, 2021 •

edited

Loading

jglick commented May 12, 2021

car-roll left a comment •

edited

Loading

bitwiseman commented May 13, 2021

jglick commented May 13, 2021

jglick May 13, 2021

bitwiseman commented May 13, 2021

jglick commented Jun 2, 2021

[JENKINS-44860] UsernameCredentials.isUsernameSecret #205

[JENKINS-44860] UsernameCredentials.isUsernameSecret #205

Conversation

jglick commented May 6, 2021

jglick May 6, 2021

Choose a reason for hiding this comment

jglick May 6, 2021

Choose a reason for hiding this comment

jglick May 6, 2021

Choose a reason for hiding this comment

jglick May 6, 2021

Choose a reason for hiding this comment

Wadeck left a comment

Choose a reason for hiding this comment

bitwiseman commented May 12, 2021 • edited Loading

jglick commented May 12, 2021

car-roll left a comment • edited Loading

Choose a reason for hiding this comment

bitwiseman commented May 13, 2021

jglick commented May 13, 2021

jglick May 13, 2021

Choose a reason for hiding this comment

bitwiseman commented May 13, 2021

jglick commented Jun 2, 2021

[JENKINS-44860] `UsernameCredentials.isUsernameSecret` #205

[JENKINS-44860] `UsernameCredentials.isUsernameSecret` #205

bitwiseman commented May 12, 2021 •

edited

Loading

car-roll left a comment •

edited

Loading