From 681abcc18577e2e61dccd64e14ffdee2eeeeb2f0 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 2 Sep 2020 10:31:50 +0200 Subject: [PATCH] Polish "Configure SAML 2.0 Service Provider via Metadata" See gh-23045 --- .../saml2/Saml2RelyingPartyProperties.java | 2 +- ...RelyingPartyRegistrationConfiguration.java | 37 ++++++++++--------- ...ml2RelyingPartyAutoConfigurationTests.java | 7 ++-- .../Saml2RelyingPartyPropertiesTests.java | 2 +- .../src/test/resources/saml/idp-metadata | 2 +- 5 files changed, 27 insertions(+), 23 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 9f87dd42a53..eb995e9bc7b 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 @@ -141,7 +141,7 @@ public class Saml2RelyingPartyProperties { private String entityId; /** - * Endpoint for discovery-based configuration. + * URI to the metadata endpoint for discovery-based configuration. */ private String metadataUri; 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 747a0d928b8..5c49b4f83cb 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 @@ -22,6 +22,7 @@ import java.security.cert.X509Certificate; import java.security.interfaces.RSAPrivateKey; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import java.util.stream.Collectors; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -37,6 +38,8 @@ import org.springframework.security.converter.RsaKeyConverters; import org.springframework.security.saml2.core.Saml2X509Credential; import org.springframework.security.saml2.provider.service.registration.InMemoryRelyingPartyRegistrationRepository; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration; +import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration.AssertingPartyDetails; +import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration.Builder; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrations; import org.springframework.security.saml2.provider.service.servlet.filter.Saml2WebSsoAuthenticationFilter; @@ -67,26 +70,13 @@ class Saml2RelyingPartyRegistrationConfiguration { } private RelyingPartyRegistration asRegistration(String id, Registration properties) { - RelyingPartyRegistration.Builder builder; boolean usingMetadata = StringUtils.hasText(properties.getIdentityprovider().getMetadataUri()); - if (usingMetadata) { - builder = RelyingPartyRegistrations.fromMetadataLocation(properties.getIdentityprovider().getMetadataUri()) - .registrationId(id); - } - else { - builder = RelyingPartyRegistration.withRegistrationId(id); - } + Builder builder = (usingMetadata) ? RelyingPartyRegistrations + .fromMetadataLocation(properties.getIdentityprovider().getMetadataUri()).registrationId(id) + : RelyingPartyRegistration.withRegistrationId(id); builder.assertionConsumerServiceLocation( "{baseUrl}" + Saml2WebSsoAuthenticationFilter.DEFAULT_FILTER_PROCESSES_URI); - Saml2RelyingPartyProperties.Identityprovider identityprovider = properties.getIdentityprovider(); - PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); - builder.assertingPartyDetails((details) -> { - map.from(identityprovider::getEntityId).to(details::entityId); - map.from(identityprovider.getSinglesignon()::getBinding).to(details::singleSignOnServiceBinding); - map.from(identityprovider.getSinglesignon()::getUrl).to(details::singleSignOnServiceLocation); - map.from(identityprovider.getSinglesignon()::isSignRequest).when((signRequest) -> !usingMetadata) - .to(details::wantAuthnRequestsSigned); - }); + builder.assertingPartyDetails(mapIdentityProvider(properties, usingMetadata)); builder.signingX509Credentials((credentials) -> properties.getSigning().getCredentials().stream() .map(this::asSigningCredential).forEach(credentials::add)); builder.assertingPartyDetails((details) -> details @@ -99,6 +89,19 @@ class Saml2RelyingPartyRegistrationConfiguration { return registration; } + private Consumer mapIdentityProvider(Registration properties, + boolean usingMetadata) { + PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); + 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()::getUrl).to(details::singleSignOnServiceLocation); + map.from(identityprovider.getSinglesignon()::isSignRequest).when((signRequest) -> !usingMetadata) + .to(details::wantAuthnRequestsSigned); + }; + } + private void validateSigningCredentials(Registration properties, boolean signRequest) { if (signRequest) { Assert.state(!properties.getSigning().getCredentials().isEmpty(), 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 ccb9e1d9481..e4676e3561d 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 @@ -35,6 +35,7 @@ import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; import org.springframework.security.config.BeanIds; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; @@ -122,7 +123,7 @@ class Saml2RelyingPartyAutoConfigurationTests { try (MockWebServer server = new MockWebServer()) { server.start(); String metadataUrl = server.url("").toString(); - setupMockResponse(server); + setupMockResponse(server, new ClassPathResource("saml/idp-metadata")); this.contextRunner.withPropertyValues(PREFIX + ".foo.identityprovider.metadata-uri=" + metadataUrl) .run((context) -> { assertThat(context).hasSingleBean(RelyingPartyRegistrationRepository.class); @@ -195,8 +196,8 @@ class Saml2RelyingPartyAutoConfigurationTests { return filters.stream().anyMatch(filter::isInstance); } - private void setupMockResponse(MockWebServer server) throws Exception { - try (InputStream metadataSource = new ClassPathResource("saml/idp-metadata").getInputStream()) { + private void setupMockResponse(MockWebServer server, Resource resourceBody) throws Exception { + try (InputStream metadataSource = resourceBody.getInputStream()) { Buffer metadataBuffer = new Buffer().readFrom(metadataSource); MockResponse metadataResponse = new MockResponse().setBody(metadataBuffer); server.enqueue(metadataResponse); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyPropertiesTests.java index 2027ff7eddd..e28faa2f16c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyPropertiesTests.java @@ -103,7 +103,7 @@ class Saml2RelyingPartyPropertiesTests { } @Test - void customizeIdentityProviderMetadataUrl() { + void customizeIdentityProviderMetadataUri() { bind("spring.security.saml2.relyingparty.registration.simplesamlphp.identityprovider.metadata-uri", "https://idp.example.org/metadata"); assertThat(this.properties.getRegistration().get("simplesamlphp").getIdentityprovider().getMetadataUri()) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/resources/saml/idp-metadata b/spring-boot-project/spring-boot-autoconfigure/src/test/resources/saml/idp-metadata index f11c34b2d4f..e6785d15edc 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/resources/saml/idp-metadata +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/resources/saml/idp-metadata @@ -39,4 +39,4 @@ mailto:technical.contact@example.com - \ No newline at end of file +