From f34db5ca24429efe14f1201ed23be9578e3a9fc5 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Fri, 11 Mar 2022 17:31:51 +0100 Subject: [PATCH] RelyingPartyRegistrations#fromMetadataLocation prioritizes REDIRECT binding for sso and logout --- ...etadataAssertingPartyDetailsConverter.java | 8 ++- ...taAssertingPartyDetailsConverterTests.java | 64 +++++++++++++++---- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverter.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverter.java index dbe5cff5822..9f48a6cf394 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverter.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverter.java @@ -139,7 +139,9 @@ else if (singleSignOnService.getBinding().equals(Saml2MessageBinding.REDIRECT.ge continue; } party.singleSignOnServiceLocation(singleSignOnService.getLocation()).singleSignOnServiceBinding(binding); - break; + if (binding.equals(Saml2MessageBinding.REDIRECT)) { + break; + } } for (SingleLogoutService singleLogoutService : idpssoDescriptor.getSingleLogoutServices()) { Saml2MessageBinding binding; @@ -156,7 +158,9 @@ else if (singleLogoutService.getBinding().equals(Saml2MessageBinding.REDIRECT.ge ? singleLogoutService.getLocation() : singleLogoutService.getResponseLocation(); party.singleLogoutServiceLocation(singleLogoutService.getLocation()) .singleLogoutServiceResponseLocation(responseLocation).singleLogoutServiceBinding(binding); - break; + if (binding.equals(Saml2MessageBinding.REDIRECT)) { + break; + } } return party; } diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverterTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverterTests.java index 0c3c0a5e5cd..b5d09901534 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverterTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/OpenSamlMetadataAssertingPartyDetailsConverterTests.java @@ -54,8 +54,10 @@ public class OpenSamlMetadataAssertingPartyDetailsConverterTests { private static final String EXTENSIONS_TEMPLATE = "" + "" + ""; - private static final String SINGLE_SIGN_ON_SERVICE_TEMPLATE = ""; + private static final String SINGLE_SIGN_ON_SERVICE_TEMPLATE = ""; + + private static final String SINGLE_LOGOUT_SERVICE_TEMPLATE = ""; private OpenSamlMetadataAssertingPartyDetailsConverter converter; @@ -94,10 +96,10 @@ public void readWhenMissingSingleSignOnServiceThenException() { @Test public void readWhenDescriptorFullySpecifiedThenConfigures() throws Exception { String payload = String.format(ENTITY_DESCRIPTOR_TEMPLATE, - String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, - String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"signing\"") - + String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"encryption\"") + EXTENSIONS_TEMPLATE - + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE))); + String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"signing\"") + + String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"encryption\"") + EXTENSIONS_TEMPLATE + + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE, Saml2MessageBinding.REDIRECT.getUrn()) + + String.format(SINGLE_LOGOUT_SERVICE_TEMPLATE, Saml2MessageBinding.REDIRECT.getUrn()))); InputStream inputStream = new ByteArrayInputStream(payload.getBytes()); RelyingPartyRegistration.AssertingPartyDetails details = this.converter.convert(inputStream).iterator().next() .build(); @@ -105,6 +107,9 @@ public void readWhenDescriptorFullySpecifiedThenConfigures() throws Exception { assertThat(details.getSigningAlgorithms()).containsExactly(SignatureConstants.ALGO_ID_DIGEST_SHA512); assertThat(details.getSingleSignOnServiceLocation()).isEqualTo("sso-location"); assertThat(details.getSingleSignOnServiceBinding()).isEqualTo(Saml2MessageBinding.REDIRECT); + assertThat(details.getSingleLogoutServiceLocation()).isEqualTo("logout-location"); + assertThat(details.getSingleLogoutServiceResponseLocation()).isEqualTo("logout-response-location"); + assertThat(details.getSingleLogoutServiceBinding()).isEqualTo(Saml2MessageBinding.REDIRECT); assertThat(details.getEntityId()).isEqualTo("entity-id"); assertThat(details.getVerificationX509Credentials()).hasSize(1); assertThat(details.getVerificationX509Credentials().iterator().next().getCertificate()) @@ -122,12 +127,10 @@ public void readWhenDescriptorFullySpecifiedThenConfigures() throws Exception { // gh-9051 @Test public void readWhenEntitiesDescriptorThenConfigures() throws Exception { - String payload = String.format(ENTITIES_DESCRIPTOR_TEMPLATE, - String.format(ENTITY_DESCRIPTOR_TEMPLATE, - String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, - String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"signing\"") - + String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"encryption\"") - + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE)))); + String payload = String.format(ENTITIES_DESCRIPTOR_TEMPLATE, String.format(ENTITY_DESCRIPTOR_TEMPLATE, + String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"signing\"") + + String.format(KEY_DESCRIPTOR_TEMPLATE, "use=\"encryption\"") + + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE, Saml2MessageBinding.REDIRECT.getUrn())))); InputStream inputStream = new ByteArrayInputStream(payload.getBytes()); RelyingPartyRegistration.AssertingPartyDetails details = this.converter.convert(inputStream).iterator().next() .build(); @@ -145,8 +148,9 @@ public void readWhenEntitiesDescriptorThenConfigures() throws Exception { @Test public void readWhenKeyDescriptorHasNoUseThenConfiguresBothKeyTypes() throws Exception { - String payload = String.format(ENTITY_DESCRIPTOR_TEMPLATE, String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, - String.format(KEY_DESCRIPTOR_TEMPLATE, "") + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE))); + String payload = String.format(ENTITY_DESCRIPTOR_TEMPLATE, + String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, String.format(KEY_DESCRIPTOR_TEMPLATE, "") + + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE, Saml2MessageBinding.REDIRECT.getUrn()))); InputStream inputStream = new ByteArrayInputStream(payload.getBytes()); RelyingPartyRegistration.AssertingPartyDetails details = this.converter.convert(inputStream).iterator().next() .build(); @@ -157,6 +161,38 @@ public void readWhenKeyDescriptorHasNoUseThenConfiguresBothKeyTypes() throws Exc .isEqualTo(x509Certificate(CERTIFICATE)); } + @Test + public void readWhenSSOBindingSupportsPostAndRedirectThenConfiguresRedirect() throws Exception { + String payload = String.format(ENTITY_DESCRIPTOR_TEMPLATE, + String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, + String.format(KEY_DESCRIPTOR_TEMPLATE, "") + + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE, Saml2MessageBinding.POST.getUrn()) + + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE, Saml2MessageBinding.REDIRECT.getUrn()) + + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE, Saml2MessageBinding.POST.getUrn()))); + InputStream inputStream = new ByteArrayInputStream(payload.getBytes()); + RelyingPartyRegistration.AssertingPartyDetails details = this.converter.convert(inputStream).iterator().next() + .build(); + assertThat(details.getSingleSignOnServiceLocation()).isEqualTo("sso-location"); + assertThat(details.getSingleSignOnServiceBinding()).isEqualTo(Saml2MessageBinding.REDIRECT); + } + + @Test + public void readWhenLogoutBindingSupportsPostAndRedirectThenConfiguresRedirect() throws Exception { + String payload = String.format(ENTITY_DESCRIPTOR_TEMPLATE, + String.format(IDP_SSO_DESCRIPTOR_TEMPLATE, + String.format(KEY_DESCRIPTOR_TEMPLATE, "") + + String.format(SINGLE_SIGN_ON_SERVICE_TEMPLATE, Saml2MessageBinding.REDIRECT.getUrn()) + + String.format(SINGLE_LOGOUT_SERVICE_TEMPLATE, Saml2MessageBinding.POST.getUrn()) + + String.format(SINGLE_LOGOUT_SERVICE_TEMPLATE, Saml2MessageBinding.REDIRECT.getUrn()) + + String.format(SINGLE_LOGOUT_SERVICE_TEMPLATE, Saml2MessageBinding.POST.getUrn()))); + InputStream inputStream = new ByteArrayInputStream(payload.getBytes()); + RelyingPartyRegistration.AssertingPartyDetails details = this.converter.convert(inputStream).iterator().next() + .build(); + assertThat(details.getSingleLogoutServiceLocation()).isEqualTo("logout-location"); + assertThat(details.getSingleLogoutServiceResponseLocation()).isEqualTo("logout-response-location"); + assertThat(details.getSingleLogoutServiceBinding()).isEqualTo(Saml2MessageBinding.REDIRECT); + } + X509Certificate x509Certificate(String data) { try { InputStream certificate = new ByteArrayInputStream(Base64.getDecoder().decode(data.getBytes()));