diff --git a/pom.xml b/pom.xml index 5ccdb7bdb..4b62e4093 100644 --- a/pom.xml +++ b/pom.xml @@ -65,7 +65,7 @@ - 2.3.20 + 2.4 -SNAPSHOT 2.222.4 8 diff --git a/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java b/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java index ba48c86e3..96e4f0909 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java +++ b/src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java @@ -35,7 +35,6 @@ import hudson.model.Descriptor; import hudson.model.Items; import hudson.util.FormValidation; -import hudson.util.HttpResponses; import hudson.util.IOUtils; import hudson.util.Secret; import java.io.ByteArrayInputStream; @@ -45,6 +44,7 @@ import java.io.InputStream; import java.io.ObjectStreamException; import java.io.Serializable; +import java.nio.charset.StandardCharsets; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; @@ -68,6 +68,7 @@ import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.interceptor.RequirePOST; public class CertificateCredentialsImpl extends BaseStandardCredentials implements StandardCertificateCredentials { @@ -445,13 +446,32 @@ public UploadedKeyStoreSource(String uploadedKeystore) { * Our constructor. * * @param uploadedKeystore the keystore content. + * @deprecated */ @SuppressWarnings("unused") // by stapler - @DataBoundConstructor + @Deprecated public UploadedKeyStoreSource(SecretBytes uploadedKeystore) { this.uploadedKeystoreBytes = uploadedKeystore; } + /** + * Constructor able to receive file directly + * + * @param uploadedCertFile the keystore content from the file upload + * @param uploadedKeystore the keystore encrypted data, in case the file is not uploaded (e.g. update of the password / description) + */ + @SuppressWarnings("unused") // by stapler + @DataBoundConstructor + public UploadedKeyStoreSource(FileItem uploadedCertFile, SecretBytes uploadedKeystore) { + if (uploadedCertFile != null) { + byte[] fileBytes = uploadedCertFile.get(); + if (fileBytes.length != 0) { + uploadedKeystore = SecretBytes.fromBytes(fileBytes); + } + } + this.uploadedKeystoreBytes = uploadedKeystore; + } + /** * Migrate to the new field. * @@ -513,6 +533,7 @@ public String toString() { */ @Extension public static class DescriptorImpl extends KeyStoreSourceDescriptor { + public static final String DEFAULT_VALUE = UploadedKeyStoreSource.class.getName() + ".default-value"; /** * Decode the {@link Base64} keystore wrapped in a {@link Secret}. @@ -565,12 +586,24 @@ public String getDisplayName() { */ @SuppressWarnings("unused") // stapler form validation @Restricted(NoExternalUse.class) + @RequirePOST public FormValidation doCheckUploadedKeystore(@QueryParameter String value, + @QueryParameter String uploadedCertFile, @QueryParameter String password) { + // Priority for the file, to cover the (re-)upload cases + if (StringUtils.isNotEmpty(uploadedCertFile)) { + byte[] uploadedCertFileBytes = Base64.getDecoder().decode(uploadedCertFile.getBytes(StandardCharsets.UTF_8)); + return validateCertificateKeystore("PKCS12", uploadedCertFileBytes, password); + } + if (StringUtils.isBlank(value)) { return FormValidation.error(Messages.CertificateCredentialsImpl_NoCertificateUploaded()); } + if (DEFAULT_VALUE.equals(value)) { + return FormValidation.ok(); + } + // If no file, we rely on the previous value, stored as SecretBytes in an hidden input SecretBytes secretBytes = SecretBytes.fromString(value); byte[] keystoreBytes = secretBytes.getPlainData(); if (keystoreBytes == null || keystoreBytes.length == 0) { @@ -596,7 +629,11 @@ public Upload getUpload(String divId) { /** * Stapler binding object to handle a pop-up window for file upload. + * + * @deprecated since 2.4. This is no longer required/supported due to the inlining of the file input. + * Deprecated for removal soon. */ + @Deprecated public static class Upload { /** @@ -656,18 +693,9 @@ public SecretBytes getUploadedKeystore() { */ @NonNull public HttpResponse doUpload(@NonNull StaplerRequest req) throws ServletException, IOException { - FileItem file = req.getFileItem("certificate.file"); - if (file == null) { - throw new ServletException("no file upload"); - } - // Here is the trick, if we have a successful upload we replace ourselves in the stapler view - // with an instance that has the uploaded content and request stapler to render the "complete" - // view for that instance. The "complete" view can then do the injection and close itself so that - // the user experience is the pop-up then click upload and finally we inject back in the content to - // the form. - SecretBytes uploadedKeystore = SecretBytes.fromBytes(file.get()); - return HttpResponses.forwardToView( - new Upload(getDivId(), uploadedKeystore), "complete"); + return FormValidation.ok("This endpoint is no longer required/supported due to the inlining of the file input. " + + "If you came to this endpoint due to another plugin, you will have to update that plugin to be compatible with Credentials Plugin 2.4+. " + + "It will be deleted soon."); } } } diff --git a/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/complete.jelly b/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/complete.jelly index 9f1dfcef8..c03fd9134 100644 --- a/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/complete.jelly +++ b/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/complete.jelly @@ -24,31 +24,13 @@ ~ THE SOFTWARE. --> - + - -
- - - - - -
- + View no longer required/supported due to the inlining of the file input. + If you came to this page due to another plugin, you will have to update that plugin to be compatible + with Credentials Plugin 2.4+ + It will be deleted soon. \ No newline at end of file diff --git a/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/index.jelly b/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/index.jelly index 98938ab58..2c3832479 100644 --- a/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/index.jelly +++ b/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/index.jelly @@ -24,22 +24,13 @@ ~ THE SOFTWARE. --> - + -

${%Upload PKCS#12 certificate}

- - - - - - - - - - + View no longer required/supported due to the inlining of the file input. + If you came to this page due to another plugin, you will have to update that plugin to be compatible + with Credentials Plugin 2.4+ + It will be deleted soon. \ No newline at end of file diff --git a/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/config.jelly b/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/config.jelly index 9516c5fe3..876196522 100644 --- a/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/config.jelly +++ b/src/main/resources/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/config.jelly @@ -24,23 +24,71 @@ ~ THE SOFTWARE. --> - - - - - - - - - - - - - + ]]> diff --git a/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java index a50bf39c8..f0b9c1f6f 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java @@ -33,11 +33,28 @@ import com.cloudbees.plugins.credentials.SecretBytes; import com.cloudbees.plugins.credentials.SystemCredentialsProvider; import com.cloudbees.plugins.credentials.common.CertificateCredentials; +import com.cloudbees.plugins.credentials.common.StandardCertificateCredentials; import com.cloudbees.plugins.credentials.domains.Domain; +import com.gargoylesoftware.htmlunit.FormEncodingType; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.Page; +import com.gargoylesoftware.htmlunit.WebRequest; +import com.gargoylesoftware.htmlunit.html.DomNode; +import com.gargoylesoftware.htmlunit.html.DomNodeList; +import com.gargoylesoftware.htmlunit.html.HtmlElementUtil; +import com.gargoylesoftware.htmlunit.html.HtmlFileInput; +import com.gargoylesoftware.htmlunit.html.HtmlForm; +import com.gargoylesoftware.htmlunit.html.HtmlOption; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import com.gargoylesoftware.htmlunit.html.HtmlRadioButtonInput; import hudson.FilePath; +import hudson.Util; import hudson.cli.CLICommandInvoker; import hudson.cli.UpdateJobCommand; +import hudson.model.ItemGroup; import hudson.model.Job; +import hudson.security.ACL; +import hudson.util.Secret; import jenkins.model.Jenkins; import org.apache.commons.io.FileUtils; import org.junit.Before; @@ -50,7 +67,11 @@ import java.io.File; import java.io.IOException; +import java.net.URL; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.util.Base64; import java.util.Collections; import java.util.List; @@ -69,18 +90,27 @@ public class CertificateCredentialsImplTest { public TemporaryFolder tmp = new TemporaryFolder(); private File p12; + private File p12Invalid; + + private static final String VALID_PASSWORD = "password"; + private static final String INVALID_PASSWORD = "blabla"; + private static final String EXPECTED_DISPLAY_NAME = "EMAILADDRESS=me@myhost.mydomain, CN=pkcs12, O=Fort-Funston, L=SanFrancisco, ST=CA, C=US"; @Before public void setup() throws IOException { p12 = tmp.newFile("test.p12"); FileUtils.copyURLToFile(CertificateCredentialsImplTest.class.getResource("test.p12"), p12); + p12Invalid = tmp.newFile("invalid.p12"); + FileUtils.copyURLToFile(CertificateCredentialsImplTest.class.getResource("invalid.p12"), p12Invalid); + + r.jenkins.setCrumbIssuer(null); } @Test public void displayName() throws IOException { SecretBytes uploadedKeystore = SecretBytes.fromBytes(Files.readAllBytes(p12.toPath())); CertificateCredentialsImpl.UploadedKeyStoreSource storeSource = new CertificateCredentialsImpl.UploadedKeyStoreSource(uploadedKeystore); - assertEquals("EMAILADDRESS=me@myhost.mydomain, CN=pkcs12, O=Fort-Funston, L=SanFrancisco, ST=CA, C=US", CredentialsNameProvider.name(new CertificateCredentialsImpl(null, "abc123", null, "password", storeSource))); + assertEquals(EXPECTED_DISPLAY_NAME, CredentialsNameProvider.name(new CertificateCredentialsImpl(null, "abc123", null, "password", storeSource))); } @Test @@ -178,6 +208,184 @@ public void verifyUserWithRunScriptsCanUploadMasterKeySource() throws Exception assertThat(reloadedSource, instanceOf(CertificateCredentialsImpl.UploadedKeyStoreSource.class)); } + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_uploadedFileValid() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore("", getValidP12_base64(), VALID_PASSWORD); + assertThat(content, containsString("ok")); + assertThat(content, containsString(EXPECTED_DISPLAY_NAME)); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_uploadedFileValid_encryptedPassword() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore("", getValidP12_base64(), Secret.fromString(VALID_PASSWORD).getEncryptedValue()); + assertThat(content, containsString("ok")); + 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 { + String content = getContentFrom_doCheckUploadedKeystore("", getValidP12_base64(), INVALID_PASSWORD); + assertThat(content, containsString("warning")); + assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_LoadKeystoreFailed()))); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_uploadedFileInvalid() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore("", getInvalidP12_base64(), VALID_PASSWORD); + assertThat(content, containsString("warning")); + assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_LoadKeystoreFailed()))); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_keyStoreBlank() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore("", "", VALID_PASSWORD); + assertThat(content, containsString("error")); + assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_NoCertificateUploaded()))); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_keyStoreDefault() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore(CertificateCredentialsImpl.UploadedKeyStoreSource.DescriptorImpl.DEFAULT_VALUE, "", VALID_PASSWORD); + assertThat(content, not(allOf(containsString("warning"), containsString("error")))); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_keyStoreInvalidSecret() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore("", "", VALID_PASSWORD); + assertThat(content, containsString("error")); + assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_NoCertificateUploaded()))); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_keyStoreValid() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore(getValidP12_secretBytes(), "", VALID_PASSWORD); + assertThat(content, containsString("ok")); + assertThat(content, containsString(EXPECTED_DISPLAY_NAME)); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_keyStoreValid_encryptedPassword() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore(getValidP12_secretBytes(), "", Secret.fromString(VALID_PASSWORD).getEncryptedValue()); + assertThat(content, containsString("ok")); + assertThat(content, containsString(EXPECTED_DISPLAY_NAME)); + } + + @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")))); + } + + @Test + @Issue("JENKINS-64542") + public void doCheckUploadedKeystore_keyStoreInvalid() throws Exception { + String content = getContentFrom_doCheckUploadedKeystore(getInvalidP12_secretBytes(), "", VALID_PASSWORD); + assertThat(content, containsString("warning")); + assertThat(content, containsString(Util.escape(Messages.CertificateCredentialsImpl_LoadKeystoreFailed()))); + } + + @Test + @Issue("JENKINS-63761") + public void fullSubmitOfUploadedKeystore() throws Exception { + String certificateDisplayName = r.jenkins.getDescriptor(CertificateCredentialsImpl.class).getDisplayName(); + + JenkinsRule.WebClient wc = r.createWebClient(); + HtmlPage htmlPage = wc.goTo("credentials/store/system/domain/_/newCredentials"); + HtmlForm newCredentialsForm = htmlPage.getFormByName("newCredentials"); + + DomNodeList allOptions = htmlPage.getDocumentElement().querySelectorAll("select.dropdownList option"); + boolean optionFound = allOptions.stream().anyMatch(domNode -> { + if (domNode instanceof HtmlOption) { + HtmlOption option = (HtmlOption) domNode; + if (option.getVisibleText().equals(certificateDisplayName)) { + try { + HtmlElementUtil.click(option); + } catch (IOException e) { + throw new RuntimeException(e); + } + return true; + } + } + + return false; + }); + assertTrue("The Certificate option was not found in the credentials type select", optionFound); + + HtmlRadioButtonInput keyStoreRadio = htmlPage.getDocumentElement().querySelector("input[name$=keyStoreSource]"); + HtmlElementUtil.click(keyStoreRadio); + + HtmlFileInput uploadedCertFileInput = htmlPage.getDocumentElement().querySelector("input[type=file][name=uploadedCertFile]"); + uploadedCertFileInput.setFiles(p12); + + // for all the types of credentials + newCredentialsForm.getInputsByName("_.password").forEach(input -> input.setValueAttribute(VALID_PASSWORD)); + htmlPage.getDocumentElement().querySelector("input[type=file][name=uploadedCertFile]"); + + List certificateCredentials = CredentialsProvider.lookupCredentials(CertificateCredentials.class, (ItemGroup) null, ACL.SYSTEM); + assertThat(certificateCredentials, hasSize(0)); + + r.submit(newCredentialsForm); + + certificateCredentials = CredentialsProvider.lookupCredentials(CertificateCredentials.class, (ItemGroup) null, ACL.SYSTEM); + assertThat(certificateCredentials, hasSize(1)); + + CertificateCredentials certificate = certificateCredentials.get(0); + String displayName = StandardCertificateCredentials.NameProvider.getSubjectDN(certificate.getKeyStore()); + assertEquals(EXPECTED_DISPLAY_NAME, displayName); + } + + private String getValidP12_base64() throws Exception { + return Base64.getEncoder().encodeToString(Files.readAllBytes(p12.toPath())); + } + + private String getValidP12_secretBytes() throws Exception { + return SecretBytes.fromBytes(Files.readAllBytes(p12.toPath())).toString(); + } + + private String getInvalidP12_base64() throws Exception { + return Base64.getEncoder().encodeToString(Files.readAllBytes(p12Invalid.toPath())); + } + + private String getInvalidP12_secretBytes() throws Exception { + return SecretBytes.fromBytes(Files.readAllBytes(p12Invalid.toPath())).toString(); + } + + private String getContentFrom_doCheckUploadedKeystore(String value, String uploadedCertFile, String password) throws Exception { + String descriptorUrl = r.jenkins.getDescriptor(CertificateCredentialsImpl.UploadedKeyStoreSource.class).getDescriptorUrl(); + WebRequest request = new WebRequest(new URL(r.getURL() + descriptorUrl + "/checkUploadedKeystore"), HttpMethod.POST); + request.setEncodingType(FormEncodingType.URL_ENCODED); + request.setRequestBody( + "value="+URLEncoder.encode(value, StandardCharsets.UTF_8.name())+ + "&uploadedCertFile="+URLEncoder.encode(uploadedCertFile, StandardCharsets.UTF_8.name())+ + "&password="+URLEncoder.encode(password, StandardCharsets.UTF_8.name()) + ); + + JenkinsRule.WebClient wc = r.createWebClient(); + Page page = wc.getPage(request); + + String content = page.getWebResponse().getContentAsString(); + return content; + } + private CredentialsStore getFolderStore(Folder f) { Iterable stores = CredentialsProvider.lookupStores(f); CredentialsStore folderStore = null; diff --git a/src/test/resources/com/cloudbees/plugins/credentials/impl/invalid.p12 b/src/test/resources/com/cloudbees/plugins/credentials/impl/invalid.p12 new file mode 100644 index 000000000..ae717573c --- /dev/null +++ b/src/test/resources/com/cloudbees/plugins/credentials/impl/invalid.p12 @@ -0,0 +1,2 @@ +0� +blablabla \ No newline at end of file