Skip to content

Commit

Permalink
[SECURITY-2053] Prevent plain text credentials of Basic/Digest authen…
Browse files Browse the repository at this point in the history
…tication (#105)
  • Loading branch information
offa authored Aug 1, 2022
1 parent 68d65f6 commit a2cf345
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 86 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ THE SOFTWARE.
<changelist>-SNAPSHOT</changelist>
<gitHubRepo>jenkinsci/http-request-plugin</gitHubRepo>
<jenkins.version>2.249.1</jenkins.version>
<hpi.compatibleSinceVersion>1.8.17</hpi.compatibleSinceVersion>
<hpi.compatibleSinceVersion>1.16</hpi.compatibleSinceVersion>
</properties>

<repositories>
Expand Down
5 changes: 0 additions & 5 deletions src/main/java/jenkins/plugins/http_request/HttpRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import hudson.util.ListBoxModel;
import hudson.util.ListBoxModel.Option;

import jenkins.plugins.http_request.auth.BasicDigestAuthentication;
import jenkins.plugins.http_request.auth.FormAuthentication;
import jenkins.plugins.http_request.util.HttpClientUtil;
import jenkins.plugins.http_request.util.HttpRequestFormDataPart;
Expand Down Expand Up @@ -536,10 +535,6 @@ public static ListBoxModel fillAuthenticationItems(Item project, String url) {
}

List<Option> options = new ArrayList<>();
for (BasicDigestAuthentication basic : HttpRequestGlobalConfig.get().getBasicDigestAuthentications()) {
options.add(new Option("(deprecated - use Jenkins Credentials) " +
basic.getKeyName(), basic.getKeyName()));
}

for (FormAuthentication formAuthentication : HttpRequestGlobalConfig.get().getFormAuthentications()) {
options.add(new Option(formAuthentication.getKeyName()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@
@Extension
public class HttpRequestGlobalConfig extends GlobalConfiguration {

private List<BasicDigestAuthentication> basicDigestAuthentications = new ArrayList<>();
/**
* @deprecated removed without replacement
*/
@Deprecated
private transient List<BasicDigestAuthentication> basicDigestAuthentications = new ArrayList<>();
private List<FormAuthentication> formAuthentications = new ArrayList<>();

private static final XStream2 XSTREAM2 = new XStream2();
Expand Down Expand Up @@ -78,10 +82,18 @@ public static HttpRequestGlobalConfig get() {
return GlobalConfiguration.all().get(HttpRequestGlobalConfig.class);
}

/**
* @deprecated removed without replacement
*/
@Deprecated
public List<BasicDigestAuthentication> getBasicDigestAuthentications() {
return basicDigestAuthentications;
}

/**
* @deprecated removed without replacement
*/
@Deprecated
public void setBasicDigestAuthentications(
List<BasicDigestAuthentication> basicDigestAuthentications) {
this.basicDigestAuthentications = basicDigestAuthentications;
Expand All @@ -97,10 +109,7 @@ public void setFormAuthentications(
}

public List<Authenticator> getAuthentications() {
List<Authenticator> list = new ArrayList<>();
list.addAll(basicDigestAuthentications);
list.addAll(formAuthentications);
return list;
return new ArrayList<>(formAuthentications);
}

public Authenticator getAuthentication(String keyName) {
Expand All @@ -111,4 +120,11 @@ public Authenticator getAuthentication(String keyName) {
}
return null;
}

protected Object readResolve() {
if (this.basicDigestAuthentications != null) {
this.basicDigestAuthentications = new ArrayList<>();
}
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,6 @@
</d:tag>
</d:taglib>
<f:section title="HTTP Request">
<f:entry title="DEPRECATED (use Jenkins credentials) Basic/Digest Authentication">
<f:repeatable field="basicDigestAuthentications">
<local:blockWrapper>
<f:entry title="Key Name" field="keyName" help="/plugin/http_request/help-keyName.html">
<f:textbox />
</f:entry>
<f:entry title="Username" field="userName" help="/plugin/http_request/help-userName.html">
<f:textbox />
</f:entry>
<f:entry title="Password" field="password" help="/plugin/http_request/help-password.html">
<f:password />
</f:entry>
<f:entry title="">
<div align="right">
<f:repeatableDeleteButton />
</div>
</f:entry>
</local:blockWrapper>
</f:repeatable>
</f:entry>

<f:entry title="Form Authentication">
<f:repeatable field="formAuthentications">
<local:blockWrapper>
Expand Down
3 changes: 0 additions & 3 deletions src/main/webapp/help-password.html

This file was deleted.

3 changes: 0 additions & 3 deletions src/main/webapp/help-userName.html

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import hudson.model.FreeStyleProject;
import hudson.tasks.Builder;

import jenkins.plugins.http_request.auth.BasicDigestAuthentication;
import jenkins.plugins.http_request.auth.FormAuthentication;
import jenkins.plugins.http_request.util.HttpRequestNameValuePair;
import jenkins.plugins.http_request.util.RequestAction;
Expand All @@ -33,7 +32,6 @@ public class HttpRequestBackwardCompatibilityTest {
public void defaultGlobalConfig() {
// Test that config from 1.8.6 can be loaded
HttpRequestGlobalConfig cfg = HttpRequestGlobalConfig.get();
assertEquals(Collections.emptyList(), cfg.getBasicDigestAuthentications());
assertEquals(Collections.emptyList(), cfg.getFormAuthentications());
assertEquals("jenkins.plugins.http_request.HttpRequest.xml", cfg.getConfigFile().getFile().getName());
}
Expand All @@ -46,18 +44,6 @@ public void populatedGlobalConfig() {
// and the HttpRequestGlobalConfig.getConfigFile() method
HttpRequestGlobalConfig cfg = HttpRequestGlobalConfig.get();

List<BasicDigestAuthentication> bdas = cfg.getBasicDigestAuthentications();
assertEquals(2,bdas.size());
Iterator<BasicDigestAuthentication> itr = bdas.iterator();
BasicDigestAuthentication bda = itr.next();
assertEquals("k1",bda.getKeyName());
assertEquals("u1",bda.getUserName());
assertEquals("p1",bda.getPassword());
bda = itr.next();
assertEquals("k2",bda.getKeyName());
assertEquals("u2",bda.getUserName());
assertEquals("p2",bda.getPassword());

List<FormAuthentication> fas = cfg.getFormAuthentications();
assertEquals(1,fas.size());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import java.util.ArrayList;
import java.util.List;

import jenkins.plugins.http_request.auth.BasicDigestAuthentication;
import jenkins.plugins.http_request.auth.FormAuthentication;
import jenkins.plugins.http_request.util.HttpRequestNameValuePair;
import jenkins.plugins.http_request.util.RequestAction;
Expand Down Expand Up @@ -62,10 +61,6 @@ public void configRoundtripGroup2() throws Exception {

@Test
public void configRoundtripGroup3() throws Exception {
List<BasicDigestAuthentication> bda = new ArrayList<>();
bda.add(new BasicDigestAuthentication("keyname1","username1","password1"));
bda.add(new BasicDigestAuthentication("keyname2","username2","password2"));
HttpRequestGlobalConfig.get().setBasicDigestAuthentications(bda);
configRoundTrip(before);

List<HttpRequestNameValuePair> params = new ArrayList<>();
Expand Down Expand Up @@ -117,18 +112,6 @@ private void configRoundTrip(HttpRequest before) throws Exception {
assertEquals(bnvp.getValue(),anvp.getValue());
}

// Basic authentication check
List<BasicDigestAuthentication> beforeBdas = HttpRequestGlobalConfig.get().getBasicDigestAuthentications();
List<BasicDigestAuthentication> afterBdas = HttpRequestGlobalConfig.get().getBasicDigestAuthentications();
assertEquals(beforeBdas.size(), afterBdas.size());
for (int idx = 0; idx < beforeBdas.size(); idx++) {
BasicDigestAuthentication beforeBda = beforeBdas.get(idx);
BasicDigestAuthentication afterBda = afterBdas.get(idx);
assertEquals(beforeBda.getKeyName(), afterBda.getKeyName());
assertEquals(beforeBda.getUserName(),afterBda.getUserName());
assertEquals(beforeBda.getPassword(),afterBda.getPassword());
}

// Form authentication check
List<FormAuthentication> beforeFas = HttpRequestGlobalConfig.get().getFormAuthentications();
List<FormAuthentication> afterFas = HttpRequestGlobalConfig.get().getFormAuthentications();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.util.ArrayList;
import java.util.List;

import jenkins.plugins.http_request.auth.BasicDigestAuthentication;
import jenkins.plugins.http_request.auth.FormAuthentication;
import jenkins.plugins.http_request.util.HttpRequestNameValuePair;
import jenkins.plugins.http_request.util.RequestAction;
Expand Down Expand Up @@ -54,10 +53,6 @@ public void configRoundtripGroup2() throws Exception {

@Test
public void configRoundtripGroup3() throws Exception {
List<BasicDigestAuthentication> bda = new ArrayList<>();
bda.add(new BasicDigestAuthentication("keyname1","username1","password1"));
bda.add(new BasicDigestAuthentication("keyname2","username2","password2"));
HttpRequestGlobalConfig.get().setBasicDigestAuthentications(bda);
configRoundTrip(before);

List<HttpRequestNameValuePair> params = new ArrayList<>();
Expand Down Expand Up @@ -112,18 +107,6 @@ private void configRoundTrip(HttpRequestStep before) throws Exception {
assertEquals(bnvp.getValue(),anvp.getValue());
}

// Basic authentication check
List<BasicDigestAuthentication> beforeBdas = HttpRequestGlobalConfig.get().getBasicDigestAuthentications();
List<BasicDigestAuthentication> afterBdas = HttpRequestGlobalConfig.get().getBasicDigestAuthentications();
assertEquals(beforeBdas.size(), afterBdas.size());
for (int idx = 0; idx < beforeBdas.size(); idx++) {
BasicDigestAuthentication beforeBda = beforeBdas.get(idx);
BasicDigestAuthentication afterBda = afterBdas.get(idx);
assertEquals(beforeBda.getKeyName(), afterBda.getKeyName());
assertEquals(beforeBda.getUserName(),afterBda.getUserName());
assertEquals(beforeBda.getPassword(),afterBda.getPassword());
}

// Form authentication check
List<FormAuthentication> beforeFas = HttpRequestGlobalConfig.get().getFormAuthentications();
List<FormAuthentication> afterFas = HttpRequestGlobalConfig.get().getFormAuthentications();
Expand Down

0 comments on commit a2cf345

Please sign in to comment.