Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check request method to determine correct response binding #302

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Check request method to determine correct response binding #302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions core/src/main/java/com/onelogin/saml2/http/HttpRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@
*/
public final class HttpRequest {

public static final Map<String, List<String>> EMPTY_PARAMETERS = Collections.<String, List<String>>emptyMap();
public static final Map<String, List<String>> EMPTY_PARAMETERS = Collections.emptyMap();

private final String requestURL;
private final Map<String, List<String>> parameters;
private final String queryString;
private final String method;

/**
* Creates a new HttpRequest.
Expand All @@ -38,8 +39,8 @@ public final class HttpRequest {
* @deprecated Not providing a queryString can cause HTTP Redirect binding to fail.
*/
@Deprecated
public HttpRequest(String requestURL) {
this(requestURL, EMPTY_PARAMETERS);
public HttpRequest(String method, String requestURL) {
this(method, requestURL, EMPTY_PARAMETERS);
}

/**
Expand All @@ -48,8 +49,8 @@ public HttpRequest(String requestURL) {
* @param requestURL the request URL (up to but not including query parameters)
* @param queryString string that is contained in the request URL after the path
*/
public HttpRequest(String requestURL, String queryString) {
this(requestURL, EMPTY_PARAMETERS, queryString);
public HttpRequest(String method, String requestURL, String queryString) {
this(method, requestURL, EMPTY_PARAMETERS, queryString);
}

/**
Expand All @@ -61,8 +62,8 @@ public HttpRequest(String requestURL, String queryString) {
* @deprecated Not providing a queryString can cause HTTP Redirect binding to fail.
*/
@Deprecated
public HttpRequest(String requestURL, Map<String, List<String>> parameters) {
this(requestURL, parameters, null);
public HttpRequest(String method, String requestURL, Map<String, List<String>> parameters) {
this(method, requestURL, parameters, null);
}

/**
Expand All @@ -73,10 +74,11 @@ public HttpRequest(String requestURL, Map<String, List<String>> parameters) {
* @param queryString string that is contained in the request URL after the path
* @throws NullPointerException if any of the parameters is null
*/
public HttpRequest(String requestURL, Map<String, List<String>> parameters, String queryString) {
public HttpRequest(String method, String requestURL, Map<String, List<String>> parameters, String queryString) {
this.requestURL = checkNotNull(requestURL, "requestURL");
this.parameters = unmodifiableCopyOf(checkNotNull(parameters, "queryParams"));
this.queryString = StringUtils.trimToEmpty(queryString);
this.method = method == null ? null : method.toUpperCase();
}

/**
Expand All @@ -89,13 +91,13 @@ public HttpRequest addParameter(String name, String value) {
checkNotNull(name, "name");
checkNotNull(value, "value");

final List<String> oldValues = parameters.containsKey(name) ? parameters.get(name) : new ArrayList<String>();
final List<String> oldValues = parameters.containsKey(name) ? parameters.get(name) : new ArrayList<>();
final List<String> newValues = new ArrayList<>(oldValues);
newValues.add(value);
final Map<String, List<String>> params = new HashMap<>(parameters);
params.put(name, newValues);

return new HttpRequest(requestURL, params, queryString);
return new HttpRequest(method, requestURL, params, queryString);
}

/**
Expand All @@ -109,7 +111,7 @@ public HttpRequest removeParameter(String name) {
final Map<String, List<String>> params = new HashMap<>(parameters);
params.remove(name);

return new HttpRequest(requestURL, params, queryString);
return new HttpRequest(method, requestURL, params, queryString);
}

/**
Expand Down Expand Up @@ -137,7 +139,7 @@ public String getParameter(String name) {
*/
public List<String> getParameters(String name) {
List<String> values = parameters.get(name);
return values != null ? values : Collections.<String>emptyList();
return values != null ? values : Collections.emptyList();
}

/**
Expand Down Expand Up @@ -178,6 +180,10 @@ public String getEncodedParameter(String name, String defaultValue) {
return (value != null ? value : Util.urlEncoder(defaultValue));
}

public boolean isGet() {
return Objects.equals(method, "GET");
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -189,7 +195,8 @@ public boolean equals(Object o) {
}

HttpRequest that = (HttpRequest) o;
return Objects.equals(requestURL, that.requestURL) &&
return Objects.equals(method, that.method) &&
Objects.equals(requestURL, that.requestURL) &&
Objects.equals(parameters, that.parameters) &&
Objects.equals(queryString, that.queryString);
}
Expand Down
51 changes: 39 additions & 12 deletions core/src/test/java/com/onelogin/saml2/http/HttpRequestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;

import java.util.Arrays;
import java.util.Collections;
Expand All @@ -22,17 +23,33 @@
public class HttpRequestTest {
@Test
public void testConstructorWithNoQueryParams() throws Exception {
final String method = "get";
final String url = "url";

final HttpRequest request = new HttpRequest(url, (String)null);
final HttpRequest request = new HttpRequest(method, url, (String)null);
assertThat(request.getRequestURL(), equalTo(url));
assertThat(request.getParameters(), equalTo(Collections.<String, List<String>>emptyMap()));
assertThat(request.getParameters("x"), equalTo(Collections.<String>emptyList()));
assertThat(request.getParameter("x"), nullValue());
}

@Test
public void testMethod() {
final String get = "get";
final String post = "post";
final String url = "url";

final HttpRequest request = new HttpRequest(get, url, (String)null);
assertTrue(request.isGet());
final HttpRequest request2 = new HttpRequest(post, url, (String)null);
assertFalse(request2.isGet());
final HttpRequest request3 = new HttpRequest(get.toUpperCase(), url, (String)null);
assertTrue(request3.isGet());
}

@Test
public void testConstructorWithQueryParams() throws Exception {
final String method = "get";
final String url = "url";
final String name = "name";
final String value1 = "val1";
Expand All @@ -41,7 +58,7 @@ public void testConstructorWithQueryParams() throws Exception {
final List<String> values = Arrays.asList(value1, value2);
final Map<String, List<String>> parametersMap = singletonMap(name, values);

final HttpRequest request = new HttpRequest(url, parametersMap, null);
final HttpRequest request = new HttpRequest(method, url, parametersMap, null);
assertThat(request.getRequestURL(), equalTo(url));
assertThat(request.getParameters(), equalTo(parametersMap));
assertThat(request.getParameters(name), equalTo(values));
Expand All @@ -50,11 +67,12 @@ public void testConstructorWithQueryParams() throws Exception {

@Test
public void testAddParameter() throws Exception {
final String method = "get";
final String url = "some_url";
final String name = "name";
final String value = "value";

final HttpRequest request = new HttpRequest(url, (String)null).addParameter(name, value);
final HttpRequest request = new HttpRequest(method, url, (String)null).addParameter(name, value);
assertThat(request.getRequestURL(), equalTo(url));
assertThat(request.getParameters(), equalTo(singletonMap(name, singletonList(value))));
assertThat(request.getParameters(name), equalTo(singletonList(value)));
Expand All @@ -66,11 +84,12 @@ public void testAddParameter() throws Exception {

@Test
public void testRemoveParameter() throws Exception {
final String method = "get";
final String url = "some_url";
final String name = "name";
final String value = "value";

HttpRequest request = new HttpRequest(url, (String)null).addParameter(name, value);
HttpRequest request = new HttpRequest(method, url, (String)null).addParameter(name, value);
assertThat(request.getRequestURL(), equalTo(url));
assertThat(request.getParameters(), equalTo(singletonMap(name, singletonList(value))));
assertThat(request.getParameters(name), equalTo(singletonList(value)));
Expand All @@ -85,6 +104,7 @@ public void testRemoveParameter() throws Exception {

@Test
public void testGetEncodedParameter_encodesParametersNotOnQueryString() throws Exception {
final String method = "post";
final String url = "url";
final String name = "name";
final String value1 = "val/1!";
Expand All @@ -94,14 +114,15 @@ public void testGetEncodedParameter_encodesParametersNotOnQueryString() throws E
final List<String> values = Arrays.asList(value1);
final Map<String, List<String>> parametersMap = singletonMap(name, values);

final HttpRequest request = new HttpRequest(url, parametersMap, null).addParameter(addedName, addedValue);
final HttpRequest request = new HttpRequest(method, url, parametersMap, null).addParameter(addedName, addedValue);

assertThat(request.getEncodedParameter(name), equalTo(Util.urlEncoder(value1)));
assertThat(request.getEncodedParameter(addedName), equalTo(Util.urlEncoder(addedValue)));
}

@Test
public void testGetEncodedParameter_prefersValueFromQueryString() throws Exception {
final String method = "get";
final String url = "url";
final String name = "name";
final String value1 = "value1";
Expand All @@ -111,31 +132,33 @@ public void testGetEncodedParameter_prefersValueFromQueryString() throws Excepti
final List<String> values = Arrays.asList(value1);
final Map<String, List<String>> parametersMap = singletonMap(name, values);

final HttpRequest request = new HttpRequest(url, parametersMap, queryString);
final HttpRequest request = new HttpRequest(method, url, parametersMap, queryString);

assertThat(request.getEncodedParameter(name), equalTo(urlValue1));
assertThat(request.getParameter(name), equalTo(value1));
}

@Test
public void testGetEncodedParameter_returnsExactAsGivenInQueryString() throws Exception {
final String method = "get";
final String url = "url";
final String name = "name";
String encodedValue1 = NaiveUrlEncoder.encode("do not alter!");
final String queryString = name + "=" + encodedValue1;

final HttpRequest request = new HttpRequest(url, queryString);
final HttpRequest request = new HttpRequest(method, url, queryString);

assertThat(request.getEncodedParameter(name), equalTo(encodedValue1));
}

@Test
public void testGetEncodedParameter_handlesMultipleValuesOnQueryString() throws Exception {
final String method = "get";
final String url = "url";
final String queryString = "k1=v1&k2=v2&k3=v3";

final Map<String, List<String>> parametersMap = new HashMap<>();
final HttpRequest request = new HttpRequest(url, parametersMap, queryString);
final HttpRequest request = new HttpRequest(method, url, parametersMap, queryString);

assertThat(request.getEncodedParameter("k1"), equalTo("v1"));
assertThat(request.getEncodedParameter("k2"), equalTo("v2"));
Expand All @@ -144,40 +167,44 @@ public void testGetEncodedParameter_handlesMultipleValuesOnQueryString() throws

@Test
public void testGetEncodedParameter_stopsAtUrlFragment() throws Exception {
final String method = "get";
final String url = "url";
final String queryString = "first=&foo=bar#ignore";

final HttpRequest request = new HttpRequest(url, queryString);
final HttpRequest request = new HttpRequest(method, url, queryString);

assertThat(request.getEncodedParameter("foo"), equalTo("bar"));
}

@Test
public void testGetEncodedParameter_withDefault_usesDefaultWhenParameterMissing() throws Exception {
final String method = "get";
final String url = "url";
final String foobar = "foo/bar!";

final HttpRequest request = new HttpRequest(url, (String)null);
final HttpRequest request = new HttpRequest(method, url, (String)null);
assertThat(request.getEncodedParameter("missing", foobar), equalTo(Util.urlEncoder(foobar)));
}


@Test
public void testAddParameter_preservesQueryString() throws Exception {
final String method = "get";
final String url = "url";
final String name = "name";
final String value1 = "val/1!";
String encodedValue1 = NaiveUrlEncoder.encode(value1);
final String queryString = name + "=" + encodedValue1;

final Map<String, List<String>> parametersMap = new HashMap<>();
final HttpRequest request = new HttpRequest(url, parametersMap, queryString).addParameter(name, value1);
final HttpRequest request = new HttpRequest(method, url, parametersMap, queryString).addParameter(name, value1);

assertThat(request.getEncodedParameter(name), equalTo(encodedValue1));
}

@Test
public void testRemoveParameter_preservesQueryString() throws Exception {
final String method = "get";
final String url = "url";
final String name = "name";
final String value1 = "val/1!";
Expand All @@ -187,7 +214,7 @@ public void testRemoveParameter_preservesQueryString() throws Exception {
final List<String> values = Arrays.asList(value1);
final Map<String, List<String>> parametersMap = singletonMap(name, values);

final HttpRequest request = new HttpRequest(url, parametersMap, queryString).removeParameter(name);
final HttpRequest request = new HttpRequest(method, url, parametersMap, queryString).removeParameter(name);

assertThat(request.getEncodedParameter(name), equalTo(encodedValue1));
}
Expand Down
9 changes: 3 additions & 6 deletions core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@
import com.onelogin.saml2.exception.SettingsException;
import com.onelogin.saml2.exception.ValidationError;
import com.onelogin.saml2.http.HttpRequest;
import com.onelogin.saml2.logout.LogoutRequest;
import com.onelogin.saml2.model.SamlResponseStatus;
import com.onelogin.saml2.settings.Saml2Settings;
import com.onelogin.saml2.settings.SettingsBuilder;
import com.onelogin.saml2.util.Constants;
import com.onelogin.saml2.util.Util;

import org.hamcrest.Matchers;
import org.joda.time.DateTime;
import org.joda.time.DateTimeUtils;
Expand All @@ -27,6 +25,8 @@
import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.xpath.XPathExpressionException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
Expand All @@ -38,9 +38,6 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.xpath.XPathExpressionException;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.Matchers.contains;
Expand Down Expand Up @@ -3027,7 +3024,7 @@ private static HttpRequest newHttpRequest(String samlResponseEncoded) {
}

private static HttpRequest newHttpRequest(String requestURL, String samlResponseEncoded) {
return new HttpRequest(requestURL, (String)null).addParameter("SAMLResponse", samlResponseEncoded);
return new HttpRequest("GET", requestURL, (String)null).addParameter("SAMLResponse", samlResponseEncoded);
}

private void setDateTime(String ISOTimeStamp) {
Expand Down
10 changes: 5 additions & 5 deletions core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ public void testIsInValidSign_defaultUrlEncode() throws Exception {
//This signature is based on the query string above
String signature = "cxDTcLRHhXJKGYcjZE2RRz5p7tVg/irNimq48KkJ0n10wiGwAmuzUByxEm4OHbetDrHGtxI5ygjrR0/HcrD8IkYyI5Ie4r5tJYkfdtpUrvOQ7khbBvP9GzEbZIrz7eH1ALdCDchORaRB/cs6v+OZbBj5uPTrN//wOhZl2k9H2xVW/SYy17jDoIKh/wvqtQ9FF+h2UxdUEhxeB/UUXOC6nVLMo+RGaamSviYkUE1Zu1tmalO+F6FivNQ31T/TkqzWz0KEjmnFs3eKbHakPVuUHpDQm7Gf2gBS1TXwVQsL7e2axtvv4RH5djlq1Z2WH2V+PwGOkIvLxf3igGUSR1A8bw==";

HttpRequest httpRequest = new HttpRequest(requestURL, queryString)
HttpRequest httpRequest = new HttpRequest("GET", requestURL, queryString)
.addParameter("SAMLRequest", samlRequestEncoded)
.addParameter("RelayState", relayState)
.addParameter("SigAlg", sigAlg)
Expand Down Expand Up @@ -692,7 +692,7 @@ public void testIsInValidSign_naiveUrlEncoding() throws Exception {
//This signature is based on the query string above
String signatureNaiveEncoding = "Gj2mUq6RBPAPXI9VjDDlwAxueSEBlOfgpWKLpsQbqIp+2XPFtC/vPAZpuPjHCDNNnAI3WKZa4l8ijwQBTqQwKz88k9gTx6vcLxPl2L4SrWdLOokiGrIVYJ+0sK2hapHHMa7WzGiTgpeTuejHbD4ptneaRXl4nrJAEo0WJ/rNTSWbJpnb+ENtgBnsfkmj+6z1KFY70ruo7W/vme21Jg+4XNfBSGl6LLSjEnZHJG0ET80HKvJEZayv4BQGZ3MShcSMyab/w+rLfDvDRA5RcRxw+NHOXo/kxZ3qhpa6daOwG69+PiiWmusmB2gaSq6jy2L55zFks9a36Pt5l5fYA2dE4g==";

HttpRequest httpRequest = new HttpRequest(requestURL, queryString)
HttpRequest httpRequest = new HttpRequest("GET", requestURL, queryString)
.addParameter("SAMLRequest", samlRequestEncoded)
.addParameter("RelayState", relayState)
.addParameter("SigAlg", sigAlg)
Expand Down Expand Up @@ -721,7 +721,7 @@ public void testIsInValidSign() throws Exception {
String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
String signature = "XCwCyI5cs7WhiJlB5ktSlWxSBxv+6q2xT3c8L7dLV6NQG9LHWhN7gf8qNsahSXfCzA0Ey9dp5BQ0EdRvAk2DIzKmJY6e3hvAIEp1zglHNjzkgcQmZCcrkK9Czi2Y1WkjOwR/WgUTUWsGJAVqVvlRZuS3zk3nxMrLH6f7toyvuJc=";

HttpRequest httpRequest = new HttpRequest(requestURL, (String)null)
HttpRequest httpRequest = new HttpRequest("GET", requestURL, (String)null)
.addParameter("SAMLRequest", samlRequestEncoded)
.addParameter("RelayState", relayState)
.addParameter("SigAlg", sigAlg)
Expand Down Expand Up @@ -786,7 +786,7 @@ public void testIsInValidSignWithDeprecatedAlg() throws Exception {
String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
String signature = "XCwCyI5cs7WhiJlB5ktSlWxSBxv+6q2xT3c8L7dLV6NQG9LHWhN7gf8qNsahSXfCzA0Ey9dp5BQ0EdRvAk2DIzKmJY6e3hvAIEp1zglHNjzkgcQmZCcrkK9Czi2Y1WkjOwR/WgUTUWsGJAVqVvlRZuS3zk3nxMrLH6f7toyvuJc=";

HttpRequest httpRequest = new HttpRequest(requestURL, (String)null)
HttpRequest httpRequest = new HttpRequest("GET", requestURL, (String)null)
.addParameter("SAMLRequest", samlRequestEncoded)
.addParameter("RelayState", relayState)
.addParameter("SigAlg", sigAlg)
Expand Down Expand Up @@ -877,6 +877,6 @@ public void testGetError() throws Exception {
}

private static HttpRequest newHttpRequest(String requestURL, String samlRequestEncoded) {
return new HttpRequest(requestURL, (String)null).addParameter("SAMLRequest", samlRequestEncoded);
return new HttpRequest("GET", requestURL, (String)null).addParameter("SAMLRequest", samlRequestEncoded);
}
}
Loading