diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a5ac38a2505..b485b33ca7d 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -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" diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidator.java index 20ff4fec1e0..0406d70ee4e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidator.java @@ -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; @@ -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(); + } + Saml2LogoutValidatorResult result = delegate.validate(parameters); if (!result.hasErrors()) { return result; diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidator.java index 23e0f679182..ed8e708432d 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidator.java @@ -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; @@ -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(); + } + Saml2LogoutValidatorResult result = delegate.validate(parameters); if (!result.hasErrors()) { return result; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidatorTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidatorTest.java index ce065169036..fbf484942df 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidatorTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutRequestValidatorTest.java @@ -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) @@ -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); + + Saml2LogoutValidatorResult result = validator.validate(parameters); + + assertThat(result.hasErrors()).isFalse(); + verify(delegate, never()).validate(any()); + } } \ No newline at end of file diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidatorTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidatorTest.java index 3aab59044cf..166324210ba 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidatorTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/SamlLogoutResponseValidatorTest.java @@ -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) @@ -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); + + Saml2LogoutValidatorResult result = validator.validate(parameters); + + assertThat(result.hasErrors()).isFalse(); + verify(delegate, never()).validate(any()); + } } \ No newline at end of file diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/saml/SamlAuthenticationMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/saml/SamlAuthenticationMockMvcTests.java index 58606024856..3398f722902 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/saml/SamlAuthenticationMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/saml/SamlAuthenticationMockMvcTests.java @@ -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)); + } + private static void createUser( JdbcScimUserProvisioning jdbcScimUserProvisioning, IdentityZone identityZone @@ -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") @@ -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("