From b9453da34385c5bf71de244035cdb795cfc25dce Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Fri, 17 Dec 2021 15:18:41 -0700 Subject: [PATCH] Support No SingleLogoutServiceLocation Closes gh-10674 --- .../metadata/OpenSamlMetadataResolver.java | 4 +++- .../RelyingPartyRegistration.java | 2 ++ .../logout/OpenSamlLogoutRequestResolver.java | 3 +++ .../OpenSamlLogoutResponseResolver.java | 3 +++ .../logout/Saml2LogoutRequestFilter.java | 6 ++++++ .../logout/Saml2LogoutResponseFilter.java | 6 ++++++ .../OpenSaml3LogoutRequestResolverTests.java | 4 +++- .../OpenSaml3LogoutResponseResolverTests.java | 5 ++++- .../OpenSaml4LogoutRequestResolverTests.java | 4 +++- .../OpenSaml4LogoutResponseResolverTests.java | 5 ++++- .../OpenSamlMetadataResolverTests.java | 9 +++++++++ .../logout/Saml2LogoutRequestFilterTests.java | 16 +++++++++++++++ .../Saml2LogoutResponseFilterTests.java | 20 +++++++++++++++++++ 13 files changed, 82 insertions(+), 5 deletions(-) diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolver.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolver.java index 7b850a5483..731b9c8cf8 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolver.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolver.java @@ -87,7 +87,9 @@ public final class OpenSamlMetadataResolver implements Saml2MetadataResolver { spSsoDescriptor.getKeyDescriptors() .addAll(buildKeys(registration.getDecryptionX509Credentials(), UsageType.ENCRYPTION)); spSsoDescriptor.getAssertionConsumerServices().add(buildAssertionConsumerService(registration)); - spSsoDescriptor.getSingleLogoutServices().add(buildSingleLogoutService(registration)); + if (registration.getSingleLogoutServiceLocation() != null) { + spSsoDescriptor.getSingleLogoutServices().add(buildSingleLogoutService(registration)); + } if (registration.getNameIdFormat() != null) { spSsoDescriptor.getNameIDFormats().add(buildNameIDFormat(registration)); } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistration.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistration.java index 43e61b11e1..fa0a4a59d2 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistration.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistration.java @@ -108,6 +108,8 @@ public final class RelyingPartyRegistration { Assert.hasText(entityId, "entityId cannot be empty"); Assert.hasText(assertionConsumerServiceLocation, "assertionConsumerServiceLocation cannot be empty"); Assert.notNull(assertionConsumerServiceBinding, "assertionConsumerServiceBinding cannot be null"); + Assert.isTrue(singleLogoutServiceLocation == null || singleLogoutServiceBinding != null, + "singleLogoutServiceBinding cannot be null when singleLogoutServiceLocation is set"); Assert.notNull(providerDetails, "providerDetails cannot be null"); Assert.notEmpty(credentials, "credentials cannot be empty"); for (org.springframework.security.saml2.credentials.Saml2X509Credential c : credentials) { diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutRequestResolver.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutRequestResolver.java index 5a5e64c6e3..5fd296b5a8 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutRequestResolver.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutRequestResolver.java @@ -111,6 +111,9 @@ final class OpenSamlLogoutRequestResolver { if (registration == null) { return null; } + if (registration.getAssertingPartyDetails().getSingleLogoutServiceLocation() == null) { + return null; + } LogoutRequest logoutRequest = this.logoutRequestBuilder.buildObject(); logoutRequest.setDestination(registration.getAssertingPartyDetails().getSingleLogoutServiceLocation()); Issuer issuer = this.issuerBuilder.buildObject(); diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutResponseResolver.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutResponseResolver.java index 935fb1febf..864eed0943 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutResponseResolver.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSamlLogoutResponseResolver.java @@ -132,6 +132,9 @@ final class OpenSamlLogoutResponseResolver { if (registration == null) { return null; } + if (registration.getAssertingPartyDetails().getSingleLogoutServiceResponseLocation() == null) { + return null; + } String serialized = request.getParameter(Saml2ParameterNames.SAML_REQUEST); byte[] b = Saml2Utils.samlDecode(serialized); LogoutRequest logoutRequest = parse(inflateIfRequired(registration, b)); diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilter.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilter.java index 99e88c9ae8..1aa5e69b37 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilter.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilter.java @@ -120,6 +120,12 @@ public final class Saml2LogoutRequestFilter extends OncePerRequestFilter { response.sendError(HttpServletResponse.SC_BAD_REQUEST); return; } + if (registration.getSingleLogoutServiceLocation() == null) { + this.logger.trace( + "Did not process logout request since RelyingPartyRegistration has not been configured with a logout request endpoint"); + response.sendError(HttpServletResponse.SC_UNAUTHORIZED); + return; + } if (!isCorrectBinding(request, registration)) { this.logger.trace("Did not process logout request since used incorrect binding"); response.sendError(HttpServletResponse.SC_UNAUTHORIZED); diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilter.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilter.java index 83b4c8eccd..239249719a 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilter.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilter.java @@ -120,6 +120,12 @@ public final class Saml2LogoutResponseFilter extends OncePerRequestFilter { response.sendError(HttpServletResponse.SC_BAD_REQUEST, error.toString()); return; } + if (registration.getSingleLogoutServiceResponseLocation() == null) { + this.logger.trace( + "Did not process logout response since RelyingPartyRegistration has not been configured with a logout response endpoint"); + response.sendError(HttpServletResponse.SC_UNAUTHORIZED); + return; + } if (!isCorrectBinding(request, registration)) { this.logger.trace("Did not process logout request since used incorrect binding"); response.sendError(HttpServletResponse.SC_UNAUTHORIZED); diff --git a/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutRequestResolverTests.java b/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutRequestResolverTests.java index 99e5d225b1..57a7e7247b 100644 --- a/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutRequestResolverTests.java +++ b/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutRequestResolverTests.java @@ -47,7 +47,9 @@ public class OpenSaml3LogoutRequestResolverTests { this.relyingPartyRegistrationResolver); logoutRequestResolver.setParametersConsumer((parameters) -> parameters.getLogoutRequest().setID("myid")); HttpServletRequest request = new MockHttpServletRequest(); - RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration().build(); + RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration() + .assertingPartyDetails((party) -> party.singleLogoutServiceLocation("https://ap.example.com/logout")) + .build(); Authentication authentication = new TestingAuthenticationToken("user", "password"); given(this.relyingPartyRegistrationResolver.resolve(any(), any())).willReturn(registration); Saml2LogoutRequest logoutRequest = logoutRequestResolver.resolve(request, authentication); diff --git a/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutResponseResolverTests.java b/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutResponseResolverTests.java index 2e5a4a0a43..628d2401e4 100644 --- a/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutResponseResolverTests.java +++ b/saml2/saml2-service-provider/src/opensaml3Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml3LogoutResponseResolverTests.java @@ -53,7 +53,10 @@ public class OpenSaml3LogoutResponseResolverTests { Consumer parametersConsumer = mock(Consumer.class); logoutResponseResolver.setParametersConsumer(parametersConsumer); MockHttpServletRequest request = new MockHttpServletRequest(); - RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration().build(); + RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration() + .assertingPartyDetails( + (party) -> party.singleLogoutServiceResponseLocation("https://ap.example.com/logout")) + .build(); Authentication authentication = new TestingAuthenticationToken("user", "password"); LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); request.setParameter(Saml2ParameterNames.SAML_REQUEST, diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutRequestResolverTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutRequestResolverTests.java index 6ea35b4716..4d7c5c266e 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutRequestResolverTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutRequestResolverTests.java @@ -47,7 +47,9 @@ public class OpenSaml4LogoutRequestResolverTests { this.relyingPartyRegistrationResolver); logoutRequestResolver.setParametersConsumer((parameters) -> parameters.getLogoutRequest().setID("myid")); HttpServletRequest request = new MockHttpServletRequest(); - RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration().build(); + RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration() + .assertingPartyDetails((party) -> party.singleLogoutServiceLocation("https://ap.example.com/logout")) + .build(); Authentication authentication = new TestingAuthenticationToken("user", "password"); given(this.relyingPartyRegistrationResolver.resolve(any(), any())).willReturn(registration); Saml2LogoutRequest logoutRequest = logoutRequestResolver.resolve(request, authentication); diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutResponseResolverTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutResponseResolverTests.java index 7353318fb9..20d1801857 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutResponseResolverTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4LogoutResponseResolverTests.java @@ -53,7 +53,10 @@ public class OpenSaml4LogoutResponseResolverTests { Consumer parametersConsumer = mock(Consumer.class); logoutResponseResolver.setParametersConsumer(parametersConsumer); MockHttpServletRequest request = new MockHttpServletRequest(); - RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration().build(); + RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration() + .assertingPartyDetails( + (party) -> party.singleLogoutServiceResponseLocation("https://ap.example.com/logout")) + .build(); Authentication authentication = new TestingAuthenticationToken("user", "password"); LogoutRequest logoutRequest = TestOpenSamlObjects.assertingPartyLogoutRequest(registration); request.setParameter(Saml2ParameterNames.SAML_REQUEST, diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolverTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolverTests.java index be2069ab94..f5e6e44560 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolverTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/metadata/OpenSamlMetadataResolverTests.java @@ -70,4 +70,13 @@ public class OpenSamlMetadataResolverTests { assertThat(metadata).contains("format"); } + @Test + public void resolveWhenRelyingPartyNoLogoutThenMetadataMatches() { + RelyingPartyRegistration relyingPartyRegistration = TestRelyingPartyRegistrations.full() + .singleLogoutServiceLocation(null).nameIdFormat("format").build(); + OpenSamlMetadataResolver openSamlMetadataResolver = new OpenSamlMetadataResolver(); + String metadata = openSamlMetadataResolver.resolve(relyingPartyRegistration); + assertThat(metadata).doesNotContain("ResponseLocation"); + } + } diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilterTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilterTests.java index 2f08d6c122..bb604c4775 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilterTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilterTests.java @@ -153,4 +153,20 @@ public class Saml2LogoutRequestFilterTests { verifyNoInteractions(this.logoutHandler); } + @Test + public void doFilterWhenNoRelyingPartyLogoutThen401() throws Exception { + Authentication authentication = new TestingAuthenticationToken("user", "password"); + SecurityContextHolder.getContext().setAuthentication(authentication); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/logout/saml2/slo"); + request.setServletPath("/logout/saml2/slo"); + request.setParameter(Saml2ParameterNames.SAML_REQUEST, "request"); + MockHttpServletResponse response = new MockHttpServletResponse(); + RelyingPartyRegistration registration = TestRelyingPartyRegistrations.full().singleLogoutServiceLocation(null) + .build(); + given(this.relyingPartyRegistrationResolver.resolve(any(), any())).willReturn(registration); + this.logoutRequestProcessingFilter.doFilterInternal(request, response, new MockFilterChain()); + assertThat(response.getStatus()).isEqualTo(401); + verifyNoInteractions(this.logoutHandler); + } + } diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilterTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilterTests.java index 2a86a06a26..b201e52a05 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilterTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutResponseFilterTests.java @@ -37,6 +37,7 @@ import org.springframework.security.saml2.provider.service.registration.TestRely import org.springframework.security.saml2.provider.service.web.RelyingPartyRegistrationResolver; import org.springframework.security.web.authentication.logout.LogoutSuccessHandler; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.mock; @@ -151,4 +152,23 @@ public class Saml2LogoutResponseFilterTests { verifyNoInteractions(this.logoutSuccessHandler); } + @Test + public void doFilterWhenNoRelyingPartyLogoutThen401() throws Exception { + Authentication authentication = new TestingAuthenticationToken("user", "password"); + SecurityContextHolder.getContext().setAuthentication(authentication); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/logout/saml2/slo"); + request.setServletPath("/logout/saml2/slo"); + request.setParameter(Saml2ParameterNames.SAML_RESPONSE, "response"); + MockHttpServletResponse response = new MockHttpServletResponse(); + RelyingPartyRegistration registration = TestRelyingPartyRegistrations.full().singleLogoutServiceLocation(null) + .singleLogoutServiceResponseLocation(null).build(); + given(this.relyingPartyRegistrationResolver.resolve(any(), any())).willReturn(registration); + Saml2LogoutRequest logoutRequest = Saml2LogoutRequest.withRelyingPartyRegistration(registration) + .samlRequest("request").build(); + given(this.logoutRequestRepository.removeLogoutRequest(request, response)).willReturn(logoutRequest); + this.logoutResponseProcessingFilter.doFilterInternal(request, response, new MockFilterChain()); + assertThat(response.getStatus()).isEqualTo(401); + verifyNoInteractions(this.logoutSuccessHandler); + } + }