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

Author: jzheaux

saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactory.java

@@ -274,7 +274,7 @@ private String serialize(AuthnRequest authnRequest) {
 
 	private SignatureSigningParameters resolveSigningParameters(RelyingPartyRegistration relyingPartyRegistration) {
 		List<Credential> credentials = resolveSigningCredentials(relyingPartyRegistration);
-		List<String> algorithms = relyingPartyRegistration.getAssertingPartyDetails().getSigningMethodAlgorithms();
+		List<String> algorithms = relyingPartyRegistration.getAssertingPartyDetails().getSigningAlgorithms();
 		List<String> digests = Collections.singletonList(SignatureConstants.ALGO_ID_DIGEST_SHA256);
 		String canonicalization = SignatureConstants.ALGO_ID_C14N_EXCL_OMIT_COMMENTS;
 		SignatureSigningParametersResolver resolver = new SAMLMetadataSignatureSigningParametersResolver();

saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/RelyingPartyRegistration.java

@@ -19,7 +19,6 @@
 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 static Builder withRelyingPartyRegistration(RelyingPartyRegistration regi
 				.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 static final class AssertingPartyDetails {
 
 		private final boolean wantAuthnRequestsSigned;
 
+		private List<String> signingAlgorithms;
+
 		private final Collection<Saml2X509Credential> verificationX509Credentials;
 
 		private final Collection<Saml2X509Credential> encryptionX509Credentials;
@@ -442,13 +445,12 @@ public static final class AssertingPartyDetails {
 
 		private final Saml2MessageBinding singleSignOnServiceBinding;
 
-		private List<String> signingMethodAlgorithms;
-
-		private AssertingPartyDetails(String entityId, boolean wantAuthnRequestsSigned,
+		private AssertingPartyDetails(String entityId, boolean wantAuthnRequestsSigned, List<String> signingAlgorithms,
 				Collection<Saml2X509Credential> verificationX509Credentials,
 				Collection<Saml2X509Credential> encryptionX509Credentials, String singleSignOnServiceLocation,
-				Saml2MessageBinding singleSignOnServiceBinding, List<String> 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 @@ private AssertingPartyDetails(String entityId, boolean wantAuthnRequestsSigned,
 			}
 			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 boolean getWantAuthnRequestsSigned() {
 			return this.wantAuthnRequestsSigned;
 		}
 
+		/**
+		 * Get the list of org.opensaml.saml.ext.saml2alg.SigningMethod Algorithms for
+		 * this asserting party, in preference order.
+		 *
+		 * <p>
+		 * Equivalent to the values found in &lt;SigningMethod Algorithm="..."/&gt; in the
+		 * asserting party's &lt;IDPSSODescriptor&gt;.
+		 * @return the list of SigningMethod Algorithms
+		 * @since 5.5
+		 */
+		public List<String> getSigningAlgorithms() {
+			return this.signingAlgorithms;
+		}
+
 		/**
 		 * Get all verification {@link Saml2X509Credential}s associated with this
 		 * asserting party
@@ -550,21 +565,14 @@ public Saml2MessageBinding getSingleSignOnServiceBinding() {
 			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<String> getSigningMethodAlgorithms() {
-			return this.signingMethodAlgorithms;
-		}
-
 		public static final class Builder {
 
 			private String entityId;
 
 			private boolean wantAuthnRequestsSigned = true;
 
+			private List<String> signingAlgorithms = new ArrayList<>();
+
 			private Collection<Saml2X509Credential> verificationX509Credentials = new HashSet<>();
 
 			private Collection<Saml2X509Credential> encryptionX509Credentials = new HashSet<>();
@@ -573,8 +581,6 @@ public static final class Builder {
 
 			private Saml2MessageBinding singleSignOnServiceBinding = Saml2MessageBinding.REDIRECT;
 
-			private List<String> signingMethodAlgorithms = new ArrayList<>();
-
 			/**
 			 * Set the asserting party's <a href=
 			 * "https://wiki.shibboleth.net/confluence/display/CONCEPT/EntityNaming">EntityID</a>.
@@ -600,6 +606,19 @@ public Builder wantAuthnRequestsSigned(boolean wantAuthnRequestsSigned) {
 				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<List<String>> 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 Builder singleSignOnServiceBinding(Saml2MessageBinding singleSignOnServic
 				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<List<String>> 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<String> signingMethodAlgorithmsCopy = this.signingMethodAlgorithms.isEmpty()
-						? Arrays.asList(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256)
-						: Collections.unmodifiableList(this.signingMethodAlgorithms);
+				List<String> 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);
 			}
 
 		}

saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/OpenSamlAuthenticationRequestFactoryTests.java

@@ -244,7 +244,7 @@ public void createPostAuthenticationRequestWhenAssertionConsumerServiceBindingTh
 	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();

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 @@ private void compareRegistrations(RelyingPartyRegistration registration, Relying
 				.isEqualTo(registration.getAssertingPartyDetails().getEncryptionX509Credentials());
 		assertThat(copy.getAssertingPartyDetails().getVerificationX509Credentials())
 				.isEqualTo(registration.getAssertingPartyDetails().getVerificationX509Credentials());
+		assertThat(copy.getAssertingPartyDetails().getSigningAlgorithms())
+				.isEqualTo(registration.getAssertingPartyDetails().getSigningAlgorithms());
 	}
 
 	@Test