From 41eea741b8cbb95749f244d2b15c494d4beb9189 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 18 Jul 2016 15:13:10 -0700 Subject: [PATCH] Ensure index audit trail is bound for security lifecycle service Original commit: elastic/x-pack-elasticsearch@bbe7ec0802c41ac8fe009a6e895d4630d3e314d7 --- .../xpack/security/Security.java | 7 ++++-- .../security/SecurityLifecycleService.java | 23 +++++++++++-------- .../RemoteIndexAuditTrailStartingTests.java | 8 +++---- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 0a0af76b4ee..343c392ec35 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -217,7 +217,8 @@ public class Security implements ActionPlugin { b.bind(CryptoService.class).toInstance(cryptoService); if (auditingEnabled(settings)) { b.bind(AuditTrail.class).to(AuditTrailService.class); // interface used by some actions... - } else { + } + if (indexAuditLoggingEnabled(settings) == false) { // TODO: remove this once we can construct SecurityLifecycleService without guice b.bind(IndexAuditTrail.class).toProvider(Providers.of(null)); } @@ -287,7 +288,9 @@ public class Security implements ActionPlugin { auditTrails.add(new LoggingAuditTrail(settings, clusterService, threadPool)); break; case IndexAuditTrail.NAME: - auditTrails.add(new IndexAuditTrail(settings, client, threadPool, clusterService)); + IndexAuditTrail indexAuditTrail = new IndexAuditTrail(settings, client, threadPool, clusterService); + auditTrails.add(indexAuditTrail); + components.add(indexAuditTrail); // SecurityLifecycleService needs this.... break; default: throw new IllegalArgumentException("Unknown audit trail output [" + output + "]"); diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java index a43c5e6872f..1a1276b91e3 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java @@ -11,6 +11,7 @@ import org.elasticsearch.cluster.ClusterStateListener; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.component.LifecycleListener; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.inject.internal.Nullable; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.xpack.security.audit.index.IndexAuditTrail; @@ -39,7 +40,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust @Inject public SecurityLifecycleService(Settings settings, ClusterService clusterService, ThreadPool threadPool, - IndexAuditTrail indexAuditTrail, NativeUsersStore nativeUserStore, + @Nullable IndexAuditTrail indexAuditTrail, NativeUsersStore nativeUserStore, NativeRolesStore nativeRolesStore, InternalClient client) { super(settings); this.settings = settings; @@ -144,19 +145,23 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust } catch (Exception e) { logger.error("failed to stop native roles module", e); } - try { - indexAuditTrail.stop(); - } catch (Exception e) { - logger.error("failed to stop audit trail module", e); + if (indexAuditTrail != null) { + try { + indexAuditTrail.stop(); + } catch (Exception e) { + logger.error("failed to stop audit trail module", e); + } } } public void close() { // There is no .close() method for the roles module - try { - indexAuditTrail.close(); - } catch (Exception e) { - logger.error("failed to close audit trail module", e); + if (indexAuditTrail != null) { + try { + indexAuditTrail.close(); + } catch (Exception e) { + logger.error("failed to close audit trail module", e); + } } } } diff --git a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java index aa4d5989103..530242a0150 100644 --- a/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java +++ b/elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java @@ -114,10 +114,10 @@ public class RemoteIndexAuditTrailStartingTests extends SecurityIntegTestCase { @After public void stopRemoteCluster() throws Exception { if (remoteCluster != null) { - Iterable auditTrails = internalCluster().getInstances(IndexAuditTrail.class); + /*Iterable auditTrails = internalCluster().getInstances(IndexAuditTrail.class); for (IndexAuditTrail auditTrail : auditTrails) { auditTrail.close(); - } + }*/ try { remoteCluster.wipe(Collections.emptySet()); @@ -128,12 +128,12 @@ public class RemoteIndexAuditTrailStartingTests extends SecurityIntegTestCase { } // stop the index audit trail so that the shards aren't locked causing the test to fail - if (outputs.contains("index")) { + /*if (outputs.contains("index")) { Iterable auditTrails = internalCluster().getInstances(IndexAuditTrail.class); for (IndexAuditTrail auditTrail : auditTrails) { auditTrail.close(); } - } + }*/ } public void testThatRemoteAuditInstancesAreStarted() throws Exception {