From 1261c229a3695edf29d4a1a2a5cd284fa6089731 Mon Sep 17 00:00:00 2001 From: Robert Winch <362503+rwinch@users.noreply.github.com> Date: Tue, 3 Mar 2026 14:44:54 -0600 Subject: [PATCH] Fix Flaky Crypto Tests Previously the RsaSecretEncryptorTests were flaky because the assumed that a BadPaddigException would be thrown when using things like different salt. However, given that the tests had random inputs (e.g. keys) there is the possibility that, despite the fact that it can never be properly decrypted, the final bytes look like a valid encrypted value. This updates the tests to ensure that decrypt either throws an Exception or is not equal to the original plaintext. --- crypto/spring-security-crypto.gradle | 6 ++ .../assertions/CryptoAssertionsTests.java | 47 ++++++++++++++++ .../encrypt/RsaSecretEncryptorTests.java | 12 ++-- .../crypto/assertions/CryptoAssertions.java | 44 +++++++++++++++ .../crypto/assertions/CryptoStringAssert.java | 56 +++++++++++++++++++ 5 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 crypto/src/test/java/org/springframework/security/crypto/assertions/CryptoAssertionsTests.java create mode 100644 crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoAssertions.java create mode 100644 crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoStringAssert.java diff --git a/crypto/spring-security-crypto.gradle b/crypto/spring-security-crypto.gradle index 78a8fd31d0..b62d524af0 100644 --- a/crypto/spring-security-crypto.gradle +++ b/crypto/spring-security-crypto.gradle @@ -1,3 +1,7 @@ +plugins { + id 'java-test-fixtures' +} + apply plugin: 'io.spring.convention.spring-module' dependencies { @@ -6,6 +10,8 @@ dependencies { optional 'org.springframework:spring-core' optional 'org.bouncycastle:bcpkix-jdk18on' + testFixturesImplementation "org.assertj:assertj-core" + testImplementation "org.assertj:assertj-core" testImplementation "org.junit.jupiter:junit-jupiter-api" testImplementation "org.junit.jupiter:junit-jupiter-params" diff --git a/crypto/src/test/java/org/springframework/security/crypto/assertions/CryptoAssertionsTests.java b/crypto/src/test/java/org/springframework/security/crypto/assertions/CryptoAssertionsTests.java new file mode 100644 index 0000000000..935639f12b --- /dev/null +++ b/crypto/src/test/java/org/springframework/security/crypto/assertions/CryptoAssertionsTests.java @@ -0,0 +1,47 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.crypto.assertions; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * Tests for {@link CryptoAssertions} and {@link CryptoStringAssert}. + */ +class CryptoAssertionsTests { + + @Test + void doesNotDecryptToPassesWhenSupplierThrows() { + CryptoAssertions.assertThat((() -> { + throw new RuntimeException("decrypt failed"); + })).doesNotDecryptTo("any"); + } + + @Test + void doesNotDecryptToPassesWhenResultDiffersFromExpected() { + CryptoAssertions.assertThat(() -> "other").doesNotDecryptTo("plaintext"); + } + + @Test + void doesNotDecryptToFailsWhenResultEqualsExpected() { + assertThatExceptionOfType(AssertionError.class) + .isThrownBy(() -> CryptoAssertions.assertThat(() -> "plaintext").doesNotDecryptTo("plaintext")) + .withMessageContaining("Expected supplier not to return but it did"); + } + +} diff --git a/crypto/src/test/java/org/springframework/security/crypto/encrypt/RsaSecretEncryptorTests.java b/crypto/src/test/java/org/springframework/security/crypto/encrypt/RsaSecretEncryptorTests.java index 6a366f2be6..70b2314c55 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/encrypt/RsaSecretEncryptorTests.java +++ b/crypto/src/test/java/org/springframework/security/crypto/encrypt/RsaSecretEncryptorTests.java @@ -22,8 +22,9 @@ import java.security.interfaces.RSAPublicKey; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.security.crypto.assertions.CryptoAssertions; + import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * @author Dave Syer @@ -86,13 +87,15 @@ public class RsaSecretEncryptorTests { @Test public void roundTripWithMixedAlgorithm() { RsaSecretEncryptor oaep = new RsaSecretEncryptor(RsaAlgorithm.OAEP); - assertThatIllegalStateException().isThrownBy(() -> oaep.decrypt(this.encryptor.encrypt("encryptor"))); + CryptoAssertions.assertThat(() -> oaep.decrypt(this.encryptor.encrypt("encryptor"))) + .doesNotDecryptTo("encryptor"); } @Test public void roundTripWithMixedSalt() { RsaSecretEncryptor other = new RsaSecretEncryptor(this.encryptor.getPublicKey(), RsaAlgorithm.DEFAULT, "salt"); - assertThatIllegalStateException().isThrownBy(() -> this.encryptor.decrypt(other.encrypt("encryptor"))); + CryptoAssertions.assertThat(() -> this.encryptor.decrypt(other.encrypt("encryptor"))) + .doesNotDecryptTo("encryptor"); } @Test @@ -106,7 +109,8 @@ public class RsaSecretEncryptorTests { public void publicKeyCannotDecrypt() { RsaSecretEncryptor encryptor = new RsaSecretEncryptor(this.encryptor.getPublicKey()); assertThat(encryptor.canDecrypt()).as("Encryptor schould not be able to decrypt").isFalse(); - assertThatIllegalStateException().isThrownBy(() -> encryptor.decrypt(encryptor.encrypt("encryptor"))); + CryptoAssertions.assertThat(() -> encryptor.decrypt(encryptor.encrypt("encryptor"))) + .doesNotDecryptTo("encryptor"); } @Test diff --git a/crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoAssertions.java b/crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoAssertions.java new file mode 100644 index 0000000000..8bfeff4191 --- /dev/null +++ b/crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoAssertions.java @@ -0,0 +1,44 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.crypto.assertions; + +import java.util.function.Supplier; + +/** + * AssertJ entry point for crypto-related assertions. Use {@link #assertThat(Supplier)} to + * assert on a {@link Supplier}&lt;{@link String}&gt; (e.g. a decryption lambda). + * <p> + * Example: <pre> + * assertThat(() -&gt; encryptor.decrypt(ciphertext)).doesNotDecryptTo("plaintext"); + * </pre> + */ +public final class CryptoAssertions { + + private CryptoAssertions() { + } + + /** + * Create assertions for the given supplier (e.g. a decryption expression). + * @param actual the supplier to assert on + * @return assertion object with methods like + * {@link CryptoStringAssert#doesNotDecryptTo(String)} + */ + public static CryptoStringAssert assertThat(Supplier<String> actual) { + return new CryptoStringAssert(actual); + } + +} diff --git a/crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoStringAssert.java b/crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoStringAssert.java new file mode 100644 index 0000000000..715dfd2e0c --- /dev/null +++ b/crypto/src/testFixtures/java/org/springframework/security/crypto/assertions/CryptoStringAssert.java @@ -0,0 +1,56 @@ +/* + * Copyright 2004-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.crypto.assertions; + +import java.util.Objects; +import java.util.function.Supplier; + +import org.assertj.core.api.AbstractObjectAssert; + +/** + * AssertJ assertion for {@link Supplier}&lt;{@link String}&gt;, supporting + * decryption-related checks such as {@link #doesNotDecryptTo(String)}. + */ +public final class CryptoStringAssert extends AbstractObjectAssert<CryptoStringAssert, Supplier<String>> { + + CryptoStringAssert(Supplier<String> actual) { + super(actual, CryptoStringAssert.class); + } + + /** + * Asserts that either the supplier throws an exception when invoked, or the value it + * returns is not equal to the given string. Use this to assert that a decryption + * attempt does not yield a specific plaintext (e.g. wrong key or tampered + * ciphertext). + * @param expected the value that the supplier must not return + * @return this assertion for chaining + */ + public CryptoStringAssert doesNotDecryptTo(String expected) { + isNotNull(); + try { + String result = this.actual.get(); + if (Objects.equals(result, expected)) { + failWithMessage("Expected supplier not to return <%s> but it did", expected); + } + } + catch (Exception ex) { + // Exception thrown: supplier does not "decrypt to" the expected value + } + return this; + } + +}