diff --git a/build-tools/src/main/resources/roles.yml b/build-tools/src/main/resources/roles.yml index 433dfdf79719f..9fadedf23214e 100644 --- a/build-tools/src/main/resources/roles.yml +++ b/build-tools/src/main/resources/roles.yml @@ -5,7 +5,7 @@ _es_test_root: indices: - names: [ "*" ] allow_restricted_indices: true - privileges: [ "ALL" ] + privileges: [ "ALL", "read_failures" ] run_as: [ "*" ] applications: - application: "*" diff --git a/docs/changelog/119915.yaml b/docs/changelog/119915.yaml new file mode 100644 index 0000000000000..44821f0d92bb0 --- /dev/null +++ b/docs/changelog/119915.yaml @@ -0,0 +1,5 @@ +pr: 119915 +summary: Add `read_failures` privilege for authorizing failure store access +area: Authorization +type: enhancement +issues: [] diff --git a/docs/reference/rest-api/security/get-builtin-privileges.asciidoc b/docs/reference/rest-api/security/get-builtin-privileges.asciidoc index 08a03a5b1e830..ad1ca80843649 100644 --- a/docs/reference/rest-api/security/get-builtin-privileges.asciidoc +++ b/docs/reference/rest-api/security/get-builtin-privileges.asciidoc @@ -155,6 +155,7 @@ A successful call returns an object with "cluster", "index", and "remote_cluster "none", "read", "read_cross_cluster", + "read_failures", "view_index_metadata", "write" ], diff --git a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java index 7c04c38eff63c..e38aae6641adc 100644 --- a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java +++ b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java @@ -1953,10 +1953,6 @@ public void onFailure(Exception e) { latch.await(); var ghostReference = brokenDataStreamHolder.get().getIndices().get(0); - // Many APIs fail with NPE, because of broken data stream: - expectThrows(NullPointerException.class, indicesAdmin().stats(new IndicesStatsRequest())); - expectThrows(NullPointerException.class, client().search(new SearchRequest())); - assertAcked( client().execute( ModifyDataStreamsAction.INSTANCE, diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 9ad00b517d51c..e5c45a0871ee6 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -1698,7 +1698,7 @@ private static Set expandToOpenClosed( for (int i = 0, n = indexAbstraction.getIndices().size(); i < n; i++) { Index index = indexAbstraction.getIndices().get(i); IndexMetadata indexMetadata = context.state.metadata().index(index); - if (indexMetadata.getState() != excludeState) { + if (indexMetadata != null && indexMetadata.getState() != excludeState) { resources.add( new ResolvedExpression( index.getName(), diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Group.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Group.java new file mode 100644 index 0000000000000..357947c84ece7 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Group.java @@ -0,0 +1,281 @@ +/* + * 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.core.security.authz.permission; + +import org.apache.lucene.util.automaton.Automaton; +import org.elasticsearch.cluster.metadata.DataStream; +import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.logging.DeprecationCategory; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.core.Nullable; +import org.elasticsearch.index.Index; +import org.elasticsearch.xpack.core.security.authz.RestrictedIndices; +import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; +import org.elasticsearch.xpack.core.security.support.Automatons; +import org.elasticsearch.xpack.core.security.support.StringMatcher; + +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Predicate; +import java.util.function.Supplier; + +import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.READ; +import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.READ_FAILURES_PRIVILEGE_NAME; + +/** + * Represents an {@link IndicesPermission} group, as in one set of index name patterns and the privileges granted for the indices + * covered by those patterns. + */ +public class Group { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(Group.class); + + public static final Group[] EMPTY_ARRAY = new Group[0]; + + final IndexPrivilege privilege; // ATHE make this private again + private final Predicate actionMatcher; + private final String[] indices; + final StringMatcher indexNameMatcher; // ATHE make this private again + private final Supplier indexNameAutomaton; + // TODO: Use FieldPermissionsDefinition instead of FieldPermissions. The former is a better counterpart to query + private final FieldPermissions fieldPermissions; + private final Set query; + // by default certain restricted indices are exempted when granting privileges, as they should generally be hidden for ordinary + // 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; + // These two flags are just a cache so we don't have to constantly re-check strings + private final boolean hasMappingUpdateBwcPermissions; + private final boolean hasReadFailuresPrivilege; + + public Group( + IndexPrivilege privilege, + FieldPermissions fieldPermissions, + @Nullable Set query, + boolean allowRestrictedIndices, + RestrictedIndices restrictedIndices, + String... indices + ) { + assert indices.length != 0; + this.privilege = privilege; + this.actionMatcher = privilege.predicate(); + this.indices = indices; + this.allowRestrictedIndices = allowRestrictedIndices; + ConcurrentHashMap indexNameAutomatonMemo = new ConcurrentHashMap<>(1); + if (allowRestrictedIndices) { + this.indexNameMatcher = StringMatcher.of(indices); + this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent(indices, k -> Automatons.patterns(indices)); + } else { + this.indexNameMatcher = StringMatcher.of(indices).and(name -> restrictedIndices.isRestricted(name) == false); + this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent( + indices, + k -> Automatons.minusAndMinimize(Automatons.patterns(indices), restrictedIndices.getAutomaton()) + ); + } + this.fieldPermissions = Objects.requireNonNull(fieldPermissions); + this.query = query; + this.hasMappingUpdateBwcPermissions = IndicesPermission.containsPrivilegeThatGrantsMappingUpdatesForBwc(privilege.name()); + this.hasReadFailuresPrivilege = this.privilege.name().stream().anyMatch(READ_FAILURES_PRIVILEGE_NAME::equals); + } + + public IndexPrivilege privilege() { + return privilege; + } + + public String[] indices() { + return indices; + } + + @Nullable + public Set getQuery() { + return query; + } + + public FieldPermissions getFieldPermissions() { + return fieldPermissions; + } + + IsResourceAuthorizedPredicate allowedIndicesPredicate(String action) { + return new GroupChecker(action, this); + } + + static class GroupChecker extends IsResourceAuthorizedPredicate { + private final String action; + private final Group group; + private final boolean isReadAction; + private final boolean isMappingUpdateAction; + private final boolean actionMatches; + private final boolean actionAuthorized; + private final AtomicBoolean deprecationLogEmitted = new AtomicBoolean(false); + + GroupChecker(String action, Group group) { + this.action = action; + this.group = group; + isReadAction = READ.predicate().test(action); + isMappingUpdateAction = IndicesPermission.isMappingUpdateAction(action); + actionMatches = group.actionMatcher.test(action); + actionAuthorized = actionMatches || (isReadAction && group.hasReadFailuresPrivilege); + } + + @Override + public IndicesPermission.AuthorizedComponents check(String name, IndexAbstraction resource, boolean authByDataStream) { + if (actionAuthorized == false) { + if (isMappingUpdateAction + && group.hasMappingUpdateBwcPermissions + && resource != null + && resource.getParentDataStream() == null + && resource.getType() != IndexAbstraction.Type.DATA_STREAM + && group.indexNameMatcher.test(name)) { + boolean alreadyLogged = deprecationLogEmitted.getAndSet(true); + if (alreadyLogged == false) { + for (String privilegeName : group.privilege.name()) { + if (IndicesPermission.PRIVILEGE_NAME_SET_BWC_ALLOW_MAPPING_UPDATE.contains(privilegeName)) { + deprecationLogger.warn( + DeprecationCategory.SECURITY, + "[" + resource.getName() + "] mapping update for ingest privilege [" + privilegeName + "]", + "the index privilege [" + + privilegeName + + "] allowed the update " + + "mapping action [" + + action + + "] on index [" + + resource.getName() + + "], this privilege " + + "will not permit mapping updates in the next major release - users who require access " + + "to update mappings must be granted explicit privileges" + ); + } + } + } + return IndicesPermission.AuthorizedComponents.ALL; + } + return IndicesPermission.AuthorizedComponents.NONE; + } else { + if (resource == null) { + if (group.indexNameMatcher.test(name)) { + if (isReadAction) { + if (actionMatches && group.hasReadFailuresPrivilege) { + return IndicesPermission.AuthorizedComponents.ALL; + } else if (actionMatches) { + return IndicesPermission.AuthorizedComponents.DATA; + } else if (group.hasReadFailuresPrivilege) { + return IndicesPermission.AuthorizedComponents.FAILURES; + } + } else { // not a read action + return IndicesPermission.AuthorizedComponents.ALL; + } + } else { + return IndicesPermission.AuthorizedComponents.NONE; + } + } + assert name.equals(resource.getName()); + return switch (resource.getType()) { + case ALIAS -> group.checkMultiIndexAbstraction(isReadAction, actionMatches, resource); + case DATA_STREAM -> group.checkMultiIndexAbstraction(isReadAction, actionMatches, resource); + case CONCRETE_INDEX -> { + final DataStream ds = resource.getParentDataStream(); + + if (ds != null) { // This index is owned by a data stream + // Since this is a concrete index, the write index is the index itself + Index indexObj = resource.getWriteIndex(); + boolean isFailureStoreIndex = indexObj != null && ds.getFailureIndices().contains(indexObj); + + if (isReadAction) { + // If we're trying to read a failure store index, we need to have read_failures for the data stream + if (isFailureStoreIndex) { + if ((group.hasReadFailuresPrivilege && group.indexNameMatcher.test(ds.getName()))) { + // And authorize it as a failure store index (i.e. no DLS/FLS) + yield IndicesPermission.AuthorizedComponents.FAILURES; + } + } else if (actionMatches) { // not a failure store index + if ((authByDataStream && group.indexNameMatcher.test(ds.getName())) + || group.indexNameMatcher.test(resource.getName())) { + yield IndicesPermission.AuthorizedComponents.DATA; + } + } + } else { // Not a read action, authenticate as normal + String indexName = resource.getName(); + if ((authByDataStream && group.indexNameMatcher.test(ds.getName())) + || group.indexNameMatcher.test(indexName)) { + yield IndicesPermission.AuthorizedComponents.DATA; + } + } + } else if (group.indexNameMatcher.test(resource.getName())) { + yield IndicesPermission.AuthorizedComponents.DATA; + } + yield IndicesPermission.AuthorizedComponents.NONE; + } + case null -> group.indexNameMatcher.test(name) + ? IndicesPermission.AuthorizedComponents.DATA + : IndicesPermission.AuthorizedComponents.NONE; + }; + } + } + } + + private IndicesPermission.AuthorizedComponents checkMultiIndexAbstraction( + boolean isReadAction, + boolean actionMatches, + IndexAbstraction resource + ) { + if (indexNameMatcher.test(resource.getName())) { + if (actionMatches && (isReadAction == false || hasReadFailuresPrivilege)) { + // User has both normal read privileges and read_failures OR normal privileges and action is not read + return IndicesPermission.AuthorizedComponents.ALL; + } else if (actionMatches && hasReadFailuresPrivilege == false) { + return IndicesPermission.AuthorizedComponents.DATA; + } else if (hasReadFailuresPrivilege) { // action not authorized by typical match + return IndicesPermission.AuthorizedComponents.FAILURES; + } + } + return IndicesPermission.AuthorizedComponents.NONE; + } + + boolean checkAction(String action) { + return actionMatcher.test(action) || (hasReadFailuresPrivilege && READ.predicate().test(action)); + } + + boolean hasQuery() { + return query != null; + } + + public boolean allowRestrictedIndices() { + return allowRestrictedIndices; + } + + Automaton getIndexMatcherAutomaton() { + return indexNameAutomaton.get(); + } + + boolean allowsTotalDataIndexAccess() { + return allowRestrictedIndices + && indexNameMatcher.isTotal() + && privilege.name().contains("all") // ATHE: probably make this into a constant + && hasReadFailuresPrivilege + && query == null; + } + + @Override + public String toString() { + return "Group{" + + "privilege=" + + privilege + + ", indices=" + + Strings.arrayToCommaDelimitedString(indices) + + ", fieldPermissions=" + + fieldPermissions + + ", query=" + + query + + ", allowRestrictedIndices=" + + allowRestrictedIndices + + '}'; + } +} 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..718fd82b19b68 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 @@ -15,10 +15,7 @@ import org.elasticsearch.cluster.metadata.IndexAbstraction; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.Metadata; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.logging.DeprecationCategory; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.set.Sets; @@ -29,7 +26,6 @@ import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; import org.elasticsearch.xpack.core.security.support.Automatons; -import org.elasticsearch.xpack.core.security.support.StringMatcher; import java.util.ArrayList; import java.util.Arrays; @@ -39,26 +35,22 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.BiPredicate; -import java.util.function.Predicate; import java.util.function.Supplier; import static java.util.Collections.unmodifiableMap; +import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.READ_FAILURES; +import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.READ_FAILURES_PRIVILEGE_NAME; /** * A permission that is based on privileges for index related actions executed * on specific indices */ public final class IndicesPermission { - - private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(IndicesPermission.class); - public static final IndicesPermission NONE = new IndicesPermission(new RestrictedIndices(Automatons.EMPTY), Group.EMPTY_ARRAY); - private static final Set PRIVILEGE_NAME_SET_BWC_ALLOW_MAPPING_UPDATE = Set.of("create", "create_doc", "index", "write"); + static final Set PRIVILEGE_NAME_SET_BWC_ALLOW_MAPPING_UPDATE = Set.of("create", "create_doc", "index", "write"); private final Map allowedIndicesMatchersForAction = new ConcurrentHashMap<>(); @@ -66,6 +58,67 @@ public final class IndicesPermission { private final Group[] groups; private final boolean hasFieldOrDocumentLevelSecurity; + public enum AuthorizedComponents { + NONE, + DATA, + FAILURES, + ALL; + + public AuthorizedComponents union(AuthorizedComponents other) { + if (other == null) { + return this; + } + return switch (this) { + case ALL -> ALL; + case NONE -> other; + case DATA -> other.isFailuresAuthorized() ? ALL : DATA; + case FAILURES -> other.isDataAuthorized() ? ALL : FAILURES; + }; + } + + public AuthorizedComponents intersection(AuthorizedComponents other) { + if (other == null) { + return NONE; + } + return switch (this) { + case ALL -> other; + case NONE -> NONE; + case DATA -> other.isDataAuthorized() ? DATA : NONE; + case FAILURES -> other.isFailuresAuthorized() ? FAILURES : NONE; + }; + } + + /** + * @return True if data components are authorized for the resource in question + */ + public boolean isDataAuthorized() { + return switch (this) { + case ALL, DATA -> true; + case NONE, FAILURES -> false; + }; + } + + /** + * @return True if failure components are authorized for the resource in question + */ + public boolean isFailuresAuthorized() { + return switch (this) { + case ALL, FAILURES -> true; + case NONE, DATA -> false; + }; + } + + /** + * @return True if any components, data or failure, are authorized for the resource in question. Mostly for visibility checks. + */ + public boolean isAnyAuthorized() { + return switch (this) { + case ALL, DATA, FAILURES -> true; + case NONE -> false; + }; + } + } + public static class Builder { RestrictedIndices restrictedIndices; @@ -95,34 +148,8 @@ public IndicesPermission build() { private IndicesPermission(RestrictedIndices restrictedIndices, Group[] groups) { this.restrictedIndices = restrictedIndices; this.groups = groups; - this.hasFieldOrDocumentLevelSecurity = Arrays.stream(groups).noneMatch(Group::isTotal) - && Arrays.stream(groups).anyMatch(g -> g.hasQuery() || g.fieldPermissions.hasFieldLevelSecurity()); - } - - /** - * This function constructs an index matcher that can be used to find indices allowed by - * permissions groups. - * - * @param ordinaryIndices A list of ordinary indices. If this collection contains restricted indices, - * according to the restrictedNamesAutomaton, they will not be matched. - * @param restrictedIndices A list of restricted index names. All of these will be matched. - * @return A matcher that will match all non-restricted index names in the ordinaryIndices - * collection and all index names in the restrictedIndices collection. - */ - private StringMatcher indexMatcher(Collection ordinaryIndices, Collection restrictedIndices) { - StringMatcher matcher; - if (ordinaryIndices.isEmpty()) { - matcher = StringMatcher.of(restrictedIndices); - } else { - matcher = StringMatcher.of(ordinaryIndices); - if (this.restrictedIndices != null) { - matcher = matcher.and("", name -> this.restrictedIndices.isRestricted(name) == false); - } - if (restrictedIndices.isEmpty() == false) { - matcher = StringMatcher.of(restrictedIndices).or(matcher); - } - } - return matcher; + this.hasFieldOrDocumentLevelSecurity = Arrays.stream(groups).noneMatch(Group::allowsTotalDataIndexAccess) + && Arrays.stream(groups).anyMatch(g -> g.hasQuery() || g.getFieldPermissions().hasFieldLevelSecurity()); } public Group[] groups() { @@ -142,88 +169,16 @@ public boolean hasFieldOrDocumentLevelSecurity() { } private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String action) { - final Set ordinaryIndices = new HashSet<>(); - final Set restrictedIndices = 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())); - } 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 - // the privilege definition does not currently allow it - if (group.allowRestrictedIndices) { - grantMappingUpdatesOnRestrictedIndices.addAll(Arrays.asList(group.indices())); - } else { - grantMappingUpdatesOnIndices.addAll(Arrays.asList(group.indices())); - } - } - } - final StringMatcher nameMatcher = indexMatcher(ordinaryIndices, restrictedIndices); - final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices, grantMappingUpdatesOnRestrictedIndices); - return new IsResourceAuthorizedPredicate(nameMatcher, bwcSpecialCaseMatcher); - } - - /** - * 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 { - - 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()); - return resourceNameMatcher.test(name) - || (isPartOfDatastream(indexAbstraction) == false && additionalNonDatastreamNameMatcher.test(name)); - }); + if (groups.length == 0) { + return new IsResourceAuthorizedPredicate.NoResourcesAuthorizedChecker(); } - private IsResourceAuthorizedPredicate(BiPredicate biPredicate) { - this.biPredicate = biPredicate; + IsResourceAuthorizedPredicate predicate = groups[0].allowedIndicesPredicate(action); + for (int i = 1; i < groups.length; i++) { + predicate = predicate.orAllowIf(groups[i].allowedIndicesPredicate(action)); } - /** - * 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)); - } - - /** - * Verifies if access is authorized to the given {@param indexAbstraction} resource. - * The resource must exist. Otherwise, use the {@link #test(String, IndexAbstraction)} method. - * Returns {@code true} if access to the given resource is authorized or {@code false} otherwise. - */ - public final boolean test(IndexAbstraction indexAbstraction) { - return test(indexAbstraction.getName(), indexAbstraction); - } - - /** - * Verifies if access is authorized to the resource with the given {@param name}. - * The {@param indexAbstraction}, which is the resource to be accessed, must be supplied if the resource exists or be {@code null} - * 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); - } - - private static boolean isPartOfDatastream(IndexAbstraction indexAbstraction) { - return indexAbstraction != null - && (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM || indexAbstraction.getParentDataStream() != null); - } + return predicate; } /** @@ -235,7 +190,8 @@ private static boolean isPartOfDatastream(IndexAbstraction indexAbstraction) { public boolean check(String action) { final boolean isMappingUpdateAction = isMappingUpdateAction(action); for (Group group : groups) { - if (group.checkAction(action) || (isMappingUpdateAction && containsPrivilegeThatGrantsMappingUpdatesForBwc(group))) { + if (group.checkAction(action) + || (isMappingUpdateAction && containsPrivilegeThatGrantsMappingUpdatesForBwc(group.privilege().name()))) { return true; } } @@ -387,37 +343,6 @@ private IndexResource(String name, @Nullable IndexAbstraction abstraction, @Null this.selector = selector; } - /** - * @return {@code true} if-and-only-if this object is related to a data-stream, either by having a - * {@link IndexAbstraction#getType()} of {@link IndexAbstraction.Type#DATA_STREAM} or by being the backing index for a - * {@link IndexAbstraction#getParentDataStream()} data-stream}. - */ - public boolean isPartOfDataStream() { - if (indexAbstraction == null) { - return false; - } - return switch (indexAbstraction.getType()) { - case DATA_STREAM -> true; - case CONCRETE_INDEX -> indexAbstraction.getParentDataStream() != null; - default -> false; - }; - } - - /** - * Check whether this object is covered by the provided permission {@link Group}. - * For indices that are part of a data-stream, this checks both the index name and the parent data-stream name. - * In all other cases, it checks the name of this object only. - */ - public boolean checkIndex(Group group) { - final DataStream ds = indexAbstraction == null ? null : indexAbstraction.getParentDataStream(); - if (ds != null) { - if (group.checkIndex(ds.getName())) { - return true; - } - } - return group.checkIndex(name); - } - /** * @return the number of distinct objects to which this expansion refers. */ @@ -489,13 +414,6 @@ public IndicesAccessControl authorize( Metadata metadata, FieldPermissionsCache fieldPermissionsCache ) { - // Short circuit if the indicesPermission allows all access to every index - for (Group group : groups) { - if (group.isTotal()) { - return IndicesAccessControl.allowAll(); - } - } - final Map resources = Maps.newMapWithExpectedSize(requestedIndicesOrAliases.size()); int totalResourceCount = 0; Map lookup = metadata.getIndicesLookup(); @@ -506,6 +424,7 @@ public IndicesAccessControl authorize( IndexComponentSelector selector = expressionAndSelector.v2() == null ? null : IndexComponentSelector.getByKey(expressionAndSelector.v2()); + final IndexResource resource = new IndexResource(indexOrAlias, lookup.get(indexOrAlias), selector); resources.put(resource.name, resource); totalResourceCount += resource.size(lookup); @@ -538,62 +457,56 @@ private Map buildIndicesAccessC final Map roleQueriesByIndex = Maps.newMapWithExpectedSize(totalResourceCount); final Set grantedResources = Sets.newHashSetWithExpectedSize(totalResourceCount); - final boolean isMappingUpdateAction = isMappingUpdateAction(action); - for (IndexResource resource : requestedResources.values()) { // true if ANY group covers the given index AND the given action boolean granted = false; final Collection concreteIndices = resource.resolveConcreteIndices(metadata); for (Group group : groups) { + AuthorizedComponents authorizedComponents = group.allowedIndicesPredicate(action) + .check(resource.name, resource.indexAbstraction); // the group covers the given index OR the given index is a backing index and the group covers the parent data stream - if (resource.checkIndex(group)) { - if (group.checkAction(action) - || (isMappingUpdateAction // for BWC reasons, mapping updates are exceptionally allowed for certain privileges on - // indices and aliases (but not on data streams) - && false == resource.isPartOfDataStream() - && containsPrivilegeThatGrantsMappingUpdatesForBwc(group))) { - granted = true; - // propagate DLS and FLS permissions over the concrete indices - for (String index : concreteIndices) { - final Set fieldPermissions = fieldPermissionsByIndex.compute(index, (k, existingSet) -> { - if (existingSet == null) { - // Most indices rely on the default (empty) field permissions object, so we optimize for that case - // Using an immutable single item set is significantly faster because it avoids any of the hashing - // and backing set creation. - return Set.of(group.getFieldPermissions()); - } else if (existingSet.size() == 1) { - FieldPermissions fp = group.getFieldPermissions(); - if (existingSet.contains(fp)) { - return existingSet; - } - // This index doesn't have a single field permissions object, replace the singleton with a real Set - final Set hashSet = new HashSet<>(existingSet); - hashSet.add(fp); - return hashSet; - } else { - existingSet.add(group.getFieldPermissions()); + if (authorizedComponents != null && authorizedComponents != AuthorizedComponents.NONE) { + granted = true; + // propagate DLS and FLS permissions over the concrete indices + for (String index : concreteIndices) { + final Set fieldPermissions = fieldPermissionsByIndex.compute(index, (k, existingSet) -> { + if (existingSet == null) { + // Most indices rely on the default (empty) field permissions object, so we optimize for that case + // Using an immutable single item set is significantly faster because it avoids any of the hashing + // and backing set creation. + return Set.of(group.getFieldPermissions()); + } else if (existingSet.size() == 1) { + FieldPermissions fp = group.getFieldPermissions(); + if (existingSet.contains(fp)) { return existingSet; } - }); - - DocumentLevelPermissions docPermissions; - if (group.hasQuery()) { - docPermissions = roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions()); - docPermissions.addAll(group.getQuery()); + // This index doesn't have a single field permissions object, replace the singleton with a real Set + final Set hashSet = new HashSet<>(existingSet); + hashSet.add(fp); + return hashSet; } else { - // if more than one permission matches for a concrete index here and if - // a single permission doesn't have a role query then DLS will not be - // applied even when other permissions do have a role query - docPermissions = DocumentLevelPermissions.ALLOW_ALL; - // don't worry about what's already there - just overwrite it, it avoids doing a 2nd hash lookup. - roleQueriesByIndex.put(index, docPermissions); + existingSet.add(group.getFieldPermissions()); + return existingSet; } + }); - if (index.equals(resource.name) == false) { - fieldPermissionsByIndex.put(resource.name, fieldPermissions); - roleQueriesByIndex.put(resource.name, docPermissions); - } + DocumentLevelPermissions docPermissions; + if (group.hasQuery()) { + docPermissions = roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions()); + docPermissions.addAll(group.getQuery()); + } else { + // if more than one permission matches for a concrete index here and if + // a single permission doesn't have a role query then DLS will not be + // applied even when other permissions do have a role query + docPermissions = DocumentLevelPermissions.ALLOW_ALL; + // don't worry about what's already there - just overwrite it, it avoids doing a 2nd hash lookup. + roleQueriesByIndex.put(index, docPermissions); + } + + if (index.equals(resource.name) == false) { + fieldPermissionsByIndex.put(resource.name, fieldPermissions); + roleQueriesByIndex.put(resource.name, docPermissions); } } } @@ -640,46 +553,18 @@ && containsPrivilegeThatGrantsMappingUpdatesForBwc(group))) { * If action is not granted for at least one resource, this method will return {@code false}. */ private boolean isActionGranted(final String action, final Map requestedResources) { - - final boolean isMappingUpdateAction = isMappingUpdateAction(action); - for (IndexResource resource : requestedResources.values()) { // true if ANY group covers the given index AND the given action boolean granted = false; - // true if ANY group, which contains certain ingest privileges, covers the given index AND the action is a mapping update for - // an index or an alias (but not for a data stream) - boolean bwcGrantMappingUpdate = false; - final List bwcDeprecationLogActions = new ArrayList<>(); for (Group group : groups) { - // the group covers the given index OR the given index is a backing index and the group covers the parent data stream - if (resource.checkIndex(group)) { - boolean actionCheck = group.checkAction(action); - // If action is granted we don't have to check for BWC and can stop at first granting group. - if (actionCheck) { - granted = true; - break; - } else { - // mapping updates are allowed for certain privileges on indices and aliases (but not on data streams), - // outside of the privilege definition - boolean bwcMappingActionCheck = isMappingUpdateAction - && false == resource.isPartOfDataStream() - && containsPrivilegeThatGrantsMappingUpdatesForBwc(group); - bwcGrantMappingUpdate = bwcGrantMappingUpdate || bwcMappingActionCheck; - - if (bwcMappingActionCheck) { - logDeprecatedBwcPrivilegeUsage(action, resource, group, bwcDeprecationLogActions); - } - } + AuthorizedComponents authResult = group.allowedIndicesPredicate(action).check(resource.name, resource.indexAbstraction); + if (authResult != null && authResult.isAnyAuthorized()) { + granted = true; + break; } } - if (false == granted && bwcGrantMappingUpdate) { - // the action is granted only due to the deprecated behaviour of certain privileges - granted = true; - bwcDeprecationLogActions.forEach(Runnable::run); - } - if (granted == false) { // We stop and return at first not granted resource. return false; @@ -690,34 +575,6 @@ private boolean isActionGranted(final String action, final Map bwcDeprecationLogActions - ) { - for (String privilegeName : group.privilege.name()) { - if (PRIVILEGE_NAME_SET_BWC_ALLOW_MAPPING_UPDATE.contains(privilegeName)) { - bwcDeprecationLogActions.add( - () -> deprecationLogger.warn( - DeprecationCategory.SECURITY, - "[" + resource.name + "] mapping update for ingest privilege [" + privilegeName + "]", - "the index privilege [" - + privilegeName - + "] allowed the update " - + "mapping action [" - + action - + "] on index [" - + resource.name - + "], this privilege " - + "will not permit mapping updates in the next major release - users who require access " - + "to update mappings must be granted explicit privileges" - ) - ); - } - } - } - private boolean isConcreteRestrictedIndex(String indexPattern) { if (Regex.isSimpleMatchPattern(indexPattern) || Automatons.isLuceneRegex(indexPattern)) { return false; @@ -725,12 +582,20 @@ private boolean isConcreteRestrictedIndex(String indexPattern) { return restrictedIndices.isRestricted(indexPattern); } - private static boolean isMappingUpdateAction(String action) { + static boolean isMappingUpdateAction(String action) { return action.equals(TransportPutMappingAction.TYPE.name()) || action.equals(TransportAutoPutMappingAction.TYPE.name()); } - private static boolean containsPrivilegeThatGrantsMappingUpdatesForBwc(Group group) { - return group.privilege().name().stream().anyMatch(PRIVILEGE_NAME_SET_BWC_ALLOW_MAPPING_UPDATE::contains); + static boolean containsPrivilegeThatGrantsMappingUpdatesForBwc(Set names) { + return names.stream().anyMatch(PRIVILEGE_NAME_SET_BWC_ALLOW_MAPPING_UPDATE::contains); + } + + private static boolean isFailureReadAction(String action) { + return READ_FAILURES.predicate().test(action); + } + + private static boolean containsReadFailuresPrivilege(Group group) { + return group.privilege().name().contains(READ_FAILURES_PRIVILEGE_NAME); } /** @@ -788,113 +653,6 @@ private Map indexGroupAutomatons(boolean combine) { return allAutomatons; } - public static class Group { - public static final Group[] EMPTY_ARRAY = new Group[0]; - - private final IndexPrivilege privilege; - private final Predicate actionMatcher; - private final String[] indices; - private final StringMatcher indexNameMatcher; - private final Supplier indexNameAutomaton; - // TODO: Use FieldPermissionsDefinition instead of FieldPermissions. The former is a better counterpart to query - private final FieldPermissions fieldPermissions; - private final Set query; - // by default certain restricted indices are exempted when granting privileges, as they should generally be hidden for ordinary - // 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 Group( - IndexPrivilege privilege, - FieldPermissions fieldPermissions, - @Nullable Set query, - boolean allowRestrictedIndices, - RestrictedIndices restrictedIndices, - String... indices - ) { - assert indices.length != 0; - this.privilege = privilege; - this.actionMatcher = privilege.predicate(); - this.indices = indices; - this.allowRestrictedIndices = allowRestrictedIndices; - ConcurrentHashMap indexNameAutomatonMemo = new ConcurrentHashMap<>(1); - if (allowRestrictedIndices) { - this.indexNameMatcher = StringMatcher.of(indices); - this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent(indices, k -> Automatons.patterns(indices)); - } else { - this.indexNameMatcher = StringMatcher.of(indices).and(name -> restrictedIndices.isRestricted(name) == false); - this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent( - indices, - k -> Automatons.minusAndMinimize(Automatons.patterns(indices), restrictedIndices.getAutomaton()) - ); - } - this.fieldPermissions = Objects.requireNonNull(fieldPermissions); - this.query = query; - } - - public IndexPrivilege privilege() { - return privilege; - } - - public String[] indices() { - return indices; - } - - @Nullable - public Set getQuery() { - return query; - } - - public FieldPermissions getFieldPermissions() { - return fieldPermissions; - } - - private boolean checkAction(String action) { - return actionMatcher.test(action); - } - - private boolean checkIndex(String index) { - assert index != null; - return indexNameMatcher.test(index); - } - - boolean hasQuery() { - return query != null; - } - - public boolean allowRestrictedIndices() { - return allowRestrictedIndices; - } - - public Automaton getIndexMatcherAutomaton() { - return indexNameAutomaton.get(); - } - - boolean isTotal() { - return allowRestrictedIndices - && indexNameMatcher.isTotal() - && privilege == IndexPrivilege.ALL - && query == null - && false == fieldPermissions.hasFieldLevelSecurity(); - } - - @Override - public String toString() { - return "Group{" - + "privilege=" - + privilege - + ", indices=" - + Strings.arrayToCommaDelimitedString(indices) - + ", fieldPermissions=" - + fieldPermissions - + ", query=" - + query - + ", allowRestrictedIndices=" - + allowRestrictedIndices - + '}'; - } - } - private static class DocumentLevelPermissions { public static final DocumentLevelPermissions ALLOW_ALL = new DocumentLevelPermissions(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IsResourceAuthorizedPredicate.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IsResourceAuthorizedPredicate.java new file mode 100644 index 0000000000000..c67d885589efd --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IsResourceAuthorizedPredicate.java @@ -0,0 +1,119 @@ +/* + * 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.core.security.authz.permission; + +import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.core.Nullable; + +import java.util.Objects; + +/** + * 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 abstract class IsResourceAuthorizedPredicate { + /** + * A checker that allows access to no resources. + */ + static class NoResourcesAuthorizedChecker extends IsResourceAuthorizedPredicate { + @Override + public IndicesPermission.AuthorizedComponents check(String name, IndexAbstraction indexAbstraction, boolean authByDataStream) { + return IndicesPermission.AuthorizedComponents.NONE; + } + } + + /** + * A checker which combines the results of two checkers into a single result, authorizing access if either of the two given checkers + * grant access, and denying only if neither grants access. + */ + static class OrChecker extends IsResourceAuthorizedPredicate { + private final IsResourceAuthorizedPredicate a; + private final IsResourceAuthorizedPredicate b; + + OrChecker(IsResourceAuthorizedPredicate a, IsResourceAuthorizedPredicate b) { + this.a = Objects.requireNonNull(a); + this.b = Objects.requireNonNull(b); + } + + @Override + public IndicesPermission.AuthorizedComponents check(String name, IndexAbstraction abstraction, boolean authByDataStream) { + IndicesPermission.AuthorizedComponents authResult = a.check(name, abstraction); + // If we're only authorized for some components, other predicates might authorize us for the rest + return switch (authResult) { + case null -> b.check(name, abstraction, authByDataStream); + case NONE -> b.check(name, abstraction, authByDataStream); // Can't do worse than totally unauthorized, thank u NEXT + case ALL -> IndicesPermission.AuthorizedComponents.ALL; // Can't do better than ALL, so short circuit + case DATA, FAILURES -> authResult.union(b.check(name, abstraction, authByDataStream)); + }; + } + } + + /** + * A checker which combines the results of two checkers into a single result, authorizing access only if both of the given checkers + * authorize access, and denying access otherwise. + */ + static class AndChecker extends IsResourceAuthorizedPredicate { + private final IsResourceAuthorizedPredicate a; + private final IsResourceAuthorizedPredicate b; + + AndChecker(IsResourceAuthorizedPredicate a, IsResourceAuthorizedPredicate b) { + this.a = Objects.requireNonNull(a); + this.b = Objects.requireNonNull(b); + } + + @Override + public IndicesPermission.AuthorizedComponents check(String name, IndexAbstraction abstraction, boolean authByDataStream) { + IndicesPermission.AuthorizedComponents authResult = a.check(name, abstraction, authByDataStream); + // We can short circuit out if anything is unauthorized here + return switch (authResult) { + case null -> IndicesPermission.AuthorizedComponents.NONE; + case NONE -> IndicesPermission.AuthorizedComponents.NONE; + case ALL -> b.check(name, abstraction, authByDataStream); + case DATA, FAILURES -> authResult.intersection(b.check(name, abstraction, authByDataStream)); + }; + } + } + + /** + * 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 orAllowIf(IsResourceAuthorizedPredicate other) { + return new OrChecker(this, other); + } + + public final IsResourceAuthorizedPredicate alsoRequire(IsResourceAuthorizedPredicate other) { + return new AndChecker(this, other); + } + + /** + * Check which components of the given {@param indexAbstraction} resource is authorized. + * The resource must exist. Otherwise, use the {@link #check(String, IndexAbstraction)} method. + * + * @return An object representing which components of this index abstraction the user is authorized to access + */ + public final IndicesPermission.AuthorizedComponents check(IndexAbstraction indexAbstraction) { + return check(indexAbstraction.getName(), indexAbstraction); + } + + public final boolean test(IndexAbstraction indexAbstraction) { + IndicesPermission.AuthorizedComponents authResult = check(indexAbstraction); + return authResult != null && authResult.isDataAuthorized(); + } + + public IndicesPermission.AuthorizedComponents check(String name, @Nullable IndexAbstraction indexAbstraction) { + return check(name, indexAbstraction, true); + } + + public abstract IndicesPermission.AuthorizedComponents check( + String name, + @Nullable IndexAbstraction indexAbstraction, + boolean allowAuthorizationViaDataStream + ); +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java index 010e08b0d4db6..86f1b2862224a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java @@ -19,7 +19,6 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptorsIntersection; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; -import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.IsResourceAuthorizedPredicate; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -199,7 +198,7 @@ public RoleDescriptorsIntersection getRoleDescriptorsIntersectionForRemoteCluste */ @Override public IsResourceAuthorizedPredicate allowedIndicesMatcher(String action) { - return baseRole.allowedIndicesMatcher(action).and(limitedByRole.allowedIndicesMatcher(action)); + return baseRole.allowedIndicesMatcher(action).alsoRequire(limitedByRole.allowedIndicesMatcher(action)); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteIndicesPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteIndicesPermission.java index 2abc93997fa9d..06bf920fa5068 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteIndicesPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteIndicesPermission.java @@ -37,7 +37,7 @@ public static Builder builder() { } public static class Builder { - final Map, List> remoteIndicesGroups; + final Map, List> remoteIndicesGroups; public Builder() { this.remoteIndicesGroups = new HashMap<>(); @@ -58,7 +58,7 @@ public Builder addGroup( : "remote indices groups only support up to one FLS field-grant-exclude group"; remoteIndicesGroups.computeIfAbsent(remoteClusterAliases, k -> new ArrayList<>()) .add( - new IndicesPermission.Group( + new Group( privilege, fieldPermissions, query, @@ -82,9 +82,9 @@ public RemoteIndicesPermission build() { public record RemoteIndicesGroup( Set remoteClusterAliases, StringMatcher remoteClusterAliasMatcher, - List indicesPermissionGroups + List indicesPermissionGroups ) { - public RemoteIndicesGroup(Set remoteClusterAliases, List indicesPermissionGroups) { + public RemoteIndicesGroup(Set remoteClusterAliases, List indicesPermissionGroups) { this(remoteClusterAliases, StringMatcher.of(remoteClusterAliases), indicesPermissionGroups); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java index fe97b152a2ee7..f2008cf586e01 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java @@ -20,7 +20,6 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptorsIntersection; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; -import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.IsResourceAuthorizedPredicate; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java index 9b63b73d7801b..d7e83480d66bf 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java @@ -22,7 +22,6 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptorsIntersection; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; -import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.IsResourceAuthorizedPredicate; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.restriction.WorkflowsRestriction; @@ -215,7 +214,7 @@ public RoleDescriptorsIntersection getRoleDescriptorsIntersectionForRemoteCluste final List indicesPrivileges = new ArrayList<>(); for (RemoteIndicesPermission.RemoteIndicesGroup remoteIndicesGroup : remoteIndicesPermission.remoteIndicesGroups()) { - for (IndicesPermission.Group indicesGroup : remoteIndicesGroup.indicesPermissionGroups()) { + for (Group indicesGroup : remoteIndicesGroup.indicesPermissionGroups()) { indicesPrivileges.add(toIndicesPrivileges(indicesGroup)); } } @@ -237,7 +236,7 @@ public RoleDescriptorsIntersection getRoleDescriptorsIntersectionForRemoteCluste ); } - private static Set getFieldGrantExcludeGroups(IndicesPermission.Group group) { + private static Set getFieldGrantExcludeGroups(Group group) { if (group.getFieldPermissions().hasFieldLevelSecurity()) { final List fieldPermissionsDefinitions = group.getFieldPermissions() .getFieldPermissionsDefinitions(); @@ -250,7 +249,7 @@ private static Set getFieldGr } } - private static RoleDescriptor.IndicesPrivileges toIndicesPrivileges(final IndicesPermission.Group indicesGroup) { + private static RoleDescriptor.IndicesPrivileges toIndicesPrivileges(final Group indicesGroup) { final Set queries = indicesGroup.getQuery(); final Set fieldGrantExcludeGroups = getFieldGrantExcludeGroups(indicesGroup); assert queries == null || queries.size() <= 1 diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java index 7174b2f616c2a..4c3ad50a705ff 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java @@ -81,6 +81,9 @@ public final class IndexPrivilege extends Privilege { ResolveIndexAction.NAME, TransportResolveClusterAction.NAME ); + // This is a special case: read_failures acts like `read` *only* for failure store indices in authorized data streams. + // This internal action is not used, but having it makes automaton subset checks work as expected with this privilege. + private static final Automaton READ_FAILURES_AUTOMATON = patterns("internal:special/read_failures"); private static final Automaton READ_CROSS_CLUSTER_AUTOMATON = patterns( "internal:transport/proxy/indices:data/read/*", TransportClusterSearchShardsAction.TYPE.name(), @@ -178,7 +181,11 @@ public final class IndexPrivilege extends Privilege { public static final IndexPrivilege NONE = new IndexPrivilege("none", Automatons.EMPTY); public static final IndexPrivilege ALL = new IndexPrivilege("all", ALL_AUTOMATON); - public static final IndexPrivilege READ = new IndexPrivilege("read", READ_AUTOMATON); + public static final String READ_PRIVILEGE_NAME = "read"; + public static final IndexPrivilege READ = new IndexPrivilege(READ_PRIVILEGE_NAME, READ_AUTOMATON); + public static final String READ_FAILURES_PRIVILEGE_NAME = "read_failures"; + // read_failures is a special case - it should act like `read`, but adjusted to only allow access to failure indices + public static final IndexPrivilege READ_FAILURES = new IndexPrivilege(READ_FAILURES_PRIVILEGE_NAME, READ_FAILURES_AUTOMATON); public static final IndexPrivilege READ_CROSS_CLUSTER = new IndexPrivilege("read_cross_cluster", READ_CROSS_CLUSTER_AUTOMATON); public static final IndexPrivilege CREATE = new IndexPrivilege("create", CREATE_AUTOMATON); public static final IndexPrivilege INDEX = new IndexPrivilege("index", INDEX_AUTOMATON); @@ -221,6 +228,7 @@ public final class IndexPrivilege extends Privilege { entry("create_index", CREATE_INDEX), entry("monitor", MONITOR), entry("read", READ), + entry("read_failures", READ_FAILURES), entry("index", INDEX), entry("delete", DELETE), entry("write", WRITE), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/test/SecuritySettingsSourceField.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/test/SecuritySettingsSourceField.java index 537aac3497cd7..b17863fed5c59 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/test/SecuritySettingsSourceField.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/test/SecuritySettingsSourceField.java @@ -23,7 +23,7 @@ public final class SecuritySettingsSourceField { indices: - names: [ "*" ] allow_restricted_indices: true - privileges: [ "ALL" ] + privileges: [ "ALL", "read_failures" ] run_as: [ "*" ] applications: - application: "*" @@ -35,7 +35,11 @@ public final class SecuritySettingsSourceField { "_es_test_root", new String[] { "ALL" }, new RoleDescriptor.IndicesPrivileges[] { - RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("ALL").allowRestrictedIndices(true).build() }, + RoleDescriptor.IndicesPrivileges.builder() + .indices("*") + .privileges("ALL", "read_failures") + .allowRestrictedIndices(true) + .build() }, new RoleDescriptor.ApplicationResourcePrivileges[] { RoleDescriptor.ApplicationResourcePrivileges.builder().application("*").privileges("*").resources("*").build() }, null, diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java index 107953557f3ea..24634b50bbe5d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java @@ -413,6 +413,7 @@ public void testIngestAdminRole() { assertNoAccessAllowed(ingestAdminRole, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2)); } + // ATHE: A bunch of put mapping calls emit warnings here now and I'm not sure if they should or not public void testKibanaSystemRole() { final TransportRequest request = mock(TransportRequest.class); final Authentication authentication = randomValueOtherThanMany( @@ -974,6 +975,15 @@ public void testKibanaSystemRole() { is(true) ); assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + if (indexAbstraction.getType() == IndexAbstraction.Type.CONCRETE_INDEX) { + assertWarnings( + "the index privilege [write] allowed the update mapping action [indices:admin/mapping/put] on index [" + + index + + "], this privilege will not permit mapping updates in the next major release - users who require access to update" + + " mappings must be granted explicit privileges" + ); + } + assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -998,6 +1008,15 @@ public void testKibanaSystemRole() { is(true) ); assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + if (indexAbstraction.getType() == IndexAbstraction.Type.CONCRETE_INDEX) { + assertWarnings( + "the index privilege [index] allowed the update mapping action [indices:admin/mapping/put] on index [" + + index + + "], this privilege will not permit mapping updates in the next major release - users who require access to update" + + " mappings must be granted explicit privileges" + ); + } + assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -1024,7 +1043,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -1081,6 +1101,15 @@ public void testKibanaSystemRole() { is(true) ); assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + if (indexAbstraction.getType() == IndexAbstraction.Type.CONCRETE_INDEX) { + assertWarnings( + "the index privilege [write] allowed the update mapping action [indices:admin/mapping/put] on index [" + + index + + "], this privilege will not permit mapping updates in the next major release - users who require access to update" + + " mappings must be granted explicit privileges" + ); + } + assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -1319,7 +1348,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); assertThat( kibanaRole.indices().allowedIndicesMatcher("indices:admin/data_stream/lifecycle/put").test(indexAbstraction), @@ -1453,10 +1483,10 @@ public void testKibanaSystemRole() { ); // Granted by bwc for index privilege - assertThat( - kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), - is(indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM) - ); + // assertThat( + // kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM) + // ); // Deny deleting documents and rollover assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportDeleteAction.NAME).test(indexAbstraction), is(false)); @@ -1516,7 +1546,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportDeleteIndexAction.TYPE.name()).test(indexAbstraction), is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportSearchAction.TYPE.name()).test(indexAbstraction), is(true)); @@ -1580,7 +1611,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -1606,7 +1638,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -1632,7 +1665,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -1671,7 +1705,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); @@ -1743,7 +1778,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); assertViewIndexMetadata(kibanaRole, indexName); }); @@ -1836,7 +1872,8 @@ public void testKibanaSystemRole() { kibanaRole.indices().allowedIndicesMatcher(TransportUpdateSettingsAction.TYPE.name()).test(indexAbstraction), is(true) ); - assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), is(true)); + // assertThat(kibanaRole.indices().allowedIndicesMatcher(TransportPutMappingAction.TYPE.name()).test(indexAbstraction), + // is(true)); assertThat(kibanaRole.indices().allowedIndicesMatcher(RolloverAction.NAME).test(indexAbstraction), is(true)); }); } @@ -4176,6 +4213,7 @@ private IndexAbstraction mockIndexAbstraction(String name) { IndexAbstraction mock = mock(IndexAbstraction.class); when(mock.getName()).thenReturn(name); when(mock.getType()).thenReturn( + // IndexAbstraction.Type.CONCRETE_INDEX randomFrom(IndexAbstraction.Type.CONCRETE_INDEX, IndexAbstraction.Type.ALIAS, IndexAbstraction.Type.DATA_STREAM) ); return mock; diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DataStreamSecurityIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DataStreamSecurityIT.java index 550578161302b..05e23bf078615 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DataStreamSecurityIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DataStreamSecurityIT.java @@ -8,27 +8,43 @@ package org.elasticsearch.integration; import org.elasticsearch.ElasticsearchSecurityException; +import org.elasticsearch.action.DocWriteRequest; +import org.elasticsearch.action.admin.indices.get.GetIndexRequestBuilder; +import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; import org.elasticsearch.action.admin.indices.stats.IndicesStatsRequest; import org.elasticsearch.action.admin.indices.template.put.TransportPutComposableIndexTemplateAction; +import org.elasticsearch.action.bulk.BulkItemResponse; +import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.datastreams.CreateDataStreamAction; import org.elasticsearch.action.datastreams.ModifyDataStreamsAction; +import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.metadata.DataStream; import org.elasticsearch.cluster.metadata.DataStreamAction; +import org.elasticsearch.cluster.metadata.DataStreamFailureStore; +import org.elasticsearch.cluster.metadata.DataStreamOptions; import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.ResettableValue; +import org.elasticsearch.cluster.metadata.Template; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.core.Strings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.datastreams.DataStreamsPlugin; import org.elasticsearch.index.Index; import org.elasticsearch.index.mapper.extras.MapperExtrasPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.SecurityIntegTestCase; import org.elasticsearch.test.SecuritySettingsSource; import org.elasticsearch.test.SecuritySettingsSourceField; import org.elasticsearch.transport.netty4.Netty4Plugin; +import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.security.LocalStateSecurity; import org.elasticsearch.xpack.wildcard.Wildcard; @@ -43,6 +59,7 @@ import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.nullValue; public class DataStreamSecurityIT extends SecurityIntegTestCase { @@ -51,6 +68,31 @@ protected Collection> nodePlugins() { return List.of(LocalStateSecurity.class, Netty4Plugin.class, MapperExtrasPlugin.class, DataStreamsPlugin.class, Wildcard.class); } + @Override + protected String configUsers() { + final String usersPasswdHashed = new String( + getFastStoredHashAlgoForTests().hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING) + ); + return super.configUsers() + "only_failures:" + usersPasswdHashed + "\n"; + } + + @Override + protected String configUsersRoles() { + return super.configUsersRoles() + "only_failures:only_failures\n"; + } + + @Override + protected String configRoles() { + // role that has analyze indices privileges only + return Strings.format(""" + %s + only_failures: + indices: + - names: '*' + privileges: [ 'read_failures', 'write' ] + """, super.configRoles()); + } + public void testRemoveGhostReference() throws Exception { var headers = Map.of( BASIC_AUTH_HEADER, @@ -142,4 +184,64 @@ public void onFailure(Exception e) { assertThat(indicesStatsResponse.getIndices().size(), equalTo(shouldBreakIndexName ? 1 : 2)); } + public void testFailureStoreAuthorziation() throws Exception { + var adminHeaders = Map.of( + BASIC_AUTH_HEADER, + basicAuthHeaderValue(SecuritySettingsSource.TEST_USER_NAME, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING) + ); + final var adminClient = client().filterWithHeader(adminHeaders); + var onlyFailHeaders = Map.of( + BASIC_AUTH_HEADER, + basicAuthHeaderValue(SecuritySettingsSource.TEST_USER_NAME, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING) + ); + final var failuresClient = client().filterWithHeader(onlyFailHeaders); + + var putTemplateRequest = new TransportPutComposableIndexTemplateAction.Request("id"); + putTemplateRequest.indexTemplate( + ComposableIndexTemplate.builder() + .indexPatterns(List.of("stuff-*")) + .template( + Template.builder() + .mappings(CompressedXContent.fromJSON("{\"properties\": {\"code\": {\"type\": \"integer\"}}}")) + .dataStreamOptions( + new DataStreamOptions.Template( + ResettableValue.create(new DataStreamFailureStore.Template(ResettableValue.create(true))) + ) + ) + ) + .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate(false, false)) + .build() + ); + assertAcked(adminClient.execute(TransportPutComposableIndexTemplateAction.TYPE, putTemplateRequest).actionGet()); + + String dataStreamName = "stuff-es"; + var request = new CreateDataStreamAction.Request(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, dataStreamName); + assertAcked(adminClient.execute(CreateDataStreamAction.INSTANCE, request).actionGet()); + + BulkRequest bulkRequest = new BulkRequest(); + bulkRequest.add( + new IndexRequest(dataStreamName).opType(DocWriteRequest.OpType.CREATE) + .source("{\"code\": \"well this aint right\"}", XContentType.JSON), + new IndexRequest(dataStreamName).opType(DocWriteRequest.OpType.CREATE) + .source("{\"@timestamp\": \"2015-01-01T12:10:30Z\", \"code\": 404}", XContentType.JSON) + ); + BulkResponse bulkResponse = adminClient.bulk(bulkRequest).actionGet(); + assertThat(bulkResponse.getItems().length, equalTo(2)); + String backingIndexPrefix = DataStream.BACKING_INDEX_PREFIX + dataStreamName; + String failureIndexPrefix = DataStream.FAILURE_STORE_PREFIX + dataStreamName; + + for (BulkItemResponse itemResponse : bulkResponse) { + assertThat(itemResponse.getFailure(), nullValue()); + assertThat(itemResponse.status(), equalTo(RestStatus.CREATED)); + // assertThat(itemResponse.getIndex(), anyOf(startsWith(backingIndexPrefix), startsWith(failureIndexPrefix))); + } + + indicesAdmin().refresh(new RefreshRequest(dataStreamName)).actionGet(); + var getResp = failuresClient.admin() + .indices() + .getIndex(new GetIndexRequestBuilder(failuresClient, TimeValue.THIRTY_SECONDS).addIndices(dataStreamName + "::*").request()); + var searchResponse = failuresClient.prepareSearch(dataStreamName + "::failures").get(); + assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(1L)); + } + } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java index e2a52b1c83131..9ca4329a7b630 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java @@ -43,7 +43,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.SortedMap; import java.util.concurrent.CopyOnWriteArraySet; @@ -435,9 +434,8 @@ static String getPutMappingIndexOrAlias(PutMappingRequest request, Predicate> foundAliases = metadata.findAllAliases(new String[] { concreteIndexName }); - List aliasMetadata = foundAliases.get(concreteIndexName); - if (aliasMetadata != null) { - Optional foundAlias = aliasMetadata.stream().map(AliasMetadata::alias).filter(isAuthorized).filter(aliasName -> { - IndexAbstraction alias = metadata.getIndicesLookup().get(aliasName); - List indices = alias.getIndices(); - if (indices.size() == 1) { - return true; - } else { - assert alias.getType() == IndexAbstraction.Type.ALIAS; - Index writeIndex = alias.getWriteIndex(); - return writeIndex != null && writeIndex.getName().equals(concreteIndexName); + Map foundAliases = metadata.index(indexAbstraction.getName()).getAliases(); + if (foundAliases != null && foundAliases.isEmpty() == false) { + for (var aliasMd : foundAliases.values()) { + String aliasName = aliasMd.alias(); + IndexAbstraction aliasAbstraction = metadata.getIndicesLookup().get(aliasName); + assert aliasAbstraction.getType() == IndexAbstraction.Type.ALIAS; + Index writeIndex = aliasAbstraction.getWriteIndex(); + if (writeIndex != null) { + if (concreteIndexName.equals(writeIndex.getName()) && isAuthorized.test(aliasName)) { + return aliasName; + } } - }).findFirst(); - resolvedAliasOrIndex = foundAlias.orElse(concreteIndexName); + } } else { - resolvedAliasOrIndex = concreteIndexName; + return concreteIndexName; } } - - return resolvedAliasOrIndex; + return concreteIndexName; } private static List loadAuthorizedAliases(Supplier> authorizedIndices, Metadata metadata) { 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 614401770cfb7..d847117162d4b 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 @@ -73,8 +73,9 @@ import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition; +import org.elasticsearch.xpack.core.security.authz.permission.Group; import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission; -import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.IsResourceAuthorizedPredicate; +import org.elasticsearch.xpack.core.security.authz.permission.IsResourceAuthorizedPredicate; import org.elasticsearch.xpack.core.security.authz.permission.RemoteIndicesPermission; import org.elasticsearch.xpack.core.security.authz.permission.ResourcePrivileges; import org.elasticsearch.xpack.core.security.authz.permission.ResourcePrivilegesMap; @@ -784,13 +785,13 @@ static GetUserPrivilegesResponse buildUserPrivilegesResponseObject(Role userRole } final Set indices = new LinkedHashSet<>(); - for (IndicesPermission.Group group : userRole.indices().groups()) { + for (Group group : userRole.indices().groups()) { indices.add(toIndices(group)); } final Set remoteIndices = new LinkedHashSet<>(); for (RemoteIndicesPermission.RemoteIndicesGroup remoteIndicesGroup : userRole.remoteIndices().remoteIndicesGroups()) { - for (IndicesPermission.Group group : remoteIndicesGroup.indicesPermissionGroups()) { + for (Group group : remoteIndicesGroup.indicesPermissionGroups()) { remoteIndices.add(new GetUserPrivilegesResponse.RemoteIndices(toIndices(group), remoteIndicesGroup.remoteClusterAliases())); } } @@ -832,7 +833,7 @@ static GetUserPrivilegesResponse buildUserPrivilegesResponseObject(Role userRole ); } - private static GetUserPrivilegesResponse.Indices toIndices(final IndicesPermission.Group group) { + private static GetUserPrivilegesResponse.Indices toIndices(final Group group) { final Set queries = group.getQuery() == null ? Collections.emptySet() : group.getQuery(); final Set fieldSecurity = getFieldGrantExcludeGroups(group); return new GetUserPrivilegesResponse.Indices( @@ -844,7 +845,7 @@ private static GetUserPrivilegesResponse.Indices toIndices(final IndicesPermissi ); } - private static Set getFieldGrantExcludeGroups(IndicesPermission.Group group) { + private static Set getFieldGrantExcludeGroups(Group group) { if (group.getFieldPermissions().hasFieldLevelSecurity()) { final List fieldPermissionsDefinitions = group.getFieldPermissions() .getFieldPermissionsDefinitions(); @@ -875,42 +876,47 @@ 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)) { + IndicesPermission.AuthorizedComponents authResult = predicate.check(indexAbstraction); + if (authResult != null && authResult != IndicesPermission.AuthorizedComponents.NONE) { indicesAndAliases.add(indexAbstraction.getName()); 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 (authResult.isDataAuthorized()) { + 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 (authResult.isFailuresAuthorized()) { + for (Index index : ((DataStream) indexAbstraction).getFailureIndices()) { + indicesAndAliases.add(index.getName()); + } } + } } } } else { for (IndexAbstraction indexAbstraction : lookup.values()) { - if (indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM && predicate.test(indexAbstraction)) { - indicesAndAliases.add(indexAbstraction.getName()); + if (indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM) { + IndicesPermission.AuthorizedComponents authResult = predicate.check( + indexAbstraction.getName(), + indexAbstraction, + false + ); + if (indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM + && authResult != null + && authResult.isDataAuthorized()) { + indicesAndAliases.add(indexAbstraction.getName()); + } } + } } timeChecker.accept(indicesAndAliases); return indicesAndAliases; }, name -> { 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 { - // 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. - return (indexAbstraction.getParentDataStream() != null && predicate.test(indexAbstraction.getParentDataStream())) - || predicate.test(indexAbstraction); - } + return predicate.check(name, indexAbstraction).isAnyAuthorized(); }); } 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..499122986743b 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 @@ -342,7 +342,7 @@ public void setup() { new RoleDescriptor( "data_stream_test3", null, - new IndicesPrivileges[] { IndicesPrivileges.builder().indices("logs*").privileges("all").build() }, + new IndicesPrivileges[] { IndicesPrivileges.builder().indices("logs*").privileges("all", "read_failures").build() }, null ) ); @@ -1943,6 +1943,11 @@ public void testDynamicPutMappingRequestFromAlias() { request = new PutMappingRequest(Strings.EMPTY_ARRAY).setConcreteIndex(new Index(index, UUIDs.base64UUID())); putMappingIndexOrAlias = IndicesAndAliasesResolver.getPutMappingIndexOrAlias(request, authorizedIndices::check, metadata); assertEquals(index, putMappingIndexOrAlias); + assertWarnings( + "the index privilege [write] allowed the update mapping action [indices:admin/mapping/put] on " + + "index [barbaz], this privilege will not permit mapping updates in the next major release - users who require access to " + + "update mappings must be granted explicit privileges" + ); } public void testWhenAliasToMultipleIndicesAndUserIsAuthorizedUsingAliasReturnsAliasNameForDynamicPutMappingRequestOnWriteIndex() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java index 4488c28750dc0..50834fc5d9de7 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java @@ -33,7 +33,9 @@ import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition; +import org.elasticsearch.xpack.core.security.authz.permission.Group; import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission; +import org.elasticsearch.xpack.core.security.authz.permission.IsResourceAuthorizedPredicate; import org.elasticsearch.xpack.core.security.authz.permission.Role; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; import org.elasticsearch.xpack.core.security.support.StringMatcher; @@ -45,6 +47,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -335,7 +338,7 @@ public void testErrorMessageIfIndexPatternIsTooComplex() { } final ElasticsearchSecurityException e = expectThrows( ElasticsearchSecurityException.class, - () -> new IndicesPermission.Group( + () -> new Group( IndexPrivilege.ALL, FieldPermissions.DEFAULT, null, @@ -594,6 +597,11 @@ public void testAuthorizationForMappingUpdates() { + TransportAutoPutMappingAction.TYPE.name() + "] on " + "index [test1], this privilege will not permit mapping updates in the next major release - " + + "users who require access to update mappings must be granted explicit privileges", + "the index privilege [index] allowed the update mapping action [" + + TransportAutoPutMappingAction.TYPE.name() + + "] on " + + "index [test_write1], this privilege will not permit mapping updates in the next major release - " + "users who require access to update mappings must be granted explicit privileges" ); @@ -659,9 +667,10 @@ public void testIndicesPermissionHasFieldOrDocumentLevelSecurity() { ).build(); assertThat(indicesPermission2.hasFieldOrDocumentLevelSecurity(), is(false)); - // IsTotal means NO DLS/FLS even when there is another group that has DLS/FLS + // allowsTotalDataIndexAccess means NO DLS/FLS even when there is another group that has DLS/FLS + // failure store indices are not considered as final IndicesPermission indicesPermission3 = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( - IndexPrivilege.ALL, + IndexPrivilege.get(Set.of("all", "read_failures")), FieldPermissions.DEFAULT, null, true, @@ -670,55 +679,124 @@ public void testIndicesPermissionHasFieldOrDocumentLevelSecurity() { assertThat(indicesPermission3.hasFieldOrDocumentLevelSecurity(), is(false)); } - public void testResourceAuthorizedPredicateForDatastreams() { + public void testFailureStoreReadAuthorization() { String dataStreamName = "logs-datastream"; - Metadata.Builder mb = Metadata.builder( - DataStreamTestHelper.getClusterStateWithDataStreams( - List.of(Tuple.tuple(dataStreamName, 1)), - List.of(), - Instant.now().toEpochMilli(), - builder().build(), - 1 - ).getMetadata() - ); - DataStream dataStream = mb.dataStream(dataStreamName); - IndexAbstraction backingIndex = new IndexAbstraction.ConcreteIndex( - DataStreamTestHelper.createBackingIndex(dataStreamName, 1).build(), - dataStream - ); - IndexAbstraction concreteIndex = new IndexAbstraction.ConcreteIndex( - IndexMetadata.builder("logs-index").settings(indexSettings(IndexVersion.current(), 1, 0)).build() - ); - AliasMetadata aliasMetadata = new AliasMetadata.Builder("logs-alias").build(); - IndexAbstraction alias = new IndexAbstraction.Alias( - aliasMetadata, - List.of( - IndexMetadata.builder("logs-index").settings(indexSettings(IndexVersion.current(), 1, 0)).putAlias(aliasMetadata).build() + + IndexMetadata backingIndexMd = DataStreamTestHelper.createBackingIndex(dataStreamName, 1).build(); + IndexMetadata failureIndexMd = DataStreamTestHelper.createFailureStore(dataStreamName, 1).build(); + IndexMetadata regularIndex = IndexMetadata.builder("logs-index").settings(indexSettings(IndexVersion.current(), 1, 0)).build(); + + Metadata.Builder mb = Metadata.builder() + .dataStreams( + Map.of( + dataStreamName, + DataStreamTestHelper.newInstance(dataStreamName, List.of(backingIndexMd.getIndex()), List.of(failureIndexMd.getIndex())) + ), + Map.of() ) - ); - IndicesPermission.IsResourceAuthorizedPredicate predicate = new IndicesPermission.IsResourceAuthorizedPredicate( - StringMatcher.of("other"), - StringMatcher.of(dataStreamName, backingIndex.getName(), concreteIndex.getName(), alias.getName()) - ); - assertThat(predicate.test(dataStream), is(false)); - // test authorization for a missing resource with the datastream's name - assertThat(predicate.test(dataStream.getName(), null), is(true)); - assertThat(predicate.test(backingIndex), is(false)); - // test authorization for a missing resource with the backing index's name - assertThat(predicate.test(backingIndex.getName(), null), is(true)); - assertThat(predicate.test(concreteIndex), is(true)); - assertThat(predicate.test(alias), is(true)); + .indices( + Map.of( + backingIndexMd.getIndex().getName(), + backingIndexMd, + failureIndexMd.getIndex().getName(), + failureIndexMd, + regularIndex.getIndex().getName(), + regularIndex + ) + ); + DataStream dataStream = mb.dataStream(dataStreamName); + + IndexAbstraction backingIndex = new IndexAbstraction.ConcreteIndex(backingIndexMd, dataStream); + IndexAbstraction failureIndex = new IndexAbstraction.ConcreteIndex(failureIndexMd, dataStream); + + IndexAbstraction concreteIndex = new IndexAbstraction.ConcreteIndex(regularIndex); + final IndicesPermission everythingPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( + IndexPrivilege.get(Set.of("all", "read_failures")), + FieldPermissions.DEFAULT, + null, + true, + "logs-data*" + ).addGroup(IndexPrivilege.NONE, FieldPermissions.DEFAULT, null, randomBoolean(), "logs-data*").build(); + final IndicesPermission dataOnlyPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( + IndexPrivilege.get(Set.of("all")), + FieldPermissions.DEFAULT, + null, + true, + "logs-data*" + ).addGroup(IndexPrivilege.NONE, FieldPermissions.DEFAULT, null, randomBoolean(), "logs-data*").build(); + final IndicesPermission failuresOnlyPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup( + IndexPrivilege.get(Set.of("read_failures")), + FieldPermissions.DEFAULT, + null, + true, + "logs-data*" + ).addGroup(IndexPrivilege.NONE, FieldPermissions.DEFAULT, null, randomBoolean(), "logs-data*").build(); + + IsResourceAuthorizedPredicate everythingChecker = everythingPermission.allowedIndicesMatcher("indices:data/read/search"); + IsResourceAuthorizedPredicate onlyDataChecker = dataOnlyPermission.allowedIndicesMatcher("indices:data/read/search"); + IsResourceAuthorizedPredicate onlyFailuresChecker = failuresOnlyPermission.allowedIndicesMatcher("indices:data/read/search"); + + // Baseline check, nothing should be authorized for the concrete index + assertThat(everythingChecker.check(concreteIndex).isDataAuthorized(), is(false)); + assertThat(onlyDataChecker.check(concreteIndex).isDataAuthorized(), is(false)); + assertThat(onlyFailuresChecker.check(concreteIndex).isDataAuthorized(), is(false)); + assertThat(everythingChecker.check(concreteIndex).isFailuresAuthorized(), is(false)); + assertThat(onlyDataChecker.check(concreteIndex).isFailuresAuthorized(), is(false)); + assertThat(onlyFailuresChecker.check(concreteIndex).isFailuresAuthorized(), is(false)); + + // Check the data stream name directly + assertThat(everythingChecker.check(dataStreamName, null).isDataAuthorized(), is(true)); + assertThat(everythingChecker.check(dataStreamName, null).isFailuresAuthorized(), is(true)); + assertThat(onlyDataChecker.check(dataStreamName, null).isDataAuthorized(), is(true)); + assertThat(onlyDataChecker.check(dataStreamName, null).isFailuresAuthorized(), is(false)); + assertThat(onlyFailuresChecker.check(dataStreamName, null).isDataAuthorized(), is(false)); + assertThat(onlyFailuresChecker.check(dataStreamName, null).isFailuresAuthorized(), is(true)); + + assertThat(everythingChecker.check(concreteIndex).isFailuresAuthorized(), is(false)); + assertThat(onlyDataChecker.check(concreteIndex).isFailuresAuthorized(), is(false)); + assertThat(onlyFailuresChecker.check(concreteIndex).isFailuresAuthorized(), is(false)); + + // Check data index + assertThat(everythingChecker.check(backingIndex).isDataAuthorized(), is(true)); + assertThat(onlyDataChecker.check(backingIndex).isDataAuthorized(), is(true)); + assertThat(onlyFailuresChecker.check(backingIndex).isDataAuthorized(), is(false)); + + // Check failures index + assertThat(everythingChecker.check(failureIndex).isFailuresAuthorized(), is(true)); + assertThat(onlyDataChecker.check(failureIndex).isFailuresAuthorized(), is(false)); + assertThat(onlyFailuresChecker.check(failureIndex).isFailuresAuthorized(), is(true)); } public void testResourceAuthorizedPredicateAnd() { - IndicesPermission.IsResourceAuthorizedPredicate predicate1 = new IndicesPermission.IsResourceAuthorizedPredicate( - StringMatcher.of("c", "a"), - StringMatcher.of("b", "d") - ); - IndicesPermission.IsResourceAuthorizedPredicate predicate2 = new IndicesPermission.IsResourceAuthorizedPredicate( - StringMatcher.of("c", "b"), - StringMatcher.of("a", "d") - ); + IsResourceAuthorizedPredicate predicate1 = new IsResourceAuthorizedPredicate() { + @Override + public IndicesPermission.AuthorizedComponents check(String name, IndexAbstraction abstraction, boolean authByDataStream) { + assertTrue(authByDataStream); + StringMatcher regularNames = StringMatcher.of("c", "a"); + StringMatcher nonDatastreamNames = StringMatcher.of("b", "d"); + if (abstraction.getType() != IndexAbstraction.Type.DATA_STREAM && abstraction.getParentDataStream() == null) { + if (nonDatastreamNames.test(name)) { + return IndicesPermission.AuthorizedComponents.DATA; + } + } + return regularNames.test(name) ? IndicesPermission.AuthorizedComponents.DATA : IndicesPermission.AuthorizedComponents.NONE; + } + }; + IsResourceAuthorizedPredicate predicate2 = new IsResourceAuthorizedPredicate() { + @Override + public IndicesPermission.AuthorizedComponents check(String name, IndexAbstraction abstraction, boolean authByDataStream) { + assertTrue(authByDataStream); + StringMatcher regularNames = StringMatcher.of("c", "b"); + StringMatcher nonDatastreamNames = StringMatcher.of("a", "d"); + if (abstraction.getType() != IndexAbstraction.Type.DATA_STREAM && abstraction.getParentDataStream() == null) { + if (nonDatastreamNames.test(name)) { + return IndicesPermission.AuthorizedComponents.DATA; + } + } + return regularNames.test(name) ? IndicesPermission.AuthorizedComponents.DATA : IndicesPermission.AuthorizedComponents.NONE; + + } + }; Metadata.Builder mb = Metadata.builder( DataStreamTestHelper.getClusterStateWithDataStreams( List.of(Tuple.tuple("a", 1), Tuple.tuple("b", 1), Tuple.tuple("c", 1), Tuple.tuple("d", 1)), @@ -736,7 +814,7 @@ public void testResourceAuthorizedPredicateAnd() { IndexAbstraction concreteIndexB = concreteIndexAbstraction("b"); IndexAbstraction concreteIndexC = concreteIndexAbstraction("c"); IndexAbstraction concreteIndexD = concreteIndexAbstraction("d"); - IndicesPermission.IsResourceAuthorizedPredicate predicate = predicate1.and(predicate2); + IsResourceAuthorizedPredicate predicate = predicate1.alsoRequire(predicate2); assertThat(predicate.test(dataStreamA), is(false)); assertThat(predicate.test(dataStreamB), is(false)); assertThat(predicate.test(dataStreamC), is(true)); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/permission/PermissionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/permission/PermissionTests.java index 6356cde16b3cc..2d711e85fc5e2 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/permission/PermissionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/permission/PermissionTests.java @@ -15,7 +15,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.RestrictedIndices; -import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.IsResourceAuthorizedPredicate; +import org.elasticsearch.xpack.core.security.authz.permission.IsResourceAuthorizedPredicate; import org.elasticsearch.xpack.core.security.authz.permission.Role; import org.elasticsearch.xpack.core.security.authz.privilege.Privilege; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -57,6 +57,15 @@ public void testAllowedIndicesMatcherForMappingUpdates() throws Exception { when(mockIndexAbstraction.getName()).thenReturn("ingest_foo" + randomAlphaOfLength(3)); when(mockIndexAbstraction.getType()).thenReturn(IndexAbstraction.Type.CONCRETE_INDEX); assertThat(indexPredicate.test(mockIndexAbstraction), is(true)); + assertWarnings( + "the index privilege [create] allowed the update mapping action [" + + mappingUpdateActionName + + "] on " + + "index [" + + mockIndexAbstraction.getName() + + "], this privilege will not permit mapping updates in the next major release - " + + "users who require access to update mappings must be granted explicit privileges" + ); when(mockIndexAbstraction.getType()).thenReturn(IndexAbstraction.Type.ALIAS); assertThat(indexPredicate.test(mockIndexAbstraction), is(true)); // mapping updates are NOT permitted on data streams and backing indices diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index ed173d8e2b127..2a4b5c414ed2b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -75,8 +75,9 @@ import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDefinition; +import org.elasticsearch.xpack.core.security.authz.permission.Group; import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission; -import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission.IsResourceAuthorizedPredicate; +import org.elasticsearch.xpack.core.security.authz.permission.IsResourceAuthorizedPredicate; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissionGroup; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; import org.elasticsearch.xpack.core.security.authz.permission.RemoteIndicesPermission; @@ -3237,6 +3238,7 @@ private IndexAbstraction mockIndexAbstraction(String name) { when(mock.getType()).thenReturn( randomFrom(IndexAbstraction.Type.CONCRETE_INDEX, IndexAbstraction.Type.ALIAS, IndexAbstraction.Type.DATA_STREAM) ); + when(mock.getParentDataStream()).thenReturn(null); return mock; } @@ -3344,7 +3346,7 @@ private void assertValidRemoteClusterPermissionsGroups(List remoteClustersAliases, - final Matcher... matchers + final Matcher... matchers ) { assertThat( permission.remoteIndicesGroups() @@ -3357,11 +3359,11 @@ private void assertHasRemoteIndexGroupsForClusters( ); } - private static Matcher indexGroup(final String... indices) { + private static Matcher indexGroup(final String... indices) { return indexGroup(IndexPrivilege.READ, false, indices); } - private static Matcher indexGroup( + private static Matcher indexGroup( final IndexPrivilege privilege, final boolean allowRestrictedIndices, final String... indices @@ -3375,7 +3377,7 @@ private static Matcher indexGroup( ); } - private static Matcher indexGroup( + private static Matcher indexGroup( final IndexPrivilege privilege, final boolean allowRestrictedIndices, @Nullable final String query, @@ -3385,10 +3387,10 @@ private static Matcher indexGroup( return new BaseMatcher<>() { @Override public boolean matches(Object o) { - if (false == o instanceof IndicesPermission.Group) { + if (false == o instanceof Group) { return false; } - final IndicesPermission.Group group = (IndicesPermission.Group) o; + final Group group = (Group) o; return equalTo(query == null ? null : Set.of(new BytesArray(query))).matches(group.getQuery()) && equalTo(privilege).matches(group.privilege()) && equalTo(allowRestrictedIndices).matches(group.allowRestrictedIndices()) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java index af5f44b5989fb..a5837d49c7bb1 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; +import org.elasticsearch.xpack.core.security.authz.permission.Group; import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; import org.elasticsearch.xpack.core.security.authz.permission.Role; @@ -124,7 +125,7 @@ public void testParseFile() throws Exception { assertThat(role.indices().groups().length, is(2)); assertThat(role.runAs(), is(RunAsPermission.NONE)); - IndicesPermission.Group group = role.indices().groups()[0]; + Group group = role.indices().groups()[0]; assertThat(group.indices(), notNullValue()); assertThat(group.indices().length, is(2)); assertThat(group.indices()[0], equalTo("idx1")); diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml index d03e6925cab1f..aa11ed63602c7 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml @@ -16,4 +16,4 @@ setup: # I would much prefer we could just check that specific entries are in the array, but we don't have # an assertion for that - length: { "cluster" : 62 } - - length: { "index" : 22 } + - length: { "index" : 23 }