From 32c02fbedb8ac2c79a1df3b05884077ee329e243 Mon Sep 17 00:00:00 2001 From: Clement Stoquart Date: Tue, 26 Nov 2019 09:20:27 +0100 Subject: [PATCH] Remove empty relay state from redirect url --- ...aml2WebSsoAuthenticationRequestFilter.java | 13 +- ...ebSsoAuthenticationRequestFilterTests.java | 91 ++++++++++++++ .../filter/TestSaml2SigningCredentials.java | 118 ++++++++++++++++++ 3 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilterTests.java create mode 100644 saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/TestSaml2SigningCredentials.java diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilter.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilter.java index 7bc25a0994..b27927647f 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilter.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilter.java @@ -25,6 +25,7 @@ import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher.MatchResult; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.util.UriComponentsBuilder; import org.springframework.web.util.UriUtils; @@ -94,13 +95,17 @@ public class Saml2WebSsoAuthenticationRequestFilter extends OncePerRequestFilter String xml = this.authenticationRequestFactory.createAuthenticationRequest(authNRequest); String encoded = encode(deflate(xml)); String relayState = request.getParameter("RelayState"); - String redirect = UriComponentsBuilder + UriComponentsBuilder uriBuilder = UriComponentsBuilder .fromUriString(relyingParty.getIdpWebSsoUrl()) - .queryParam("SAMLRequest", UriUtils.encode(encoded, StandardCharsets.ISO_8859_1)) - .queryParam("RelayState", UriUtils.encode(relayState, StandardCharsets.ISO_8859_1)) + .queryParam("SAMLRequest", UriUtils.encode(encoded, StandardCharsets.ISO_8859_1)); + + if (StringUtils.hasText(relayState)) { + uriBuilder.queryParam("RelayState", UriUtils.encode(relayState, StandardCharsets.ISO_8859_1)); + } + + return uriBuilder .build(true) .toUriString(); - return redirect; } private Saml2AuthenticationRequest createAuthenticationRequest(RelyingPartyRegistration relyingParty, HttpServletRequest request) { diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilterTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilterTests.java new file mode 100644 index 0000000000..c17e9c820c --- /dev/null +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilterTests.java @@ -0,0 +1,91 @@ +/* + * Copyright 2002-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.saml2.provider.service.servlet.filter; + +import java.io.IOException; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.springframework.mock.web.MockFilterChain; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration; +import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.springframework.security.saml2.provider.service.servlet.filter.TestSaml2SigningCredentials.signingCredential; + +public class Saml2WebSsoAuthenticationRequestFilterTests { + + private Saml2WebSsoAuthenticationRequestFilter filter; + private RelyingPartyRegistrationRepository repository = mock(RelyingPartyRegistrationRepository.class); + private MockHttpServletRequest request; + private HttpServletResponse response; + private MockFilterChain filterChain; + + @Before + public void setup() { + filter = new Saml2WebSsoAuthenticationRequestFilter(repository); + request = new MockHttpServletRequest(); + response = new MockHttpServletResponse(); + request.setPathInfo("/saml2/authenticate/registration-id"); + + filterChain = new MockFilterChain(); + } + + @Test + public void createSamlRequestRedirectUrlAndReturnUrlWithoutRelayState() throws ServletException, IOException { + RelyingPartyRegistration relyingPartyRegistration = RelyingPartyRegistration + .withRegistrationId("registration-id") + .remoteIdpEntityId("idp-entity-id") + .idpWebSsoUrl("sso-url") + .assertionConsumerServiceUrlTemplate("template") + .credentials(c -> c.add(signingCredential())) + .build(); + + when(repository.findByRegistrationId("registration-id")) + .thenReturn(relyingPartyRegistration); + + filter.doFilterInternal(request, response, filterChain); + + Assert.assertFalse(response.getHeader("Location").contains("RelayState=")); + } + + @Test + public void createSamlRequestRedirectUrlAndReturnUrlWithRelayState() throws ServletException, IOException { + RelyingPartyRegistration relyingPartyRegistration = RelyingPartyRegistration + .withRegistrationId("registration-id") + .remoteIdpEntityId("idp-entity-id") + .idpWebSsoUrl("sso-url") + .assertionConsumerServiceUrlTemplate("template") + .credentials(c -> c.add(signingCredential())) + .build(); + + when(repository.findByRegistrationId("registration-id")) + .thenReturn(relyingPartyRegistration); + + request.setParameter("RelayState", "my-relay-state"); + + filter.doFilterInternal(request, response, filterChain); + + Assert.assertTrue(response.getHeader("Location").contains("RelayState=my-relay-state")); + } +} diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/TestSaml2SigningCredentials.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/TestSaml2SigningCredentials.java new file mode 100644 index 0000000000..3aa718227e --- /dev/null +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/servlet/filter/TestSaml2SigningCredentials.java @@ -0,0 +1,118 @@ +/* + * Copyright 2002-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.saml2.provider.service.servlet.filter; + +import java.io.ByteArrayInputStream; +import java.security.KeyException; +import java.security.PrivateKey; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; + +import org.opensaml.security.crypto.KeySupport; +import org.springframework.security.saml2.Saml2Exception; +import org.springframework.security.saml2.credentials.Saml2X509Credential; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.springframework.security.saml2.credentials.Saml2X509Credential.Saml2X509CredentialType.DECRYPTION; +import static org.springframework.security.saml2.credentials.Saml2X509Credential.Saml2X509CredentialType.SIGNING; + +final class TestSaml2SigningCredentials { + + static Saml2X509Credential signingCredential() { + return new Saml2X509Credential(idpPrivateKey(), idpCertificate(), SIGNING, DECRYPTION); + } + + private static X509Certificate certificate(String cert) { + ByteArrayInputStream certBytes = new ByteArrayInputStream(cert.getBytes()); + try { + return (X509Certificate) CertificateFactory + .getInstance("X.509") + .generateCertificate(certBytes); + } + catch (CertificateException e) { + throw new Saml2Exception(e); + } + } + + private static PrivateKey privateKey(String key) { + try { + return KeySupport.decodePrivateKey(key.getBytes(UTF_8), new char[0]); + } + catch (KeyException e) { + throw new Saml2Exception(e); + } + } + + private static X509Certificate idpCertificate() { + return certificate("-----BEGIN CERTIFICATE-----\n" + + "MIIEEzCCAvugAwIBAgIJAIc1qzLrv+5nMA0GCSqGSIb3DQEBCwUAMIGfMQswCQYD\n" + + "VQQGEwJVUzELMAkGA1UECAwCQ08xFDASBgNVBAcMC0Nhc3RsZSBSb2NrMRwwGgYD\n" + + "VQQKDBNTYW1sIFRlc3RpbmcgU2VydmVyMQswCQYDVQQLDAJJVDEgMB4GA1UEAwwX\n" + + "c2ltcGxlc2FtbHBocC5jZmFwcHMuaW8xIDAeBgkqhkiG9w0BCQEWEWZoYW5pa0Bw\n" + + "aXZvdGFsLmlvMB4XDTE1MDIyMzIyNDUwM1oXDTI1MDIyMjIyNDUwM1owgZ8xCzAJ\n" + + "BgNVBAYTAlVTMQswCQYDVQQIDAJDTzEUMBIGA1UEBwwLQ2FzdGxlIFJvY2sxHDAa\n" + + "BgNVBAoME1NhbWwgVGVzdGluZyBTZXJ2ZXIxCzAJBgNVBAsMAklUMSAwHgYDVQQD\n" + + "DBdzaW1wbGVzYW1scGhwLmNmYXBwcy5pbzEgMB4GCSqGSIb3DQEJARYRZmhhbmlr\n" + + "QHBpdm90YWwuaW8wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC4cn62\n" + + "E1xLqpN34PmbrKBbkOXFjzWgJ9b+pXuaRft6A339uuIQeoeH5qeSKRVTl32L0gdz\n" + + "2ZivLwZXW+cqvftVW1tvEHvzJFyxeTW3fCUeCQsebLnA2qRa07RkxTo6Nf244mWW\n" + + "RDodcoHEfDUSbxfTZ6IExSojSIU2RnD6WllYWFdD1GFpBJOmQB8rAc8wJIBdHFdQ\n" + + "nX8Ttl7hZ6rtgqEYMzYVMuJ2F2r1HSU1zSAvwpdYP6rRGFRJEfdA9mm3WKfNLSc5\n" + + "cljz0X/TXy0vVlAV95l9qcfFzPmrkNIst9FZSwpvB49LyAVke04FQPPwLgVH4gph\n" + + "iJH3jvZ7I+J5lS8VAgMBAAGjUDBOMB0GA1UdDgQWBBTTyP6Cc5HlBJ5+ucVCwGc5\n" + + "ogKNGzAfBgNVHSMEGDAWgBTTyP6Cc5HlBJ5+ucVCwGc5ogKNGzAMBgNVHRMEBTAD\n" + + "AQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAvMS4EQeP/ipV4jOG5lO6/tYCb/iJeAduO\n" + + "nRhkJk0DbX329lDLZhTTL/x/w/9muCVcvLrzEp6PN+VWfw5E5FWtZN0yhGtP9R+v\n" + + "ZnrV+oc2zGD+no1/ySFOe3EiJCO5dehxKjYEmBRv5sU/LZFKZpozKN/BMEa6CqLu\n" + + "xbzb7ykxVr7EVFXwltPxzE9TmL9OACNNyF5eJHWMRMllarUvkcXlh4pux4ks9e6z\n" + + "V9DQBy2zds9f1I3qxg0eX6JnGrXi/ZiCT+lJgVe3ZFXiejiLAiKB04sXW3ti0LW3\n" + + "lx13Y1YlQ4/tlpgTgfIJxKV6nyPiLoK0nywbMd+vpAirDt2Oc+hk\n" + + "-----END CERTIFICATE-----\n"); + } + + private static PrivateKey idpPrivateKey() { + return privateKey("-----BEGIN PRIVATE KEY-----\n" + + "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC4cn62E1xLqpN3\n" + + "4PmbrKBbkOXFjzWgJ9b+pXuaRft6A339uuIQeoeH5qeSKRVTl32L0gdz2ZivLwZX\n" + + "W+cqvftVW1tvEHvzJFyxeTW3fCUeCQsebLnA2qRa07RkxTo6Nf244mWWRDodcoHE\n" + + "fDUSbxfTZ6IExSojSIU2RnD6WllYWFdD1GFpBJOmQB8rAc8wJIBdHFdQnX8Ttl7h\n" + + "Z6rtgqEYMzYVMuJ2F2r1HSU1zSAvwpdYP6rRGFRJEfdA9mm3WKfNLSc5cljz0X/T\n" + + "Xy0vVlAV95l9qcfFzPmrkNIst9FZSwpvB49LyAVke04FQPPwLgVH4gphiJH3jvZ7\n" + + "I+J5lS8VAgMBAAECggEBAKyxBlIS7mcp3chvq0RF7B3PHFJMMzkwE+t3pLJcs4cZ\n" + + "nezh/KbREfP70QjXzk/llnZCvxeIs5vRu24vbdBm79qLHqBuHp8XfHHtuo2AfoAQ\n" + + "l4h047Xc/+TKMivnPQ0jX9qqndKDLqZDf5wnbslDmlskvF0a/MjsLU0TxtOfo+dB\n" + + "t55FW11cGqxZwhS5Gnr+cbw3OkHz23b9gEOt9qfwPVepeysbmm9FjU+k4yVa7rAN\n" + + "xcbzVb6Y7GCITe2tgvvEHmjB9BLmWrH3mZ3Af17YU/iN6TrpPd6Sj3QoS+2wGtAe\n" + + "HbUs3CKJu7bIHcj4poal6Kh8519S+erJTtqQ8M0ZiEECgYEA43hLYAPaUueFkdfh\n" + + "9K/7ClH6436CUH3VdizwUXi26fdhhV/I/ot6zLfU2mgEHU22LBECWQGtAFm8kv0P\n" + + "zPn+qjaR3e62l5PIlSYbnkIidzoDZ2ztu4jF5LgStlTJQPteFEGgZVl5o9DaSZOq\n" + + "Yd7G3XqXuQ1VGMW58G5FYJPtA1cCgYEAz5TPUtK+R2KXHMjUwlGY9AefQYRYmyX2\n" + + "Tn/OFgKvY8lpAkMrhPKONq7SMYc8E9v9G7A0dIOXvW7QOYSapNhKU+np3lUafR5F\n" + + "4ZN0bxZ9qjHbn3AMYeraKjeutHvlLtbHdIc1j3sxe/EzltRsYmiqLdEBW0p6hwWg\n" + + "tyGhYWVyaXMCgYAfDOKtHpmEy5nOCLwNXKBWDk7DExfSyPqEgSnk1SeS1HP5ctPK\n" + + "+1st6sIhdiVpopwFc+TwJWxqKdW18tlfT5jVv1E2DEnccw3kXilS9xAhWkfwrEvf\n" + + "V5I74GydewFl32o+NZ8hdo9GL1I8zO1rIq/et8dSOWGuWf9BtKu/vTGTTQKBgFxU\n" + + "VjsCnbvmsEwPUAL2hE/WrBFaKocnxXx5AFNt8lEyHtDwy4Sg1nygGcIJ4sD6koQk\n" + + "RdClT3LkvR04TAiSY80bN/i6ZcPNGUwSaDGZEWAIOSWbkwZijZNFnSGOEgxZX/IG\n" + + "yd39766vREEMTwEeiMNEOZQ/dmxkJm4OOVe25cLdAoGACOtPnq1Fxay80UYBf4rQ\n" + + "+bJ9yX1ulB8WIree1hD7OHSB2lRHxrVYWrglrTvkh63Lgx+EcsTV788OsvAVfPPz\n" + + "BZrn8SdDlQqalMxUBYEFwnsYD3cQ8yOUnijFVC4xNcdDv8OIqVgSk4KKxU5AshaA\n" + + "xk6Mox+u8Cc2eAK12H13i+8=\n" + + "-----END PRIVATE KEY-----\n"); + } +}