From 8f2b180eb622c3c3707c16de342037cbe20db3b8 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Mon, 24 May 2021 16:37:31 -0700 Subject: [PATCH] Use Sso Binding from SAML metadata uri if present If the property is explicitly configured, that gets used. If none are present, we rely on Spring Security's default value of REDIRECT. Fixes gh-26454 --- .../saml2/Saml2RelyingPartyProperties.java | 2 +- ...RelyingPartyRegistrationConfiguration.java | 3 +- ...ml2RelyingPartyAutoConfigurationTests.java | 52 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyProperties.java index dc49c2dd652..7aec53207fa 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyProperties.java @@ -289,7 +289,7 @@ public class Saml2RelyingPartyProperties { /** * Whether to redirect or post authentication requests. */ - private Saml2MessageBinding binding = Saml2MessageBinding.REDIRECT; + private Saml2MessageBinding binding; /** * Whether to sign authentication requests. diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java index 4ebe6876922..c7469754977 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java @@ -98,7 +98,8 @@ class Saml2RelyingPartyRegistrationConfiguration { Saml2RelyingPartyProperties.Identityprovider identityprovider = properties.getIdentityprovider(); return (details) -> { map.from(identityprovider::getEntityId).to(details::entityId); - map.from(identityprovider.getSinglesignon()::getBinding).to(details::singleSignOnServiceBinding); + map.from(identityprovider.getSinglesignon()::getBinding).whenNonNull() + .to(details::singleSignOnServiceBinding); map.from(identityprovider.getSinglesignon()::getUrl).to(details::singleSignOnServiceLocation); map.from(identityprovider.getSinglesignon()::isSignRequest).when((signRequest) -> !usingMetadata) .to(details::wantAuthnRequestsSigned); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java index 27f6e16aaf6..36c07194ed1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java @@ -135,6 +135,50 @@ class Saml2RelyingPartyAutoConfigurationTests { } } + @Test + void autoconfigurationShouldUseBindingFromMetadataUrlIfPresent() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.start(); + String metadataUrl = server.url("").toString(); + setupMockResponse(server, new ClassPathResource("saml/idp-metadata")); + this.contextRunner.withPropertyValues(PREFIX + ".foo.identityprovider.metadata-uri=" + metadataUrl) + .run((context) -> { + RelyingPartyRegistrationRepository repository = context + .getBean(RelyingPartyRegistrationRepository.class); + RelyingPartyRegistration registration = repository.findByRegistrationId("foo"); + assertThat(registration.getAssertingPartyDetails().getSingleSignOnServiceBinding()) + .isEqualTo(Saml2MessageBinding.POST); + }); + } + } + + @Test + void autoconfigurationWhenMetadataUrlAndPropertyPresentShouldUseBindingFromProperty() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.start(); + String metadataUrl = server.url("").toString(); + setupMockResponse(server, new ClassPathResource("saml/idp-metadata")); + this.contextRunner.withPropertyValues(PREFIX + ".foo.identityprovider.metadata-uri=" + metadataUrl, + PREFIX + ".foo.identityprovider.singlesignon.binding=redirect").run((context) -> { + RelyingPartyRegistrationRepository repository = context + .getBean(RelyingPartyRegistrationRepository.class); + RelyingPartyRegistration registration = repository.findByRegistrationId("foo"); + assertThat(registration.getAssertingPartyDetails().getSingleSignOnServiceBinding()) + .isEqualTo(Saml2MessageBinding.REDIRECT); + }); + } + } + + @Test + void autoconfigurationWhenNoMetadataUrlOrPropertyPresentShouldUseRedirectBinding() { + this.contextRunner.withPropertyValues(getPropertyValuesWithoutSsoBinding()).run((context) -> { + RelyingPartyRegistrationRepository repository = context.getBean(RelyingPartyRegistrationRepository.class); + RelyingPartyRegistration registration = repository.findByRegistrationId("foo"); + assertThat(registration.getAssertingPartyDetails().getSingleSignOnServiceBinding()) + .isEqualTo(Saml2MessageBinding.REDIRECT); + }); + } + @Test void relyingPartyRegistrationRepositoryShouldBeConditionalOnMissingBean() { this.contextRunner.withPropertyValues(getPropertyValues()) @@ -180,6 +224,14 @@ class Saml2RelyingPartyAutoConfigurationTests { PREFIX + ".foo.identityprovider.verification.credentials[0].certificate-location=classpath:saml/certificate-location" }; } + private String[] getPropertyValuesWithoutSsoBinding() { + return new String[] { PREFIX + + ".foo.identityprovider.singlesignon.url=https://simplesaml-for-spring-saml.cfapps.io/saml2/idp/SSOService.php", + PREFIX + ".foo.identityprovider.singlesignon.sign-request=false", + PREFIX + ".foo.identityprovider.entity-id=https://simplesaml-for-spring-saml.cfapps.io/saml2/idp/metadata.php", + PREFIX + ".foo.identityprovider.verification.credentials[0].certificate-location=classpath:saml/certificate-location" }; + } + private String[] getPropertyValues() { return new String[] { PREFIX + ".foo.signing.credentials[0].private-key-location=classpath:saml/private-key-location",