Skip to content

Commit f7447bf

Browse files
committed
[JENKINS-74964] Fix: Display error message when adding invalid certificate credentials
1 parent f51f5d5 commit f7447bf

File tree

7 files changed

+87
-7
lines changed

7 files changed

+87
-7
lines changed

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.kohsuke.args4j.Argument;
5858
import org.kohsuke.args4j.CmdLineException;
5959
import org.kohsuke.args4j.Localizable;
60+
import org.kohsuke.stapler.HttpResponses;
6061
import org.kohsuke.stapler.Stapler;
6162
import org.kohsuke.stapler.StaplerRequest2;
6263
import org.kohsuke.stapler.StaplerResponse2;
@@ -608,8 +609,14 @@ public JSONObject doAddCredentials(StaplerRequest2 req, StaplerResponse2 rsp) th
608609
.element("notificationType", "ERROR");
609610
}
610611
store.checkPermission(CredentialsStoreAction.CREATE);
611-
Credentials credentials = Descriptor.bindJSON(req, Credentials.class, data.getJSONObject("credentials"));
612-
boolean credentialsWereAdded = store.addCredentials(wrapper.getDomain(), credentials);
612+
boolean credentialsWereAdded;
613+
try {
614+
Credentials credentials = Descriptor.bindJSON(req, Credentials.class,
615+
data.getJSONObject("credentials"));
616+
credentialsWereAdded = store.addCredentials(wrapper.getDomain(), credentials);
617+
} catch (HttpResponses.HttpResponseException e) {
618+
return new JSONObject().element("message", e.getMessage()).element("notificationType", "ERROR");
619+
}
613620
if (credentialsWereAdded) {
614621
return new JSONObject()
615622
.element("message", "Credentials created")

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,22 @@ public class CertificateCredentialsImpl extends BaseStandardCredentials implemen
129129
public CertificateCredentialsImpl(@CheckForNull CredentialsScope scope,
130130
@CheckForNull String id, @CheckForNull String description,
131131
@CheckForNull String password,
132-
@NonNull KeyStoreSource keyStoreSource) {
132+
@NonNull KeyStoreSource keyStoreSource) throws Descriptor.FormException {
133133
super(scope, id, description);
134134
Objects.requireNonNull(keyStoreSource);
135+
if (FIPS140.useCompliantAlgorithms() && StringUtils.isNotBlank(password) && password.length() < 14) {
136+
throw new Descriptor.FormException(Messages.CertificateCredentialsImpl_ShortPasswordFIPS(), "password");
137+
}
135138
this.password = Secret.fromString(password);
136139
this.keyStoreSource = keyStoreSource;
137140
// ensure the keySore is valid
138141
// we check here as otherwise it will lead to hard to diagnose errors when used
139142
try {
140143
keyStoreSource.toKeyStore(toCharArray(this.password));
141144
} catch (GeneralSecurityException | IOException e) {
142-
throw new IllegalArgumentException("KeyStore is not valid.", e);
145+
LOGGER.log(Level.WARNING, Messages.CertificateCredentialsImpl_InvalidKeystore(), e);
146+
throw new Descriptor.FormException(Messages.CertificateCredentialsImpl_InvalidKeystore(), e,
147+
"keyStoreSource");
143148
}
144149
}
145150

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@
4242
import org.kohsuke.stapler.QueryParameter;
4343
import org.kohsuke.stapler.interceptor.RequirePOST;
4444

45-
import java.util.Objects;
46-
4745
/**
4846
* Concrete implementation of {@link StandardUsernamePasswordCredentials}.
4947
*

src/main/resources/com/cloudbees/plugins/credentials/impl/Messages.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
UsernamePasswordCredentialsImpl.DisplayName=Username with password
2525
CertificateCredentialsImpl.DisplayName=Certificate
2626
CertificateCredentialsImpl.EmptyKeystore=Empty keystore
27+
CertificateCredentialsImpl.InvalidKeystore=KeyStore is not valid
2728
CertificateCredentialsImpl.LoadKeyFailed=Could retrieve key "{0}"
2829
CertificateCredentialsImpl.LoadKeyFailedQueryEmptyPassword=Could retrieve key "{0}". You may need to provide a password
2930
CertificateCredentialsImpl.LoadKeystoreFailed=Could not load keystore

src/main/resources/lib/credentials/select/select.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ window.credentials.addSubmit = function (_) {
123123
.catch((e) => {
124124
// notificationBar.show(...) with logging ID could be handy here?
125125
console.error("Could not add credentials:", e);
126+
window.notificationBar.show("Credentials creation failed.", window.notificationBar["ERROR"]);
126127
})
127128
}
128129
};

src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,32 @@
11
package com.cloudbees.plugins.credentials;
22

33
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.hasSize;
45
import static org.hamcrest.Matchers.is;
6+
import static org.junit.Assert.assertTrue;
57

68
import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials;
9+
import com.cloudbees.plugins.credentials.impl.CertificateCredentialsImpl;
10+
import com.cloudbees.plugins.credentials.impl.Messages;
711
import hudson.model.UnprotectedRootAction;
12+
import java.io.IOException;
813
import java.util.List;
14+
import net.sf.json.JSONObject;
915
import org.hamcrest.Matchers;
16+
import org.htmlunit.Page;
17+
import org.htmlunit.html.DomNode;
18+
import org.htmlunit.html.DomNodeList;
1019
import org.htmlunit.html.HtmlButton;
20+
import org.htmlunit.html.HtmlElementUtil;
1121
import org.htmlunit.html.HtmlForm;
22+
import org.htmlunit.html.HtmlFormUtil;
1223
import org.htmlunit.html.HtmlInput;
24+
import org.htmlunit.html.HtmlOption;
1325
import org.htmlunit.html.HtmlPage;
26+
import org.htmlunit.html.HtmlRadioButtonInput;
1427
import org.junit.Rule;
1528
import org.junit.Test;
29+
import org.jvnet.hudson.test.Issue;
1630
import org.jvnet.hudson.test.JenkinsRule;
1731
import org.jvnet.hudson.test.TestExtension;
1832

@@ -56,6 +70,59 @@ public void doAddCredentialsFromPopupWorksAsExpected() throws Exception {
5670
}
5771
}
5872

73+
@Test
74+
@Issue("JENKINS-74964")
75+
public void doAddCredentialsFromPopupForInvalidPEMCertificateKeystore() throws Exception {
76+
77+
try (JenkinsRule.WebClient wc = j.createWebClient()) {
78+
HtmlPage htmlPage = wc.goTo("credentials-selection");
79+
80+
HtmlButton addCredentialsButton = htmlPage.querySelector(".credentials-add-menu");
81+
addCredentialsButton.fireEvent("mouseenter");
82+
addCredentialsButton.click();
83+
84+
HtmlButton jenkinsCredentialsOption = htmlPage.querySelector(".jenkins-dropdown__item");
85+
jenkinsCredentialsOption.click();
86+
87+
wc.waitForBackgroundJavaScript(4000);
88+
HtmlForm form = htmlPage.querySelector("#credentials-dialog-form");
89+
String certificateDisplayName = j.jenkins.getDescriptor(CertificateCredentialsImpl.class).getDisplayName();
90+
String KeyStoreSourceDisplayName = j.jenkins.getDescriptor(
91+
CertificateCredentialsImpl.PEMEntryKeyStoreSource.class).getDisplayName();
92+
DomNodeList<DomNode> allOptions = htmlPage.getDocumentElement().querySelectorAll(
93+
"select.dropdownList option");
94+
boolean optionFound = selectOption(allOptions, certificateDisplayName);
95+
assertTrue("The Certificate option was not found in the credentials type select", optionFound);
96+
List<HtmlRadioButtonInput> inputs = htmlPage.getDocumentElement().getByXPath(
97+
"//input[contains(@name, 'keyStoreSource') and following-sibling::label[contains(.,'"
98+
+ KeyStoreSourceDisplayName + "')]]");
99+
assertThat("query should return only a singular input", inputs, hasSize(1));
100+
HtmlElementUtil.click(inputs.get(0));
101+
wc.waitForBackgroundJavaScript(4000);
102+
Page submit = HtmlFormUtil.submit(form);
103+
JSONObject responseJson = JSONObject.fromObject(submit.getWebResponse().getContentAsString());
104+
assertThat(responseJson.getString("message"), is(Messages.CertificateCredentialsImpl_InvalidKeystore()));
105+
assertThat(responseJson.getString("notificationType"), is("ERROR"));
106+
}
107+
}
108+
109+
private static boolean selectOption(DomNodeList<DomNode> allOptions, String optionName) {
110+
return allOptions.stream().anyMatch(domNode -> {
111+
if (domNode instanceof HtmlOption option) {
112+
if (option.getVisibleText().equals(optionName)) {
113+
try {
114+
HtmlElementUtil.click(option);
115+
} catch (IOException e) {
116+
throw new RuntimeException(e);
117+
}
118+
return true;
119+
}
120+
}
121+
122+
return false;
123+
});
124+
}
125+
59126
@TestExtension
60127
public static class CredentialsSelectionAction implements UnprotectedRootAction {
61128
@Override

src/test/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImplTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.htmlunit.html.HtmlRadioButtonInput;
4848

4949
import hudson.Util;
50+
import hudson.model.Descriptor;
5051
import hudson.model.ItemGroup;
5152
import hudson.security.ACL;
5253
import hudson.util.Secret;
@@ -106,7 +107,7 @@ public void setup() throws IOException {
106107
}
107108

108109
@Test
109-
public void displayName() throws IOException {
110+
public void displayName() throws IOException, Descriptor.FormException {
110111
SecretBytes uploadedKeystore = SecretBytes.fromBytes(Files.readAllBytes(p12.toPath()));
111112
CertificateCredentialsImpl.UploadedKeyStoreSource storeSource = new CertificateCredentialsImpl.UploadedKeyStoreSource(uploadedKeystore);
112113
assertEquals(EXPECTED_DISPLAY_NAME, CredentialsNameProvider.name(new CertificateCredentialsImpl(null, "abc123", null, "password", storeSource)));

0 commit comments

Comments
 (0)