Skip to content

Commit 5cecca1

Browse files
pjdartonpjdarton
committed
Refactor base json code to make extending easier
1 parent 63b47df commit 5cecca1

File tree

4 files changed

+240
-54
lines changed

4 files changed

+240
-54
lines changed

‎src/main/java/io/jenkins/plugins/credentials/secretsmanager/factory/BaseAwsJsonCredentials.java‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.cloudbees.plugins.credentials.CredentialsUnavailableException;
99
import com.cloudbees.plugins.credentials.impl.BaseStandardCredentials;
1010

11+
import edu.umd.cs.findbugs.annotations.NonNull;
1112
import hudson.util.Secret;
1213
import io.jenkins.plugins.credentials.secretsmanager.Messages;
1314
import net.sf.json.JSON;
@@ -49,22 +50,51 @@ protected BaseAwsJsonCredentials(BaseAwsJsonCredentials toSnapshot) {
4950
this.supplierOfSecretData = new Snapshot<Secret>(secretDataToSnapshot);
5051
}
5152

53+
// Note:
54+
// We MUST NOT tell anyone what the JSON is, or give any hints as to its
55+
// contents, as that could then leak sensitive data so we have to suppress the
56+
// informative exception(s) and just tell the user that it didn't work.
57+
58+
/**
59+
* Reads the secret JSON and returns the field requested.
60+
*
61+
* @param fieldname The (top-level) field that we want (which must be a
62+
* {@link String}).
63+
* @return The contents of that JSON field.
64+
* @throws CredentialsUnavailableException if the JSON is missing the field, or
65+
* the field is not a {@link String}.
66+
*/
67+
protected String getMandatoryField(@NonNull JSONObject secretJson, @NonNull String fieldname) {
68+
final String fieldValue;
69+
try {
70+
fieldValue = secretJson.getString(fieldname);
71+
} catch (JSONException | NullPointerException ex) {
72+
throw new CredentialsUnavailableException("secret", Messages.wrongJsonError(getId(), fieldname));
73+
}
74+
return fieldValue;
75+
}
76+
5277
/**
5378
* Reads the secret JSON and returns the field requested.
5479
*
5580
* @param fieldname The (top-level) field that we want (which must be a
5681
* {@link String}).
5782
* @return The contents of that JSON field.
83+
*/
84+
protected String getOptionalField(@NonNull JSONObject secretJson, @NonNull String fieldname) {
85+
final String fieldValue = secretJson.optString(fieldname);
86+
return fieldValue;
87+
}
88+
89+
/**
90+
* Reads the secret JSON and returns it.
91+
*
92+
* @return The contents of that JSON field.
5893
* @throws CredentialsUnavailableException if there is no JSON, or it is not
59-
* valid JSON, or it is missing the
60-
* field, or the field is not a
61-
* {@link String}.
94+
* valid JSON.
6295
*/
63-
protected String getFieldFromSecretJson(String fieldname) {
64-
// Note:
65-
// We MUST NOT tell anyone what the JSON is, or give any hints as to its
66-
// contents, as that could then leak sensitive data so we have to suppress the
67-
// informative exception(s) and just tell the user that it didn't work.
96+
@NonNull
97+
protected JSONObject getSecretJson() {
6898
final Secret secret = supplierOfSecretData.get();
6999
final String rawSecretJson = secret == null ? "" : secret.getPlainText();
70100
if (rawSecretJson.isEmpty()) {
@@ -78,13 +108,12 @@ protected String getFieldFromSecretJson(String fieldname) {
78108
}
79109
// if we got this far then we have some syntactically-valid JSON
80110
// ... but it might not be a JSON object containing the field we wanted.
81-
final String fieldValue;
111+
final JSONObject jsonObject;
82112
try {
83-
final JSONObject jsonObject = (JSONObject) parsedJson;
84-
fieldValue = jsonObject.getString(fieldname);
85-
} catch (JSONException | ClassCastException | NullPointerException ex) {
86-
throw new CredentialsUnavailableException("secret", Messages.wrongJsonError(getId(), fieldname));
113+
jsonObject = (JSONObject) parsedJson;
114+
} catch (ClassCastException ex) {
115+
throw new CredentialsUnavailableException("secret", Messages.noValidJsonError(getId()));
87116
}
88-
return fieldValue;
117+
return jsonObject;
89118
}
90119
}

‎src/main/java/io/jenkins/plugins/credentials/secretsmanager/factory/username_password/AwsJsonUsernamePasswordCredentials.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,13 @@ public AwsJsonUsernamePasswordCredentials(String id, String description, Supplie
6767
@NonNull
6868
@Override
6969
public Secret getPassword() {
70-
return Secret.fromString(getFieldFromSecretJson(JSON_FIELDNAME_FOR_PASSWORD));
70+
return Secret.fromString(getMandatoryField(getSecretJson(), JSON_FIELDNAME_FOR_PASSWORD));
7171
}
7272

7373
@NonNull
7474
@Override
7575
public String getUsername() {
76-
return getFieldFromSecretJson(JSON_FIELDNAME_FOR_USERNAME);
76+
return getMandatoryField(getSecretJson(), JSON_FIELDNAME_FOR_USERNAME);
7777
}
7878

7979
@Override

‎src/test/java/io/jenkins/plugins/credentials/secretsmanager/factory/BaseAwsJsonCredentialsTest.java‎

Lines changed: 149 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@
1515
import hudson.util.Secret;
1616
import io.jenkins.plugins.credentials.secretsmanager.AwsCredentialsProvider;
1717
import io.jenkins.plugins.credentials.secretsmanager.Messages;
18+
import net.sf.json.JSONObject;
1819

1920
public class BaseAwsJsonCredentialsTest {
2021
@Test
21-
public void getFieldFromSecretJsonGivenValidJsonThenReturnsValue() {
22+
public void getMandatoryFieldGivenValidJsonThenReturnsValue() {
2223
// Given
2324
final String expected = "myFieldValue";
2425
final String fieldName = "myFieldName";
@@ -28,14 +29,15 @@ public void getFieldFromSecretJsonGivenValidJsonThenReturnsValue() {
2829
final TestClass instance = new TestClass(stubSupplier);
2930

3031
// When
31-
final String actual = instance.getFieldFromSecretJson(fieldName);
32+
final JSONObject jsonData = instance.getSecretJson();
33+
final String actual = instance.getMandatoryField(jsonData, fieldName);
3234

3335
// Then
3436
assertThat(actual).isEqualTo(expected);
3537
}
3638

3739
@Test
38-
public void getFieldFromSecretJsonGivenValidJsonWithUnwantedDataThenReturnsValue() {
40+
public void getMandatoryFieldGivenValidJsonWithUnwantedDataThenReturnsValue() {
3941
// Given
4042
final String expected = "myFieldValue";
4143
final String fieldName = "myFieldName";
@@ -45,24 +47,60 @@ public void getFieldFromSecretJsonGivenValidJsonWithUnwantedDataThenReturnsValue
4547
final TestClass instance = new TestClass(stubSupplier);
4648

4749
// When
48-
final String actual = instance.getFieldFromSecretJson(fieldName);
50+
final JSONObject jsonData = instance.getSecretJson();
51+
final String actual = instance.getMandatoryField(jsonData, fieldName);
4952

5053
// Then
5154
assertThat(actual).isEqualTo(expected);
5255
}
5356

5457
@Test
55-
public void getFieldFromSecretJsonGivenMissingJsonThenReturnsThrows() {
58+
public void getOptionalFieldGivenValidJsonThenReturnsValue() {
59+
// Given
60+
final String expected = "";
61+
final String fieldName = "fieldWeDontHaveInTheJson";
62+
final String json = mkJson("oneField", "oneValue");
63+
final Secret secretJson = Secret.fromString(json);
64+
final Supplier<Secret> stubSupplier = new StubSupplier<>(secretJson);
65+
final TestClass instance = new TestClass(stubSupplier);
66+
67+
// When
68+
final JSONObject jsonData = instance.getSecretJson();
69+
final String actual = instance.getOptionalField(jsonData, fieldName);
70+
71+
// Then
72+
assertThat(actual).isEqualTo(expected);
73+
}
74+
75+
@Test
76+
public void getOptionalFieldGivenValidJsonWithoutFieldThenReturnsEmptyString() {
77+
// Given
78+
final String expected = "";
79+
final String fieldName = "fieldWeDontHaveInTheJson";
80+
final String json = mkJson("oneField", "oneValue");
81+
final Secret secretJson = Secret.fromString(json);
82+
final Supplier<Secret> stubSupplier = new StubSupplier<>(secretJson);
83+
final TestClass instance = new TestClass(stubSupplier);
84+
85+
// When
86+
final JSONObject jsonData = instance.getSecretJson();
87+
final String actual = instance.getOptionalField(jsonData, fieldName);
88+
89+
// Then
90+
assertThat(actual).isEqualTo(expected);
91+
}
92+
93+
@Test
94+
public void getSecretJsonGivenMissingJsonThenReturnsThrows() {
5695
// Given
5796
final Secret secretJson = null;
5897
final Supplier<Secret> stubSupplier = new StubSupplier<>(secretJson);
5998
final TestClass instance = new TestClass(stubSupplier);
60-
final String missingFieldName = "someFieldName";
6199

62100
// When
63101
CredentialsUnavailableException actual = null;
64102
try {
65-
instance.getFieldFromSecretJson(missingFieldName);
103+
instance.getSecretJson();
66104
} catch (CredentialsUnavailableException ex) {
67105
actual = ex;
68106
}
@@ -74,18 +112,17 @@ public void getFieldFromSecretJsonGivenMissingJsonThenReturnsThrows() {
74112
}
75113

76114
@Test
77-
public void getFieldFromSecretJsonGivenEmptyJsonThenReturnsThrows() {
115+
public void getSecretJsonGivenEmptyJsonThenReturnsThrows() {
78116
// Given
79117
final String json = "";
80118
final Secret secretJson = Secret.fromString(json);
81119
final Supplier<Secret> stubSupplier = new StubSupplier<>(secretJson);
82120
final TestClass instance = new TestClass(stubSupplier);
83-
final String missingFieldName = "someFieldName";
84121

85122
// When
86123
CredentialsUnavailableException actual = null;
87124
try {
88-
instance.getFieldFromSecretJson(missingFieldName);
125+
instance.getSecretJson();
89126
} catch (CredentialsUnavailableException ex) {
90127
actual = ex;
91128
}
@@ -97,17 +134,16 @@ public void getFieldFromSecretJsonGivenEmptyJsonThenReturnsThrows() {
97134
}
98135

99136
@Test
100-
public void getFieldFromSecretJsonGivenInvalidJsonThenReturnsThrows() {
137+
public void getSecretJsonGivenInvalidJsonThenReturnsThrows() {
101138
final String json = "1234 is not valid JSON";
102139
final Secret secretJson = Secret.fromString(json);
103140
final Supplier<Secret> stubSupplier = new StubSupplier<>(secretJson);
104141
final TestClass instance = new TestClass(stubSupplier);
105-
final String missingFieldName = "fieldName";
106142

107143
// When
108144
CredentialsUnavailableException actual = null;
109145
try {
110-
instance.getFieldFromSecretJson(missingFieldName);
146+
instance.getSecretJson();
111147
} catch (CredentialsUnavailableException ex) {
112148
actual = ex;
113149
}
@@ -121,7 +157,30 @@ public void getFieldFromSecretJsonGivenInvalidJsonThenReturnsThrows() {
121157
}
122158

123159
@Test
124-
public void getFieldFromSecretJsonGivenValidJsonMissingDesiredFieldThenReturnsThrows() {
160+
public void getSecretJsonGivenUnexpectedJsonThenReturnsThrows() {
161+
final String json = "[ \"hello\", \"world\" ]";
162+
final Secret secretJson = Secret.fromString(json);
163+
final Supplier<Secret> stubSupplier = new StubSupplier<>(secretJson);
164+
final TestClass instance = new TestClass(stubSupplier);
165+
166+
// When
167+
CredentialsUnavailableException actual = null;
168+
try {
169+
instance.getSecretJson();
170+
} catch (CredentialsUnavailableException ex) {
171+
actual = ex;
172+
}
173+
174+
// Then
175+
assertThat(actual).isNotNull();
176+
assertThat(actual.getProperty()).isEqualTo("secret");
177+
assertThat(actual.getMessage()).contains(instance.getId());
178+
assertThat(actual.getMessage()).contains(Messages.noValidJsonError(instance.getId()));
179+
assertThat(actual.getMessage()).doesNotContain("hello");
180+
}
181+
182+
@Test
183+
public void getMandatoryFieldGivenValidJsonMissingDesiredFieldThenReturnsThrows() {
125184
// Given
126185
final String unexpectedFieldName = "potentiallySecretFieldName";
127186
final String unexpectedValue = "potentiallySecretValue";
@@ -132,9 +191,10 @@ public void getFieldFromSecretJsonGivenValidJsonMissingDesiredFieldThenReturnsTh
132191
final String missingFieldName = "someOtherFieldName";
133192

134193
// When
194+
final JSONObject jsonObject = instance.getSecretJson();
135195
CredentialsUnavailableException actual = null;
136196
try {
137-
instance.getFieldFromSecretJson(missingFieldName);
197+
instance.getMandatoryField(jsonObject, missingFieldName);
138198
} catch (CredentialsUnavailableException ex) {
139199
actual = ex;
140200
}
@@ -149,6 +209,30 @@ public void getFieldFromSecretJsonGivenValidJsonMissingDesiredFieldThenReturnsTh
149209
assertThat(actual.getMessage()).doesNotContain(unexpectedValue);
150210
}
151211

212+
@Test
213+
public void snapshotConstructorGivenInstanceToSnapshotThenReturnsClone() {
214+
// Given
215+
final String json = mkJson("oneField", "oneValue");
216+
final Secret secretJson = Secret.fromString(json);
217+
final StubSingleShotSupplier<Secret> stubSupplier = new StubSingleShotSupplier<Secret>(secretJson);
218+
final TestClass original = new TestClass(stubSupplier);
219+
final JSONObject expectedSecretJson = original.getSecretJson();
220+
final String expectedId = original.getId();
221+
final String expectedDescription = original.getDescription();
222+
stubSupplier.forgetPreviousCalls();
223+
224+
// When
225+
final TestClass actual = new TestClass(original);
226+
227+
// Then
228+
final JSONObject actualSecretJson = actual.getSecretJson();
229+
assertThat((Comparable<?>) actualSecretJson).isEqualTo((Comparable<?>) expectedSecretJson);
230+
final String actualId = actual.getId();
231+
assertThat(actualId).isEqualTo(expectedId);
232+
final String actualDescription = actual.getDescription();
233+
assertThat(actualDescription).isEqualTo(expectedDescription);
234+
}
235+
152236
public static void assertThatJsonCredentialsDescriptorIsTheSameAsTheDescriptorForNonJsonCredentials(
153237
CredentialsDescriptor instanceUnderTest, CredentialsDescriptor nonJsonEquivalent) {
154238
// Given
@@ -179,11 +263,17 @@ public static void assertThatJsonCredentialsDescriptorIsTheSameAsTheDescriptorFo
179263
}
180264

181265
public static String mkJson(String fieldName1, String fieldValue1) {
182-
return "{ " + fieldName1 + ": '" + fieldValue1 + "' }";
266+
return "{ \"" + fieldName1 + "\": \"" + fieldValue1 + "\" }";
183267
}
184268

185269
public static String mkJson(String fieldName1, String fieldValue1, String fieldName2, String fieldValue2) {
186-
return "{ " + fieldName1 + ": '" + fieldValue1 + "', " + fieldName2 + ": '" + fieldValue2 + "' }";
270+
return "{ \"" + fieldName1 + "\": \"" + fieldValue1 + "\", \"" + fieldName2 + "\": \"" + fieldValue2 + "\" }";
271+
}
272+
273+
public static String mkJson(String fieldName1, String fieldValue1, String fieldName2, String fieldValue2,
274+
String fieldName3, String fieldValue3) {
275+
return "{ \"" + fieldName1 + "\": \"" + fieldValue1 + "\", \"" + fieldName2 + "\": \"" + fieldValue2 + "\", \""
276+
+ fieldName3 + "\": \"" + fieldValue3 + "\" }";
187277
}
188278

189279
private static String[] toClassAgnosticMethodDescription(Method[] m) {
@@ -199,13 +289,29 @@ private static String[] toClassAgnosticMethodDescription(Method[] m) {
199289
}
200290

201291
private static class TestClass extends BaseAwsJsonCredentials {
202-
protected TestClass(Supplier<Secret> usernameAndPasswordJson) {
292+
TestClass(Supplier<Secret> usernameAndPasswordJson) {
203293
super("TestId", "TestDescription", usernameAndPasswordJson);
204294
}
205295

296+
TestClass(BaseAwsJsonCredentials toSnapshot) {
297+
super(toSnapshot);
298+
}
299+
206300
// expose so we can test it
207-
public String getFieldFromSecretJson(String fieldname) {
208-
return super.getFieldFromSecretJson(fieldname);
301+
302+
@Override
303+
public String getMandatoryField(JSONObject secretJson, String fieldname) {
304+
return super.getMandatoryField(secretJson, fieldname);
305+
}
306+
307+
@Override
308+
public String getOptionalField(JSONObject secretJson, String fieldname) {
309+
return super.getOptionalField(secretJson, fieldname);
310+
}
311+
312+
@Override
313+
public JSONObject getSecretJson() {
314+
return super.getSecretJson();
209315
}
210316
}
211317

@@ -223,4 +329,27 @@ public T get() {
223329
return supplied;
224330
}
225331
}
332+
333+
public static class StubSingleShotSupplier<T> implements Supplier<T> {
334+
private final T supplied;
335+
private Throwable whereWeWereCalledFromTheFirstTime = null;
336+
337+
public StubSingleShotSupplier(T supplied) {
338+
this.supplied = supplied;
339+
}
340+
341+
public void forgetPreviousCalls() {
342+
whereWeWereCalledFromTheFirstTime = null;
343+
}
344+
345+
@Override
346+
public T get() {
347+
if (whereWeWereCalledFromTheFirstTime != null) {
348+
throw new IllegalStateException("This provider has already been called before",
349+
whereWeWereCalledFromTheFirstTime);
350+
}
351+
whereWeWereCalledFromTheFirstTime = new Throwable("First call to the supplier was from here.");
352+
return supplied;
353+
}
354+
}
226355
}

0 commit comments

Comments
 (0)