diff --git a/core/src/main/java/com/onelogin/saml2/http/HttpRequest.java b/core/src/main/java/com/onelogin/saml2/http/HttpRequest.java index 6567bcf5..7902682e 100644 --- a/core/src/main/java/com/onelogin/saml2/http/HttpRequest.java +++ b/core/src/main/java/com/onelogin/saml2/http/HttpRequest.java @@ -24,11 +24,12 @@ */ public final class HttpRequest { - public static final Map> EMPTY_PARAMETERS = Collections.>emptyMap(); + public static final Map> EMPTY_PARAMETERS = Collections.emptyMap(); private final String requestURL; private final Map> parameters; private final String queryString; + private final String method; /** * Creates a new HttpRequest. @@ -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); } /** @@ -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); } /** @@ -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> parameters) { - this(requestURL, parameters, null); + public HttpRequest(String method, String requestURL, Map> parameters) { + this(method, requestURL, parameters, null); } /** @@ -73,10 +74,11 @@ public HttpRequest(String requestURL, Map> 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> parameters, String queryString) { + public HttpRequest(String method, String requestURL, Map> 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(); } /** @@ -89,13 +91,13 @@ public HttpRequest addParameter(String name, String value) { checkNotNull(name, "name"); checkNotNull(value, "value"); - final List oldValues = parameters.containsKey(name) ? parameters.get(name) : new ArrayList(); + final List oldValues = parameters.containsKey(name) ? parameters.get(name) : new ArrayList<>(); final List newValues = new ArrayList<>(oldValues); newValues.add(value); final Map> params = new HashMap<>(parameters); params.put(name, newValues); - return new HttpRequest(requestURL, params, queryString); + return new HttpRequest(method, requestURL, params, queryString); } /** @@ -109,7 +111,7 @@ public HttpRequest removeParameter(String name) { final Map> params = new HashMap<>(parameters); params.remove(name); - return new HttpRequest(requestURL, params, queryString); + return new HttpRequest(method, requestURL, params, queryString); } /** @@ -137,7 +139,7 @@ public String getParameter(String name) { */ public List getParameters(String name) { List values = parameters.get(name); - return values != null ? values : Collections.emptyList(); + return values != null ? values : Collections.emptyList(); } /** @@ -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) { @@ -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); } diff --git a/core/src/test/java/com/onelogin/saml2/http/HttpRequestTest.java b/core/src/test/java/com/onelogin/saml2/http/HttpRequestTest.java index edf04c9f..28072fe9 100644 --- a/core/src/test/java/com/onelogin/saml2/http/HttpRequestTest.java +++ b/core/src/test/java/com/onelogin/saml2/http/HttpRequestTest.java @@ -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; @@ -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.>emptyMap())); assertThat(request.getParameters("x"), equalTo(Collections.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"; @@ -41,7 +58,7 @@ public void testConstructorWithQueryParams() throws Exception { final List values = Arrays.asList(value1, value2); final Map> 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)); @@ -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))); @@ -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))); @@ -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!"; @@ -94,7 +114,7 @@ public void testGetEncodedParameter_encodesParametersNotOnQueryString() throws E final List values = Arrays.asList(value1); final Map> 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))); @@ -102,6 +122,7 @@ public void testGetEncodedParameter_encodesParametersNotOnQueryString() throws E @Test public void testGetEncodedParameter_prefersValueFromQueryString() throws Exception { + final String method = "get"; final String url = "url"; final String name = "name"; final String value1 = "value1"; @@ -111,7 +132,7 @@ public void testGetEncodedParameter_prefersValueFromQueryString() throws Excepti final List values = Arrays.asList(value1); final Map> 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)); @@ -119,23 +140,25 @@ public void testGetEncodedParameter_prefersValueFromQueryString() throws Excepti @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> 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")); @@ -144,26 +167,29 @@ 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!"; @@ -171,13 +197,14 @@ public void testAddParameter_preservesQueryString() throws Exception { final String queryString = name + "=" + encodedValue1; final Map> 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!"; @@ -187,7 +214,7 @@ public void testRemoveParameter_preservesQueryString() throws Exception { final List values = Arrays.asList(value1); final Map> 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)); } diff --git a/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java b/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java index cae24ce2..3f0a5ebe 100644 --- a/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java @@ -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; @@ -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; @@ -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; @@ -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) { diff --git a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java index cf9fab1f..e694422d 100644 --- a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java @@ -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) @@ -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) @@ -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) @@ -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) @@ -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); } } diff --git a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java index fe04cb7c..5d1dc6a7 100644 --- a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java @@ -41,7 +41,7 @@ public void testGetEncodedLogoutResponseSimulated() throws Exception { Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); final String logoutResponseString = Util.getFileAsString("data/logout_responses/logout_response.xml"); final String requestURL = "/"; - HttpRequest httpRequest = new HttpRequest(requestURL, (String)null); + HttpRequest httpRequest = new HttpRequest("GET", requestURL, (String)null); LogoutResponse logoutResponseBuilder = new LogoutResponse(settings, httpRequest) { @Override @@ -137,7 +137,7 @@ public void testBuild() throws IOException, XMLEntityException, URISyntaxExcepti Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); final String requestURL = "/"; - HttpRequest httpRequest = new HttpRequest(requestURL, (String)null); + HttpRequest httpRequest = new HttpRequest("GET", requestURL, (String)null); LogoutResponse logoutResponse = new LogoutResponse(settings, httpRequest); assertFalse(logoutResponse.isValid()); @@ -266,7 +266,7 @@ public void testIsValidNoResponse() throws XMLEntityException, IOException, Erro assertFalse(logoutResponse.isValid()); assertEquals("SAML Logout Response is not loaded", logoutResponse.getError()); - httpRequest = new HttpRequest(requestURL, (String)null); + httpRequest = new HttpRequest("GET", requestURL, (String)null); logoutResponse = new LogoutResponse(settings, httpRequest); assertFalse(logoutResponse.isValid()); assertEquals("SAML Logout Response is not loaded", logoutResponse.getError()); @@ -449,7 +449,7 @@ public void testIsInValidSign_defaultUrlEncode() throws Exception { //This signature is based on the query string above String signature = "czxEy2WDRZS1U4b2PQFpE4KRhRs8jt5bBKdTFx5oIXpte6qtm0Lk/5lzw/2S6Y1NJpj5DJvSLJvylgNE+RYfJR1GX0zQplm2dZYtlo7CZUyfS3JCLsWviEtPXaon+8Z0lQQkPt4yxCf9v8Qd0pvxHglTUCK/sU0NXnZQdpSxxfsaNCcjQf5gTg/gj8oI7xdrnamBPFtsaH6tAirkjGMoYS4Otju3mcrdcNBIHG40wrffUDnE83Jw4AOFCp8Vsf0zPTQOQsxS4HF4VS78OvGn7jLi2MdabeAQcK5+tP3mUB4vO8AAt8QbkEEiWQbcvA9i1Ezma92CdNYgaf4B3JYpPA=="; - HttpRequest httpRequest = new HttpRequest(requestURL, queryString) + HttpRequest httpRequest = new HttpRequest("GET", requestURL, queryString) .addParameter("SAMLResponse", samlResponseEncoded) .addParameter("RelayState", relayState) .addParameter("SigAlg", sigAlg) @@ -477,7 +477,7 @@ public void testIsInValidSign_naiveUrlEncoding() throws Exception { //This signature is based on the query string above String signature = "eSoTB+0GA/HfncASEFk7ONHbB3+9YrOBgK9xUyRoCDY97oXw49JYoXOL07kHrVvbngKmKFNx5fnYtDaL8WCe5LfRRgjJz1LLacriHn2ggeMmY/fTaXPoy2zQW0Fv1H362QXicTWQXgWFS5cJAIcBa2I7TLgNwXsMgjdBF2hyacW0IwfkAceGiBwDDTy6XIBAZk2Ff7w5lbZh+fa5JLNKrbvoveJk2NS3KK6INYO7UW5hukWz2cpzbHsx9lfxUJi8/ZCwUtFWZ4rdXVN+Qiw5y8S2eE2BIEfFmz7IfvrMRXa2la/rXFQfmteQo+N1sO3K1YZyoT/aA3k36glXvnj3kw=="; - HttpRequest httpRequest = new HttpRequest(requestURL, queryString) + HttpRequest httpRequest = new HttpRequest("GET", requestURL, queryString) .addParameter("SAMLResponse", samlResponseEncoded) .addParameter("RelayState", relayState) .addParameter("SigAlg", sigAlg) @@ -509,7 +509,7 @@ public void testIsInValidSign() throws URISyntaxException, IOException, XMLEntit String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; String signature = "vfWbbc47PkP3ejx4bjKsRX7lo9Ml1WRoE5J5owF/0mnyKHfSY6XbhO1wwjBV5vWdrUVX+xp6slHyAf4YoAsXFS0qhan6txDiZY4Oec6yE+l10iZbzvie06I4GPak4QrQ4gAyXOSzwCrRmJu4gnpeUxZ6IqKtdrKfAYRAcVfNKGA="; - HttpRequest httpRequest = new HttpRequest(requestURL, (String)null) + HttpRequest httpRequest = new HttpRequest("GET", requestURL, (String)null) .addParameter("SAMLResponse", samlResponseEncoded) .addParameter("RelayState", relayState) .addParameter("SigAlg", sigAlg) @@ -575,7 +575,7 @@ public void testIsInValidSignWithDeprecatedAlg() throws URISyntaxException, IOEx String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; String signature = "vfWbbc47PkP3ejx4bjKsRX7lo9Ml1WRoE5J5owF/0mnyKHfSY6XbhO1wwjBV5vWdrUVX+xp6slHyAf4YoAsXFS0qhan6txDiZY4Oec6yE+l10iZbzvie06I4GPak4QrQ4gAyXOSzwCrRmJu4gnpeUxZ6IqKtdrKfAYRAcVfNKGA="; - HttpRequest httpRequest = new HttpRequest(requestURL, (String)null) + HttpRequest httpRequest = new HttpRequest("GET", requestURL, (String)null) .addParameter("SAMLResponse", samlResponseEncoded) .addParameter("RelayState", relayState) .addParameter("SigAlg", sigAlg) @@ -644,6 +644,6 @@ public void testGetError() throws URISyntaxException, IOException, XMLEntityExce } 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); } } diff --git a/toolkit/src/main/java/com/onelogin/saml2/Auth.java b/toolkit/src/main/java/com/onelogin/saml2/Auth.java index 092f9324..e7b71929 100644 --- a/toolkit/src/main/java/com/onelogin/saml2/Auth.java +++ b/toolkit/src/main/java/com/onelogin/saml2/Auth.java @@ -869,8 +869,19 @@ public void processResponse() throws Exception { public String processSLO(Boolean keepLocalSession, String requestId, Boolean stay) throws Exception { final HttpRequest httpRequest = ServletUtils.makeHttpRequest(this.request); - final String samlRequestParameter = httpRequest.getParameter("SAMLRequest"); - final String samlResponseParameter = httpRequest.getParameter("SAMLResponse"); + final String samlRequestParameter; + final String samlResponseParameter; + + if (httpRequest.isGet()) { + samlRequestParameter = httpRequest.getParameter("SAMLRequest"); + samlResponseParameter = httpRequest.getParameter("SAMLResponse"); + } else { + // NOTE: even if the request method is POST, it's possible that parameters were sent in the + // URL; however the Redirect binding requires a GET, so we basically use this as a mechanism + // to fail the processing on non-GET requests all together. See #299. + samlRequestParameter = null; + samlResponseParameter = null; + } if (samlResponseParameter != null) { LogoutResponse logoutResponse = new LogoutResponse(settings, httpRequest); diff --git a/toolkit/src/main/java/com/onelogin/saml2/servlet/ServletUtils.java b/toolkit/src/main/java/com/onelogin/saml2/servlet/ServletUtils.java index c62bdd71..c51df270 100644 --- a/toolkit/src/main/java/com/onelogin/saml2/servlet/ServletUtils.java +++ b/toolkit/src/main/java/com/onelogin/saml2/servlet/ServletUtils.java @@ -9,8 +9,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.lang3.StringUtils; - import com.onelogin.saml2.http.HttpRequest; import com.onelogin.saml2.util.Util; @@ -32,14 +30,13 @@ private ServletUtils() { * @return a HttpRequest */ public static HttpRequest makeHttpRequest(HttpServletRequest req) { - @SuppressWarnings("unchecked") - final Map paramsAsArray = (Map) req.getParameterMap(); + final Map paramsAsArray = req.getParameterMap(); final Map> paramsAsList = new HashMap<>(); for (Map.Entry param : paramsAsArray.entrySet()) { paramsAsList.put(param.getKey(), Arrays.asList(param.getValue())); } - return new HttpRequest(req.getRequestURL().toString(), paramsAsList, req.getQueryString()); + return new HttpRequest(req.getMethod(), req.getRequestURL().toString(), paramsAsList, req.getQueryString()); } /** @@ -52,7 +49,7 @@ public static HttpRequest makeHttpRequest(HttpServletRequest req) { * @return the HOST URL */ public static String getSelfURLhost(HttpServletRequest request) { - String hostUrl = StringUtils.EMPTY; + String hostUrl; final int serverPort = request.getServerPort(); if ((serverPort == 80) || (serverPort == 443) || serverPort == 0) { hostUrl = String.format("%s://%s", request.getScheme(), request.getServerName()); diff --git a/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java b/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java index 12b62b68..5245ad10 100644 --- a/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java +++ b/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java @@ -7,10 +7,12 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.matches; import static org.mockito.Mockito.mock; @@ -626,7 +628,7 @@ public void testProcessSLONoMessage() throws Exception { */ @Test public void testProcessSLORequestKeepSession() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -655,7 +657,7 @@ public void testProcessSLORequestKeepSession() throws Exception { */ @Test public void testProcessSLORequestRemoveSession() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -683,7 +685,7 @@ public void testProcessSLORequestRemoveSession() throws Exception { */ @Test public void testProcessSLORequestStay() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -711,7 +713,7 @@ public void testProcessSLORequestStay() throws Exception { */ @Test public void testProcessSLORequestStayFalse() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -741,7 +743,7 @@ public void testProcessSLORequestStayFalse() throws Exception { */ @Test public void testProcessSLORequestStayTrue() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -760,6 +762,36 @@ public void testProcessSLORequestStayTrue() throws Exception { assertThat(target, startsWith("http://idp.example.com/simplesaml/saml2/idp/SingleLogoutService.php?SAMLResponse=")); } + /** + * Tests the processSLO methods of Auth + * Case: message delivered over incorrect HTTP method (implying wrong binding is used) + * + * @throws Exception + * + * @see com.onelogin.saml2.Auth#processSLO + */ + @Test + public void testProcessSLOWrongMethod() throws Exception { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + HttpSession session = mock(HttpSession.class); + when(request.getMethod()).thenReturn("post"); + when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); + when(request.getSession()).thenReturn(session); + + Map params = new HashMap<>(); + params.put("SAMLRequest", new String[]{"doesnt-matter"}); + params.put("SAMLResponse", new String[]{"ditto"}); + + when(request.getParameterMap()).thenReturn(params); + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); + Auth auth = new Auth(settings, request, response); + assertFalse(auth.isAuthenticated()); + assertTrue(auth.getErrors().isEmpty()); + assertThrows(Error.class, ()->auth.processSLO(false, null, true)); + assertThat(auth.getErrors().get(0), equalTo("invalid_binding")); + } + /** * Tests the processSLO methods of Auth * Case: process LogoutRequest, with RelayState and sign response @@ -770,7 +802,7 @@ public void testProcessSLORequestStayTrue() throws Exception { */ @Test public void testProcessSLORequestSignRes() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -806,7 +838,7 @@ public void testProcessSLORequestSignRes() throws Exception { */ @Test public void testProcessSLORequestInvalid() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://localhost:8080/java-saml-jspsample/sls.jsp")); @@ -837,7 +869,7 @@ public void testProcessSLORequestInvalid() throws Exception { */ @Test public void testProcessSLOResponseKeepSession() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -864,7 +896,7 @@ public void testProcessSLOResponseKeepSession() throws Exception { */ @Test public void testProcessSLOResponseRemoveSession() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -891,7 +923,7 @@ public void testProcessSLOResponseRemoveSession() throws Exception { */ @Test public void testProcessSLOResponseWrongRequestId() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -920,7 +952,7 @@ public void testProcessSLOResponseWrongRequestId() throws Exception { */ @Test public void testProcessSLOResponseStatusResponder() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); HttpSession session = mock(HttpSession.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); @@ -2123,7 +2155,7 @@ public void testGetLastLogoutRequestSent() throws IOException, SettingsException */ @Test public void testGetLastLogoutRequestReceived() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); when(request.getRequestURL()).thenReturn(new StringBuffer("/")); String samlRequestEncoded = Util.getFileAsString("data/logout_requests/logout_request.xml.base64"); @@ -2177,7 +2209,7 @@ public void testGetLastSAMLResponse() throws Exception { */ @Test public void testGetLastLogoutResponseSent() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); when(request.getRequestURL()).thenReturn(new StringBuffer("http://stuff.com/endpoints/endpoints/sls.php")); String samlRequestEncoded = Util.getFileAsString("data/logout_requests/logout_request_deflated.xml.base64"); @@ -2201,7 +2233,7 @@ public void testGetLastLogoutResponseSent() throws Exception { */ @Test public void testGetLastLogoutResponseReceived() throws Exception { - HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletRequest request = mockGet(); HttpServletResponse response = mock(HttpServletResponse.class); when(request.getRequestURL()).thenReturn(new StringBuffer("/")); String samlResponseEncoded = Util.getFileAsString("data/logout_responses/logout_response.xml.base64"); @@ -2213,4 +2245,11 @@ public void testGetLastLogoutResponseReceived() throws Exception { String logoutResponseXML = auth.getLastResponseXML(); assertThat(logoutResponseXML, containsString("