Merge pull request #581 from timja/azure-pfx

Handle password with empty string for certificate auth
jenkinsci · Dec 10, 2024 · 6017143 · 6017143
2 parents f51f5d5 + d0177a1
commit 6017143
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 22 deletions.
diff --git a/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java b/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java
@@ -147,12 +147,10 @@ public CertificateCredentialsImpl(@CheckForNull CredentialsScope scope,
      * Helper to convert a {@link Secret} password into a {@code char[]}
      *
      * @param password the password.
-     * @return a {@code char[]} containing the password or {@code null}
+     * @return a {@code char[]} containing the password
      */
-    @CheckForNull
     private static char[] toCharArray(@NonNull Secret password) {
-        String plainText = Util.fixEmpty(password.getPlainText());
-        return plainText == null ? null : plainText.toCharArray();
+        return password.getPlainText().toCharArray();
     }
 
     /**
@@ -248,7 +246,7 @@ public FormValidation doCheckPassword(@QueryParameter String value) {
                 return FormValidation.error(Messages.CertificateCredentialsImpl_ShortPasswordFIPS());
             }
             if (pw.isEmpty()) {
-                return FormValidation.warning(Messages.CertificateCredentialsImpl_NoPassword());
+                return FormValidation.ok(Messages.CertificateCredentialsImpl_NoPassword());
             }
             if (pw.length() < 14) {
                 return FormValidation.warning(Messages.CertificateCredentialsImpl_ShortPassword());
@@ -624,9 +622,7 @@ protected static FormValidation validateCertificateKeystore(byte[] keystoreBytes
                 } catch (KeyStoreException | CertificateException | NoSuchAlgorithmException | IOException e) {
                     return FormValidation.warning(e, Messages.CertificateCredentialsImpl_LoadKeystoreFailed());
                 } finally {
-                    if (passwordChars != null) {
-                        Arrays.fill(passwordChars, ' ');
-                    }
+                    Arrays.fill(passwordChars, ' ');
                 }
             }
 
@@ -739,6 +735,9 @@ public FormValidation doCheckCertChain(@QueryParameter String value) {
                     List<PEMEncodable> pemEncodables = PEMEncodable.decodeAll(pemCerts, null);
                     long count = pemEncodables.stream().map(PEMEncodable::toCertificate).filter(Objects::nonNull).count();
                     if (count < 1) {
+                        if (Util.fixEmpty(value) == null) {
+                            return FormValidation.ok();
+                        }
                         return FormValidation.error(Messages.CertificateCredentialsImpl_PEMNoCertificates());
                     }
                     // ensure only certs are provided.
@@ -771,6 +770,9 @@ public FormValidation doCheckPrivateKey(@QueryParameter String value,
                     List<PEMEncodable> pemEncodables = PEMEncodable.decodeAll(key, toCharArray(Secret.fromString(password)));
                     long count = pemEncodables.stream().map(PEMEncodable::toPrivateKey).filter(Objects::nonNull).count();
                     if (count == 0) {
+                        if (Util.fixEmpty(value) == null) {
+                            return FormValidation.ok();
+                        }
                         return FormValidation.error(Messages.CertificateCredentialsImpl_PEMNoKeys());
                     }
                     if (count > 1) {

diff --git a/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties b/src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties
@@ -24,8 +24,8 @@
 UsernamePasswordCredentialsImpl.DisplayName=Username with password
 CertificateCredentialsImpl.DisplayName=Certificate
 CertificateCredentialsImpl.EmptyKeystore=Empty keystore
-CertificateCredentialsImpl.LoadKeyFailed=Could retrieve key "{0}"
-CertificateCredentialsImpl.LoadKeyFailedQueryEmptyPassword=Could retrieve key "{0}". You may need to provide a password
+CertificateCredentialsImpl.LoadKeyFailed=Couldn''t retrieve key for alias "{0}"
+CertificateCredentialsImpl.LoadKeyFailedQueryEmptyPassword=Couldn''t retrieve key for alias "{0}". You may need to provide a password
 CertificateCredentialsImpl.LoadKeystoreFailed=Could not load keystore
 CertificateCredentialsImpl.NoCertificateUploaded=No certificate uploaded
 CertificateCredentialsImpl.UploadedKeyStoreSourceDisplayName=Upload PKCS#12 certificate and key

diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java
@@ -32,6 +32,7 @@
 import com.cloudbees.plugins.credentials.SecretBytes;
 import com.cloudbees.plugins.credentials.common.CertificateCredentials;
 import com.cloudbees.plugins.credentials.common.StandardCertificateCredentials;
+import hudson.util.FormValidation;
 import org.htmlunit.FormEncodingType;
 import org.htmlunit.HttpMethod;
 import org.htmlunit.Page;
@@ -128,14 +129,6 @@ public void doCheckUploadedKeystore_uploadedFileValid_encryptedPassword() throws
         assertThat(content, containsString(EXPECTED_DISPLAY_NAME));
     }
 
-    @Test
-    @Issue("JENKINS-64542")
-    public void doCheckUploadedKeystore_uploadedFileValid_butMissingPassword() throws Exception {
-        String content = getContentFrom_doCheckUploadedKeystore("", getValidP12_base64(), "");
-        assertThat(content, containsString("warning"));
-        assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_LoadKeyFailedQueryEmptyPassword("1"))));
-    }
-
     @Test
     @Issue("JENKINS-64542")
     public void doCheckUploadedKeystore_uploadedFileValid_butInvalidPassword() throws Exception {
@@ -193,10 +186,11 @@ public void doCheckUploadedKeystore_keyStoreValid_encryptedPassword() throws Exc
 
     @Test
     @Issue("JENKINS-64542")
-    public void doCheckUploadedKeystore_keyStoreValid_butMissingPassword() throws Exception {
-        String content = getContentFrom_doCheckUploadedKeystore(getValidP12_secretBytes(), "", "");
-        assertThat(content, containsString("warning"));
-        assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_LoadKeyFailedQueryEmptyPassword("1"))));
+    public void doCheckPassword_missing() {
+        CertificateCredentialsImpl.DescriptorImpl descriptor = r.jenkins.getDescriptorByType(CertificateCredentialsImpl.DescriptorImpl.class);
+        FormValidation formValidation = descriptor.doCheckPassword("");
+        assertThat(formValidation.kind, is(FormValidation.Kind.OK));
+        assertThat(formValidation.getMessage(), is(Messages.CertificateCredentialsImpl_NoPassword()));
     }
 
     @Test