Skip to content

Commit 8ef027a

Browse files
jzheauxjzheaux
committed
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
1 parent 42708d2 commit 8ef027a

File tree

3 files changed

+19
-69
lines changed

3 files changed

+19
-69
lines changed

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

Lines changed: 14 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@
2525
import java.util.List;
2626
import java.util.function.Function;
2727

28-
import org.apache.commons.logging.Log;
29-
import org.apache.commons.logging.LogFactory;
30-
31-
import org.springframework.core.log.LogMessage;
3228
import org.springframework.core.serializer.DefaultDeserializer;
3329
import org.springframework.core.serializer.DefaultSerializer;
3430
import org.springframework.core.serializer.Deserializer;
@@ -53,8 +49,7 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart
5349

5450
private final JdbcOperations jdbcOperations;
5551

56-
private RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper(
57-
ResultSet::getBytes);
52+
private final RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper();
5853

5954
private final AssertingPartyMetadataParametersMapper assertingPartyMetadataParametersMapper = new AssertingPartyMetadataParametersMapper();
6055

@@ -113,18 +108,6 @@ public JdbcAssertingPartyMetadataRepository(JdbcOperations jdbcOperations) {
113108
this.jdbcOperations = jdbcOperations;
114109
}
115110

116-
/**
117-
* Sets the {@link RowMapper} used for mapping the current row in
118-
* {@code java.sql.ResultSet} to {@link AssertingPartyMetadata}. The default is
119-
* {@link AssertingPartyMetadataRowMapper}.
120-
* @param assertingPartyMetadataRowMapper the {@link RowMapper} used for mapping the
121-
* current row in {@code java.sql.ResultSet} to {@link AssertingPartyMetadata}
122-
*/
123-
public void setAssertingPartyMetadataRowMapper(RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper) {
124-
Assert.notNull(assertingPartyMetadataRowMapper, "assertingPartyMetadataRowMapper cannot be null");
125-
this.assertingPartyMetadataRowMapper = assertingPartyMetadataRowMapper;
126-
}
127-
128111
@Override
129112
public AssertingPartyMetadata findByEntityId(String entityId) {
130113
Assert.hasText(entityId, "entityId cannot be empty");
@@ -172,16 +155,8 @@ private int updateCredentialRecord(AssertingPartyMetadata metadata) {
172155
*/
173156
private static final class AssertingPartyMetadataRowMapper implements RowMapper<AssertingPartyMetadata> {
174157

175-
private final Log logger = LogFactory.getLog(AssertingPartyMetadataRowMapper.class);
176-
177158
private final Deserializer<Object> deserializer = new DefaultDeserializer();
178159

179-
private final GetBytes getBytes;
180-
181-
AssertingPartyMetadataRowMapper(GetBytes getBytes) {
182-
this.getBytes = getBytes;
183-
}
184-
185160
@Override
186161
public AssertingPartyMetadata mapRow(ResultSet rs, int rowNum) throws SQLException {
187162
String entityId = rs.getString("entity_id");
@@ -191,41 +166,26 @@ public AssertingPartyMetadata mapRow(ResultSet rs, int rowNum) throws SQLExcepti
191166
String singleLogoutUrl = rs.getString("singlelogout_url");
192167
String singleLogoutResponseUrl = rs.getString("singlelogout_response_url");
193168
Saml2MessageBinding singleLogoutBinding = Saml2MessageBinding.from(rs.getString("singlelogout_binding"));
194-
byte[] signingAlgorithmsBytes = this.getBytes.getBytes(rs, "signing_algorithms");
195-
byte[] verificationCredentialsBytes = this.getBytes.getBytes(rs, "verification_credentials");
196-
byte[] encryptionCredentialsBytes = this.getBytes.getBytes(rs, "encryption_credentials");
197-
169+
List<String> algorithms = List.of(rs.getString("signing_algorithms").split(","));
170+
byte[] verificationCredentialsBytes = rs.getBytes("verification_credentials");
171+
byte[] encryptionCredentialsBytes = rs.getBytes("encryption_credentials");
172+
ThrowingFunction<byte[], Collection<Saml2X509Credential>> credentials = (
173+
bytes) -> (Collection<Saml2X509Credential>) this.deserializer.deserializeFromByteArray(bytes);
198174
AssertingPartyMetadata.Builder<?> builder = new AssertingPartyDetails.Builder();
199-
try {
200-
if (signingAlgorithmsBytes != null) {
201-
List<String> signingAlgorithms = (List<String>) this.deserializer
202-
.deserializeFromByteArray(signingAlgorithmsBytes);
203-
builder.signingAlgorithms((algorithms) -> algorithms.addAll(signingAlgorithms));
204-
}
205-
if (verificationCredentialsBytes != null) {
206-
Collection<Saml2X509Credential> verificationCredentials = (Collection<Saml2X509Credential>) this.deserializer
207-
.deserializeFromByteArray(verificationCredentialsBytes);
208-
builder.verificationX509Credentials((credentials) -> credentials.addAll(verificationCredentials));
209-
}
210-
if (encryptionCredentialsBytes != null) {
211-
Collection<Saml2X509Credential> encryptionCredentials = (Collection<Saml2X509Credential>) this.deserializer
212-
.deserializeFromByteArray(encryptionCredentialsBytes);
213-
builder.encryptionX509Credentials((credentials) -> credentials.addAll(encryptionCredentials));
214-
}
215-
}
216-
catch (Exception ex) {
217-
this.logger.debug(LogMessage.format("Parsing serialized credentials for entity %s failed", entityId),
218-
ex);
219-
return null;
220-
}
175+
Collection<Saml2X509Credential> verificationCredentials = credentials.apply(verificationCredentialsBytes);
176+
Collection<Saml2X509Credential> encryptionCredentials = (encryptionCredentialsBytes != null)
177+
? credentials.apply(encryptionCredentialsBytes) : List.of();
221178

222179
builder.entityId(entityId)
223180
.wantAuthnRequestsSigned(singleSignOnSignRequest)
224181
.singleSignOnServiceLocation(singleSignOnUrl)
225182
.singleSignOnServiceBinding(singleSignOnBinding)
226183
.singleLogoutServiceLocation(singleLogoutUrl)
227184
.singleLogoutServiceBinding(singleLogoutBinding)
228-
.singleLogoutServiceResponseLocation(singleLogoutResponseUrl);
185+
.singleLogoutServiceResponseLocation(singleLogoutResponseUrl)
186+
.signingAlgorithms((a) -> a.addAll(algorithms))
187+
.verificationX509Credentials((c) -> c.addAll(verificationCredentials))
188+
.encryptionX509Credentials((c) -> c.addAll(encryptionCredentials));
229189
return builder.build();
230190
}
231191

@@ -244,8 +204,7 @@ public List<SqlParameterValue> apply(AssertingPartyMetadata record) {
244204
parameters.add(new SqlParameterValue(Types.VARCHAR, record.getSingleSignOnServiceLocation()));
245205
parameters.add(new SqlParameterValue(Types.VARCHAR, record.getSingleSignOnServiceBinding().getUrn()));
246206
parameters.add(new SqlParameterValue(Types.BOOLEAN, record.getWantAuthnRequestsSigned()));
247-
ThrowingFunction<List<String>, byte[]> algorithms = this.serializer::serializeToByteArray;
248-
parameters.add(new SqlParameterValue(Types.BLOB, algorithms.apply(record.getSigningAlgorithms())));
207+
parameters.add(new SqlParameterValue(Types.BLOB, String.join(",", record.getSigningAlgorithms())));
249208
ThrowingFunction<Collection<Saml2X509Credential>, byte[]> credentials = this.serializer::serializeToByteArray;
250209
parameters
251210
.add(new SqlParameterValue(Types.BLOB, credentials.apply(record.getVerificationX509Credentials())));
@@ -259,10 +218,4 @@ public List<SqlParameterValue> apply(AssertingPartyMetadata record) {
259218

260219
}
261220

262-
private interface GetBytes {
263-
264-
byte[] getBytes(ResultSet rs, String columnName) throws SQLException;
265-
266-
}
267-
268221
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CREATE TABLE saml2_asserting_party_metadata
44
singlesignon_url VARCHAR(1000) NOT NULL,
55
singlesignon_binding VARCHAR(100),
66
singlesignon_sign_request boolean,
7-
signing_algorithms blob,
7+
signing_algorithms VARCHAR(256) NOT NULL,
88
verification_credentials blob NOT NULL,
99
encryption_credentials blob,
1010
singlelogout_url VARCHAR(1000),

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ void findByEntityIdWhenEntityIdIsNullThenThrowIllegalArgumentException() {
7979
}
8080

8181
@Test
82-
void findByEntityId() {
82+
void findByEntityIdWhenEntityPresentThenReturns() {
8383
this.repository.save(this.metadata);
8484

8585
AssertingPartyMetadata found = this.repository.findByEntityId(this.metadata.getEntityId());
@@ -88,17 +88,14 @@ void findByEntityId() {
8888
}
8989

9090
@Test
91-
void findByEntityIdWhenNotExists() {
91+
void findByEntityIdWhenNotExistsThenNull() {
9292
AssertingPartyMetadata found = this.repository.findByEntityId("non-existent-entity-id");
9393
assertThat(found).isNull();
9494
}
9595

9696
@Test
97-
void iterator() {
98-
AssertingPartyMetadata second = RelyingPartyRegistration.withAssertingPartyMetadata(this.metadata)
99-
.assertingPartyMetadata((a) -> a.entityId("https://example.org/idp"))
100-
.build()
101-
.getAssertingPartyMetadata();
97+
void iteratorWhenEnitiesExistThenContains() {
98+
AssertingPartyMetadata second = this.metadata.mutate().entityId("https://example.org/idp").build();
10299
this.repository.save(this.metadata);
103100
this.repository.save(second);
104101

0 commit comments

Comments
 (0)