Skip to content

Commit

Permalink
Merge pull request #208 from Wadeck/JENKINS-62761_certificate_upload_bug
Browse files Browse the repository at this point in the history 
[JENKINS-62761] Certificate upload correction
  • Loading branch information
@Wadeck
Wadeck authored May 12, 2021
2 parents 9b36231 + eae42c1 commit 80425d4
Show file tree
Hide file tree
Showing 7 changed files with 330 additions and 72 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
</scm>

<properties>
<revision>2.3.20</revision>
<revision>2.4</revision>
<changelist>-SNAPSHOT</changelist>
<jenkins.version>2.222.4</jenkins.version>
<java.level>8</java.level>
Expand Down
56 changes: 42 additions & 14 deletions src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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}.
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {

/**
Expand Down Expand Up @@ -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.");
}
}
}
Expand Down
28 changes: 5 additions & 23 deletions .../credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/complete.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,13 @@
~ THE SOFTWARE.
-->

<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler" xmlns:l="/lib/layout" >
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" >
<l:layout>
<l:main-panel>
<j:set var="instance" value="${it}"/>
<div style="display:none">
<f:form>
<f:entry>
<f:textbox id="content" field="uploadedKeystore" disabled="true"/>
</f:entry>
</f:form>
</div>
<script>
try {
var id = "${it.divId.replaceAll('[^id0-9]', '')}";
var newInput = document.getElementById('content');
var oldInput = window.opener.document.getElementById(id);
oldInput.value = newInput.value;
oldInput.onblur();
newInput = null;
oldInput = null;
} catch (e) {
// ignore
}
window.close();
</script>
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.
</l:main-panel>
</l:layout>
</j:jelly>
19 changes: 5 additions & 14 deletions ...ins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/Upload/index.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,13 @@
~ THE SOFTWARE.
-->

<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler" xmlns:l="/lib/layout" >
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" >
<l:layout norefresh="true">
<l:main-panel>
<h1>${%Upload PKCS#12 certificate}</h1>
<f:form action="upload" method="post" name="upload">
<f:entry>
<input type="file" name="certificate.file" size="40"/>
</f:entry>
<f:block>
<f:submit value="${%Upload}"/>
<st:nbsp/>
<input type="button" value="${%Cancel}"
onclick="window.close(); return false;"
class="submit-button"/>
</f:block>
</f:form>
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.
</l:main-panel>
</l:layout>
</j:jelly>
85 changes: 66 additions & 19 deletions ...s/plugins/credentials/impl/CertificateCredentialsImpl/UploadedKeyStoreSource/config.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,71 @@
~ THE SOFTWARE.
-->

<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler">
<j:set var="divId" value="${h.generateId()}"/>
<f:entry field="uploadedKeystore">
<f:textbox id="${divId}" style="display:none" checkMethod="post"/>
</f:entry>
<tr>
<td/>
<td>
</td>
<td align="right">
<input type="button" value="${%Upload certificate}..."
onclick="window.open('${rootURL}/descriptor/${descriptor.clazz.name}/upload/${divId}', 'upload-cert-${divId}','width=640,height=400').opener=window; return false;"
class="submit-button"/>
</td>
</tr>
<script> // workaround for JENKINS-19124
window.setTimeout(function(){
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
<j:set var="divId" value="${h.generateId()}"/>
<j:set var="fileId" value="${h.generateId()}"/>
<f:entry field="uploadedKeystore">
<!--
TODO We have to use a custom behavior as the ../password is not completely supported,
the registerValidator being called before applyNameRef in hudson-behavior, we cannot rely on the built-in feature
Could be simplified when https://issues.jenkins.io/browse/JENKINS-65616 is corrected.
-->

<!-- $$ => $ after jelly interpretation -->
<f:textbox id="${divId}" style="display:none" default="${descriptor.DEFAULT_VALUE}" checkMethod="post"
checkUrl="'${rootURL}/descriptorByName/com.cloudbees.plugins.credentials.impl.CertificateCredentialsImpl$$UploadedKeyStoreSource/checkUploadedKeystore?'+fillValues_${divId}(this)"
/>
</f:entry>
<f:entry field="uploadedCertFile">
<input id="${fileId}" type="file" name="uploadedCertFile" class="setting-input" jsonAware="true" />
</f:entry>
<script><![CDATA[
var uploadedCertFile_${fileId} = '';
(function(){
// Adding a onChange method on the file input to retrieve the value of the file content in a variable
var uploadedCertFileInput = window.document.getElementById("${fileId}");
var _onchange = uploadedCertFileInput.onchange;
if(typeof _onchange === "function") {
uploadedCertFileInput.onchange = function() { fileOnChange(this); _onchange.call(this); }
} else {
uploadedCertFileInput.onchange = fileOnChange.bind(uploadedCertFileInput);
}
function fileOnChange() {
try { // inspired by https://stackoverflow.com/a/754398
var uploadedCertFileInputFile = uploadedCertFileInput.files[0];
var reader = new FileReader();
reader.onload = function (evt) {
uploadedCertFile_${fileId} = btoa(evt.target.result);
var uploadedKeystore = document.getElementById("${divId}");
uploadedKeystore.onchange(uploadedKeystore);
}
reader.onerror = function (evt) {
if (window.console !== null) {
console.warn("Error during loading uploadedCertFile content", evt);
}
uploadedCertFile_${fileId} = '';
}
reader.readAsBinaryString(uploadedCertFileInputFile);
}
catch(e){
if (window.console !== null) {
console.warn("Unable to retrieve uploadedCertFile content");
}
}
}
})();
function fillValues_${divId}(el) {
var value = el.value;
var password = findNextFormItem(el, 'password').value;
var uploadedCertFile = uploadedCertFile_${fileId};
return "value="+encodeURIComponent(value)+"&password="+encodeURIComponent(password)+"&uploadedCertFile="+encodeURIComponent(uploadedCertFile);
}
// workaround for JENKINS-19124
// without this script, the password changes will be not trigger the check on the uploadedKeystore
window.setTimeout(function(){
var r = window.document.getElementById("${divId}");
var p = findNextFormItem(r, 'password');
if (p) {
Expand All @@ -51,10 +99,9 @@
} else {
p.onchange = function() { _field.onchange(_field); };
}
_field.onchange(_field);
}
r = null;
p = null;
}, 500);
</script>
]]></script>
</j:jelly>
Loading

0 comments on commit 80425d4

Please sign in to comment.