From e1826a0bd8eee7dc63267c28d4ce6f6ff294e7e8 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Fri, 30 Oct 2020 15:48:11 -0600 Subject: [PATCH] Polish Signature Algorithm Support - Changed name to signatureAlgorithms since method and algorithm are synonymous - Re-ordered methods to follow typical IDPSSODescriptor order - Adjusted JavaDoc to refer to IDPSSODescriptor terminology Issue gh-8952 --- .../OpenSamlAuthenticationRequestFactory.java | 2 +- .../RelyingPartyRegistration.java | 77 ++++++++++--------- ...SamlAuthenticationRequestFactoryTests.java | 2 +- .../RelyingPartyRegistrationTests.java | 7 +- 4 files changed, 49 insertions(+), 39 deletions(-) diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java index 9c6a5d83f1..26bfd1a848 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java @@ -274,7 +274,7 @@ public class OpenSamlAuthenticationRequestFactory implements Saml2Authentication private SignatureSigningParameters resolveSigningParameters(RelyingPartyRegistration relyingPartyRegistration) { List credentials = resolveSigningCredentials(relyingPartyRegistration); - List algorithms = relyingPartyRegistration.getAssertingPartyDetails().getSigningMethodAlgorithms(); + List algorithms = relyingPartyRegistration.getAssertingPartyDetails().getSigningAlgorithms(); List digests = Collections.singletonList(SignatureConstants.ALGO_ID_DIGEST_SHA256); String canonicalization = SignatureConstants.ALGO_ID_C14N_EXCL_OMIT_COMMENTS; SignatureSigningParametersResolver resolver = new SAMLMetadataSignatureSigningParametersResolver(); 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 cdbef9e544..1e3e008c0c 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 @@ -19,7 +19,6 @@ package org.springframework.security.saml2.provider.service.registration; import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -368,6 +367,8 @@ public final class RelyingPartyRegistration { .assertingPartyDetails((assertingParty) -> assertingParty .entityId(registration.getAssertingPartyDetails().getEntityId()) .wantAuthnRequestsSigned(registration.getAssertingPartyDetails().getWantAuthnRequestsSigned()) + .signingAlgorithms((algorithms) -> algorithms + .addAll(registration.getAssertingPartyDetails().getSigningAlgorithms())) .verificationX509Credentials((c) -> c .addAll(registration.getAssertingPartyDetails().getVerificationX509Credentials())) .encryptionX509Credentials( @@ -434,6 +435,8 @@ public final class RelyingPartyRegistration { private final boolean wantAuthnRequestsSigned; + private List signingAlgorithms; + private final Collection verificationX509Credentials; private final Collection encryptionX509Credentials; @@ -442,13 +445,12 @@ public final class RelyingPartyRegistration { private final Saml2MessageBinding singleSignOnServiceBinding; - private List signingMethodAlgorithms; - - private AssertingPartyDetails(String entityId, boolean wantAuthnRequestsSigned, + private AssertingPartyDetails(String entityId, boolean wantAuthnRequestsSigned, List signingAlgorithms, Collection verificationX509Credentials, Collection encryptionX509Credentials, String singleSignOnServiceLocation, - Saml2MessageBinding singleSignOnServiceBinding, List signingMethodAlgorithms) { + Saml2MessageBinding singleSignOnServiceBinding) { Assert.hasText(entityId, "entityId cannot be null or empty"); + Assert.notEmpty(signingAlgorithms, "signingAlgorithms cannot be empty"); Assert.notNull(verificationX509Credentials, "verificationX509Credentials cannot be null"); for (Saml2X509Credential credential : verificationX509Credentials) { Assert.notNull(credential, "verificationX509Credentials cannot have null values"); @@ -463,14 +465,13 @@ public final class RelyingPartyRegistration { } Assert.notNull(singleSignOnServiceLocation, "singleSignOnServiceLocation cannot be null"); Assert.notNull(singleSignOnServiceBinding, "singleSignOnServiceBinding cannot be null"); - Assert.notEmpty(signingMethodAlgorithms, "signingMethodAlgorithms cannot be empty"); this.entityId = entityId; this.wantAuthnRequestsSigned = wantAuthnRequestsSigned; + this.signingAlgorithms = signingAlgorithms; this.verificationX509Credentials = verificationX509Credentials; this.encryptionX509Credentials = encryptionX509Credentials; this.singleSignOnServiceLocation = singleSignOnServiceLocation; this.singleSignOnServiceBinding = singleSignOnServiceBinding; - this.signingMethodAlgorithms = signingMethodAlgorithms; } /** @@ -500,6 +501,20 @@ public final class RelyingPartyRegistration { return this.wantAuthnRequestsSigned; } + /** + * Get the list of org.opensaml.saml.ext.saml2alg.SigningMethod Algorithms for + * this asserting party, in preference order. + * + *

+ * Equivalent to the values found in <SigningMethod Algorithm="..."/> in the + * asserting party's <IDPSSODescriptor>. + * @return the list of SigningMethod Algorithms + * @since 5.5 + */ + public List getSigningAlgorithms() { + return this.signingAlgorithms; + } + /** * Get all verification {@link Saml2X509Credential}s associated with this * asserting party @@ -550,21 +565,14 @@ public final class RelyingPartyRegistration { return this.singleSignOnServiceBinding; } - /** - * Return the list of preferred signature algorithm URIs, in preference order. - * @return the list of signature algorithm URIs - * @since 5.5 - */ - public List getSigningMethodAlgorithms() { - return this.signingMethodAlgorithms; - } - public static final class Builder { private String entityId; private boolean wantAuthnRequestsSigned = true; + private List signingAlgorithms = new ArrayList<>(); + private Collection verificationX509Credentials = new HashSet<>(); private Collection encryptionX509Credentials = new HashSet<>(); @@ -573,8 +581,6 @@ public final class RelyingPartyRegistration { private Saml2MessageBinding singleSignOnServiceBinding = Saml2MessageBinding.REDIRECT; - private List signingMethodAlgorithms = new ArrayList<>(); - /** * Set the asserting party's EntityID. @@ -600,6 +606,19 @@ public final class RelyingPartyRegistration { return this; } + /** + * Apply this {@link Consumer} to the list of SigningMethod Algorithms + * @param signingMethodAlgorithmsConsumer a {@link Consumer} of the list of + * SigningMethod Algorithms + * @return this {@link AssertingPartyDetails.Builder} for further + * configuration + * @since 5.5 + */ + public Builder signingAlgorithms(Consumer> signingMethodAlgorithmsConsumer) { + signingMethodAlgorithmsConsumer.accept(this.signingAlgorithms); + return this; + } + /** * Apply this {@link Consumer} to the list of {@link Saml2X509Credential}s * @param credentialsConsumer a {@link Consumer} of the {@link List} of @@ -658,31 +677,19 @@ public final class RelyingPartyRegistration { return this; } - /** - * Apply this {@link Consumer} to the list of signature algorithm URIs - * @param signingMethodAlgorithmsConsumer a {@link Consumer} of the list of - * signature algorithm URIs - * @return this {@code Builder} - * @since 5.5 - */ - public Builder signingMethodAlgorithms(Consumer> signingMethodAlgorithmsConsumer) { - signingMethodAlgorithmsConsumer.accept(this.signingMethodAlgorithms); - return this; - } - /** * Creates an immutable ProviderDetails object representing the configuration * for an Identity Provider, IDP * @return immutable ProviderDetails object */ public AssertingPartyDetails build() { - List signingMethodAlgorithmsCopy = this.signingMethodAlgorithms.isEmpty() - ? Arrays.asList(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256) - : Collections.unmodifiableList(this.signingMethodAlgorithms); + List signingAlgorithms = this.signingAlgorithms.isEmpty() + ? Collections.singletonList(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256) + : Collections.unmodifiableList(this.signingAlgorithms); - return new AssertingPartyDetails(this.entityId, this.wantAuthnRequestsSigned, + return new AssertingPartyDetails(this.entityId, this.wantAuthnRequestsSigned, signingAlgorithms, this.verificationX509Credentials, this.encryptionX509Credentials, - this.singleSignOnServiceLocation, this.singleSignOnServiceBinding, signingMethodAlgorithmsCopy); + this.singleSignOnServiceLocation, this.singleSignOnServiceBinding); } } diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactoryTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactoryTests.java index 67bf4a6803..f0f03819fe 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactoryTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactoryTests.java @@ -244,7 +244,7 @@ public class OpenSamlAuthenticationRequestFactoryTests { public void createRedirectAuthenticationRequestWhenSHA1SignRequestThenSignatureIsPresent() { RelyingPartyRegistration relyingPartyRegistration = this.relyingPartyRegistrationBuilder .assertingPartyDetails( - (a) -> a.signingMethodAlgorithms((c) -> c.add(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1))) + (a) -> a.signingAlgorithms((algs) -> algs.add(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1))) .build(); Saml2AuthenticationRequestContext context = this.contextBuilder.relayState("Relay State Value") .relyingPartyRegistration(relyingPartyRegistration).build(); diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistrationTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistrationTests.java index 8f6c444913..948d873c49 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistrationTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistrationTests.java @@ -28,8 +28,9 @@ public class RelyingPartyRegistrationTests { @Test public void withRelyingPartyRegistrationWorks() { RelyingPartyRegistration registration = TestRelyingPartyRegistrations.relyingPartyRegistration() - .providerDetails((p) -> p.binding(Saml2MessageBinding.POST)) - .providerDetails((p) -> p.signAuthNRequest(false)) + .assertingPartyDetails((a) -> a.singleSignOnServiceBinding(Saml2MessageBinding.POST)) + .assertingPartyDetails((a) -> a.wantAuthnRequestsSigned(false)) + .assertingPartyDetails((a) -> a.signingAlgorithms((algs) -> algs.add("alg"))) .assertionConsumerServiceBinding(Saml2MessageBinding.REDIRECT).build(); RelyingPartyRegistration copy = RelyingPartyRegistration.withRelyingPartyRegistration(registration).build(); compareRegistrations(registration, copy); @@ -71,6 +72,8 @@ public class RelyingPartyRegistrationTests { .isEqualTo(registration.getAssertingPartyDetails().getEncryptionX509Credentials()); assertThat(copy.getAssertingPartyDetails().getVerificationX509Credentials()) .isEqualTo(registration.getAssertingPartyDetails().getVerificationX509Credentials()); + assertThat(copy.getAssertingPartyDetails().getSigningAlgorithms()) + .isEqualTo(registration.getAssertingPartyDetails().getSigningAlgorithms()); } @Test