Skip to content

Commit b396ff2

Browse files
caroline-ttdcaroline-ttd
authored
Add key name in the audit log error message (#486)
1 parent 3f92981 commit b396ff2

File tree

2 files changed

+32
-42
lines changed

2 files changed

+32
-42
lines changed

‎src/main/java/com/uid2/shared/audit/Audit.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ private boolean validateJsonObjectParams(JsonObject jsonObject, String propertyN
9595
for (String key : jsonObject.fieldNames()) {
9696
String val = jsonObject.getString(key);
9797

98-
boolean containsNoSecret = validateNoSecrets(key, propertyName) && validateNoSecrets(val, propertyName);
99-
boolean containsNoSQL = validateNoSQL(key, propertyName) && validateNoSQL(val, propertyName);
98+
boolean containsNoSecret = validateNoSecrets(val, String.format("%s.%s", propertyName, key));
99+
boolean containsNoSQL = validateNoSQL(val, String.format("%s.%s", propertyName, key));
100100

101101
if (!(containsNoSecret && containsNoSQL)) {
102102
keysToRemove.add(key);
@@ -128,7 +128,7 @@ private boolean validateJsonArrayParams(JsonArray jsonArray, String propertyName
128128
newJsonArray.add(object);
129129
}
130130
} else {
131-
toJsonValidationErrorMessageBuilder.append("The request body is a JSON array, but one of its elements is not a JSON object.");
131+
toJsonValidationErrorMessageBuilder.append(String.format("The request body is a JSON array, but one of its elements is not a JSON object: %s. ", object.toString()));
132132
}
133133
}
134134

@@ -152,7 +152,7 @@ private String sanitizeRequestBody(String requestBody) {
152152
JsonArray jsonArray = new JsonArray(mapper.writeValueAsString(root));
153153
if (validateJsonArrayParams(jsonArray, "request_body")) sanitizedRequestBody = jsonArray.toString();
154154
} else {
155-
toJsonValidationErrorMessageBuilder.append("The request body of audit log is not a JSON object or array. ");
155+
toJsonValidationErrorMessageBuilder.append(String.format("The request body of audit log is not a JSON object or array: %s. ", requestBody));
156156
}
157157
} catch (Exception e) {
158158
toJsonValidationErrorMessageBuilder.append("The request body of audit log is Invalid JSON: ").append(e.getMessage());

‎src/test/java/com/uid2/shared/audit/AuditTest.java‎

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ public class AuditTest {
3838
private SocketAddress mockAddress;
3939
private Logger logger;
4040
private ListAppender<ILoggingEvent> listAppender;
41-
private String UID2_SECRET_KEY = "UID2-O-P-AB12cd34EF-zyX9_abCDEFghijklMNOPQRSTuvwxYZ0123";
42-
private String SECRET = "jhgjy6tuygiuhi";
41+
private String UID_SECRET_KEY = "UID2_SECRET_KEY";
42+
private String UID_SECRET = "UID2-O-P-AB12cd34EF-zyX9_abCDEFghijklMNOPQRSTuvwxYZ0123";
4343
private String SQL_STATEMENT = "SELECT * FROM users WHERE username = '' OR '1'='1'";
4444
private String TRACE_ID = "Root=1-6825017b-2321f2302b5ea904340c1cff";
4545
private String UID_TRACE_ID = "Root=1-6825017b-2321f2302b5ea904340c1cfa";
@@ -140,13 +140,13 @@ public void testBodyParamsAsJsobObject() {
140140
@Test
141141
public void testBodyParamsAsJsobObjectWithSecret() {
142142
Mockito.when(mockRequest.method()).thenReturn(HttpMethod.POST);
143-
AuditParams params = new AuditParams(null, Arrays.asList("name.first", "location", UID2_SECRET_KEY));
143+
AuditParams params = new AuditParams(null, Arrays.asList("name.first", "location", UID_SECRET_KEY));
144144

145145
RequestBody mockBody = Mockito.mock(RequestBody.class);
146146
JsonObject json = new JsonObject()
147147
.put("name", new JsonObject().put("first", "uid2_user"))
148148
.put("location", "seattle")
149-
.put(UID2_SECRET_KEY, SECRET);
149+
.put(UID_SECRET_KEY, UID_SECRET);
150150

151151
Mockito.when(mockCtx.body()).thenReturn(mockBody);
152152
Mockito.when(mockBody.buffer()).thenReturn(Buffer.buffer(json.toString()));
@@ -158,26 +158,25 @@ public void testBodyParamsAsJsobObjectWithSecret() {
158158
.toList();
159159

160160
assertThat(messages).allMatch(msg -> msg.contains("uid2_user") && msg.contains("seattle"));
161-
assertThat(messages).noneMatch(msg -> msg.contains(UID2_SECRET_KEY));
162-
assertThat(messages).noneMatch(msg -> msg.contains(SECRET));
161+
assertThat(messages).noneMatch(msg -> msg.contains(UID_SECRET));
163162

164163
boolean errorLogged = listAppender.list.stream()
165-
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("Secret found in the audit log: request_body."));
164+
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains(String.format("Secret found in the audit log: request_body.%s.", UID_SECRET_KEY)));
166165

167166
assertThat(errorLogged).isTrue();
168167
}
169168

170169
@Test
171170
public void testBodyParamsAsJsobObjectWithSecretNSQL() {
172171
Mockito.when(mockRequest.method()).thenReturn(HttpMethod.POST);
173-
AuditParams params = new AuditParams(null, Arrays.asList("name.first", "location", UID2_SECRET_KEY, SQL_STATEMENT));
172+
AuditParams params = new AuditParams(null, Arrays.asList("name.first", "location", UID_SECRET_KEY, "weather"));
174173

175174
RequestBody mockBody = Mockito.mock(RequestBody.class);
176175
JsonObject json = new JsonObject()
177176
.put("name", new JsonObject().put("first", "uid2_user"))
178177
.put("location", "seattle")
179-
.put(UID2_SECRET_KEY, SECRET)
180-
.put(SQL_STATEMENT, "weather");
178+
.put(UID_SECRET_KEY, UID_SECRET)
179+
.put("weather", SQL_STATEMENT);
181180

182181
Mockito.when(mockCtx.body()).thenReturn(mockBody);
183182
Mockito.when(mockBody.buffer()).thenReturn(Buffer.buffer(json.toString()));
@@ -189,13 +188,11 @@ public void testBodyParamsAsJsobObjectWithSecretNSQL() {
189188
.toList();
190189

191190
assertThat(messages).allMatch(msg -> msg.contains("uid2_user") && msg.contains("seattle"));
192-
assertThat(messages).noneMatch(msg -> msg.contains(UID2_SECRET_KEY));
193-
assertThat(messages).noneMatch(msg -> msg.contains(SECRET));
191+
assertThat(messages).noneMatch(msg -> msg.contains(UID_SECRET));
194192
assertThat(messages).noneMatch(msg -> msg.contains(SQL_STATEMENT));
195-
assertThat(messages).noneMatch(msg -> msg.contains("weather"));
196193

197194
boolean errorLogged = listAppender.list.stream()
198-
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("Secret found in the audit log: request_body.") && event.getFormattedMessage().contains("SQL injection found in the audit log: request_body."));
195+
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains(String.format("Secret found in the audit log: request_body.%s. ", UID_SECRET_KEY)) && event.getFormattedMessage().contains("SQL injection found in the audit log: request_body.weather. "));
199196

200197
assertThat(errorLogged).isTrue();
201198
}
@@ -248,13 +245,13 @@ public void testBodyParamsAsJsonArray() {
248245
@Test
249246
public void testBodyParamsAsJsonArrayWithSecret() {
250247
Mockito.when(mockRequest.method()).thenReturn(HttpMethod.POST);
251-
AuditParams params = new AuditParams(null, Arrays.asList("partner_id", "config", UID2_SECRET_KEY));
248+
AuditParams params = new AuditParams(null, Arrays.asList("partner_id", "config", UID_SECRET_KEY));
252249

253250
RequestBody mockBody = Mockito.mock(RequestBody.class);
254251
JsonArray json = new JsonArray()
255252
.add(new JsonObject().put("partner_id", "1").put("config", "config1"))
256253
.add(new JsonObject().put("partner_id", "2").put("config", "config2"))
257-
.add(new JsonObject().put(UID2_SECRET_KEY, SECRET).put("config", "config3"));
254+
.add(new JsonObject().put(UID_SECRET_KEY, UID_SECRET).put("config", "config3"));
258255

259256
Mockito.when(mockCtx.body()).thenReturn(mockBody);
260257
Mockito.when(mockBody.buffer()).thenReturn(Buffer.buffer(json.toString()));
@@ -266,25 +263,24 @@ public void testBodyParamsAsJsonArrayWithSecret() {
266263
.toList();
267264

268265
assertThat(messages).allMatch(msg -> msg.contains("partner_id") && msg.contains("config"));
269-
assertThat(messages).noneMatch(msg -> msg.contains(UID2_SECRET_KEY));
270-
assertThat(messages).noneMatch(msg -> msg.contains(SECRET));
266+
assertThat(messages).noneMatch(msg -> msg.contains(UID_SECRET));
271267

272268
boolean errorLogged = listAppender.list.stream()
273-
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("Secret found in the audit log: request_body."));
269+
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains(String.format("Secret found in the audit log: request_body.%s", UID_SECRET_KEY)));
274270

275271
assertThat(errorLogged).isTrue();
276272
}
277273

278274
@Test
279275
public void testBodyParamsAsJsonArrayWithSecretNSQL() {
280276
Mockito.when(mockRequest.method()).thenReturn(HttpMethod.POST);
281-
AuditParams params = new AuditParams(null, Arrays.asList("partner_id", "config", UID2_SECRET_KEY, "weather"));
277+
AuditParams params = new AuditParams(null, Arrays.asList("partner_id", "config", UID_SECRET_KEY, "weather"));
282278

283279
RequestBody mockBody = Mockito.mock(RequestBody.class);
284280
JsonArray json = new JsonArray()
285281
.add(new JsonObject().put("partner_id", "1").put("config", "config1"))
286282
.add(new JsonObject().put("partner_id", "2").put("config", "config2"))
287-
.add(new JsonObject().put(UID2_SECRET_KEY, SECRET).put("config", "config3"))
283+
.add(new JsonObject().put(UID_SECRET_KEY, UID_SECRET).put("config", "config3"))
288284
.add(new JsonObject().put("weather", SQL_STATEMENT).put("config", "config3"));
289285

290286
Mockito.when(mockCtx.body()).thenReturn(mockBody);
@@ -297,13 +293,11 @@ public void testBodyParamsAsJsonArrayWithSecretNSQL() {
297293
.toList();
298294

299295
assertThat(messages).allMatch(msg -> msg.contains("partner_id") && msg.contains("config"));
300-
assertThat(messages).noneMatch(msg -> msg.contains(UID2_SECRET_KEY));
301-
assertThat(messages).noneMatch(msg -> msg.contains(SECRET));
296+
assertThat(messages).noneMatch(msg -> msg.contains(UID_SECRET));
302297
assertThat(messages).noneMatch(msg -> msg.contains(SQL_STATEMENT));
303-
assertThat(messages).noneMatch(msg -> msg.contains("weather"));
304298

305299
boolean errorLogged = listAppender.list.stream()
306-
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("Secret found in the audit log: request_body.") && event.getFormattedMessage().contains("SQL injection found in the audit log: request_body."));
300+
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains(String.format("Secret found in the audit log: request_body.%s.", UID_SECRET_KEY)) && event.getFormattedMessage().contains("SQL injection found in the audit log: request_body.weather. "));
307301

308302
assertThat(errorLogged).isTrue();
309303
}
@@ -352,10 +346,10 @@ public void testQueryParams() {
352346
@Test
353347
public void testQueryParamsContainsSecrets() {
354348
Mockito.when(mockRequest.method()).thenReturn(HttpMethod.POST);
355-
AuditParams params = new AuditParams(Arrays.asList(UID2_SECRET_KEY, "location"), null);
349+
AuditParams params = new AuditParams(Arrays.asList(UID_SECRET_KEY, "location"), null);
356350

357351
MultiMap queryParams = MultiMap.caseInsensitiveMultiMap();
358-
queryParams.add(UID2_SECRET_KEY, SECRET);
352+
queryParams.add(UID_SECRET_KEY, UID_SECRET);
359353
queryParams.add("location", "seattle");
360354

361355
Mockito.when(mockCtx.request().params()).thenReturn(queryParams);
@@ -366,11 +360,10 @@ public void testQueryParamsContainsSecrets() {
366360
.toList();
367361

368362
assertThat(messages).anyMatch(msg -> msg.contains("seattle"));
369-
assertThat(messages).noneMatch(msg -> msg.contains(UID2_SECRET_KEY));
370-
assertThat(messages).noneMatch(msg -> msg.contains(SECRET));
363+
assertThat(messages).noneMatch(msg -> msg.contains(UID_SECRET));
371364

372365
boolean errorLogged = listAppender.list.stream()
373-
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("Secret found in the audit log: query_params."));
366+
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains(String.format("Secret found in the audit log: query_params.%s. ", UID_SECRET_KEY)));
374367

375368
assertThat(errorLogged).isTrue();
376369
}
@@ -392,24 +385,23 @@ public void testQueryParamsContainsSQL() {
392385
.toList();
393386

394387
assertThat(messages).anyMatch(msg -> msg.contains("seattle"));
395-
assertThat(messages).noneMatch(msg -> msg.contains("weather"));
396388
assertThat(messages).noneMatch(msg -> msg.contains(SQL_STATEMENT));
397389

398390
boolean errorLogged = listAppender.list.stream()
399-
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("SQL injection found in the audit log: query_params."));
391+
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("SQL injection found in the audit log: query_params.weather. "));
400392

401393
assertThat(errorLogged).isTrue();
402394
}
403395

404396
@Test
405397
public void testQueryParamsContainsSecretNSQL() {
406398
Mockito.when(mockRequest.method()).thenReturn(HttpMethod.POST);
407-
AuditParams params = new AuditParams(Arrays.asList(UID2_SECRET_KEY, "weather", "location"), null);
399+
AuditParams params = new AuditParams(Arrays.asList(UID_SECRET_KEY, "weather", "location"), null);
408400

409401
MultiMap queryParams = MultiMap.caseInsensitiveMultiMap();
410402
queryParams.add("weather", SQL_STATEMENT);
411403
queryParams.add("location", "seattle");
412-
queryParams.add(UID2_SECRET_KEY, SECRET);
404+
queryParams.add(UID_SECRET_KEY, UID_SECRET);
413405

414406
Mockito.when(mockCtx.request().params()).thenReturn(queryParams);
415407
new Audit("admin").log(mockCtx, params);
@@ -419,13 +411,11 @@ public void testQueryParamsContainsSecretNSQL() {
419411
.toList();
420412

421413
assertThat(messages).anyMatch(msg -> msg.contains("seattle"));
422-
assertThat(messages).noneMatch(msg -> msg.contains("weather"));
423414
assertThat(messages).noneMatch(msg -> msg.contains(SQL_STATEMENT));
424-
assertThat(messages).noneMatch(msg -> msg.contains(UID2_SECRET_KEY));
425-
assertThat(messages).noneMatch(msg -> msg.contains(SECRET));
415+
assertThat(messages).noneMatch(msg -> msg.contains(UID_SECRET));
426416

427417
boolean errorLogged = listAppender.list.stream()
428-
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains("Secret found in the audit log: query_params.") && event.getFormattedMessage().contains("SQL injection found in the audit log: query_params."));
418+
.anyMatch(event -> event.getLevel() == Level.ERROR && event.getFormattedMessage().contains(String.format("Secret found in the audit log: query_params.%s", UID_SECRET_KEY)) && event.getFormattedMessage().contains("SQL injection found in the audit log: query_params.weather. "));
429419

430420
assertThat(errorLogged).isTrue();
431421
}

0 commit comments

Comments
 (0)