Skip to content

Commit 1261c22

Browse files
committed
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.
1 parent 4501ae7 commit 1261c22

5 files changed

Lines changed: 161 additions & 4 deletions

File tree

crypto/spring-security-crypto.gradle

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
plugins {
2+
id 'java-test-fixtures'
3+
}
4+
15
apply plugin: 'io.spring.convention.spring-module'
26

37
dependencies {
@@ -6,6 +10,8 @@ dependencies {
610
optional 'org.springframework:spring-core'
711
optional 'org.bouncycastle:bcpkix-jdk18on'
812

13+
testFixturesImplementation "org.assertj:assertj-core"
14+
915
testImplementation "org.assertj:assertj-core"
1016
testImplementation "org.junit.jupiter:junit-jupiter-api"
1117
testImplementation "org.junit.jupiter:junit-jupiter-params"
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2004-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.crypto.assertions;
18+
19+
import org.junit.jupiter.api.Test;
20+
21+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
22+
23+
/**
24+
* Tests for {@link CryptoAssertions} and {@link CryptoStringAssert}.
25+
*/
26+
class CryptoAssertionsTests {
27+
28+
@Test
29+
void doesNotDecryptToPassesWhenSupplierThrows() {
30+
CryptoAssertions.assertThat((() -> {
31+
throw new RuntimeException("decrypt failed");
32+
})).doesNotDecryptTo("any");
33+
}
34+
35+
@Test
36+
void doesNotDecryptToPassesWhenResultDiffersFromExpected() {
37+
CryptoAssertions.assertThat(() -> "other").doesNotDecryptTo("plaintext");
38+
}
39+
40+
@Test
41+
void doesNotDecryptToFailsWhenResultEqualsExpected() {
42+
assertThatExceptionOfType(AssertionError.class)
43+
.isThrownBy(() -> CryptoAssertions.assertThat(() -> "plaintext").doesNotDecryptTo("plaintext"))
44+
.withMessageContaining("Expected supplier not to return <plaintext> but it did");
45+
}
46+
47+
}

crypto/src/test/java/org/springframework/security/crypto/encrypt/RsaSecretEncryptorTests.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
import org.junit.jupiter.api.BeforeEach;
2323
import org.junit.jupiter.api.Test;
2424

25+
import org.springframework.security.crypto.assertions.CryptoAssertions;
26+
2527
import static org.assertj.core.api.Assertions.assertThat;
26-
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
2728

2829
/**
2930
* @author Dave Syer
@@ -86,13 +87,15 @@ public void roundTripOaepGcm() {
8687
@Test
8788
public void roundTripWithMixedAlgorithm() {
8889
RsaSecretEncryptor oaep = new RsaSecretEncryptor(RsaAlgorithm.OAEP);
89-
assertThatIllegalStateException().isThrownBy(() -> oaep.decrypt(this.encryptor.encrypt("encryptor")));
90+
CryptoAssertions.assertThat(() -> oaep.decrypt(this.encryptor.encrypt("encryptor")))
91+
.doesNotDecryptTo("encryptor");
9092
}
9193

9294
@Test
9395
public void roundTripWithMixedSalt() {
9496
RsaSecretEncryptor other = new RsaSecretEncryptor(this.encryptor.getPublicKey(), RsaAlgorithm.DEFAULT, "salt");
95-
assertThatIllegalStateException().isThrownBy(() -> this.encryptor.decrypt(other.encrypt("encryptor")));
97+
CryptoAssertions.assertThat(() -> this.encryptor.decrypt(other.encrypt("encryptor")))
98+
.doesNotDecryptTo("encryptor");
9699
}
97100

98101
@Test
@@ -106,7 +109,8 @@ public void roundTripWithPublicKeyEncryption() {
106109
public void publicKeyCannotDecrypt() {
107110
RsaSecretEncryptor encryptor = new RsaSecretEncryptor(this.encryptor.getPublicKey());
108111
assertThat(encryptor.canDecrypt()).as("Encryptor schould not be able to decrypt").isFalse();
109-
assertThatIllegalStateException().isThrownBy(() -> encryptor.decrypt(encryptor.encrypt("encryptor")));
112+
CryptoAssertions.assertThat(() -> encryptor.decrypt(encryptor.encrypt("encryptor")))
113+
.doesNotDecryptTo("encryptor");
110114
}
111115

112116
@Test
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright 2004-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.crypto.assertions;
18+
19+
import java.util.function.Supplier;
20+
21+
/**
22+
* AssertJ entry point for crypto-related assertions. Use {@link #assertThat(Supplier)} to
23+
* assert on a {@link Supplier}&lt;{@link String}&gt; (e.g. a decryption lambda).
24+
* <p>
25+
* Example: <pre>
26+
* assertThat(() -&gt; encryptor.decrypt(ciphertext)).doesNotDecryptTo("plaintext");
27+
* </pre>
28+
*/
29+
public final class CryptoAssertions {
30+
31+
private CryptoAssertions() {
32+
}
33+
34+
/**
35+
* Create assertions for the given supplier (e.g. a decryption expression).
36+
* @param actual the supplier to assert on
37+
* @return assertion object with methods like
38+
* {@link CryptoStringAssert#doesNotDecryptTo(String)}
39+
*/
40+
public static CryptoStringAssert assertThat(Supplier<String> actual) {
41+
return new CryptoStringAssert(actual);
42+
}
43+
44+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Copyright 2004-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.crypto.assertions;
18+
19+
import java.util.Objects;
20+
import java.util.function.Supplier;
21+
22+
import org.assertj.core.api.AbstractObjectAssert;
23+
24+
/**
25+
* AssertJ assertion for {@link Supplier}&lt;{@link String}&gt;, supporting
26+
* decryption-related checks such as {@link #doesNotDecryptTo(String)}.
27+
*/
28+
public final class CryptoStringAssert extends AbstractObjectAssert<CryptoStringAssert, Supplier<String>> {
29+
30+
CryptoStringAssert(Supplier<String> actual) {
31+
super(actual, CryptoStringAssert.class);
32+
}
33+
34+
/**
35+
* Asserts that either the supplier throws an exception when invoked, or the value it
36+
* returns is not equal to the given string. Use this to assert that a decryption
37+
* attempt does not yield a specific plaintext (e.g. wrong key or tampered
38+
* ciphertext).
39+
* @param expected the value that the supplier must not return
40+
* @return this assertion for chaining
41+
*/
42+
public CryptoStringAssert doesNotDecryptTo(String expected) {
43+
isNotNull();
44+
try {
45+
String result = this.actual.get();
46+
if (Objects.equals(result, expected)) {
47+
failWithMessage("Expected supplier not to return <%s> but it did", expected);
48+
}
49+
}
50+
catch (Exception ex) {
51+
// Exception thrown: supplier does not "decrypt to" the expected value
52+
}
53+
return this;
54+
}
55+
56+
}

0 commit comments

Comments
 (0)