diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstraction.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstraction.java index 8429876f9f937..4ab94bb9d2071 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstraction.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstraction.java @@ -76,6 +76,10 @@ default Index getWriteIndex(IndexRequest request, Metadata metadata) { return getWriteIndex(); } + default boolean isFailureIndexOfDataStream() { + return false; + } + /** * @return the data stream to which this index belongs or null if this is not a concrete index or * if it is a concrete index that does not belong to a data stream. @@ -193,6 +197,11 @@ public boolean isSystem() { return isSystem; } + @Override + public boolean isFailureIndexOfDataStream() { + return dataStream != null && dataStream.isFailureStoreIndex(getName()); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java index 015c90ebe450e..cf37b3e5ba3a4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java @@ -21,7 +21,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.function.Predicate; +import java.util.function.BiPredicate; import java.util.function.Supplier; public class IndexAbstractionResolver { @@ -37,7 +37,7 @@ public List resolveIndexAbstractions( IndicesOptions indicesOptions, Metadata metadata, Supplier> allAuthorizedAndAvailable, - Predicate isAuthorized, + BiPredicate isAuthorized, boolean includeDataStreams ) { List finalIndices = new ArrayList<>(); @@ -103,7 +103,7 @@ && isIndexVisible( resolveSelectorsAndCombine(indexAbstraction, selectorString, indicesOptions, resolvedIndices, metadata); if (minus) { finalIndices.removeAll(resolvedIndices); - } else if (indicesOptions.ignoreUnavailable() == false || isAuthorized.test(indexAbstraction)) { + } else if (indicesOptions.ignoreUnavailable() == false || isAuthorized.test(indexAbstraction, selectorString)) { // Unauthorized names are considered unavailable, so if `ignoreUnavailable` is `true` they should be silently // discarded from the `finalIndices` list. Other "ways of unavailable" must be handled by the action // handler, see: https://github.com/elastic/elasticsearch/issues/90215 diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java index 5ab5ed1c23e4f..ba212e7a3dfae 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java @@ -236,6 +236,6 @@ private List resolveAbstractionsSelectorAllowed(List expressions } private List resolveAbstractions(List expressions, IndicesOptions indicesOptions, Supplier> mask) { - return indexAbstractionResolver.resolveIndexAbstractions(expressions, indicesOptions, metadata, mask, (idx) -> true, true); + return indexAbstractionResolver.resolveIndexAbstractions(expressions, indicesOptions, metadata, mask, (idx, sel) -> true, true); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java index 9d102e6954d04..3a78ee5182ed2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java @@ -295,7 +295,11 @@ interface AuthorizedIndices { /** * Checks if an index-like resource name is authorized, for an action by a user. The resource might or might not exist. */ - boolean check(String name); + default boolean check(String name) { + return check(name, null); + } + + boolean check(String name, String selector); } /** diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java index b91db5ca34366..cd9fb6ad97cc8 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java @@ -89,7 +89,6 @@ public Builder addGroup( public IndicesPermission build() { return new IndicesPermission(restrictedIndices, groups.toArray(Group.EMPTY_ARRAY)); } - } private IndicesPermission(RestrictedIndices restrictedIndices, Group[] groups) { @@ -143,16 +142,27 @@ public boolean hasFieldOrDocumentLevelSecurity() { private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String action) { final Set ordinaryIndices = new HashSet<>(); + final Set ordinaryIndicesWithFailureStoreAccess = new HashSet<>(); final Set restrictedIndices = new HashSet<>(); + final Set restrictedIndicesWithFailureStoreAccess = new HashSet<>(); + final Set grantMappingUpdatesOnIndices = new HashSet<>(); final Set grantMappingUpdatesOnRestrictedIndices = new HashSet<>(); final boolean isMappingUpdateAction = isMappingUpdateAction(action); for (final Group group : groups) { if (group.actionMatcher.test(action)) { if (group.allowRestrictedIndices) { - restrictedIndices.addAll(Arrays.asList(group.indices())); + if (group.allowFailureStoreAccess) { + restrictedIndicesWithFailureStoreAccess.addAll(Arrays.asList(group.indices())); + } else { + restrictedIndices.addAll(Arrays.asList(group.indices())); + } } else { - ordinaryIndices.addAll(Arrays.asList(group.indices())); + if (group.allowFailureStoreAccess) { + ordinaryIndicesWithFailureStoreAccess.addAll(Arrays.asList(group.indices())); + } else { + ordinaryIndices.addAll(Arrays.asList(group.indices())); + } } } else if (isMappingUpdateAction && containsPrivilegeThatGrantsMappingUpdatesForBwc(group)) { // special BWC case for certain privileges: allow put mapping on indices and aliases (but not on data streams), even if @@ -164,7 +174,10 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String } } } - final StringMatcher nameMatcher = indexMatcher(ordinaryIndices, restrictedIndices); + final StringMatcher nameMatcher = indexMatcher(ordinaryIndices, restrictedIndices).and( + "", + name -> name.endsWith("::failures") == false + ).or(indexMatcher(ordinaryIndicesWithFailureStoreAccess, restrictedIndicesWithFailureStoreAccess)); final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices, grantMappingUpdatesOnRestrictedIndices); return new IsResourceAuthorizedPredicate(nameMatcher, bwcSpecialCaseMatcher); } @@ -173,14 +186,19 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String * This encapsulates the authorization test for resources. * There is an additional test for resources that are missing or that are not a datastream or a backing index. */ - public static class IsResourceAuthorizedPredicate implements BiPredicate { + public static class IsResourceAuthorizedPredicate { private final BiPredicate biPredicate; // public for tests public IsResourceAuthorizedPredicate(StringMatcher resourceNameMatcher, StringMatcher additionalNonDatastreamNameMatcher) { this((String name, @Nullable IndexAbstraction indexAbstraction) -> { - assert indexAbstraction == null || name.equals(indexAbstraction.getName()); + // TODO this is hacky + assert indexAbstraction == null + || (name.equals(indexAbstraction.getName()) + || name.equals( + IndexNameExpressionResolver.combineSelector(indexAbstraction.getName(), IndexComponentSelector.FAILURES) + )); return resourceNameMatcher.test(name) || (isPartOfDatastream(indexAbstraction) == false && additionalNonDatastreamNameMatcher.test(name)); }); @@ -191,13 +209,12 @@ private IsResourceAuthorizedPredicate(BiPredicate biPr } /** - * Given another {@link IsResourceAuthorizedPredicate} instance in {@param other}, - * return a new {@link IsResourceAuthorizedPredicate} instance that is equivalent to the conjunction of - * authorization tests of that other instance and this one. - */ - @Override - public final IsResourceAuthorizedPredicate and(BiPredicate other) { - return new IsResourceAuthorizedPredicate(this.biPredicate.and(other)); + * Given another {@link IsResourceAuthorizedPredicate} instance in {@param other}, + * return a new {@link IsResourceAuthorizedPredicate} instance that is equivalent to the conjunction of + * authorization tests of that other instance and this one. + */ + public final IsResourceAuthorizedPredicate and(IsResourceAuthorizedPredicate other) { + return new IsResourceAuthorizedPredicate(this.biPredicate.and(other.biPredicate)); } /** @@ -215,11 +232,24 @@ public final boolean test(IndexAbstraction indexAbstraction) { * if it doesn't. * Returns {@code true} if access to the given resource is authorized or {@code false} otherwise. */ - @Override public boolean test(String name, @Nullable IndexAbstraction indexAbstraction) { return biPredicate.test(name, indexAbstraction); } + /** + * Similar to {@link #test(IndexAbstraction)} but for the failures component of a data stream. Adds ::failures to name of the + * index abstraction to test if ::failures are allowed + */ + public final boolean testDataStreamForFailureAccess(IndexAbstraction indexAbstraction) { + // TODO clean up + assert indexAbstraction != null && indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM; + return biPredicate.test( + IndexNameExpressionResolver.combineSelector(indexAbstraction.getName(), IndexComponentSelector.FAILURES), + // this cannot be `null`, since otherwise this is not treated as a data stream + indexAbstraction + ); + } + private static boolean isPartOfDatastream(IndexAbstraction indexAbstraction) { return indexAbstraction != null && (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM || indexAbstraction.getParentDataStream() != null); @@ -371,17 +401,20 @@ private static class IndexResource { private IndexResource(String name, @Nullable IndexAbstraction abstraction, @Nullable IndexComponentSelector selector) { assert name != null : "Resource name cannot be null"; - assert abstraction == null || abstraction.getName().equals(name) - : "Index abstraction has unexpected name [" + abstraction.getName() + "] vs [" + name + "]"; - assert abstraction == null - || selector == null - || IndexComponentSelector.FAILURES.equals(selector) == false - || abstraction.isDataStreamRelated() - : "Invalid index component selector [" - + selector.getKey() - + "] applied to abstraction of type [" - + abstraction.getType() - + "]"; + // assert abstraction == null || abstraction.getName().equals(name) + // : "Index abstraction has unexpected name [" + abstraction.getName() + "] vs [" + name + "]"; + // assert abstraction == null + // || selector == null + // || IndexComponentSelector.FAILURES.equals(selector) == false + // || abstraction.isDataStreamRelated() + // : "Invalid index component selector [" + // + selector.getKey() + // + "] applied to abstraction of type [" + // + abstraction.getType() + // + "]"; + var tuple = IndexNameExpressionResolver.splitSelectorExpression(name); + assert tuple.v2() == null || tuple.v2().equals(IndexComponentSelector.FAILURES.getKey()) + : "Unexpected selector [" + tuple.v2() + "] in index name [" + name + "]"; this.name = name; this.indexAbstraction = abstraction; this.selector = selector; @@ -411,7 +444,11 @@ public boolean isPartOfDataStream() { public boolean checkIndex(Group group) { final DataStream ds = indexAbstraction == null ? null : indexAbstraction.getParentDataStream(); if (ds != null) { - if (group.checkIndex(ds.getName())) { + boolean isFailureStoreIndex = ds.isFailureStoreIndex(indexAbstraction.getName()); + if (isFailureStoreIndex + && group.checkIndex(IndexNameExpressionResolver.combineSelector(ds.getName(), IndexComponentSelector.FAILURES))) { + return true; + } else if (isFailureStoreIndex == false && group.checkIndex(ds.getName())) { return true; } } @@ -500,13 +537,18 @@ public IndicesAccessControl authorize( int totalResourceCount = 0; Map lookup = metadata.getIndicesLookup(); for (String indexOrAlias : requestedIndicesOrAliases) { - // Remove any selectors from abstraction name. Discard them for this check as we do not have access control for them (yet) Tuple expressionAndSelector = IndexNameExpressionResolver.splitSelectorExpression(indexOrAlias); - indexOrAlias = expressionAndSelector.v1(); IndexComponentSelector selector = expressionAndSelector.v2() == null ? null : IndexComponentSelector.getByKey(expressionAndSelector.v2()); - final IndexResource resource = new IndexResource(indexOrAlias, lookup.get(indexOrAlias), selector); + // If the request has name::data, upstream code will remove ::data from the name + // If the request has name::* upstream code will convert to name::failures and name (without ::data) + assert selector != IndexComponentSelector.DATA : "Data selector is not allowed in this context"; + assert selector != IndexComponentSelector.ALL_APPLICABLE : "All selector is not allowed in this context"; + assert selector == null || selector == IndexComponentSelector.FAILURES + : "Only the failures selector " + "or none is not allowed in this context"; + // look up the IndexAbstraction by the name without the selector, but leave the (::failures) selector for authorization + final IndexResource resource = new IndexResource(indexOrAlias, lookup.get(expressionAndSelector.v1()), selector); resources.put(resource.name, resource); totalResourceCount += resource.size(lookup); } @@ -790,6 +832,9 @@ private Map indexGroupAutomatons(boolean combine) { public static class Group { public static final Group[] EMPTY_ARRAY = new Group[0]; + // TODO this is just a hack to avoid implementing a new field in this POC; this would be set via allow_failure_store_access on + // the role descriptor + public static final String FAILURE_STORE_ACCESS_MARKER = ".failure_store_access_marker"; private final IndexPrivilege privilege; private final Predicate actionMatcher; @@ -803,6 +848,7 @@ public static class Group { // users. Setting this flag true eliminates the special status for the purpose of this permission - restricted indices still have // to be covered by the "indices" private final boolean allowRestrictedIndices; + public final boolean allowFailureStoreAccess; public Group( IndexPrivilege privilege, @@ -816,13 +862,19 @@ public Group( this.privilege = privilege; this.actionMatcher = privilege.predicate(); this.indices = indices; + boolean allowFailureStoreAccess = allowFailureStoreAccess(indices); + this.allowFailureStoreAccess = allowFailureStoreAccess; this.allowRestrictedIndices = allowRestrictedIndices; ConcurrentHashMap indexNameAutomatonMemo = new ConcurrentHashMap<>(1); if (allowRestrictedIndices) { - this.indexNameMatcher = StringMatcher.of(indices); + this.indexNameMatcher = getIndexNameMatcher(allowFailureStoreAccess, indices); + // TODO handle this, too this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent(indices, k -> Automatons.patterns(indices)); } else { - this.indexNameMatcher = StringMatcher.of(indices).and(name -> restrictedIndices.isRestricted(name) == false); + this.indexNameMatcher = getIndexNameMatcher(allowFailureStoreAccess, indices).and( + name -> restrictedIndices.isRestricted(name) == false + ); + // TODO handle this, too this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent( indices, k -> Automatons.minusAndMinimize(Automatons.patterns(indices), restrictedIndices.getAutomaton()) @@ -832,6 +884,17 @@ public Group( this.query = query; } + private static StringMatcher getIndexNameMatcher(boolean allowFailureStoreAccess, String[] indices) { + if (false == allowFailureStoreAccess) { + return StringMatcher.of(indices).and(name -> name.endsWith("::failures") == false); + } + return StringMatcher.of(indices); + } + + private static boolean allowFailureStoreAccess(String... indices) { + return Arrays.stream(indices).anyMatch(index -> index.equals("*") || index.equals(FAILURE_STORE_ACCESS_MARKER)); + } + public IndexPrivilege privilege() { return privilege; } diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java new file mode 100644 index 0000000000000..2a9f8129b2e0a --- /dev/null +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java @@ -0,0 +1,265 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.security; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.core.Strings; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.SearchResponseUtils; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; + +public class FailureStoreSecurityRestIT extends SecurityOnTrialLicenseRestTestCase { + + private static final String DATA_ACCESS_USER = "data_access_user"; + private static final String FAILURE_STORE_ACCESS_USER = "failure_store_access_user"; + private static final SecureString PASSWORD = new SecureString("elastic-password"); + + @SuppressWarnings("unchecked") + public void testFailureStoreAccess() throws IOException { + // TODO test API keys + // TODO test role with access to concrete failure index + String dataAccessRole = "data_access"; + String failureStoreAccessRole = "failure_store_access"; + + createUser(DATA_ACCESS_USER, PASSWORD, List.of(dataAccessRole)); + createUser(FAILURE_STORE_ACCESS_USER, PASSWORD, List.of(failureStoreAccessRole)); + + upsertRole(Strings.format(""" + { + "description": "Role with data access", + "cluster": ["all"], + "indices": [{"names": ["test*"], "privileges": ["read"]}] + }"""), dataAccessRole); + upsertRole(Strings.format(""" + { + "description": "Role with failure store access", + "cluster": ["all"], + "indices": [{"names": ["test*::failures", ".failure_store_access_marker"], "privileges": ["read"]}] + }"""), failureStoreAccessRole); + + createTemplates(); + List docIds = populateDataStreamWithBulkRequest(); + assertThat(docIds.size(), equalTo(2)); + assertThat(docIds, hasItem("1")); + String successDocId = "1"; + String failedDocId = docIds.stream().filter(id -> false == id.equals(successDocId)).findFirst().get(); + + Request dataStream = new Request("GET", "/_data_stream/test1"); + Response response = adminClient().performRequest(dataStream); + Map dataStreams = entityAsMap(response); + assertEquals(Collections.singletonList("test1"), XContentMapValues.extractValue("data_streams.name", dataStreams)); + List dataIndexNames = (List) XContentMapValues.extractValue("data_streams.indices.index_name", dataStreams); + assertThat(dataIndexNames.size(), equalTo(1)); + List failureIndexNames = (List) XContentMapValues.extractValue( + "data_streams.failure_store.indices.index_name", + dataStreams + ); + assertThat(failureIndexNames.size(), equalTo(1)); + + String dataIndexName = dataIndexNames.get(0); + String failureIndexName = failureIndexNames.get(0); + + // user with access to failures index + assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::failures/_search")), failedDocId); + assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test*::failures/_search")), failedDocId); + assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1::failures/_search")), failedDocId); + assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*::failures/_search")), failedDocId); + assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/.fs*/_search")), failedDocId); + assertContainsDocIds( + performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search")), + failedDocId + ); + assertContainsDocIds( + performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::failures/_search?ignore_unavailable=true")), + failedDocId + ); + assertContainsDocIds( + performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search?ignore_unavailable=true")), + failedDocId + ); + + expectThrows404(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test12::failures/_search"))); + expectThrows404(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::failures/_search"))); + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test12::*/_search"))); + + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::data/_search"))); + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1/_search"))); + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::data/_search"))); + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2/_search"))); + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search"))); + + assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::data/_search?ignore_unavailable=true"))); + assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1/_search?ignore_unavailable=true"))); + assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::data/_search?ignore_unavailable=true"))); + assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2/_search?ignore_unavailable=true"))); + assertEmpty( + performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search?ignore_unavailable=true")) + ); + + assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/.ds*/_search"))); + // TODO is this correct? + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1::data/_search"))); + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1/_search"))); + + // user with access to data index + assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/test1/_search")), successDocId); + assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/test*/_search")), successDocId); + assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/*1/_search")), successDocId); + assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/*/_search")), successDocId); + assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/.ds*/_search")), successDocId); + assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search")), successDocId); + assertContainsDocIds( + performRequest(DATA_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search?ignore_unavailable=true")), + successDocId + ); + + expectThrows404(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test12/_search"))); + expectThrows404(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test2/_search"))); + expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test12::*/_search"))); + + expectThrows403(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test1::failures/_search"))); + expectThrows403(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test2::failures/_search"))); + expectThrows403(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search"))); + assertEmpty(performRequest(DATA_ACCESS_USER, new Request("GET", "/.fs*/_search"))); + // TODO is this correct? + expectThrows403(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/*1::failures/_search"))); + + // user with access to everything + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/test1::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/test*::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/*1::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/*::failures/_search")), failedDocId); + assertContainsDocIds(adminClient().performRequest(new Request("GET", "/.fs*/_search")), failedDocId); + + expectThrows404(() -> adminClient().performRequest(new Request("GET", "/test12::failures/_search"))); + expectThrows404(() -> adminClient().performRequest(new Request("GET", "/test2::failures/_search"))); + } + + private static void expectThrows404(ThrowingRunnable get) { + var ex = expectThrows(ResponseException.class, get); + assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(404)); + } + + private static void expectThrows403(ThrowingRunnable get) { + var ex = expectThrows(ResponseException.class, get); + assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403)); + } + + @SuppressWarnings("unchecked") + private static void assertContainsDocIds(Response response, String... docIds) throws IOException { + assertOK(response); + final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response)); + try { + SearchHit[] hits = searchResponse.getHits().getHits(); + assertThat(hits.length, equalTo(docIds.length)); + List actualDocIds = Arrays.stream(hits).map(SearchHit::getId).toList(); + assertThat(actualDocIds, containsInAnyOrder(docIds)); + } finally { + searchResponse.decRef(); + } + } + + private static void assertEmpty(Response response) throws IOException { + assertOK(response); + final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response)); + try { + SearchHit[] hits = searchResponse.getHits().getHits(); + assertThat(hits.length, equalTo(0)); + } finally { + searchResponse.decRef(); + } + } + + private void createTemplates() throws IOException { + var componentTemplateRequest = new Request("PUT", "/_component_template/component1"); + componentTemplateRequest.setJsonEntity(""" + { + "template": { + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "age": { + "type": "integer" + }, + "email": { + "type": "keyword" + }, + "name": { + "type": "text" + } + } + }, + "data_stream_options": { + "failure_store": { + "enabled": true + } + } + } + } + """); + assertOK(adminClient().performRequest(componentTemplateRequest)); + + var indexTemplateRequest = new Request("PUT", "/_index_template/template1"); + indexTemplateRequest.setJsonEntity(""" + { + "index_patterns": ["test*"], + "data_stream": {}, + "priority": 500, + "composed_of": ["component1"] + } + """); + assertOK(adminClient().performRequest(indexTemplateRequest)); + } + + @SuppressWarnings("unchecked") + private List populateDataStreamWithBulkRequest() throws IOException { + var bulkRequest = new Request("POST", "/_bulk?refresh=true"); + bulkRequest.setJsonEntity(""" + { "create" : { "_index" : "test1", "_id" : "1" } } + { "@timestamp": 1, "age" : 1, "name" : "jack", "email" : "jack@example.com" } + { "create" : { "_index" : "test1", "_id" : "2" } } + { "@timestamp": 2, "age" : "this should be an int", "name" : "jack", "email" : "jack@example.com" } + """); + Response response = adminClient().performRequest(bulkRequest); + assertOK(response); + // we need this dance because the ID for the failed document is random, **not** 2 + Map stringObjectMap = responseAsMap(response); + List items = (List) stringObjectMap.get("items"); + List ids = new ArrayList<>(); + for (Object item : items) { + Map itemMap = (Map) item; + Map create = (Map) itemMap.get("create"); + assertThat(create.get("status"), equalTo(201)); + ids.add((String) create.get("_id")); + } + return ids; + } + + private Response performRequest(String user, Request request) throws IOException { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", basicAuthHeaderValue(user, PASSWORD)).build()); + return client().performRequest(request); + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index b9270b6035680..15c7822c2ed80 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -31,6 +31,7 @@ import org.elasticsearch.action.search.TransportClosePointInTimeAction; import org.elasticsearch.action.search.TransportMultiSearchAction; import org.elasticsearch.action.search.TransportSearchScrollAction; +import org.elasticsearch.action.support.IndexComponentSelector; import org.elasticsearch.action.termvectors.MultiTermVectorsAction; import org.elasticsearch.cluster.metadata.DataStream; import org.elasticsearch.cluster.metadata.IndexAbstraction; @@ -106,6 +107,7 @@ import java.util.Objects; import java.util.Set; import java.util.TreeSet; +import java.util.function.BiPredicate; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.function.Supplier; @@ -875,16 +877,25 @@ static AuthorizedIndices resolveAuthorizedIndicesFromRole( // TODO: can this be done smarter? I think there are usually more indices/aliases in the cluster then indices defined a roles? if (includeDataStreams) { for (IndexAbstraction indexAbstraction : lookup.values()) { - if (predicate.test(indexAbstraction)) { + // the index abstraction here is from cluster state which will never have the ::failure data selector + // the first check is to see if the index/alias/data stream with no selector is authorized + // the second check is to see if the data stream with the ::failures appended to the name is authorized + boolean authorizedForDataAccess = predicate.test(indexAbstraction); + boolean authorizedForFailureAccess = indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM + && predicate.testDataStreamForFailureAccess(indexAbstraction); + if (authorizedForDataAccess || authorizedForFailureAccess) { indicesAndAliases.add(indexAbstraction.getName()); + // add data stream and its backing indices for any authorized data streams if (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) { - // add data stream and its backing indices for any authorized data streams - for (Index index : indexAbstraction.getIndices()) { - indicesAndAliases.add(index.getName()); + if (authorizedForDataAccess) { + for (Index index : indexAbstraction.getIndices()) { + indicesAndAliases.add(index.getName()); + } } - // TODO: We need to limit if a data stream's failure indices should return here. - for (Index index : ((DataStream) indexAbstraction).getFailureIndices()) { - indicesAndAliases.add(index.getName()); + if (authorizedForFailureAccess) { + for (Index index : ((DataStream) indexAbstraction).getFailureIndices()) { + indicesAndAliases.add(index.getName()); + } } } } @@ -898,13 +909,20 @@ static AuthorizedIndices resolveAuthorizedIndicesFromRole( } timeChecker.accept(indicesAndAliases); return indicesAndAliases; - }, name -> { + }, (name, selector) -> { final IndexAbstraction indexAbstraction = lookup.get(name); if (indexAbstraction == null) { // test access (by name) to a resource that does not currently exist // the action handler must handle the case of accessing resources that do not exist return predicate.test(name, null); } else { + if (indexAbstraction.isFailureIndexOfDataStream() + && predicate.testDataStreamForFailureAccess(indexAbstraction.getParentDataStream())) { + return true; + } + if (IndexComponentSelector.FAILURES.getKey().equals(selector)) { + return predicate.testDataStreamForFailureAccess(indexAbstraction); + } // We check the parent data stream first if there is one. For testing requested indices, this is most likely // more efficient than checking the index name first because we recommend grant privileges over data stream // instead of backing indices. @@ -1037,9 +1055,9 @@ private static boolean isAsyncRelatedAction(String action) { static final class AuthorizedIndices implements AuthorizationEngine.AuthorizedIndices { private final CachedSupplier> allAuthorizedAndAvailableSupplier; - private final Predicate isAuthorizedPredicate; + private final BiPredicate isAuthorizedPredicate; - AuthorizedIndices(Supplier> allAuthorizedAndAvailableSupplier, Predicate isAuthorizedPredicate) { + AuthorizedIndices(Supplier> allAuthorizedAndAvailableSupplier, BiPredicate isAuthorizedPredicate) { this.allAuthorizedAndAvailableSupplier = CachedSupplier.wrap(allAuthorizedAndAvailableSupplier); this.isAuthorizedPredicate = Objects.requireNonNull(isAuthorizedPredicate); } @@ -1050,8 +1068,8 @@ public Supplier> all() { } @Override - public boolean check(String name) { - return this.isAuthorizedPredicate.test(name); + public boolean check(String name, String selector) { + return isAuthorizedPredicate.test(name, selector); } } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java index 2e1a643bf4f4f..5a73c1d4399be 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java @@ -504,6 +504,7 @@ public static void buildRoleFromDescriptors( runAs.addAll(Arrays.asList(descriptor.getRunAs())); } + // TODO need to handle collation to not collate groups that don't match failure store access MergeableIndicesPrivilege.collatePrivilegesByIndices(descriptor.getIndicesPrivileges(), true, restrictedIndicesPrivilegesMap); MergeableIndicesPrivilege.collatePrivilegesByIndices(descriptor.getIndicesPrivileges(), false, indicesPrivilegesMap); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java index f7dc725c3f07d..d4ccb3e33f94a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java @@ -104,6 +104,7 @@ import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.newInstance; import static org.elasticsearch.test.ActionListenerUtils.anyActionListener; import static org.elasticsearch.test.TestMatchers.throwableWithMessage; +import static org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.Group.FAILURE_STORE_ACCESS_MARKER; import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.RESTRICTED_INDICES; import static org.elasticsearch.xpack.security.authz.AuthorizedIndicesTests.getRequestInfo; import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS; @@ -333,7 +334,8 @@ public void setup() { new RoleDescriptor( "data_stream_test2", null, - new IndicesPrivileges[] { IndicesPrivileges.builder().indices(otherDataStreamName + "*").privileges("all").build() }, + new IndicesPrivileges[] { + IndicesPrivileges.builder().indices(otherDataStreamName + "*", FAILURE_STORE_ACCESS_MARKER).privileges("all").build() }, null ) ); @@ -342,7 +344,8 @@ public void setup() { new RoleDescriptor( "data_stream_test3", null, - new IndicesPrivileges[] { IndicesPrivileges.builder().indices("logs*").privileges("all").build() }, + new IndicesPrivileges[] { + IndicesPrivileges.builder().indices("logs*", FAILURE_STORE_ACCESS_MARKER).privileges("all").build() }, null ) ); @@ -2413,7 +2416,7 @@ public void testBackingIndicesAreVisibleWhenIncludedByRequestWithWildcard() { public void testBackingIndicesAreNotVisibleWhenNotIncludedByRequestWithoutWildcard() { final User user = new User("data-stream-tester2", "data_stream_test2"); String dataStreamName = "logs-foobar"; - GetAliasesRequest request = new GetAliasesRequest(TEST_REQUEST_TIMEOUT, dataStreamName); + GetAliasesRequest request = new GetAliasesRequest(TEST_REQUEST_TIMEOUT, dataStreamName, dataStreamName + "::failures"); assertThat(request, instanceOf(IndicesRequest.Replaceable.class)); assertThat(request.includeDataStreams(), is(true)); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index 482715bb74c83..b3f6f0103ae0e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -1455,7 +1455,6 @@ public void testBackingIndicesAreIncludedForAuthorizedDataStreams() { public void testExplicitMappingUpdatesAreNotGrantedWithIngestPrivileges() { final String dataStreamName = "my_data_stream"; - User user = new User(randomAlphaOfLengthBetween(4, 12)); Role role = Role.builder(RESTRICTED_INDICES, "test1") .cluster(Collections.emptySet(), Collections.emptyList()) .add(IndexPrivilege.CREATE, "my_*")