Polish JdbcAssertingPartyMetadataRepository

- Remove GetBytes since it's not used yet
- Remove customizable RowMapper since this can be added
later
- Change signing_algorithms to be a String since the conversion
strategy is simple
- Standardize test names
- Simplify conversion of credentials using ThrowingFunction

Issue gh-16012

Author: jzheaux

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

@@ -25,10 +25,6 @@
 import java.util.List;
 import java.util.function.Function;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-
-import org.springframework.core.log.LogMessage;
 import org.springframework.core.serializer.DefaultDeserializer;
 import org.springframework.core.serializer.DefaultSerializer;
 import org.springframework.core.serializer.Deserializer;
@@ -53,8 +49,7 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart
 
 	private final JdbcOperations jdbcOperations;
 
-	private RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper(
-			ResultSet::getBytes);
+	private final RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper();
 
 	private final AssertingPartyMetadataParametersMapper assertingPartyMetadataParametersMapper = new AssertingPartyMetadataParametersMapper();
 
@@ -113,18 +108,6 @@ public JdbcAssertingPartyMetadataRepository(JdbcOperations jdbcOperations) {
 		this.jdbcOperations = jdbcOperations;
 	}
 
-	/**
-	 * Sets the {@link RowMapper} used for mapping the current row in
-	 * {@code java.sql.ResultSet} to {@link AssertingPartyMetadata}. The default is
-	 * {@link AssertingPartyMetadataRowMapper}.
-	 * @param assertingPartyMetadataRowMapper the {@link RowMapper} used for mapping the
-	 * current row in {@code java.sql.ResultSet} to {@link AssertingPartyMetadata}
-	 */
-	public void setAssertingPartyMetadataRowMapper(RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper) {
-		Assert.notNull(assertingPartyMetadataRowMapper, "assertingPartyMetadataRowMapper cannot be null");
-		this.assertingPartyMetadataRowMapper = assertingPartyMetadataRowMapper;
-	}
-
 	@Override
 	public AssertingPartyMetadata findByEntityId(String entityId) {
 		Assert.hasText(entityId, "entityId cannot be empty");
@@ -172,16 +155,8 @@ private int updateCredentialRecord(AssertingPartyMetadata metadata) {
 	 */
 	private static final class AssertingPartyMetadataRowMapper implements RowMapper<AssertingPartyMetadata> {
 
-		private final Log logger = LogFactory.getLog(AssertingPartyMetadataRowMapper.class);
-
 		private final Deserializer<Object> deserializer = new DefaultDeserializer();
 
-		private final GetBytes getBytes;
-
-		AssertingPartyMetadataRowMapper(GetBytes getBytes) {
-			this.getBytes = getBytes;
-		}
-
 		@Override
 		public AssertingPartyMetadata mapRow(ResultSet rs, int rowNum) throws SQLException {
 			String entityId = rs.getString("entity_id");
@@ -191,41 +166,26 @@ public AssertingPartyMetadata mapRow(ResultSet rs, int rowNum) throws SQLExcepti
 			String singleLogoutUrl = rs.getString("singlelogout_url");
 			String singleLogoutResponseUrl = rs.getString("singlelogout_response_url");
 			Saml2MessageBinding singleLogoutBinding = Saml2MessageBinding.from(rs.getString("singlelogout_binding"));
-			byte[] signingAlgorithmsBytes = this.getBytes.getBytes(rs, "signing_algorithms");
-			byte[] verificationCredentialsBytes = this.getBytes.getBytes(rs, "verification_credentials");
-			byte[] encryptionCredentialsBytes = this.getBytes.getBytes(rs, "encryption_credentials");
-
+			List<String> algorithms = List.of(rs.getString("signing_algorithms").split(","));
+			byte[] verificationCredentialsBytes = rs.getBytes("verification_credentials");
+			byte[] encryptionCredentialsBytes = rs.getBytes("encryption_credentials");
+			ThrowingFunction<byte[], Collection<Saml2X509Credential>> credentials = (
+					bytes) -> (Collection<Saml2X509Credential>) this.deserializer.deserializeFromByteArray(bytes);
 			AssertingPartyMetadata.Builder<?> builder = new AssertingPartyDetails.Builder();
-			try {
-				if (signingAlgorithmsBytes != null) {
-					List<String> signingAlgorithms = (List<String>) this.deserializer
-						.deserializeFromByteArray(signingAlgorithmsBytes);
-					builder.signingAlgorithms((algorithms) -> algorithms.addAll(signingAlgorithms));
-				}
-				if (verificationCredentialsBytes != null) {
-					Collection<Saml2X509Credential> verificationCredentials = (Collection<Saml2X509Credential>) this.deserializer
-						.deserializeFromByteArray(verificationCredentialsBytes);
-					builder.verificationX509Credentials((credentials) -> credentials.addAll(verificationCredentials));
-				}
-				if (encryptionCredentialsBytes != null) {
-					Collection<Saml2X509Credential> encryptionCredentials = (Collection<Saml2X509Credential>) this.deserializer
-						.deserializeFromByteArray(encryptionCredentialsBytes);
-					builder.encryptionX509Credentials((credentials) -> credentials.addAll(encryptionCredentials));
-				}
-			}
-			catch (Exception ex) {
-				this.logger.debug(LogMessage.format("Parsing serialized credentials for entity %s failed", entityId),
-						ex);
-				return null;
-			}
+			Collection<Saml2X509Credential> verificationCredentials = credentials.apply(verificationCredentialsBytes);
+			Collection<Saml2X509Credential> encryptionCredentials = (encryptionCredentialsBytes != null)
+					? credentials.apply(encryptionCredentialsBytes) : List.of();
 
 			builder.entityId(entityId)
 				.wantAuthnRequestsSigned(singleSignOnSignRequest)
 				.singleSignOnServiceLocation(singleSignOnUrl)
 				.singleSignOnServiceBinding(singleSignOnBinding)
 				.singleLogoutServiceLocation(singleLogoutUrl)
 				.singleLogoutServiceBinding(singleLogoutBinding)
-				.singleLogoutServiceResponseLocation(singleLogoutResponseUrl);
+				.singleLogoutServiceResponseLocation(singleLogoutResponseUrl)
+				.signingAlgorithms((a) -> a.addAll(algorithms))
+				.verificationX509Credentials((c) -> c.addAll(verificationCredentials))
+				.encryptionX509Credentials((c) -> c.addAll(encryptionCredentials));
 			return builder.build();
 		}
 
@@ -244,8 +204,7 @@ public List<SqlParameterValue> apply(AssertingPartyMetadata record) {
 			parameters.add(new SqlParameterValue(Types.VARCHAR, record.getSingleSignOnServiceLocation()));
 			parameters.add(new SqlParameterValue(Types.VARCHAR, record.getSingleSignOnServiceBinding().getUrn()));
 			parameters.add(new SqlParameterValue(Types.BOOLEAN, record.getWantAuthnRequestsSigned()));
-			ThrowingFunction<List<String>, byte[]> algorithms = this.serializer::serializeToByteArray;
-			parameters.add(new SqlParameterValue(Types.BLOB, algorithms.apply(record.getSigningAlgorithms())));
+			parameters.add(new SqlParameterValue(Types.BLOB, String.join(",", record.getSigningAlgorithms())));
 			ThrowingFunction<Collection<Saml2X509Credential>, byte[]> credentials = this.serializer::serializeToByteArray;
 			parameters
 				.add(new SqlParameterValue(Types.BLOB, credentials.apply(record.getVerificationX509Credentials())));
@@ -259,10 +218,4 @@ public List<SqlParameterValue> apply(AssertingPartyMetadata record) {
 
 	}
 
-	private interface GetBytes {
-
-		byte[] getBytes(ResultSet rs, String columnName) throws SQLException;
-
-	}
-
 }

saml2/saml2-service-provider/src/main/resources/org/springframework/security/saml2/saml2-asserting-party-metadata-schema.sql

@@ -4,7 +4,7 @@ CREATE TABLE saml2_asserting_party_metadata
     singlesignon_url          VARCHAR(1000) NOT NULL,
     singlesignon_binding      VARCHAR(100),
     singlesignon_sign_request boolean,
-    signing_algorithms        blob,
+    signing_algorithms        VARCHAR(256) NOT NULL,
     verification_credentials  blob          NOT NULL,
     encryption_credentials    blob,
     singlelogout_url          VARCHAR(1000),

saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java

@@ -79,7 +79,7 @@ void findByEntityIdWhenEntityIdIsNullThenThrowIllegalArgumentException() {
 	}
 
 	@Test
-	void findByEntityId() {
+	void findByEntityIdWhenEntityPresentThenReturns() {
 		this.repository.save(this.metadata);
 
 		AssertingPartyMetadata found = this.repository.findByEntityId(this.metadata.getEntityId());
@@ -88,17 +88,14 @@ void findByEntityId() {
 	}
 
 	@Test
-	void findByEntityIdWhenNotExists() {
+	void findByEntityIdWhenNotExistsThenNull() {
 		AssertingPartyMetadata found = this.repository.findByEntityId("non-existent-entity-id");
 		assertThat(found).isNull();
 	}
 
 	@Test
-	void iterator() {
-		AssertingPartyMetadata second = RelyingPartyRegistration.withAssertingPartyMetadata(this.metadata)
-			.assertingPartyMetadata((a) -> a.entityId("https://example.org/idp"))
-			.build()
-			.getAssertingPartyMetadata();
+	void iteratorWhenEnitiesExistThenContains() {
+		AssertingPartyMetadata second = this.metadata.mutate().entityId("https://example.org/idp").build();
 		this.repository.save(this.metadata);
 		this.repository.save(second);