diff --git a/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java b/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java index c6db8d43..81601243 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java +++ b/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java @@ -33,6 +33,7 @@ import hudson.model.ComputerSet; import hudson.model.Describable; import hudson.model.Descriptor; +import hudson.model.Failure; import hudson.model.Item; import hudson.model.ManageJenkinsAction; import hudson.model.ModelObject; @@ -656,6 +657,13 @@ public JSONObject doAddCredentials(StaplerRequest2 req, StaplerResponse2 rsp) th try { Credentials credentials = Descriptor.bindJSON(req, Credentials.class, data.getJSONObject("credentials")); + try { + CredentialsStoreAction.checkCredentialsId(credentials); + } catch (Failure f) { + return new JSONObject() + .element("message", f.getMessage()) + .element("notificationType", "ERROR"); + } credentialsWereAdded = store.addCredentials(wrapper.getDomain(), credentials); } catch (LinkageError e) { /* diff --git a/src/main/java/com/cloudbees/plugins/credentials/CredentialsStoreAction.java b/src/main/java/com/cloudbees/plugins/credentials/CredentialsStoreAction.java index ed05e808..284e158b 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/CredentialsStoreAction.java +++ b/src/main/java/com/cloudbees/plugins/credentials/CredentialsStoreAction.java @@ -529,6 +529,17 @@ public boolean hasPermission(@NonNull Permission permission) { return getACL().hasPermission(permission); } + /** + * Checks that the id of the supplied credentials does not contain illegal characters. + * + * @param credentials the credentials to check. + */ + static void checkCredentialsId(Credentials credentials) { + if (credentials instanceof IdCredentials) { + Jenkins.checkGoodName(((IdCredentials) credentials).getId()); + } + } + /** * A wrapper object to bind and expose {@link Domain} instances into the web UI. */ @@ -768,6 +779,16 @@ public HttpResponse doCreateCredentials(StaplerRequest2 req) throws ServletExcep } else { JSONObject data = req.getSubmittedForm(); Credentials credentials = Descriptor.bindJSON(req, Credentials.class, data.getJSONObject("credentials")); + try { + checkCredentialsId(credentials); + } catch (Failure f) { + if (jsonResponse) { + return HttpResponses.okJSON(new JSONObject() + .element("message", f.getMessage()) + .element("notificationType", "ERROR")); + } + throw f; + } boolean credentialsWereAdded = getStore().addCredentials(domain, credentials); if (jsonResponse) { diff --git a/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java b/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java index d23fc4da..8c933112 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java @@ -97,6 +97,41 @@ void doAddCredentialsFromPopupWorksAsExpected() throws Exception { } } + @Test + @Issue("JENKINS-75943") + void doAddCredentialsFromPopupRejectsIllegalId() throws Exception { + try (JenkinsRule.WebClient wc = j.createWebClient()) { + HtmlPage htmlPage = wc.goTo("credentials-selection"); + + HtmlButton addCredentialsButton = htmlPage.querySelector(".credentials-add-menu"); + addCredentialsButton.click(); + + HtmlButton jenkinsCredentialsOption = htmlPage.querySelector(".jenkins-dropdown__item"); + HtmlElementUtil.click(jenkinsCredentialsOption); + + HtmlRadioButtonInput item = htmlPage.querySelector(".jenkins-choice-list__item input"); + HtmlElementUtil.click(item); + + HtmlButton formSubmitButton = htmlPage.querySelector("#cr-dialog-next"); + HtmlElementUtil.click(formSubmitButton); + + HtmlForm form = htmlPage.querySelector("#credentials-dialog-form"); + + HtmlInput username = form.querySelector("input[name='_.username']"); + username.setValue("bob"); + HtmlInput password = form.querySelector("input[name='_.password']"); + password.setValue("secret"); + HtmlInput id = form.querySelector("input[name='_.id']"); + id.setValue("illegal?id"); + + formSubmitButton = htmlPage.querySelector("#cr-dialog-submit"); + HtmlElementUtil.click(formSubmitButton); + + List creds = CredentialsProvider.lookupCredentialsInItem(UsernamePasswordCredentials.class, null, ACL.SYSTEM2); + assertThat(creds, Matchers.hasSize(0)); + } + } + @Test @Issue("JENKINS-74964") void doAddCredentialsFromPopupForPEMCertificateKeystore() throws Exception { diff --git a/src/test/java/com/cloudbees/plugins/credentials/CredentialsStoreActionTest.java b/src/test/java/com/cloudbees/plugins/credentials/CredentialsStoreActionTest.java index f270971c..2420ada1 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/CredentialsStoreActionTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/CredentialsStoreActionTest.java @@ -9,6 +9,7 @@ import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; +import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; @@ -19,6 +20,7 @@ import org.apache.commons.lang3.StringUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.xmlunit.matchers.CompareMatcher; @@ -292,6 +294,42 @@ void restCRUDNonHappy() throws Exception { assertThat(con.getResponseCode(), is(HttpServletResponse.SC_CONFLICT)); } + @Test + @Issue("JENKINS-75943") + void createCredentialsFromFormRejectsIllegalId() throws Exception { + j.getInstance().setCrumbIssuer(null); + HttpURLConnection con = postCreateByForm(systemStore, null, + "{\"credentials\": {\"$class\": \"com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl\", " + + "\"scope\": \"GLOBAL\", \"id\": \"valid.id\", \"description\": \"first\", " + + "\"username\": \"bob\", \"password\": \"secret\"}}"); + assertThat(con.getResponseCode(), is(HttpServletResponse.SC_FOUND)); + assertThat(CredentialsMatchers.firstOrNull(systemStore.getCredentials(Domain.global()), + CredentialsMatchers.withId("valid.id")), notNullValue()); + + con = postCreateByForm(systemStore, null, + "{\"credentials\": {\"$class\": \"com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl\", " + + "\"scope\": \"GLOBAL\", \"id\": \"illegal?id\", \"description\": \"second\", " + + "\"username\": \"bob\", \"password\": \"secret\"}}"); + assertThat(con.getResponseCode(), is(HttpServletResponse.SC_BAD_REQUEST)); + assertThat(CredentialsMatchers.firstOrNull(systemStore.getCredentials(Domain.global()), + CredentialsMatchers.withId("illegal?id")), nullValue()); + } + + private HttpURLConnection postCreateByForm(CredentialsStore store, String domainName, String json) + throws IOException { + HttpURLConnection con = (HttpURLConnection) new URL(j.getURL(), + "credentials/store/" + store.getStoreAction().getUrlName() + "/domain/" + Util + .rawEncode(StringUtils.defaultIfBlank(domainName, "_")) + "/createCredentials") + .openConnection(); + con.setRequestMethod("POST"); + con.setInstanceFollowRedirects(false); + con.setRequestProperty("Content-Type", "application/x-www-form-urlencoded;charset=utf-8"); + con.setDoOutput(true); + con.getOutputStream() + .write(("json=" + URLEncoder.encode(json, StandardCharsets.UTF_8)).getBytes(StandardCharsets.UTF_8)); + return con; + } + private HttpURLConnection postCreateByXml(CredentialsStore store, String xml) throws IOException { HttpURLConnection con = (HttpURLConnection) new URL(j.getURL(),