Skip to content

Commit

Permalink
Refactor RestApiComplianceAuditlogTest to use TestAuditLogImpl.doThen…
Browse files Browse the repository at this point in the history 
…WaitForMessages (opensearch-project#3733)

### Description

Refactors RestApiComplianceAuditlogTest to make this test suite more
stable and re-enable an ignored test.

The primary problem in this test suite is that messages that are auto
populated on startup get polluted with other audit log messages
generated when a user calls REST APIs. To make this more stable, a sleep
was added after `setup` to ignore the auto init messages and only
isolate messages generated on the API calls in each test.

`testAutoInit` is also updated to make it stable. There are 4 automatic
messages generated on cluster startup that have to do with the seeding
of the security index and audit log index.

* Category (Enhancement, New feature, Bug fix, Test fix, Refactoring,
Maintenance, Documentation)

Test Fix

### Issues Resolved

opensearch-project#3730

### Check List
- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
@cwperks
cwperks authored Nov 28, 2023
1 parent 3b070c8 commit 6b8a3e4
Showing 1 changed file with 134 additions and 77 deletions.
211 changes: 134 additions & 77 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 @@ -11,18 +11,26 @@

package org.opensearch.security.auditlog.compliance;

import java.util.List;
import java.util.stream.Collectors;

import org.apache.http.HttpStatus;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;

import org.opensearch.common.settings.Settings;
import org.opensearch.security.auditlog.AbstractAuditlogiUnitTest;
import org.opensearch.security.auditlog.impl.AuditCategory;
import org.opensearch.security.auditlog.impl.AuditMessage;
import org.opensearch.security.auditlog.integration.TestAuditlogImpl;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.test.helper.cluster.ClusterConfiguration;
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

public class RestApiComplianceAuditlogTest extends AbstractAuditlogiUnitTest {

@Test
Expand All @@ -40,22 +48,19 @@ public void testRestApiRolesEnabled() throws Exception {
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "authenticated,GRANTED_PRIVILEGES")
.build();

setup(additionalSettings);
TestAuditlogImpl.clear();
String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }";
HttpResponse response = rh.executePutRequest(
"_opendistro/_security/api/internalusers/compuser?pretty",
body,
encodeBasicHeader("admin", "admin")
);
Thread.sleep(1500);
Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode());
Assert.assertTrue(TestAuditlogImpl.messages.size() + "", TestAuditlogImpl.messages.size() == 1);
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("audit_request_effective_user"));
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"));
validateMsgs(TestAuditlogImpl.messages);
setupAndReturnAuditMessages(additionalSettings);
final AuditMessage message = TestAuditlogImpl.doThenWaitForMessage(() -> {
String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }";
HttpResponse response = rh.executePutRequest(
"_opendistro/_security/api/internalusers/compuser?pretty",
body,
encodeBasicHeader("admin", "admin")
);
Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode());
});
validateMsgs(List.of(message));

assertThat(message.toString(), containsString("UPDATE"));
}

@Test
Expand All @@ -72,28 +77,23 @@ public void testRestApiRolesDisabled() throws Exception {
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "authenticated,GRANTED_PRIVILEGES")
.build();

setup(additionalSettings);
TestAuditlogImpl.clear();
setupAndReturnAuditMessages(additionalSettings);
String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }";

rh.enableHTTPClientSSL = true;
rh.trustHTTPServerCertificate = true;
rh.sendAdminCertificate = true;
rh.keystore = "kirk-keystore.jks";

HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", body);
Thread.sleep(1500);
Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode());
Assert.assertTrue(TestAuditlogImpl.messages.size() + "", TestAuditlogImpl.messages.size() == 1);
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("audit_request_effective_user"));
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"));
validateMsgs(TestAuditlogImpl.messages);
final AuditMessage message = TestAuditlogImpl.doThenWaitForMessage(() -> {
HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", body);
Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode());
});
validateMsgs(List.of(message));
assertThat(message.toString(), containsString("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
}

@Test
@Ignore
public void testRestApiRolesDisabledGet() throws Exception {

Settings additionalSettings = Settings.builder()
Expand All @@ -107,22 +107,42 @@ public void testRestApiRolesDisabledGet() throws Exception {
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "authenticated,GRANTED_PRIVILEGES")
.build();

setup(additionalSettings);
TestAuditlogImpl.clear();
setupAndReturnAuditMessages(additionalSettings);

rh.enableHTTPClientSSL = true;
rh.trustHTTPServerCertificate = true;
rh.sendAdminCertificate = true;
rh.keystore = "kirk-keystore.jks";
HttpResponse response = rh.executeGetRequest("_opendistro/_security/api/rolesmapping/opendistro_security_all_access?pretty");
Thread.sleep(1500);
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
Assert.assertTrue(TestAuditlogImpl.messages.size() > 2);
Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("audit_request_effective_user"));
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"));
validateMsgs(TestAuditlogImpl.messages);
final AuditMessage message = TestAuditlogImpl.doThenWaitForMessage(() -> {
HttpResponse response = rh.executeGetRequest("_opendistro/_security/api/rolesmapping/opendistro_security_all_access?pretty");
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
});
validateMsgs(List.of(message));
assertThat(message.toString(), containsString("audit_request_effective_user"));
assertThat(message.toString(), containsString("COMPLIANCE_INTERNAL_CONFIG_READ"));
}

@Test
public void testAutoInit() throws Exception {

Settings additionalSettings = Settings.builder()
.put("plugins.security.audit.type", TestAuditlogImpl.class.getName())
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_TRANSPORT, true)
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_ENABLE_REST, true)
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, false)
.put(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED, true)
.put(ConfigConstants.SECURITY_COMPLIANCE_HISTORY_INTERNAL_CONFIG_ENABLED, true)
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_TRANSPORT_CATEGORIES, "authenticated,GRANTED_PRIVILEGES")
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "authenticated,GRANTED_PRIVILEGES")
.build();

final List<AuditMessage> messages = setupAndReturnAuditMessages(additionalSettings);

validateMsgs(messages);
String allMessages = messages.stream().map(AuditMessage::toString).collect(Collectors.joining(","));
assertThat(allMessages, containsString("audit_request_effective_user"));
assertThat(allMessages, containsString("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
assertThat(allMessages, containsString("COMPLIANCE_EXTERNAL_CONFIG"));
}

@Test
Expand All @@ -136,20 +156,22 @@ public void testRestApiNewUser() throws Exception {
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_RESOLVE_BULK_REQUESTS, false)
.put(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED, false)
.put(ConfigConstants.SECURITY_COMPLIANCE_HISTORY_INTERNAL_CONFIG_ENABLED, true)
.put(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_IGNORE_USERS, "admin")
.build();

setup(additionalSettings);
TestAuditlogImpl.clear();
String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }";
HttpResponse response = rh.executePutRequest(
"_opendistro/_security/api/internalusers/compuser?pretty",
body,
encodeBasicHeader("admin", "admin")
);
Thread.sleep(1500);
Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());
Assert.assertTrue(TestAuditlogImpl.messages.size() + "", TestAuditlogImpl.messages.isEmpty());
setupAndReturnAuditMessages(additionalSettings);

final AuditMessage message = TestAuditlogImpl.doThenWaitForMessage(() -> {
String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }";
HttpResponse response = rh.executePutRequest(
"_opendistro/_security/api/internalusers/compuser?pretty",
body,
encodeBasicHeader("admin", "admin")
);
Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());
});
validateMsgs(List.of(message));
assertThat(message.toString(), containsString("audit_request_effective_user"));
assertThat(message.toString(), containsString("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
}

@Test
Expand All @@ -167,23 +189,21 @@ public void testRestInternalConfigRead() throws Exception {
.put(ConfigConstants.OPENDISTRO_SECURITY_AUDIT_CONFIG_DISABLED_REST_CATEGORIES, "authenticated,GRANTED_PRIVILEGES")
.build();

setup(additionalSettings);
TestAuditlogImpl.clear();
setupAndReturnAuditMessages(additionalSettings);

rh.enableHTTPClientSSL = true;
rh.trustHTTPServerCertificate = true;
rh.sendAdminCertificate = true;
rh.keystore = "kirk-keystore.jks";
HttpResponse response = rh.executeGetRequest("_opendistro/_security/api/internalusers/admin?pretty");
Thread.sleep(1500);
String auditLogImpl = TestAuditlogImpl.sb.toString();
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
Assert.assertTrue(TestAuditlogImpl.messages.size() + "", TestAuditlogImpl.messages.size() == 1);
Assert.assertTrue(auditLogImpl.contains("audit_request_effective_user"));
Assert.assertTrue(auditLogImpl.contains("COMPLIANCE_INTERNAL_CONFIG_READ"));
Assert.assertFalse(auditLogImpl.contains("COMPLIANCE_INTERNAL_CONFIG_WRITE"));
Assert.assertFalse(auditLogImpl.contains("UPDATE"));
validateMsgs(TestAuditlogImpl.messages);

final AuditMessage message = TestAuditlogImpl.doThenWaitForMessage(() -> {
HttpResponse response = rh.executeGetRequest("_opendistro/_security/api/internalusers/admin?pretty");
String auditLogImpl = TestAuditlogImpl.sb.toString();
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
Assert.assertTrue(auditLogImpl.contains("COMPLIANCE_INTERNAL_CONFIG_READ"));
});
validateMsgs(List.of(message));
assertThat(message.toString(), containsString("COMPLIANCE_INTERNAL_CONFIG_READ"));
}

@Test
Expand All @@ -196,26 +216,63 @@ public void testBCryptHashRedaction() throws Exception {
.put(ConfigConstants.SECURITY_COMPLIANCE_HISTORY_INTERNAL_CONFIG_ENABLED, true)
.put(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_LOG_DIFFS, true)
.build();
setup(settings);
setupAndReturnAuditMessages(settings);
rh.sendAdminCertificate = true;
rh.keystore = "kirk-keystore.jks";

// read internal users and verify no BCrypt hash is present in audit logs
TestAuditlogImpl.clear();
rh.executeGetRequest("/_opendistro/_security/api/internalusers");
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(TestAuditlogImpl.sb.toString()).matches());
final AuditMessage message1 = TestAuditlogImpl.doThenWaitForMessage(() -> {
rh.executeGetRequest("/_opendistro/_security/api/internalusers");
});

Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(message1.toString()).matches());

// read internal user worf and verify no BCrypt hash is present in audit logs
TestAuditlogImpl.clear();
rh.executeGetRequest("/_opendistro/_security/api/internalusers/worf");
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(TestAuditlogImpl.sb.toString()).matches());
final AuditMessage message2 = TestAuditlogImpl.doThenWaitForMessage(() -> {
rh.executeGetRequest("/_opendistro/_security/api/internalusers/worf");
Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(TestAuditlogImpl.sb.toString()).matches());
});

Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(message2.toString()).matches());

// create internal user and verify no BCrypt hash is present in audit logs
TestAuditlogImpl.clear();
rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}");
Assert.assertEquals(1, TestAuditlogImpl.messages.size());
Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(TestAuditlogImpl.sb.toString()).matches());
final AuditMessage message3 = TestAuditlogImpl.doThenWaitForMessage(() -> {
rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}");
});

Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(message3.toString()).matches());
}

private List<AuditMessage> setupAndReturnAuditMessages(Settings settings) {
// When OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED is set to true, there is:
// - 1 message with COMPLIANCE_INTERNAL_CONFIG_WRITE as category.
// - 1 message with COMPLIANCE_EXTERNAL_CONFIG as category for each node.
int numNodes = ClusterConfiguration.DEFAULT.getNodes();
boolean externalConfigEnabled = settings.getAsBoolean(
ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED,
false
);
int expectedMessageCount = externalConfigEnabled ? (numNodes + 1) : 1;
final List<AuditMessage> messages = TestAuditlogImpl.doThenWaitForMessages(() -> {
try {
setup(settings);
} catch (final Exception ex) {
throw new RuntimeException(ex);
}
}, expectedMessageCount);
int complianceInternalConfigWriteCount = 0;
int complianceExternalConfigCount = 0;
for (AuditMessage message : messages) {
if (AuditCategory.COMPLIANCE_INTERNAL_CONFIG_WRITE.equals(message.getCategory())) {
complianceInternalConfigWriteCount++;
} else if (AuditCategory.COMPLIANCE_EXTERNAL_CONFIG.equals(message.getCategory())) {
complianceExternalConfigCount++;
}
}
assertThat(complianceInternalConfigWriteCount, equalTo(1));
if (externalConfigEnabled) {
assertThat(complianceExternalConfigCount, equalTo(numNodes));
}
return messages;
}
}

0 comments on commit 6b8a3e4

Please sign in to comment.