Skip to content

Commit

Permalink
[Feature-2188] AuditMessage validation in tests should be more useful…
Browse files Browse the repository at this point in the history 
… / removed (#3596)

Changed validateMsgs in AbstractAuditlogUnitTest.java to throw
exceptions with descriptions instead of just returning true or false.

Signed-off-by: Prabhas Kurapati <prabhask@berkeley.edu>
  • Loading branch information
@prabhask5
prabhask5 authored Nov 16, 2023
1 parent b7f47b1 commit deff842
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 47 deletions.
30 changes: 12 additions & 18 deletions src/test/java/org/opensearch/security/auditlog/AbstractAuditlogiUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,34 +84,28 @@ protected void setupStarfleetIndex() {
rh.keystore = keystore;
}

protected boolean validateMsgs(final Collection<AuditMessage> msgs) {
boolean valid = true;
protected void validateMsgs(final Collection<AuditMessage> msgs) throws Exception {
for (AuditMessage msg : msgs) {
valid = validateMsg(msg) && valid;
validateMsg(msg);
}
return valid;
}

protected boolean validateMsg(final AuditMessage msg) {
return validateJson(msg.toJson()) && validateJson(msg.toPrettyString());
}

protected boolean validateJson(final String json) {
protected void validateMsg(final AuditMessage msg) throws Exception {
validateJson(msg.toJson());
validateJson(msg.toPrettyString());
}

protected void validateJson(final String json) throws Exception { // this function can throw either IllegalArgumentException,
// JsonMappingException
if (json == null || json.isEmpty()) {
return false;
throw new IllegalArgumentException("json is either null or empty");
}

try {
JsonNode node = DefaultObjectMapper.objectMapper.readTree(json);

if (node.get("audit_request_body") != null) {
DefaultObjectMapper.objectMapper.readTree(node.get("audit_request_body").asText());
}
JsonNode node = DefaultObjectMapper.objectMapper.readTree(json);

return true;
} catch (Exception e) {
return false;
if (node.get("audit_request_body") != null) {
DefaultObjectMapper.objectMapper.readTree(node.get("audit_request_body").asText());
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/opensearch/security/auditlog/compliance/ComplianceAuditlogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void testSourceFilter() throws Exception {
assertThat(message.getRequestBody(), not(containsString("Salary")));
assertThat(message.getRequestBody(), containsString("Gender"));

Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -245,7 +245,7 @@ public void testSourceFilterMsearch() throws Exception {
assertThat(genderMsg.getRequestBody(), containsString("Gender"));
assertThat(genderMsg.getRequestBody(), not(containsString("Salary")));

Assert.assertTrue(validateMsgs(messages));
validateMsgs(messages);
}

@Test
Expand Down Expand Up @@ -302,7 +302,7 @@ public void testInternalConfig() throws Exception {
);
});

Assert.assertTrue(validateMsgs(messages));
validateMsgs(messages);
}

@Test
Expand Down Expand Up @@ -346,7 +346,7 @@ public void testExternalConfig() throws Exception {
assertThat(messages.get(1).getNodeId(), not(equalTo(messages.get(2).getNodeId())));
assertThat(messages.get(2).getNodeId(), not(equalTo(messages.get(3).getNodeId())));

Assert.assertTrue(validateMsgs(messages));
validateMsgs(messages);
}

@Test
Expand Down Expand Up @@ -399,7 +399,7 @@ public void testUpdate() throws Exception {
assertThat(ex2.getMissingCount(), equalTo(1));

Assert.assertTrue(TestAuditlogImpl.messages.isEmpty());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void testRestApiRolesEnabled() throws Exception {
Assert.assertFalse(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_INTERNAL_CONFIG_READ"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("UPDATE"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -89,7 +89,7 @@ public void testRestApiRolesDisabled() throws Exception {
Assert.assertFalse(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_INTERNAL_CONFIG_READ"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("UPDATE"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -122,7 +122,7 @@ public void testRestApiRolesDisabledGet() throws Exception {
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_INTERNAL_CONFIG_READ"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("UPDATE"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand All @@ -147,7 +147,7 @@ public void testAutoInit() throws Exception {
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("audit_request_effective_user"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("COMPLIANCE_EXTERNAL_CONFIG"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -208,7 +208,7 @@ public void testRestInternalConfigRead() throws Exception {
Assert.assertTrue(auditLogImpl.contains("COMPLIANCE_INTERNAL_CONFIG_READ"));
Assert.assertFalse(auditLogImpl.contains("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
Assert.assertFalse(auditLogImpl.contains("UPDATE"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down
38 changes: 19 additions & 19 deletions src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public void testSSLPlainText() throws Exception {
Assert.assertEquals(AuditCategory.SSL_EXCEPTION, message.getCategory());
Assert.assertTrue(message.getExceptionStackTrace().contains("not an SSL/TLS record"));
});
Assert.assertTrue(validateMsgs(messages));
validateMsgs(messages);
}

@Test
Expand Down Expand Up @@ -182,7 +182,7 @@ public void testTaskId() throws Exception {
TestAuditlogImpl.messages.get(1).getAsMap().get(AuditMessage.TASK_ID),
TestAuditlogImpl.messages.get(1).getAsMap().get(AuditMessage.TASK_ID)
);
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -211,7 +211,7 @@ public void testDefaultsRest() throws Exception {
Assert.assertTrue(auditLogImpl.contains("\"audit_request_effective_user\" : \"admin\""));
Assert.assertTrue(auditLogImpl.contains("REST"));
Assert.assertFalse(auditLogImpl.toLowerCase().contains("authorization"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -317,7 +317,7 @@ public void testWrongUser() throws Exception {
Assert.assertTrue(TestAuditlogImpl.sb.toString(), TestAuditlogImpl.sb.toString().contains(AuditMessage.UTC_TIMESTAMP));
Assert.assertFalse(TestAuditlogImpl.sb.toString(), TestAuditlogImpl.sb.toString().contains("AUTHENTICATED"));
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testUnknownAuthorization() throws Exception {
Expand All @@ -329,7 +329,7 @@ public void testUnknownAuthorization() throws Exception {
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains(AuditMessage.UTC_TIMESTAMP));
Assert.assertFalse(TestAuditlogImpl.sb.toString().contains("AUTHENTICATED"));
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testUnauthenticated() throws Exception {
Expand All @@ -345,15 +345,15 @@ public void testUnauthenticated() throws Exception {
Assert.assertTrue(auditLogImpl.contains("/_search"));
Assert.assertTrue(auditLogImpl.contains(AuditMessage.UTC_TIMESTAMP));
Assert.assertFalse(auditLogImpl.contains("AUTHENTICATED"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);

}

public void testJustAuthenticated() throws Exception {
HttpResponse response = rh.executeGetRequest("", encodeBasicHeader("admin", "admin"));
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
Assert.assertEquals(0, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testSecurityIndexAttempt() throws Exception {
Expand All @@ -366,7 +366,7 @@ public void testSecurityIndexAttempt() throws Exception {
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains(AuditMessage.UTC_TIMESTAMP));
Assert.assertFalse(TestAuditlogImpl.sb.toString().contains("AUTHENTICATED"));
Assert.assertEquals(2, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testBadHeader() throws Exception {
Expand All @@ -381,7 +381,7 @@ public void testBadHeader() throws Exception {
Assert.assertTrue(TestAuditlogImpl.sb.toString(), TestAuditlogImpl.sb.toString().contains("BAD_HEADERS"));
Assert.assertTrue(TestAuditlogImpl.sb.toString(), TestAuditlogImpl.sb.toString().contains("_opendistro_security_bad"));
Assert.assertEquals(TestAuditlogImpl.sb.toString(), 1, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testMissingPriv() throws Exception {
Expand All @@ -395,7 +395,7 @@ public void testMissingPriv() throws Exception {
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains(AuditMessage.UTC_TIMESTAMP));
Assert.assertFalse(TestAuditlogImpl.sb.toString().contains("AUTHENTICATED"));
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testMsearch() throws Exception {
Expand All @@ -419,7 +419,7 @@ public void testMsearch() throws Exception {
Assert.assertTrue(auditLogImpl.contains("audit_trace_task_id"));
Assert.assertEquals(auditLogImpl, 4, TestAuditlogImpl.messages.size());
Assert.assertFalse(auditLogImpl.toLowerCase().contains("authorization"));
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testBulkAuth() throws Exception {
Expand Down Expand Up @@ -458,7 +458,7 @@ public void testBulkAuth() throws Exception {
Assert.assertTrue(auditLogImpl.contains("audit_trace_task_id"));
// may vary because we log shardrequests which are not predictable here
Assert.assertTrue(TestAuditlogImpl.messages.size() >= 17);
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testBulkNonAuth() throws Exception {
Expand Down Expand Up @@ -496,7 +496,7 @@ public void testBulkNonAuth() throws Exception {
Assert.assertTrue(auditLogImpl.contains("IndexRequest"));
// may vary because we log shardrequests which are not predictable here
Assert.assertTrue(TestAuditlogImpl.messages.size() >= 7);
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

public void testUpdateSettings() throws Exception {
Expand All @@ -521,7 +521,7 @@ public void testUpdateSettings() throws Exception {
Assert.assertTrue(auditLogImpl.contains(expectedRequestBodyLog));
// may vary because we log may hit cluster manager directly or not
Assert.assertTrue(TestAuditlogImpl.messages.size() > 1);
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -620,7 +620,7 @@ public void testAliases() throws Exception {
Assert.assertTrue(auditLogImpl.contains("starfleet"));
Assert.assertTrue(auditLogImpl.contains("sf"));
Assert.assertEquals(2, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -682,7 +682,7 @@ public void testScroll() throws Exception {
Assert.assertTrue(auditLogImpl.contains("InternalScrollSearchRequest"));
Assert.assertTrue(auditLogImpl.contains("MISSING_PRIVILEGES"));
Assert.assertTrue(TestAuditlogImpl.messages.size() > 2);
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down Expand Up @@ -718,7 +718,7 @@ public void testAliasResolution() throws Exception {
Assert.assertTrue(auditLogImpl.contains("audit_trace_resolved_indices"));
Assert.assertTrue(auditLogImpl.contains("vulcangov"));
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
TestAuditlogImpl.clear();
}

Expand Down Expand Up @@ -747,7 +747,7 @@ public void testAliasBadHeaders() throws Exception {
Assert.assertTrue(auditLogImpl.contains("BAD_HEADERS"));
Assert.assertTrue(auditLogImpl.contains("xxx"));
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
TestAuditlogImpl.clear();
}

Expand Down Expand Up @@ -780,7 +780,7 @@ public void testIndexCloseDelete() throws Exception {
Assert.assertTrue(auditLogImpl.contains("indices:admin/close"));
Assert.assertTrue(auditLogImpl.contains("indices:admin/delete"));
Assert.assertTrue(auditLogImpl, TestAuditlogImpl.messages.size() >= 2);
Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages));
validateMsgs(TestAuditlogImpl.messages);
}

@Test
Expand Down

0 comments on commit deff842

Please sign in to comment.