Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ orgJson = "20260522"
passay = "2.0.0"
selenium = "4.44.0"
sonarqube = "7.3.1.8318"
springBoot = "4.0.6"
springBoot = "4.1.0"
springDependencyManagement = "1.1.7"
springDocOpenapi = "3.0.3"
statsdClient = "4.4.5"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.cloudfoundry.identity.uaa.authentication;

import org.springframework.security.saml2.core.Saml2Error;
import org.springframework.security.saml2.core.Saml2ParameterNames;
import org.springframework.security.saml2.provider.service.authentication.logout.OpenSaml5LogoutRequestValidator;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutRequestValidator;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutRequestValidatorParameters;
Expand All @@ -26,6 +27,13 @@ public SamlLogoutRequestValidator(Saml2LogoutRequestValidator delegate) {

@Override
public Saml2LogoutValidatorResult validate(Saml2LogoutRequestValidatorParameters parameters) {
// Spring Security 7.1.0 throws NPE in RedirectParameters when SigAlg is absent (unsigned
// redirect-binding logout request). Treat the absence of a signature as acceptable — consistent
// with this validator's policy of not requiring signatures on logout messages.
if (parameters != null && parameters.getLogoutRequest().getParameters().get(Saml2ParameterNames.SIG_ALG) == null) {
return Saml2LogoutValidatorResult.success();
}
Comment on lines +30 to +35

Saml2LogoutValidatorResult result = delegate.validate(parameters);
if (!result.hasErrors()) {
return result;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.cloudfoundry.identity.uaa.authentication;

import org.springframework.security.saml2.core.Saml2Error;
import org.springframework.security.saml2.core.Saml2ParameterNames;
import org.springframework.security.saml2.provider.service.authentication.logout.OpenSaml5LogoutResponseValidator;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutResponseValidator;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutResponseValidatorParameters;
Expand All @@ -27,6 +28,13 @@ public SamlLogoutResponseValidator(Saml2LogoutResponseValidator delegate) {

@Override
public Saml2LogoutValidatorResult validate(Saml2LogoutResponseValidatorParameters parameters) {
// Spring Security 7.1.0 throws NPE in RedirectParameters when SigAlg is absent (unsigned
// redirect-binding logout response). Treat the absence of a signature as acceptable — consistent
// with this validator's policy of not requiring signatures on logout messages.
if (parameters != null && parameters.getLogoutResponse().getParameters().get(Saml2ParameterNames.SIG_ALG) == null) {
return Saml2LogoutValidatorResult.success();
}
Comment on lines +31 to +36

Saml2LogoutValidatorResult result = delegate.validate(parameters);
if (!result.hasErrors()) {
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.security.saml2.core.Saml2Error;
import org.springframework.security.saml2.core.Saml2ErrorCodes;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutRequest;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutRequestValidator;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutRequestValidatorParameters;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutValidatorResult;

import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -49,4 +56,17 @@ void validateDifferentErrorIsPassedThru() {
Saml2LogoutValidatorResult result = validator.validate(null);
assertThat(result.hasErrors()).isTrue();
}

@Test
void unsignedRequestSucceedsWithoutCallingDelegate() {
Saml2LogoutRequest logoutRequest = mock(Saml2LogoutRequest.class);
when(logoutRequest.getParameters()).thenReturn(Collections.emptyMap());
Saml2LogoutRequestValidatorParameters parameters = mock(Saml2LogoutRequestValidatorParameters.class);
when(parameters.getLogoutRequest()).thenReturn(logoutRequest);

Comment on lines +61 to +66
Saml2LogoutValidatorResult result = validator.validate(parameters);

assertThat(result.hasErrors()).isFalse();
verify(delegate, never()).validate(any());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.security.saml2.core.Saml2Error;
import org.springframework.security.saml2.core.Saml2ErrorCodes;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutResponse;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutResponseValidator;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutResponseValidatorParameters;
import org.springframework.security.saml2.provider.service.authentication.logout.Saml2LogoutValidatorResult;

import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -49,4 +56,17 @@ void validateDifferentErrorIsPassedThru() {
Saml2LogoutValidatorResult result = validator.validate(null);
assertThat(result.hasErrors()).isTrue();
}

@Test
void unsignedResponseSucceedsWithoutCallingDelegate() {
Saml2LogoutResponse logoutResponse = mock(Saml2LogoutResponse.class);
when(logoutResponse.getParameters()).thenReturn(Collections.emptyMap());
Saml2LogoutResponseValidatorParameters parameters = mock(Saml2LogoutResponseValidatorParameters.class);
when(parameters.getLogoutResponse()).thenReturn(logoutResponse);

Comment on lines +61 to +66
Saml2LogoutValidatorResult result = validator.validate(parameters);

assertThat(result.hasErrors()).isFalse();
verify(delegate, never()).validate(any());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ class SamlAuthenticationMockMvcTests {

private JdbcIdentityProviderProvisioning jdbcIdentityProviderProvisioning;

private static String extractSamlRequestFromPostForm(String html) {
int nameIdx = html.indexOf("name=\"SAMLRequest\"");
int valueStart = html.indexOf("value=\"", nameIdx) + "value=\"".length();
return html.substring(valueStart, html.indexOf("\"", valueStart));
}
Comment on lines +87 to +91

private static void createUser(
JdbcScimUserProvisioning jdbcScimUserProvisioning,
IdentityZone identityZone
Expand Down Expand Up @@ -150,8 +156,6 @@ void sendAuthnRequestToIdpRedirectBindingMode() throws Exception {

@Test
void sendAuthnRequestToIdpPostBindingMode() throws Exception {
final String samlRequestMatch = "name=\"SAMLRequest\" value=\"";

MvcResult mvcResult = mockMvc.perform(get("/uaa/saml2/authenticate/%s".formatted("testsaml-post-binding"))
.contextPath("/uaa")
.header(HOST, "localhost:8080")
Expand All @@ -163,10 +167,7 @@ void sendAuthnRequestToIdpPostBindingMode() throws Exception {
.andReturn();

// Decode the SAMLRequest and check the AssertionConsumerServiceURL
String contentHtml = mvcResult.getResponse().getContentAsString();
contentHtml = contentHtml.substring(contentHtml.indexOf(samlRequestMatch) + samlRequestMatch.length());
contentHtml = contentHtml.substring(0, contentHtml.indexOf("\""));
String samlRequestXml = new String(samlDecode(contentHtml), StandardCharsets.UTF_8);
String samlRequestXml = new String(samlDecode(extractSamlRequestFromPostForm(mvcResult.getResponse().getContentAsString())), StandardCharsets.UTF_8);
assertThat(samlRequestXml).contains("<saml2p:AuthnRequest");

// In the post-binding, Signature is part of the SAML AuthnRequest
Expand Down Expand Up @@ -271,8 +272,6 @@ void sendAuthnRequestFromNonDefaultZoneToIdpPostBindingMode() throws Exception {
// create IDP in non-default zone
createMockSamlIdpInSpZone("classpath:test-saml-idp-metadata-post-binding.xml", "testsaml-post-binding");

final String samlRequestMatch = "name=\"SAMLRequest\" value=\"";

MvcResult mvcResult = mockMvc.perform(get("/uaa/saml2/authenticate/%s".formatted("testsaml-post-binding"))
.contextPath("/uaa")
.header(HOST, "%s.localhost:8080".formatted(spZone.getSubdomain()))
Expand All @@ -284,10 +283,7 @@ void sendAuthnRequestFromNonDefaultZoneToIdpPostBindingMode() throws Exception {
.andReturn();

// Decode the SAMLRequest and check the AssertionConsumerServiceURL
String contentHtml = mvcResult.getResponse().getContentAsString();
contentHtml = contentHtml.substring(contentHtml.indexOf(samlRequestMatch) + samlRequestMatch.length());
contentHtml = contentHtml.substring(0, contentHtml.indexOf("\""));
String samlRequestXml = new String(samlDecode(contentHtml), StandardCharsets.UTF_8);
String samlRequestXml = new String(samlDecode(extractSamlRequestFromPostForm(mvcResult.getResponse().getContentAsString())), StandardCharsets.UTF_8);
assertThat(samlRequestXml).contains("<saml2p:AuthnRequest");

XmlAssert xmlAssert = XmlAssert.assertThat(samlRequestXml)
Expand Down Expand Up @@ -419,8 +415,6 @@ void unsignedAuthnRequestViaIdpRedirectBindingMode() throws Exception {

@Test
void unsignedAuthnRequestViaIdpPostBindingMode() throws Exception {
final String samlRequestMatch = "name=\"SAMLRequest\" value=\"";

MvcResult mvcResult = mockMvc.perform(get("/uaa/saml2/authenticate/%s".formatted("testsaml-post-binding"))
.contextPath("/uaa")
.header(HOST, "localhost:8080")
Expand All @@ -432,10 +426,7 @@ void unsignedAuthnRequestViaIdpPostBindingMode() throws Exception {
.andReturn();

// Decode the SAMLRequest and check the AssertionConsumerServiceURL
String contentHtml = mvcResult.getResponse().getContentAsString();
contentHtml = contentHtml.substring(contentHtml.indexOf(samlRequestMatch) + samlRequestMatch.length());
contentHtml = contentHtml.substring(0, contentHtml.indexOf("\""));
String samlRequestXml = new String(samlDecode(contentHtml), StandardCharsets.UTF_8);
String samlRequestXml = new String(samlDecode(extractSamlRequestFromPostForm(mvcResult.getResponse().getContentAsString())), StandardCharsets.UTF_8);
assertThat(samlRequestXml).contains("<saml2p:AuthnRequest");

// In the post-binding, Signature is part of the SAML AuthnRequest
Expand Down
Loading