Skip to content

Commit acd0716

Browse files
ketanketan
committed
Add some unit tests and improve overall performance of reading passwords
- Lazily remember the decrypted secrets in a map as more secrets are decrypted - Perform a `stat()` like operation to check if the file was modified in the last 5 seconds before reloading the DB file from disk.
1 parent fd79955 commit acd0716

File tree

11 files changed

+641
-12
lines changed

11 files changed

+641
-12
lines changed

‎db/build.gradle

+8
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,12 @@
1717
dependencies {
1818
compile group: 'com.google.code.gson', name: 'gson', version: '2.8.5'
1919
compile group: 'commons-io', name: 'commons-io', version: '2.6'
20+
21+
testCompile group: 'org.assertj', name: 'assertj-core', version: '3.12.2'
22+
testCompile group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: '5.4.2'
23+
testRuntime group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: '5.4.2'
24+
}
25+
26+
test {
27+
useJUnitPlatform()
2028
}

‎db/src/main/java/cd/go/plugin/secret/filebased/db/SecretsDatabase.java

+34-6
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,13 @@
3434
import static org.apache.commons.io.FileUtils.readFileToString;
3535

3636
public class SecretsDatabase {
37+
3738
private static final Gson GSON = new GsonBuilder()
3839
.excludeFieldsWithoutExposeAnnotation()
3940
.serializeNulls()
4041
.setPrettyPrinting()
4142
.create();
43+
4244
@Expose
4345
@SerializedName("secret_key")
4446
private final String secretKey;
@@ -47,6 +49,8 @@ public class SecretsDatabase {
4749
@SerializedName("secrets")
4850
private final LinkedHashMap<String, String> secrets = new LinkedHashMap<>();
4951

52+
final LinkedHashMap<String, String> decryptedSecrets = new LinkedHashMap<>();
53+
5054
public SecretsDatabase(String secretKey) {
5155
this.secretKey = secretKey;
5256
}
@@ -56,23 +60,37 @@ public SecretsDatabase() throws NoSuchAlgorithmException {
5660
}
5761

5862
public SecretsDatabase addSecret(String name, String value) throws GeneralSecurityException {
59-
secrets.put(name, Cipher.encrypt(secretKey, value));
63+
synchronized (this) {
64+
secrets.put(name, Cipher.encrypt(secretKey, value));
65+
decryptedSecrets.remove(name);
66+
}
6067
return this;
6168
}
6269

63-
public String getSecret(String name) throws BadSecretException, GeneralSecurityException {
64-
if (secrets.containsKey(name)) {
65-
return Cipher.decrypt(secretKey, secrets.get(name));
70+
public String getSecret(String name) {
71+
synchronized (this) {
72+
return decryptedSecrets.computeIfAbsent(name, key -> {
73+
if (secrets.containsKey(name)) {
74+
try {
75+
return Cipher.decrypt(secretKey, secrets.get(name));
76+
} catch (BadSecretException | GeneralSecurityException e) {
77+
throw new RuntimeException(e);
78+
}
79+
}
80+
return null;
81+
});
6682
}
67-
return null;
6883
}
6984

7085
public Set<String> getAllSecretKeys() {
7186
return secrets.keySet();
7287
}
7388

7489
public SecretsDatabase removeSecret(String name) {
75-
secrets.remove(name);
90+
synchronized (this) {
91+
secrets.remove(name);
92+
decryptedSecrets.remove(name);
93+
}
7694
return this;
7795
}
7896

@@ -88,4 +106,14 @@ public SecretsDatabase saveTo(File secretFile) throws IOException {
88106
public String toJSON() {
89107
return GSON.toJson(this);
90108
}
109+
110+
111+
// for testing
112+
String getSecretKey() {
113+
return secretKey;
114+
}
115+
116+
LinkedHashMap<String, String> getSecrets() {
117+
return secrets;
118+
}
91119
}

‎db/src/test/java/cd/go/plugin/secret/filebased/db/CipherTest.java

+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright 2019 ThoughtWorks, Inc.
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+
* http://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 cd.go.plugin.secret.filebased.db;
18+
19+
import org.junit.jupiter.api.Nested;
20+
import org.junit.jupiter.api.Test;
21+
22+
import java.security.GeneralSecurityException;
23+
import java.security.NoSuchAlgorithmException;
24+
import java.util.Base64;
25+
26+
import static org.assertj.core.api.Assertions.assertThat;
27+
import static org.assertj.core.api.Assertions.assertThatCode;
28+
29+
class CipherTest {
30+
31+
@Nested
32+
class GenerateKey {
33+
34+
@Test
35+
void shouldGenerate128BitKey() throws NoSuchAlgorithmException {
36+
assertThat(Cipher.generateKey())
37+
.hasSize(128 / 8);
38+
}
39+
40+
@Test
41+
void shouldGenerateRandomKeyEveryTime() throws NoSuchAlgorithmException {
42+
assertThat(Cipher.generateKey())
43+
.isNotEqualTo(Cipher.generateKey());
44+
}
45+
}
46+
47+
@Nested
48+
class Encrypt {
49+
50+
@Test
51+
void shouldEncrypt() throws GeneralSecurityException {
52+
String clearText = "foo";
53+
54+
String key = Base64.getEncoder().encodeToString(Cipher.generateKey());
55+
56+
assertThat(Cipher.encrypt(key, clearText))
57+
.doesNotContain(clearText);
58+
}
59+
60+
@Test
61+
void shouldEncryptToDifferentValueEveryTime() throws GeneralSecurityException {
62+
String clearText = "foo";
63+
64+
String key = Base64.getEncoder().encodeToString(Cipher.generateKey());
65+
66+
assertThat(Cipher.encrypt(key, clearText))
67+
.isNotEqualTo(Cipher.encrypt(key, clearText));
68+
}
69+
}
70+
71+
@Nested
72+
class Decrypt {
73+
74+
@Test
75+
void shouldDecrypt() throws GeneralSecurityException, BadSecretException {
76+
String clearText = "foo";
77+
78+
String key = Base64.getEncoder().encodeToString(Cipher.generateKey());
79+
80+
String encryptedValue = Cipher.encrypt(key, clearText);
81+
82+
assertThat(Cipher.decrypt(key, encryptedValue)).isEqualTo(clearText);
83+
}
84+
85+
@Test
86+
void shouldFailIfEncryptedValueIsTampered() throws GeneralSecurityException {
87+
String key = Base64.getEncoder().encodeToString(Cipher.generateKey());
88+
89+
assertThatCode(() -> Cipher.decrypt(key, "junk")).hasMessage("Bad cipher text")
90+
.isInstanceOf(BadSecretException.class);
91+
}
92+
}
93+
}

‎db/src/test/java/cd/go/plugin/secret/filebased/db/SecretsDatabaseTest.java

+101
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/*
2+
* Copyright 2019 ThoughtWorks, Inc.
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+
* http://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 cd.go.plugin.secret.filebased.db;
18+
19+
import org.junit.jupiter.api.Nested;
20+
import org.junit.jupiter.api.Test;
21+
import org.junit.jupiter.api.io.TempDir;
22+
23+
import java.io.File;
24+
import java.io.IOException;
25+
import java.security.GeneralSecurityException;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
29+
class SecretsDatabaseTest {
30+
31+
@Test
32+
void shouldAddASecret() throws GeneralSecurityException {
33+
SecretsDatabase secretsDatabase = new SecretsDatabase();
34+
secretsDatabase.addSecret("foo", "bar");
35+
assertThat(secretsDatabase.getSecret("foo")).isEqualTo("bar");
36+
37+
}
38+
39+
@Test
40+
void decryptingASecretShouldCacheIt() throws GeneralSecurityException {
41+
SecretsDatabase secretsDatabase = new SecretsDatabase();
42+
43+
secretsDatabase.addSecret("foo", "bar");
44+
assertThat(secretsDatabase.decryptedSecrets).doesNotContainKeys("foo");
45+
46+
secretsDatabase.getSecret("foo");
47+
assertThat(secretsDatabase.decryptedSecrets).containsKeys("foo");
48+
}
49+
50+
@Test
51+
void removingASecretShouldRemoveItFromCache() throws GeneralSecurityException {
52+
SecretsDatabase secretsDatabase = new SecretsDatabase();
53+
54+
secretsDatabase.addSecret("foo", "bar");
55+
56+
// populate cache
57+
secretsDatabase.getSecret("foo");
58+
assertThat(secretsDatabase.decryptedSecrets).containsKeys("foo");
59+
60+
// remove secret
61+
secretsDatabase.removeSecret("foo");
62+
assertThat(secretsDatabase.decryptedSecrets).doesNotContainKeys("foo");
63+
}
64+
65+
@Test
66+
void addingASecretShouldRemoveItFromCache() throws GeneralSecurityException {
67+
SecretsDatabase secretsDatabase = new SecretsDatabase();
68+
69+
secretsDatabase.addSecret("foo", "bar");
70+
71+
// populate cache
72+
secretsDatabase.getSecret("foo");
73+
assertThat(secretsDatabase.decryptedSecrets).containsKeys("foo");
74+
75+
// add secret
76+
secretsDatabase.addSecret("foo", "bar");
77+
assertThat(secretsDatabase.decryptedSecrets).doesNotContainKeys("foo");
78+
}
79+
80+
@Nested
81+
class Persistance {
82+
83+
@Test
84+
void shouldPersistDBToDisk(@TempDir File tempDir) throws GeneralSecurityException, IOException {
85+
SecretsDatabase secretsDatabase = new SecretsDatabase();
86+
87+
secretsDatabase.addSecret("foo", "bar");
88+
89+
secretsDatabase.saveTo(new File(tempDir, "db.json"));
90+
91+
SecretsDatabase loadedDB = SecretsDatabase.readFrom(new File(tempDir, "db.json"));
92+
93+
// is able to decrypt
94+
assertThat(loadedDB.getSecret("foo")).isEqualTo("bar");
95+
96+
// and saves the passphrase and all secrets
97+
assertThat(secretsDatabase.getSecretKey()).isEqualTo(loadedDB.getSecretKey());
98+
assertThat(secretsDatabase.getSecrets()).isEqualTo(loadedDB.getSecrets());
99+
}
100+
}
101+
}

‎gocd-file-based-secrets-plugin/build.gradle

+13-1
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,25 @@
1414
* limitations under the License.
1515
*/
1616

17+
plugins {
18+
id "me.champeau.gradle.jmh" version "0.4.8"
19+
}
20+
1721
configurations {
1822
extractedAtTopLevel
1923
}
2024

2125
dependencies {
22-
compile group: 'cd.go.plugin.base', name: 'gocd-plugin-base', version: '0.0.1'
2326
compileOnly group: 'cd.go.plugin', name: 'go-plugin-api', version: '18.9.0'
27+
compile group: 'cd.go.plugin.base', name: 'gocd-plugin-base', version: '0.0.1'
2428
compile group: 'org.apache.commons', name: 'commons-lang3', version: '3.9'
29+
2530
compile project(':db')
2631
compile project(':cli')
2732

33+
compileOnly group: 'org.projectlombok', name: 'lombok', version: '1.18.8'
34+
annotationProcessor group: 'org.projectlombok', name: 'lombok', version: '1.18.8'
35+
2836
extractedAtTopLevel project(':jar-class-loader')
2937

3038
testCompile group: 'org.assertj', name: 'assertj-core', version: '3.12.2'
@@ -55,3 +63,7 @@ jar {
5563
into("/")
5664
}
5765
}
66+
67+
test {
68+
useJUnitPlatform()
69+
}

0 commit comments

Comments
 (0)