Skip to content

Commit 88f9529

Browse files
jzheauxjzheaux
committed
Correctly encode query parameters
Issue gh-11235
1 parent 5540bbc commit 88f9529

File tree

2 files changed

+62
-16
lines changed

2 files changed

+62
-16
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurerTests.java

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.time.Instant;
2121
import java.util.Collection;
2222
import java.util.Collections;
23+
import java.util.Map;
2324
import java.util.function.Consumer;
2425

2526
import org.junit.jupiter.api.AfterEach;
@@ -72,6 +73,9 @@
7273
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
7374
import org.springframework.test.web.servlet.MockMvc;
7475
import org.springframework.test.web.servlet.MvcResult;
76+
import org.springframework.test.web.servlet.request.RequestPostProcessor;
77+
import org.springframework.web.util.UriComponentsBuilder;
78+
import org.springframework.web.util.UriUtils;
7579

7680
import static org.assertj.core.api.Assertions.assertThat;
7781
import static org.hamcrest.Matchers.containsString;
@@ -254,10 +258,9 @@ public void saml2LogoutRequestWhenDefaultsThenLogsOutAndSendsLogoutResponse() th
254258
principal.setRelyingPartyRegistrationId("get");
255259
Saml2Authentication user = new Saml2Authentication(principal, "response",
256260
AuthorityUtils.createAuthorityList("ROLE_USER"));
257-
MvcResult result = this.mvc
258-
.perform(get("/logout/saml2/slo").param("SAMLRequest", this.apLogoutRequest)
259-
.param("RelayState", this.apLogoutRequestRelayState).param("SigAlg", this.apLogoutRequestSigAlg)
260-
.param("Signature", this.apLogoutRequestSignature).with(authentication(user)))
261+
MvcResult result = this.mvc.perform(get("/logout/saml2/slo").param("SAMLRequest", this.apLogoutRequest)
262+
.param("RelayState", this.apLogoutRequestRelayState).param("SigAlg", this.apLogoutRequestSigAlg)
263+
.param("Signature", this.apLogoutRequestSignature).with(samlQueryString()).with(authentication(user)))
261264
.andExpect(status().isFound()).andReturn();
262265
String location = result.getResponse().getHeader("Location");
263266
assertThat(location).startsWith("https://ap.example.org/logout/saml2/response");
@@ -316,8 +319,8 @@ public void saml2LogoutResponseWhenDefaultsThenRedirects() throws Exception {
316319
assertThat(this.logoutRequestRepository.loadLogoutRequest(this.request)).isNotNull();
317320
this.mvc.perform(get("/logout/saml2/slo").session(((MockHttpSession) this.request.getSession()))
318321
.param("SAMLResponse", this.apLogoutResponse).param("RelayState", this.apLogoutResponseRelayState)
319-
.param("SigAlg", this.apLogoutResponseSigAlg).param("Signature", this.apLogoutResponseSignature))
320-
.andExpect(status().isFound()).andExpect(redirectedUrl("/login?logout"));
322+
.param("SigAlg", this.apLogoutResponseSigAlg).param("Signature", this.apLogoutResponseSignature)
323+
.with(samlQueryString())).andExpect(status().isFound()).andExpect(redirectedUrl("/login?logout"));
321324
verifyNoInteractions(getBean(LogoutHandler.class));
322325
assertThat(this.logoutRequestRepository.loadLogoutRequest(this.request)).isNull();
323326
}
@@ -334,8 +337,9 @@ public void saml2LogoutResponseWhenInvalidSamlResponseThen401() throws Exception
334337
Saml2Utils.samlInflate(Saml2Utils.samlDecode(this.apLogoutResponse)).getBytes(StandardCharsets.UTF_8));
335338
this.mvc.perform(post("/logout/saml2/slo").session((MockHttpSession) this.request.getSession())
336339
.param("SAMLResponse", deflatedApLogoutResponse).param("RelayState", this.rpLogoutRequestRelayState)
337-
.param("SigAlg", this.apLogoutRequestSigAlg).param("Signature", this.apLogoutResponseSignature))
338-
.andExpect(status().reason(containsString("invalid_signature"))).andExpect(status().isUnauthorized());
340+
.param("SigAlg", this.apLogoutRequestSigAlg).param("Signature", this.apLogoutResponseSignature)
341+
.with(samlQueryString())).andExpect(status().reason(containsString("invalid_signature")))
342+
.andExpect(status().isUnauthorized());
339343
verifyNoInteractions(getBean(LogoutHandler.class));
340344
}
341345

@@ -398,6 +402,10 @@ private <T> T getBean(Class<T> clazz) {
398402
return this.spring.getContext().getBean(clazz);
399403
}
400404

405+
private SamlQueryStringRequestPostProcessor samlQueryString() {
406+
return new SamlQueryStringRequestPostProcessor();
407+
}
408+
401409
@EnableWebSecurity
402410
@Import(Saml2LoginConfigBeans.class)
403411
static class Saml2LogoutDefaultsConfig {
@@ -602,4 +610,19 @@ public <O> O postProcess(O object) {
602610

603611
}
604612

613+
static class SamlQueryStringRequestPostProcessor implements RequestPostProcessor {
614+
615+
@Override
616+
public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) {
617+
UriComponentsBuilder builder = UriComponentsBuilder.newInstance();
618+
for (Map.Entry<String, String[]> entries : request.getParameterMap().entrySet()) {
619+
builder.queryParam(entries.getKey(),
620+
UriUtils.encode(entries.getValue()[0], StandardCharsets.ISO_8859_1));
621+
}
622+
request.setQueryString(builder.build(true).toUriString().substring(1));
623+
return request;
624+
}
625+
626+
}
627+
605628
}

‎config/src/test/java/org/springframework/security/config/http/Saml2LogoutBeanDefinitionParserTests.java

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.nio.charset.StandardCharsets;
2020
import java.time.Instant;
2121
import java.util.Collections;
22+
import java.util.Map;
2223

2324
import org.junit.jupiter.api.BeforeEach;
2425
import org.junit.jupiter.api.Test;
@@ -54,6 +55,9 @@
5455
import org.springframework.test.context.junit.jupiter.SpringExtension;
5556
import org.springframework.test.web.servlet.MockMvc;
5657
import org.springframework.test.web.servlet.MvcResult;
58+
import org.springframework.test.web.servlet.request.RequestPostProcessor;
59+
import org.springframework.web.util.UriComponentsBuilder;
60+
import org.springframework.web.util.UriUtils;
5761

5862
import static org.assertj.core.api.Assertions.assertThat;
5963
import static org.hamcrest.Matchers.containsString;
@@ -221,10 +225,9 @@ public void saml2LogoutRequestWhenDefaultsThenLogsOutAndSendsLogoutResponse() th
221225
principal.setRelyingPartyRegistrationId("get");
222226
Saml2Authentication user = new Saml2Authentication(principal, "response",
223227
AuthorityUtils.createAuthorityList("ROLE_USER"));
224-
MvcResult result = this.mvc
225-
.perform(get("/logout/saml2/slo").param("SAMLRequest", this.apLogoutRequest)
226-
.param("RelayState", this.apLogoutRequestRelayState).param("SigAlg", this.apLogoutRequestSigAlg)
227-
.param("Signature", this.apLogoutRequestSignature).with(authentication(user)))
228+
MvcResult result = this.mvc.perform(get("/logout/saml2/slo").param("SAMLRequest", this.apLogoutRequest)
229+
.param("RelayState", this.apLogoutRequestRelayState).param("SigAlg", this.apLogoutRequestSigAlg)
230+
.param("Signature", this.apLogoutRequestSignature).with(samlQueryString()).with(authentication(user)))
228231
.andExpect(status().isFound()).andReturn();
229232
String location = result.getResponse().getHeader("Location");
230233
assertThat(location).startsWith("https://ap.example.org/logout/saml2/response");
@@ -281,8 +284,8 @@ public void saml2LogoutResponseWhenDefaultsThenRedirects() throws Exception {
281284
assertThat(this.logoutRequestRepository.loadLogoutRequest(this.request)).isNotNull();
282285
this.mvc.perform(get("/logout/saml2/slo").session(((MockHttpSession) this.request.getSession()))
283286
.param("SAMLResponse", this.apLogoutResponse).param("RelayState", this.apLogoutResponseRelayState)
284-
.param("SigAlg", this.apLogoutResponseSigAlg).param("Signature", this.apLogoutResponseSignature))
285-
.andExpect(status().isFound()).andExpect(redirectedUrl("/login?logout"));
287+
.param("SigAlg", this.apLogoutResponseSigAlg).param("Signature", this.apLogoutResponseSignature)
288+
.with(samlQueryString())).andExpect(status().isFound()).andExpect(redirectedUrl("/login?logout"));
286289
assertThat(this.logoutRequestRepository.loadLogoutRequest(this.request)).isNull();
287290
}
288291

@@ -298,8 +301,9 @@ public void saml2LogoutResponseWhenInvalidSamlResponseThen401() throws Exception
298301
Saml2Utils.samlInflate(Saml2Utils.samlDecode(this.apLogoutResponse)).getBytes(StandardCharsets.UTF_8));
299302
this.mvc.perform(post("/logout/saml2/slo").session((MockHttpSession) this.request.getSession())
300303
.param("SAMLResponse", deflatedApLogoutResponse).param("RelayState", this.rpLogoutRequestRelayState)
301-
.param("SigAlg", this.apLogoutRequestSigAlg).param("Signature", this.apLogoutResponseSignature))
302-
.andExpect(status().reason(containsString("invalid_signature"))).andExpect(status().isUnauthorized());
304+
.param("SigAlg", this.apLogoutRequestSigAlg).param("Signature", this.apLogoutResponseSignature)
305+
.with(samlQueryString())).andExpect(status().reason(containsString("invalid_signature")))
306+
.andExpect(status().isUnauthorized());
303307
}
304308

305309
@Test
@@ -324,4 +328,23 @@ private String xml(String configName) {
324328
return CONFIG_LOCATION_PREFIX + "-" + configName + ".xml";
325329
}
326330

331+
private SamlQueryStringRequestPostProcessor samlQueryString() {
332+
return new SamlQueryStringRequestPostProcessor();
333+
}
334+
335+
static class SamlQueryStringRequestPostProcessor implements RequestPostProcessor {
336+
337+
@Override
338+
public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) {
339+
UriComponentsBuilder builder = UriComponentsBuilder.newInstance();
340+
for (Map.Entry<String, String[]> entries : request.getParameterMap().entrySet()) {
341+
builder.queryParam(entries.getKey(),
342+
UriUtils.encode(entries.getValue()[0], StandardCharsets.ISO_8859_1));
343+
}
344+
request.setQueryString(builder.build(true).toUriString().substring(1));
345+
return request;
346+
}
347+
348+
}
349+
327350
}

0 commit comments

Comments
 (0)