diff --git a/docs/en/settings/monitoring-settings.asciidoc b/docs/en/settings/monitoring-settings.asciidoc index e2764eb0f2a..7536e3ce64b 100644 --- a/docs/en/settings/monitoring-settings.asciidoc +++ b/docs/en/settings/monitoring-settings.asciidoc @@ -11,8 +11,9 @@ these monitoring settings in the `elasticsearch.yml` file. To adjust how monitoring data is displayed in the monitoring UI, configure {kibana-ref}/monitoring-settings-kb.html[`xpack.monitoring` settings] in `kibana.yml`. To control how monitoring data is collected from -Logstash, configure {logstash-ref}/settings-xpack.html#monitoring-settings[ -`xpack.monitoring` settings] in `logstash.yml`. +Logstash, configure +{logstash-ref}/configuring-logstash.html#monitoring-settings[`xpack.monitoring` settings] +in `logstash.yml`. For more information, see {xpack-ref}/xpack-monitoring.html[Monitoring the Elastic Stack]. diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/PutFilterAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/PutFilterAction.java index 6f9bf5a34fd..023d5e2073e 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/PutFilterAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/PutFilterAction.java @@ -37,7 +37,6 @@ import org.elasticsearch.xpack.ml.job.config.MlFilter; import org.elasticsearch.xpack.ml.job.messages.Messages; import org.elasticsearch.xpack.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.security.InternalClient; -import org.elasticsearch.xpack.watcher.watch.Payload; import java.io.IOException; import java.util.Collections; @@ -178,7 +177,7 @@ public class PutFilterAction extends Action handler) { + // At this point we lock the process context until the process has been started. + // The reason behind this is to ensure closing the job does not happen before + // the process is started as that can result to the job getting seemingly closed + // but the actual process is hanging alive. + processContext.tryLock(); try { - // At this point we lock the process context until the process has been started. - // The reason behind this is to ensure closing the job does not happen before - // the process is started as that can result to the job getting seemingly closed - // but the actual process is hanging alive. - processContext.tryLock(); AutodetectCommunicator communicator = create(processContext.getJobTask(), params, handler); processContext.setRunning(communicator); } finally { @@ -444,29 +444,36 @@ public class AutodetectProcessManager extends AbstractComponent { } processContext.tryLock(); - processContext.setDying(); - processContext.unlock(); - - if (reason == null) { - logger.info("Closing job [{}]", jobId); - } else { - logger.info("Closing job [{}], because [{}]", jobId, reason); - } - - AutodetectCommunicator communicator = processContext.getAutodetectCommunicator(); - if (communicator == null) { - logger.debug("Job [{}] is being closed before its process is started", jobId); - jobTask.markAsCompleted(); - return; - } - try { + if (processContext.setDying() == false) { + logger.debug("Cannot close job [{}] as it has already been closed", jobId); + return; + } + + if (reason == null) { + logger.info("Closing job [{}]", jobId); + } else { + logger.info("Closing job [{}], because [{}]", jobId, reason); + } + + AutodetectCommunicator communicator = processContext.getAutodetectCommunicator(); + if (communicator == null) { + logger.debug("Job [{}] is being closed before its process is started", jobId); + jobTask.markAsCompleted(); + return; + } + communicator.close(restart, reason); processByAllocation.remove(allocationId); } catch (Exception e) { logger.warn("[" + jobId + "] Exception closing autodetect process", e); setJobState(jobTask, JobState.FAILED); throw ExceptionsHelper.serverError("Exception closing autodetect process", e); + } finally { + // to ensure the contract that multiple simultaneous close calls for the same job wait until + // the job is closed is honoured, hold the lock throughout the close procedure so that another + // thread that gets into this method blocks until the first thread has finished closing the job + processContext.unlock(); } } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/ProcessContext.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/ProcessContext.java index 409639705ae..2657a34a3d1 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/ProcessContext.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/ProcessContext.java @@ -72,9 +72,9 @@ final class ProcessContext { state.setRunning(this, autodetectCommunicator); } - void setDying() { + boolean setDying() { assert lock.isHeldByCurrentThread(); - state.setDying(this); + return state.setDying(this); } KillBuilder newKillBuilder() { @@ -134,21 +134,29 @@ final class ProcessContext { } private interface ProcessState { - void setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator); - void setDying(ProcessContext processContext); + /** + * @return was a state change made? + * */ + boolean setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator); + /** + * @return was a state change made? + */ + boolean setDying(ProcessContext processContext); ProcessStateName getName(); } private static class ProcessNotRunningState implements ProcessState { @Override - public void setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator) { + public boolean setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator) { processContext.setAutodetectCommunicator(autodetectCommunicator); processContext.setState(new ProcessRunningState()); + return true; } @Override - public void setDying(ProcessContext processContext) { + public boolean setDying(ProcessContext processContext) { processContext.setState(new ProcessDyingState()); + return true; } @Override @@ -160,13 +168,15 @@ final class ProcessContext { private static class ProcessRunningState implements ProcessState { @Override - public void setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator) { + public boolean setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator) { LOGGER.debug("Process set to [running] while it was already in that state"); + return false; } @Override - public void setDying(ProcessContext processContext) { + public boolean setDying(ProcessContext processContext) { processContext.setState(new ProcessDyingState()); + return true; } @Override @@ -178,13 +188,15 @@ final class ProcessContext { private static class ProcessDyingState implements ProcessState { @Override - public void setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator) { + public boolean setRunning(ProcessContext processContext, AutodetectCommunicator autodetectCommunicator) { LOGGER.debug("Process set to [running] while it was in [dying]"); + return false; } @Override - public void setDying(ProcessContext processContext) { + public boolean setDying(ProcessContext processContext) { LOGGER.debug("Process set to [dying] while it was already in that state"); + return false; } @Override diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java index 548d6937a59..73c81afd125 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java @@ -46,7 +46,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust public static final String SECURITY_INDEX_NAME = ".security"; public static final String SECURITY_TEMPLATE_NAME = "security-index-template"; - public static final String NEW_SECURITY_INDEX_NAME = SECURITY_INDEX_NAME + "-" + IndexLifecycleManager.NEW_INDEX_VERSION; + public static final String INTERNAL_SECURITY_INDEX = IndexLifecycleManager.INTERNAL_SECURITY_INDEX; private static final Version MIN_READ_VERSION = Version.V_5_0_0; @@ -188,7 +188,7 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust } public static List indexNames() { - return Collections.unmodifiableList(Arrays.asList(SECURITY_INDEX_NAME, NEW_SECURITY_INDEX_NAME)); + return Collections.unmodifiableList(Arrays.asList(SECURITY_INDEX_NAME, INTERNAL_SECURITY_INDEX)); } /** diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 5b27a14e80f..4eb4994f274 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -377,6 +377,16 @@ public class AuthorizationService extends AbstractComponent { grant(authentication, action, originalRequest, null); } + private boolean hasSecurityIndexAccess(IndicesAccessControl indicesAccessControl) { + for (String index : SecurityLifecycleService.indexNames()) { + final IndicesAccessControl.IndexAccessControl indexPermissions = indicesAccessControl.getIndexPermissions(index); + if (indexPermissions != null && indexPermissions.isGranted()) { + return true; + } + } + return false; + } + /** * Performs authorization checks on the items within a {@link BulkShardRequest}. * This inspects the {@link BulkItemRequest items} within the request, computes an implied action for each item's @@ -562,8 +572,7 @@ public class AuthorizationService extends AbstractComponent { if (!indicesAccessControl.isGranted()) { throw denial(authentication, action, request, specificIndices); } - if (indicesAccessControl.getIndexPermissions(SecurityLifecycleService.SECURITY_INDEX_NAME) != null - && indicesAccessControl.getIndexPermissions(SecurityLifecycleService.SECURITY_INDEX_NAME).isGranted() + if (hasSecurityIndexAccess(indicesAccessControl) && MONITOR_INDEX_PREDICATE.test(action) == false && isSuperuser(authentication.getUser()) == false) { // only the superusers are allowed to work with this index, but we should allow indices monitoring actions through for debugging diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/support/IndexLifecycleManager.java b/plugin/src/main/java/org/elasticsearch/xpack/security/support/IndexLifecycleManager.java index 493f449ff29..264aa7228dc 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/support/IndexLifecycleManager.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/support/IndexLifecycleManager.java @@ -37,7 +37,6 @@ import org.elasticsearch.cluster.routing.IndexRoutingTable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.xpack.security.InternalClient; import org.elasticsearch.xpack.security.InternalSecurityClient; import org.elasticsearch.xpack.template.TemplateUtils; import org.elasticsearch.xpack.upgrade.IndexUpgradeCheck; @@ -50,12 +49,11 @@ import static org.elasticsearch.xpack.security.SecurityLifecycleService.SECURITY */ public class IndexLifecycleManager extends AbstractComponent { - public static final String INTERNAL_SECURITY_INDEX = ".security-v6"; + public static final String INTERNAL_SECURITY_INDEX = ".security-" + IndexUpgradeCheck.UPRADE_VERSION; public static final int INTERNAL_INDEX_FORMAT = 6; public static final String SECURITY_VERSION_STRING = "security-version"; public static final String TEMPLATE_VERSION_PATTERN = Pattern.quote("${security.template.version}"); - public static int NEW_INDEX_VERSION = IndexUpgradeCheck.UPRADE_VERSION; private final String indexName; private final String templateName; diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/user/XPackUser.java b/plugin/src/main/java/org/elasticsearch/xpack/security/user/XPackUser.java index 56b5ecce224..67e71ddf88a 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/user/XPackUser.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/user/XPackUser.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.security.user; +import org.elasticsearch.xpack.security.audit.index.IndexAuditTrail; import org.elasticsearch.xpack.security.authz.RoleDescriptor; import org.elasticsearch.xpack.security.authz.permission.Role; import org.elasticsearch.xpack.security.support.MetadataUtils; @@ -18,7 +19,9 @@ public class XPackUser extends User { public static final String ROLE_NAME = NAME; public static final Role ROLE = Role.builder(new RoleDescriptor(ROLE_NAME, new String[] { "all" }, new RoleDescriptor.IndicesPrivileges[] { - RoleDescriptor.IndicesPrivileges.builder().indices("/@&~(\\.security*)/").privileges("all").build()}, + RoleDescriptor.IndicesPrivileges.builder().indices("/@&~(\\.security.*)/").privileges("all").build(), + RoleDescriptor.IndicesPrivileges.builder().indices(IndexAuditTrail.INDEX_NAME_PREFIX + "-*").privileges("read").build() + }, new String[] { "*" }, MetadataUtils.DEFAULT_RESERVED_METADATA), null).build(); public static final XPackUser INSTANCE = new XPackUser(); diff --git a/plugin/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java b/plugin/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java index f267f40f228..1994add267a 100644 --- a/plugin/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java +++ b/plugin/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java @@ -439,7 +439,10 @@ public abstract class SecurityIntegTestCase extends ESIntegTestCase { } protected InternalSecurityClient internalSecurityClient() { - Client client = client(); + return internalSecurityClient(client()); + } + + protected InternalSecurityClient internalSecurityClient(Client client) { return new InternalSecurityClient(client.settings(), client.threadPool(), client); } @@ -501,14 +504,15 @@ public abstract class SecurityIntegTestCase extends ESIntegTestCase { } protected void deleteSecurityIndex() { + final InternalSecurityClient securityClient = internalSecurityClient(); GetIndexRequest getIndexRequest = new GetIndexRequest(); getIndexRequest.indices(SECURITY_INDEX_NAME); getIndexRequest.indicesOptions(IndicesOptions.lenientExpandOpen()); - GetIndexResponse getIndexResponse = internalClient().admin().indices().getIndex(getIndexRequest).actionGet(); + GetIndexResponse getIndexResponse = securityClient.admin().indices().getIndex(getIndexRequest).actionGet(); if (getIndexResponse.getIndices().length > 0) { // this is a hack to clean up the .security index since only the XPack user can delete it DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(getIndexResponse.getIndices()); - internalClient().admin().indices().delete(deleteIndexRequest).actionGet(); + securityClient.admin().indices().delete(deleteIndexRequest).actionGet(); } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java index 8e281c7034c..daac4fdc703 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.ml.action.OpenJobAction.JobTask; import org.elasticsearch.xpack.ml.job.JobManager; @@ -60,6 +61,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -308,6 +310,46 @@ public class AutodetectProcessManagerTests extends ESTestCase { assertEquals(0, manager.numberOfOpenJobs()); } + // DEBUG logging makes it possible to see exactly how the two threads + // interleaved in the AutodetectProcessManager.close() call + @TestLogging("org.elasticsearch.xpack.ml.job.process.autodetect:DEBUG") + public void testCanCloseClosingJob() throws Exception { + AutodetectCommunicator communicator = mock(AutodetectCommunicator.class); + AtomicInteger numberOfCommunicatorCloses = new AtomicInteger(0); + doAnswer(invocationOnMock -> { + numberOfCommunicatorCloses.incrementAndGet(); + // This increases the chance of the two threads both getting into + // the middle of the AutodetectProcessManager.close() method + Thread.yield(); + return null; + }).when(communicator).close(anyBoolean(), anyString()); + AutodetectProcessManager manager = createManager(communicator); + assertEquals(0, manager.numberOfOpenJobs()); + + JobTask jobTask = mock(JobTask.class); + when(jobTask.getJobId()).thenReturn("foo"); + manager.openJob(jobTask, e -> {}); + manager.processData(jobTask, createInputStream(""), randomFrom(XContentType.values()), + mock(DataLoadParams.class), (dataCounts1, e) -> {}); + + assertEquals(1, manager.numberOfOpenJobs()); + + // Close the job in a separate thread + Thread closeThread = new Thread(() -> manager.closeJob(jobTask, false, "in separate thread")); + closeThread.start(); + Thread.yield(); + + // Also close the job in the current thread, so that we have two simultaneous close requests + manager.closeJob(jobTask, false, "in main test thread"); + + closeThread.join(500); + assertFalse(closeThread.isAlive()); + + // Only one of the threads should have called AutodetectCommunicator.close() + assertEquals(1, numberOfCommunicatorCloses.get()); + assertEquals(0, manager.numberOfOpenJobs()); + } + public void testCanKillClosingJob() throws Exception { CountDownLatch closeStartedLatch = new CountDownLatch(1); CountDownLatch killLatch = new CountDownLatch(1); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/notification/email/attachment/ReportingAttachmentParserTests.java b/plugin/src/test/java/org/elasticsearch/xpack/notification/email/attachment/ReportingAttachmentParserTests.java index 174be4df9d3..7c62f6e2a55 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/notification/email/attachment/ReportingAttachmentParserTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/notification/email/attachment/ReportingAttachmentParserTests.java @@ -6,7 +6,9 @@ package org.elasticsearch.xpack.notification.email.attachment; import com.fasterxml.jackson.core.io.JsonEOFException; + import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -225,7 +227,7 @@ public class ReportingAttachmentParserTests extends ESTestCase { // closing json bracket is missing .thenReturn(new HttpResponse(200, "{\"path\": { \"foo\" : \"anything\"}}")); ReportingAttachment attachment = new ReportingAttachment("foo", "http://www.example.org/", randomBoolean(), null, null, null, null); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + ParsingException e = expectThrows(ParsingException.class, () -> reportingAttachmentParser.toAttachment(createWatchExecutionContext(), Payload.EMPTY, attachment)); assertThat(e.getMessage(), containsString("[reporting_attachment_kibana_payload] path doesn't support values of type: START_OBJECT")); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/SecurityTribeIT.java b/plugin/src/test/java/org/elasticsearch/xpack/security/SecurityTribeIT.java index 9025befad16..d59678bbb9f 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/SecurityTribeIT.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/SecurityTribeIT.java @@ -48,10 +48,8 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.function.Function; @@ -142,8 +140,8 @@ public class SecurityTribeIT extends NativeRealmIntegTestCase { try { cluster2.wipe(Collections.emptySet()); try { - // this is a hack to clean up the .security index since only the XPack user or superusers can delete it - cluster2.getInstance(InternalClient.class) + // this is a hack to clean up the .security index since only the XPackSecurity user or superusers can delete it + internalSecurityClient(cluster2.client()) .admin().indices().prepareDelete(IndexLifecycleManager.INTERNAL_SECURITY_INDEX).get(); } catch (IndexNotFoundException e) { // ignore it since not all tests create this index... diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java index 8706c9dd48b..e53f3a5309e 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java @@ -196,6 +196,7 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase { final boolean done = awaitBusy(() -> tokenService.isExpirationInProgress() == false); assertTrue(done); } + super.deleteSecurityIndex(); } public void testMetadataIsNotSentToClient() { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 1e8d3e8e1e9..9ad195f012b 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.security.authz; import org.elasticsearch.ElasticsearchParseException; -import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -134,8 +133,6 @@ import org.elasticsearch.xpack.security.user.User; import org.elasticsearch.xpack.security.user.XPackUser; import org.elasticsearch.xpack.sql.plugin.sql.action.SqlAction; import org.elasticsearch.xpack.sql.plugin.sql.action.SqlRequest; -import org.joda.time.Instant; -import org.joda.time.format.DateTimeFormat; import org.junit.Before; import org.mockito.Mockito; diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java index 61d4c6f8ce3..0d09d2a71e2 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecurityScrollTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.search.SearchContextMissingException; import org.elasticsearch.test.SecurityIntegTestCase; import org.elasticsearch.test.SecuritySettingsSource; import org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken; +import org.junit.After; import java.util.Collections; @@ -92,6 +93,11 @@ public class SecurityScrollTests extends SecurityIntegTestCase { } } + @After + public void cleanupSecurityIndex() throws Exception { + super.deleteSecurityIndex(); + } + @Override public String transportClientUsername() { return this.nodeClientUsername(); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/support/IndexLifecycleManagerIntegTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/support/IndexLifecycleManagerIntegTests.java index 29ce70b77cb..643e7d20a38 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/support/IndexLifecycleManagerIntegTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/support/IndexLifecycleManagerIntegTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.test.SecurityIntegTestCase; import org.elasticsearch.xpack.security.action.user.PutUserRequest; import org.elasticsearch.xpack.security.action.user.PutUserResponse; +import org.junit.After; import java.util.ArrayList; import java.util.List; @@ -37,4 +38,9 @@ public class IndexLifecycleManagerIntegTests extends SecurityIntegTestCase { assertTrue(future.actionGet().created()); } } + + @After + public void cleanupSecurityIndex() throws Exception { + super.deleteSecurityIndex(); + } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/user/XPackUserTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/user/XPackUserTests.java new file mode 100644 index 00000000000..912d7e8e962 --- /dev/null +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/user/XPackUserTests.java @@ -0,0 +1,54 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.security.user; + +import java.util.function.Predicate; + +import org.elasticsearch.action.get.GetAction; +import org.elasticsearch.action.index.IndexAction; +import org.elasticsearch.action.search.SearchAction; +import org.elasticsearch.action.update.UpdateAction; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.security.SecurityLifecycleService; +import org.elasticsearch.xpack.security.audit.index.IndexAuditTrail; +import org.elasticsearch.xpack.security.audit.index.IndexNameResolver; +import org.hamcrest.Matchers; +import org.joda.time.DateTime; + +public class XPackUserTests extends ESTestCase { + + public void testXPackUserCanAccessNonSecurityIndices() { + final String action = randomFrom(GetAction.NAME, SearchAction.NAME, IndexAction.NAME); + final Predicate predicate = XPackUser.ROLE.indices().allowedIndicesMatcher(action); + final String index = randomBoolean() ? randomAlphaOfLengthBetween(3, 12) : "." + randomAlphaOfLength(8); + assertThat(predicate.test(index), Matchers.is(true)); + } + + public void testXPackUserCannotAccessSecurityIndex() { + final String action = randomFrom(GetAction.NAME, SearchAction.NAME, IndexAction.NAME); + final Predicate predicate = XPackUser.ROLE.indices().allowedIndicesMatcher(action); + assertThat(predicate.test(SecurityLifecycleService.SECURITY_INDEX_NAME), Matchers.is(false)); + assertThat(predicate.test(SecurityLifecycleService.INTERNAL_SECURITY_INDEX), Matchers.is(false)); + } + + public void testXPackUserCanReadAuditTrail() { + final String action = randomFrom(GetAction.NAME, SearchAction.NAME); + final Predicate predicate = XPackUser.ROLE.indices().allowedIndicesMatcher(action); + assertThat(predicate.test(getAuditLogName()), Matchers.is(true)); + } + + public void testXPackUserCannotWriteToAuditTrail() { + final String action = randomFrom(IndexAction.NAME, UpdateAction.NAME); + final Predicate predicate = XPackUser.ROLE.indices().allowedIndicesMatcher(action); + assertThat(predicate.test(getAuditLogName()), Matchers.is(false)); + } + + private String getAuditLogName() { + final DateTime date = new DateTime().plusDays(randomIntBetween(1, 360)); + final IndexNameResolver.Rollover rollover = randomFrom(IndexNameResolver.Rollover.values()); + return IndexNameResolver.resolve(IndexAuditTrail.INDEX_NAME_PREFIX, date, rollover); + } +} \ No newline at end of file diff --git a/plugin/src/test/resources/rest-api-spec/test/security/hidden-index/10_security_read.yml b/plugin/src/test/resources/rest-api-spec/test/security/hidden-index/10_security_read.yml new file mode 100644 index 00000000000..c093a9fe945 --- /dev/null +++ b/plugin/src/test/resources/rest-api-spec/test/security/hidden-index/10_security_read.yml @@ -0,0 +1,85 @@ +--- +setup: + - skip: + features: headers + + - do: + cluster.health: + wait_for_status: yellow + + - do: + xpack.security.put_role: + name: "all_access" + body: > + { + "cluster": [ "all" ], + "indices": [ + { "names": ["*"], "privileges": ["all"] } + ] + } + + - do: + xpack.security.put_user: + username: "test_user" + body: > + { + "password" : "x-pack-test-password", + "roles" : [ "all_access" ], + "full_name" : "user with all possible privileges (but not superuser)" + } + +--- +teardown: + - do: + xpack.security.delete_user: + username: "test_user" + ignore: 404 + + - do: + xpack.security.delete_role: + name: "all_access" + ignore: 404 + +--- +"Test get security index metadata": + + - do: + catch: forbidden + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + indices.get: + index: ".security" + + - do: + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + indices.get: + index: ".secu*rity" + - length: { $body: 0 } + +--- +"Test get security document": + + - do: + catch: forbidden + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + get: + index: ".security" + type: "doc" + id: "user-test_user" + +--- +"Test search security index": + + - do: + catch: forbidden + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + search: + index: ".security" + type: "doc" + + - do: + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + search: + index: ".secu*rity" + type: "doc" + - match: { hits.total: 0 } + diff --git a/plugin/src/test/resources/rest-api-spec/test/security/hidden-index/11_security-6_read.yml b/plugin/src/test/resources/rest-api-spec/test/security/hidden-index/11_security-6_read.yml new file mode 100644 index 00000000000..4569987a433 --- /dev/null +++ b/plugin/src/test/resources/rest-api-spec/test/security/hidden-index/11_security-6_read.yml @@ -0,0 +1,85 @@ +--- +setup: + - skip: + features: headers + + - do: + cluster.health: + wait_for_status: yellow + + - do: + xpack.security.put_role: + name: "all_access" + body: > + { + "cluster": [ "all" ], + "indices": [ + { "names": ["*"], "privileges": ["all"] } + ] + } + + - do: + xpack.security.put_user: + username: "test_user" + body: > + { + "password" : "x-pack-test-password", + "roles" : [ "all_access" ], + "full_name" : "user with all possible privileges (but not superuser)" + } + +--- +teardown: + - do: + xpack.security.delete_user: + username: "test_user" + ignore: 404 + + - do: + xpack.security.delete_role: + name: "all_access" + ignore: 404 + +--- +"Test get security index metadata": + + - do: + catch: forbidden + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + indices.get: + index: ".security-6" + + - do: + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + indices.get: + index: ".security*6" + - length: { $body: 0 } + +--- +"Test get security document": + + - do: + catch: forbidden + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + get: + index: ".security-6" + type: "doc" + id: "user-test_user" + +--- +"Test search security index": + + - do: + catch: forbidden + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + search: + index: ".security-6" + type: "doc" + + - do: + headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user + search: + index: ".security*6" + type: "doc" + - match: { hits.total: 0 } +