Skip to content

Commit 0c47bfb

Browse files
clemstoquartfhanik
authored andcommitted
Remove empty relay state from redirect url
1 parent 24500fa commit 0c47bfb

File tree

3 files changed

+218
-4
lines changed

3 files changed

+218
-4
lines changed

saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/servlet/filter/Saml2WebSsoAuthenticationRequestFilter.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.springframework.security.web.util.matcher.RequestMatcher;
2626
import org.springframework.security.web.util.matcher.RequestMatcher.MatchResult;
2727
import org.springframework.util.Assert;
28+
import org.springframework.util.StringUtils;
2829
import org.springframework.web.filter.OncePerRequestFilter;
2930
import org.springframework.web.util.UriComponentsBuilder;
3031
import org.springframework.web.util.UriUtils;
@@ -94,13 +95,17 @@ private String createSamlRequestRedirectUrl(HttpServletRequest request, RelyingP
9495
String xml = this.authenticationRequestFactory.createAuthenticationRequest(authNRequest);
9596
String encoded = encode(deflate(xml));
9697
String relayState = request.getParameter("RelayState");
97-
String redirect = UriComponentsBuilder
98+
UriComponentsBuilder uriBuilder = UriComponentsBuilder
9899
.fromUriString(relyingParty.getIdpWebSsoUrl())
99-
.queryParam("SAMLRequest", UriUtils.encode(encoded, StandardCharsets.ISO_8859_1))
100-
.queryParam("RelayState", UriUtils.encode(relayState, StandardCharsets.ISO_8859_1))
100+
.queryParam("SAMLRequest", UriUtils.encode(encoded, StandardCharsets.ISO_8859_1));
101+
102+
if (StringUtils.hasText(relayState)) {
103+
uriBuilder.queryParam("RelayState", UriUtils.encode(relayState, StandardCharsets.ISO_8859_1));
104+
}
105+
106+
return uriBuilder
101107
.build(true)
102108
.toUriString();
103-
return redirect;
104109
}
105110

106111
private Saml2AuthenticationRequest createAuthenticationRequest(RelyingPartyRegistration relyingParty, HttpServletRequest request) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright 2002-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.saml2.provider.service.servlet.filter;
18+
19+
import java.io.IOException;
20+
import javax.servlet.ServletException;
21+
import javax.servlet.http.HttpServletResponse;
22+
23+
import org.junit.Assert;
24+
import org.junit.Before;
25+
import org.junit.Test;
26+
import org.springframework.mock.web.MockFilterChain;
27+
import org.springframework.mock.web.MockHttpServletRequest;
28+
import org.springframework.mock.web.MockHttpServletResponse;
29+
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
30+
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository;
31+
32+
import static org.mockito.Mockito.mock;
33+
import static org.mockito.Mockito.when;
34+
import static org.springframework.security.saml2.provider.service.servlet.filter.TestSaml2SigningCredentials.signingCredential;
35+
36+
public class Saml2WebSsoAuthenticationRequestFilterTests {
37+
38+
private Saml2WebSsoAuthenticationRequestFilter filter;
39+
private RelyingPartyRegistrationRepository repository = mock(RelyingPartyRegistrationRepository.class);
40+
private MockHttpServletRequest request;
41+
private HttpServletResponse response;
42+
private MockFilterChain filterChain;
43+
44+
@Before
45+
public void setup() {
46+
filter = new Saml2WebSsoAuthenticationRequestFilter(repository);
47+
request = new MockHttpServletRequest();
48+
response = new MockHttpServletResponse();
49+
request.setPathInfo("/saml2/authenticate/registration-id");
50+
51+
filterChain = new MockFilterChain();
52+
}
53+
54+
@Test
55+
public void createSamlRequestRedirectUrlAndReturnUrlWithoutRelayState() throws ServletException, IOException {
56+
RelyingPartyRegistration relyingPartyRegistration = RelyingPartyRegistration
57+
.withRegistrationId("registration-id")
58+
.remoteIdpEntityId("idp-entity-id")
59+
.idpWebSsoUrl("sso-url")
60+
.assertionConsumerServiceUrlTemplate("template")
61+
.credentials(c -> c.add(signingCredential()))
62+
.build();
63+
64+
when(repository.findByRegistrationId("registration-id"))
65+
.thenReturn(relyingPartyRegistration);
66+
67+
filter.doFilterInternal(request, response, filterChain);
68+
69+
Assert.assertFalse(response.getHeader("Location").contains("RelayState="));
70+
}
71+
72+
@Test
73+
public void createSamlRequestRedirectUrlAndReturnUrlWithRelayState() throws ServletException, IOException {
74+
RelyingPartyRegistration relyingPartyRegistration = RelyingPartyRegistration
75+
.withRegistrationId("registration-id")
76+
.remoteIdpEntityId("idp-entity-id")
77+
.idpWebSsoUrl("sso-url")
78+
.assertionConsumerServiceUrlTemplate("template")
79+
.credentials(c -> c.add(signingCredential()))
80+
.build();
81+
82+
when(repository.findByRegistrationId("registration-id"))
83+
.thenReturn(relyingPartyRegistration);
84+
85+
request.setParameter("RelayState", "my-relay-state");
86+
87+
filter.doFilterInternal(request, response, filterChain);
88+
89+
Assert.assertTrue(response.getHeader("Location").contains("RelayState=my-relay-state"));
90+
}
91+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright 2002-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.saml2.provider.service.servlet.filter;
18+
19+
import java.io.ByteArrayInputStream;
20+
import java.security.KeyException;
21+
import java.security.PrivateKey;
22+
import java.security.cert.CertificateException;
23+
import java.security.cert.CertificateFactory;
24+
import java.security.cert.X509Certificate;
25+
26+
import org.opensaml.security.crypto.KeySupport;
27+
import org.springframework.security.saml2.Saml2Exception;
28+
import org.springframework.security.saml2.credentials.Saml2X509Credential;
29+
30+
import static java.nio.charset.StandardCharsets.UTF_8;
31+
import static org.springframework.security.saml2.credentials.Saml2X509Credential.Saml2X509CredentialType.DECRYPTION;
32+
import static org.springframework.security.saml2.credentials.Saml2X509Credential.Saml2X509CredentialType.SIGNING;
33+
34+
final class TestSaml2SigningCredentials {
35+
36+
static Saml2X509Credential signingCredential() {
37+
return new Saml2X509Credential(idpPrivateKey(), idpCertificate(), SIGNING, DECRYPTION);
38+
}
39+
40+
private static X509Certificate certificate(String cert) {
41+
ByteArrayInputStream certBytes = new ByteArrayInputStream(cert.getBytes());
42+
try {
43+
return (X509Certificate) CertificateFactory
44+
.getInstance("X.509")
45+
.generateCertificate(certBytes);
46+
}
47+
catch (CertificateException e) {
48+
throw new Saml2Exception(e);
49+
}
50+
}
51+
52+
private static PrivateKey privateKey(String key) {
53+
try {
54+
return KeySupport.decodePrivateKey(key.getBytes(UTF_8), new char[0]);
55+
}
56+
catch (KeyException e) {
57+
throw new Saml2Exception(e);
58+
}
59+
}
60+
61+
private static X509Certificate idpCertificate() {
62+
return certificate("-----BEGIN CERTIFICATE-----\n"
63+
+ "MIIEEzCCAvugAwIBAgIJAIc1qzLrv+5nMA0GCSqGSIb3DQEBCwUAMIGfMQswCQYD\n"
64+
+ "VQQGEwJVUzELMAkGA1UECAwCQ08xFDASBgNVBAcMC0Nhc3RsZSBSb2NrMRwwGgYD\n"
65+
+ "VQQKDBNTYW1sIFRlc3RpbmcgU2VydmVyMQswCQYDVQQLDAJJVDEgMB4GA1UEAwwX\n"
66+
+ "c2ltcGxlc2FtbHBocC5jZmFwcHMuaW8xIDAeBgkqhkiG9w0BCQEWEWZoYW5pa0Bw\n"
67+
+ "aXZvdGFsLmlvMB4XDTE1MDIyMzIyNDUwM1oXDTI1MDIyMjIyNDUwM1owgZ8xCzAJ\n"
68+
+ "BgNVBAYTAlVTMQswCQYDVQQIDAJDTzEUMBIGA1UEBwwLQ2FzdGxlIFJvY2sxHDAa\n"
69+
+ "BgNVBAoME1NhbWwgVGVzdGluZyBTZXJ2ZXIxCzAJBgNVBAsMAklUMSAwHgYDVQQD\n"
70+
+ "DBdzaW1wbGVzYW1scGhwLmNmYXBwcy5pbzEgMB4GCSqGSIb3DQEJARYRZmhhbmlr\n"
71+
+ "QHBpdm90YWwuaW8wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC4cn62\n"
72+
+ "E1xLqpN34PmbrKBbkOXFjzWgJ9b+pXuaRft6A339uuIQeoeH5qeSKRVTl32L0gdz\n"
73+
+ "2ZivLwZXW+cqvftVW1tvEHvzJFyxeTW3fCUeCQsebLnA2qRa07RkxTo6Nf244mWW\n"
74+
+ "RDodcoHEfDUSbxfTZ6IExSojSIU2RnD6WllYWFdD1GFpBJOmQB8rAc8wJIBdHFdQ\n"
75+
+ "nX8Ttl7hZ6rtgqEYMzYVMuJ2F2r1HSU1zSAvwpdYP6rRGFRJEfdA9mm3WKfNLSc5\n"
76+
+ "cljz0X/TXy0vVlAV95l9qcfFzPmrkNIst9FZSwpvB49LyAVke04FQPPwLgVH4gph\n"
77+
+ "iJH3jvZ7I+J5lS8VAgMBAAGjUDBOMB0GA1UdDgQWBBTTyP6Cc5HlBJ5+ucVCwGc5\n"
78+
+ "ogKNGzAfBgNVHSMEGDAWgBTTyP6Cc5HlBJ5+ucVCwGc5ogKNGzAMBgNVHRMEBTAD\n"
79+
+ "AQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAvMS4EQeP/ipV4jOG5lO6/tYCb/iJeAduO\n"
80+
+ "nRhkJk0DbX329lDLZhTTL/x/w/9muCVcvLrzEp6PN+VWfw5E5FWtZN0yhGtP9R+v\n"
81+
+ "ZnrV+oc2zGD+no1/ySFOe3EiJCO5dehxKjYEmBRv5sU/LZFKZpozKN/BMEa6CqLu\n"
82+
+ "xbzb7ykxVr7EVFXwltPxzE9TmL9OACNNyF5eJHWMRMllarUvkcXlh4pux4ks9e6z\n"
83+
+ "V9DQBy2zds9f1I3qxg0eX6JnGrXi/ZiCT+lJgVe3ZFXiejiLAiKB04sXW3ti0LW3\n"
84+
+ "lx13Y1YlQ4/tlpgTgfIJxKV6nyPiLoK0nywbMd+vpAirDt2Oc+hk\n"
85+
+ "-----END CERTIFICATE-----\n");
86+
}
87+
88+
private static PrivateKey idpPrivateKey() {
89+
return privateKey("-----BEGIN PRIVATE KEY-----\n"
90+
+ "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC4cn62E1xLqpN3\n"
91+
+ "4PmbrKBbkOXFjzWgJ9b+pXuaRft6A339uuIQeoeH5qeSKRVTl32L0gdz2ZivLwZX\n"
92+
+ "W+cqvftVW1tvEHvzJFyxeTW3fCUeCQsebLnA2qRa07RkxTo6Nf244mWWRDodcoHE\n"
93+
+ "fDUSbxfTZ6IExSojSIU2RnD6WllYWFdD1GFpBJOmQB8rAc8wJIBdHFdQnX8Ttl7h\n"
94+
+ "Z6rtgqEYMzYVMuJ2F2r1HSU1zSAvwpdYP6rRGFRJEfdA9mm3WKfNLSc5cljz0X/T\n"
95+
+ "Xy0vVlAV95l9qcfFzPmrkNIst9FZSwpvB49LyAVke04FQPPwLgVH4gphiJH3jvZ7\n"
96+
+ "I+J5lS8VAgMBAAECggEBAKyxBlIS7mcp3chvq0RF7B3PHFJMMzkwE+t3pLJcs4cZ\n"
97+
+ "nezh/KbREfP70QjXzk/llnZCvxeIs5vRu24vbdBm79qLHqBuHp8XfHHtuo2AfoAQ\n"
98+
+ "l4h047Xc/+TKMivnPQ0jX9qqndKDLqZDf5wnbslDmlskvF0a/MjsLU0TxtOfo+dB\n"
99+
+ "t55FW11cGqxZwhS5Gnr+cbw3OkHz23b9gEOt9qfwPVepeysbmm9FjU+k4yVa7rAN\n"
100+
+ "xcbzVb6Y7GCITe2tgvvEHmjB9BLmWrH3mZ3Af17YU/iN6TrpPd6Sj3QoS+2wGtAe\n"
101+
+ "HbUs3CKJu7bIHcj4poal6Kh8519S+erJTtqQ8M0ZiEECgYEA43hLYAPaUueFkdfh\n"
102+
+ "9K/7ClH6436CUH3VdizwUXi26fdhhV/I/ot6zLfU2mgEHU22LBECWQGtAFm8kv0P\n"
103+
+ "zPn+qjaR3e62l5PIlSYbnkIidzoDZ2ztu4jF5LgStlTJQPteFEGgZVl5o9DaSZOq\n"
104+
+ "Yd7G3XqXuQ1VGMW58G5FYJPtA1cCgYEAz5TPUtK+R2KXHMjUwlGY9AefQYRYmyX2\n"
105+
+ "Tn/OFgKvY8lpAkMrhPKONq7SMYc8E9v9G7A0dIOXvW7QOYSapNhKU+np3lUafR5F\n"
106+
+ "4ZN0bxZ9qjHbn3AMYeraKjeutHvlLtbHdIc1j3sxe/EzltRsYmiqLdEBW0p6hwWg\n"
107+
+ "tyGhYWVyaXMCgYAfDOKtHpmEy5nOCLwNXKBWDk7DExfSyPqEgSnk1SeS1HP5ctPK\n"
108+
+ "+1st6sIhdiVpopwFc+TwJWxqKdW18tlfT5jVv1E2DEnccw3kXilS9xAhWkfwrEvf\n"
109+
+ "V5I74GydewFl32o+NZ8hdo9GL1I8zO1rIq/et8dSOWGuWf9BtKu/vTGTTQKBgFxU\n"
110+
+ "VjsCnbvmsEwPUAL2hE/WrBFaKocnxXx5AFNt8lEyHtDwy4Sg1nygGcIJ4sD6koQk\n"
111+
+ "RdClT3LkvR04TAiSY80bN/i6ZcPNGUwSaDGZEWAIOSWbkwZijZNFnSGOEgxZX/IG\n"
112+
+ "yd39766vREEMTwEeiMNEOZQ/dmxkJm4OOVe25cLdAoGACOtPnq1Fxay80UYBf4rQ\n"
113+
+ "+bJ9yX1ulB8WIree1hD7OHSB2lRHxrVYWrglrTvkh63Lgx+EcsTV788OsvAVfPPz\n"
114+
+ "BZrn8SdDlQqalMxUBYEFwnsYD3cQ8yOUnijFVC4xNcdDv8OIqVgSk4KKxU5AshaA\n"
115+
+ "xk6Mox+u8Cc2eAK12H13i+8=\n"
116+
+ "-----END PRIVATE KEY-----\n");
117+
}
118+
}

0 commit comments

Comments
 (0)