From 2b3c157c97f5cd2e3d09bcb6b7016c3503c8af9d Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 18 Jun 2015 13:26:51 -0400 Subject: [PATCH] store names of indices as an array instead of a string We currently store the names of indices as a comma separated string instead of an array. An array is the proper format for this information so this commit changes the index audit trail to store the indices as an array. Closes elastic/elasticsearch#917 Original commit: elastic/x-pack-elasticsearch@025393d91c9cce3931bababbf3a99c60904c9776 --- .../elasticsearch/shield/audit/AuditUtil.java | 5 +- .../shield/audit/index/IndexAuditTrail.java | 8 +-- .../audit/logfile/LoggingAuditTrail.java | 20 +++--- .../audit/index/IndexAuditTrailTests.java | 61 +++++++++++++++---- 4 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/elasticsearch/shield/audit/AuditUtil.java b/src/main/java/org/elasticsearch/shield/audit/AuditUtil.java index 6211c0b07fc..bf6079c56cf 100644 --- a/src/main/java/org/elasticsearch/shield/audit/AuditUtil.java +++ b/src/main/java/org/elasticsearch/shield/audit/AuditUtil.java @@ -6,7 +6,6 @@ package org.elasticsearch.shield.audit; import org.elasticsearch.action.IndicesRequest; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.transport.TransportMessage; @@ -29,9 +28,9 @@ public class AuditUtil { return ""; } - public static String indices(TransportMessage message) { + public static String[] indices(TransportMessage message) { if (message instanceof IndicesRequest) { - return Strings.arrayToCommaDelimitedString(((IndicesRequest) message).indices()); + return ((IndicesRequest) message).indices(); } return null; } diff --git a/src/main/java/org/elasticsearch/shield/audit/index/IndexAuditTrail.java b/src/main/java/org/elasticsearch/shield/audit/index/IndexAuditTrail.java index 89c1eda7179..d83b8d5c101 100644 --- a/src/main/java/org/elasticsearch/shield/audit/index/IndexAuditTrail.java +++ b/src/main/java/org/elasticsearch/shield/audit/index/IndexAuditTrail.java @@ -419,7 +419,7 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail { } private Message message(String type, @Nullable String action, @Nullable String principal, - @Nullable String realm, @Nullable String indices, TransportMessage message) throws Exception { + @Nullable String realm, @Nullable String[] indices, TransportMessage message) throws Exception { Message msg = new Message().start(); common("transport", type, msg.builder); @@ -435,7 +435,7 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail { msg.builder.field(Field.REALM, realm); } if (indices != null) { - msg.builder.field(Field.INDICES, indices); + msg.builder.array(Field.INDICES, indices); } if (logger.isDebugEnabled()) { msg.builder.field(Field.REQUEST, message.getClass().getSimpleName()); @@ -445,7 +445,7 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail { } private Message message(String type, @Nullable String action, @Nullable String principal, - @Nullable String realm, @Nullable String indices, RestRequest request) throws Exception { + @Nullable String realm, @Nullable String[] indices, RestRequest request) throws Exception { Message msg = new Message().start(); common("rest", type, msg.builder); @@ -460,7 +460,7 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail { msg.builder.field(Field.REALM, realm); } if (indices != null) { - msg.builder.field(Field.INDICES, indices); + msg.builder.array(Field.INDICES, indices); } if (logger.isDebugEnabled()) { msg.builder.field(Field.REQUEST_BODY, restRequestContent(request)); diff --git a/src/main/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrail.java b/src/main/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrail.java index 2ac48287744..18493da4db2 100644 --- a/src/main/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrail.java +++ b/src/main/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrail.java @@ -26,6 +26,7 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; +import static org.elasticsearch.common.Strings.arrayToCommaDelimitedString; import static org.elasticsearch.shield.audit.AuditUtil.indices; import static org.elasticsearch.shield.audit.AuditUtil.restRequestContent; @@ -60,7 +61,7 @@ public class LoggingAuditTrail implements AuditTrail { @Override public void anonymousAccessDenied(String action, TransportMessage message) { - String indices = indices(message); + String indices = indicesString(message); if (indices != null) { if (logger.isDebugEnabled()) { logger.debug("{}[transport] [anonymous_access_denied]\t{}, action=[{}], indices=[{}], request=[{}]", prefix, originAttributes(message), action, indices, message.getClass().getSimpleName()); @@ -87,7 +88,7 @@ public class LoggingAuditTrail implements AuditTrail { @Override public void authenticationFailed(AuthenticationToken token, String action, TransportMessage message) { - String indices = indices(message); + String indices = indicesString(message); if (indices != null) { if (logger.isDebugEnabled()) { logger.debug("{}[transport] [authentication_failed]\t{}, principal=[{}], action=[{}], indices=[{}], request=[{}]", prefix, originAttributes(message), token.principal(), action, indices, message.getClass().getSimpleName()); @@ -114,7 +115,7 @@ public class LoggingAuditTrail implements AuditTrail { @Override public void authenticationFailed(String action, TransportMessage message) { - String indices = indices(message); + String indices = indicesString(message); if (indices != null) { if (logger.isDebugEnabled()) { logger.debug("{}[transport] [authentication_failed]\t{}, action=[{}], indices=[{}], request=[{}]", prefix, originAttributes(message), action, indices, message.getClass().getSimpleName()); @@ -142,7 +143,7 @@ public class LoggingAuditTrail implements AuditTrail { @Override public void authenticationFailed(String realm, AuthenticationToken token, String action, TransportMessage message) { if (logger.isTraceEnabled()) { - String indices = indices(message); + String indices = indicesString(message); if (indices != null) { logger.trace("{}[transport] [authentication_failed]\trealm=[{}], {}, principal=[{}], action=[{}], indices=[{}], request=[{}]", prefix, realm, originAttributes(message), token.principal(), action, indices, message.getClass().getSimpleName()); } else { @@ -160,7 +161,7 @@ public class LoggingAuditTrail implements AuditTrail { @Override public void accessGranted(User user, String action, TransportMessage message) { - String indices = indices(message); + String indices = indicesString(message); // special treatment for internal system actions - only log on trace if (user.isSystem() && Privilege.SYSTEM.predicate().apply(action)) { @@ -191,7 +192,7 @@ public class LoggingAuditTrail implements AuditTrail { @Override public void accessDenied(User user, String action, TransportMessage message) { - String indices = indices(message); + String indices = indicesString(message); if (indices != null) { if (logger.isDebugEnabled()) { logger.debug("{}[transport] [access_denied]\t{}, principal=[{}], action=[{}], indices=[{}], request=[{}]", prefix, originAttributes(message), user.principal(), action, indices, message.getClass().getSimpleName()); @@ -209,7 +210,7 @@ public class LoggingAuditTrail implements AuditTrail { @Override public void tamperedRequest(User user, String action, TransportRequest request) { - String indices = indices(request); + String indices = indicesString(request); if (indices != null) { if (logger.isDebugEnabled()) { logger.debug("{}[transport] [tampered_request]\t{}, principal=[{}], action=[{}], indices=[{}], request=[{}]", prefix, request.remoteAddress(), user.principal(), action, indices, request.getClass().getSimpleName()); @@ -296,4 +297,9 @@ public class LoggingAuditTrail implements AuditTrail { } return builder.toString(); } + + static String indicesString(TransportMessage message) { + String[] indices = indices(message); + return indices == null ? null : arrayToCommaDelimitedString(indices); + } } diff --git a/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java b/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java index 7435c3be0fb..8e17196d824 100644 --- a/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java +++ b/src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java @@ -6,11 +6,13 @@ package org.elasticsearch.shield.audit.index; import com.google.common.base.Predicate; +import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse; import org.elasticsearch.action.admin.indices.delete.DeleteIndexResponse; import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; import org.elasticsearch.action.exists.ExistsResponse; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.common.inject.util.Providers; import org.elasticsearch.common.settings.Settings; @@ -36,6 +38,7 @@ import org.junit.Test; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.util.List; import java.util.Locale; import static org.elasticsearch.node.NodeBuilder.nodeBuilder; @@ -163,7 +166,7 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { public void testAnonymousAccessDenied_Transport() throws Exception { initialize(); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.anonymousAccessDenied("_action", message); awaitIndexCreation(resolveIndexName()); @@ -178,12 +181,16 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { assertEquals("_action", hit.field("action").getValue()); assertEquals("transport", hit.field("origin_type").getValue()); + if (message instanceof IndicesRequest) { + List indices = hit.field("indices").getValues(); + assertThat(indices, contains((Object[]) ((IndicesRequest)message).indices())); + } } @Test(expected = IndexMissingException.class) public void testAnonymousAccessDenied_Transport_Muted() throws Exception { initialize("anonymous_access_denied"); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.anonymousAccessDenied("_action", message); getClient().prepareExists(resolveIndexName()).execute().actionGet(); } @@ -237,7 +244,7 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { @Test public void testAuthenticationFailed_Transport_NoToken() throws Exception { initialize(); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.authenticationFailed("_action", message); awaitIndexCreation(resolveIndexName()); @@ -254,12 +261,16 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { assertThat(hit.field("principal"), nullValue()); assertEquals("_action", hit.field("action").getValue()); assertEquals("transport", hit.field("origin_type").getValue()); + if (message instanceof IndicesRequest) { + List indices = hit.field("indices").getValues(); + assertThat(indices, contains((Object[]) ((IndicesRequest)message).indices())); + } } @Test(expected = IndexMissingException.class) public void testAuthenticationFailed_Transport_Muted() throws Exception { initialize("authentication_failed"); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.authenticationFailed(new MockToken(), "_action", message); getClient().prepareExists(resolveIndexName()).execute().actionGet(); } @@ -267,7 +278,7 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { @Test(expected = IndexMissingException.class) public void testAuthenticationFailed_Transport_NoToken_Muted() throws Exception { initialize("authentication_failed"); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.authenticationFailed("_action", message); getClient().prepareExists(resolveIndexName()).execute().actionGet(); } @@ -323,7 +334,7 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { public void testAuthenticationFailed_Transport_Realm() throws Exception { initialize(); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.authenticationFailed("_realm", new MockToken(), "_action", message); awaitIndexCreation(resolveIndexName()); @@ -341,12 +352,16 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { assertEquals("_principal", hit.field("principal").getValue()); assertEquals("_action", hit.field("action").getValue()); assertEquals("_realm", hit.field("realm").getValue()); + if (message instanceof IndicesRequest) { + List indices = hit.field("indices").getValues(); + assertThat(indices, contains((Object[]) ((IndicesRequest)message).indices())); + } } @Test(expected = IndexMissingException.class) public void testAuthenticationFailed_Transport_Realm_Muted() throws Exception { initialize("authentication_failed"); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.authenticationFailed("_realm", new MockToken(), "_action", message); getClient().prepareExists(resolveIndexName()).execute().actionGet(); } @@ -379,7 +394,7 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { public void testAccessGranted() throws Exception { initialize(); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.accessGranted(new User.Simple("_username", "r1"), "_action", message); awaitIndexCreation(resolveIndexName()); @@ -388,12 +403,16 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { assertEquals("transport", hit.field("origin_type").getValue()); assertEquals("_username", hit.field("principal").getValue()); assertEquals("_action", hit.field("action").getValue()); + if (message instanceof IndicesRequest) { + List indices = hit.field("indices").getValues(); + assertThat(indices, contains((Object[]) ((IndicesRequest)message).indices())); + } } @Test(expected = IndexMissingException.class) public void testAccessGranted_Muted() throws Exception { initialize("access_granted"); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.accessGranted(new User.Simple("_username", "r1"), "_action", message); getClient().prepareExists(resolveIndexName()).execute().actionGet(); } @@ -425,7 +444,7 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { public void testAccessDenied() throws Exception { initialize(); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.accessDenied(new User.Simple("_username", "r1"), "_action", message); awaitIndexCreation(resolveIndexName()); @@ -434,12 +453,16 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { assertEquals("transport", hit.field("origin_type").getValue()); assertEquals("_username", hit.field("principal").getValue()); assertEquals("_action", hit.field("action").getValue()); + if (message instanceof IndicesRequest) { + List indices = hit.field("indices").getValues(); + assertThat(indices, contains((Object[]) ((IndicesRequest)message).indices())); + } } @Test(expected = IndexMissingException.class) public void testAccessDenied_Muted() throws Exception { initialize("access_denied"); - TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); + TransportMessage message = randomFrom(new RemoteHostMockMessage(), new LocalHostMockMessage(), new MockIndicesTransportMessage()); auditor.accessDenied(new User.Simple("_username", "r1"), "_action", message); getClient().prepareExists(resolveIndexName()).execute().actionGet(); } @@ -546,6 +569,22 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest { } } + private static class MockIndicesTransportMessage extends RemoteHostMockMessage implements IndicesRequest { + MockIndicesTransportMessage() throws Exception { + super(); + } + + @Override + public String[] indices() { + return new String[] { "foo", "bar", "baz" }; + } + + @Override + public IndicesOptions indicesOptions() { + return null; + } + } + private static class MockToken implements AuthenticationToken { @Override public String principal() {