diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index bde558e33dc98..2eec561020969 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -298,39 +298,32 @@ private void authorizeAction(final RequestInfo requestInfo, final String request }, clusterAuthzListener::onFailure)); } else if (isIndexAction(action)) { final Metadata metadata = clusterService.state().metadata(); - final AsyncSupplier> authorizedIndicesSupplier = new CachingAsyncSupplier<>(authzIndicesListener -> { - LoadAuthorizedIndiciesTimeChecker timeChecker = LoadAuthorizedIndiciesTimeChecker.start(requestInfo, authzInfo); + final AsyncSupplier resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>(resolvedIndicesListener -> { + final LoadAuthorizedIndiciesTimeChecker timeChecker = LoadAuthorizedIndiciesTimeChecker.start(requestInfo, authzInfo); authzEngine.loadAuthorizedIndices( - requestInfo, - authzInfo, - metadata.getIndicesLookup(), - authzIndicesListener.map(authzIndices -> { - timeChecker.done(authzIndices); - return authzIndices; - }) - ); - }); - final AsyncSupplier resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>(resolvedIndicesListener -> - authorizedIndicesSupplier.getAsync( + requestInfo, + authzInfo, + metadata.getIndicesLookup(), ActionListener.wrap( - authorizedIndices -> - resolvedIndicesListener.onResponse( - indicesAndAliasesResolver.resolve(action, request, metadata, authorizedIndices) - ), - e -> { - auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); - if (e instanceof IndexNotFoundException) { - listener.onFailure(e); - } else { - listener.onFailure(denialException(authentication, action, request, e)); + authorizedIndices -> { + timeChecker.done(authorizedIndices); + resolvedIndicesListener.onResponse( + indicesAndAliasesResolver.resolve(action, request, metadata, authorizedIndices) + ); + }, + e -> { + auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); + if (e instanceof IndexNotFoundException) { + listener.onFailure(e); + } else { + listener.onFailure(denialException(authentication, action, request, e)); + } } - } - ) - ) - ); + )); + }); authzEngine.authorizeIndexAction(requestInfo, authzInfo, resolvedIndicesAsyncSupplier, metadata.getIndicesLookup(), wrapPreservingContext(new AuthorizationResultListener<>(result -> - handleIndexActionAuthorizationResult(result, requestInfo, requestId, authzInfo, authzEngine, authorizedIndicesSupplier, + handleIndexActionAuthorizationResult(result, requestInfo, requestId, authzInfo, authzEngine, resolvedIndicesAsyncSupplier, metadata, listener), listener::onFailure, requestInfo, requestId, authzInfo), threadContext)); } else { @@ -343,7 +336,6 @@ private void authorizeAction(final RequestInfo requestInfo, final String request private void handleIndexActionAuthorizationResult(final IndexAuthorizationResult result, final RequestInfo requestInfo, final String requestId, final AuthorizationInfo authzInfo, final AuthorizationEngine authzEngine, - final AsyncSupplier> authorizedIndicesSupplier, final AsyncSupplier resolvedIndicesAsyncSupplier, final Metadata metadata, final ActionListener listener) { @@ -383,7 +375,7 @@ private void handleIndexActionAuthorizationResult(final IndexAuthorizationResult // if this is performing multiple actions on the index, then check each of those actions. assert request instanceof BulkShardRequest : "Action " + action + " requires " + BulkShardRequest.class + " but was " + request.getClass(); - authorizeBulkItems(requestInfo, authzInfo, authzEngine, resolvedIndicesAsyncSupplier, authorizedIndicesSupplier, metadata, + authorizeBulkItems(requestInfo, authzInfo, authzEngine, resolvedIndicesAsyncSupplier, metadata, requestId, wrapPreservingContext( ActionListener.wrap(ignore -> runRequestInterceptors(requestInfo, authzInfo, authorizationEngine, listener), @@ -509,7 +501,6 @@ private void authorizeRunAs(final RequestInfo requestInfo, final AuthorizationIn */ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authzInfo, AuthorizationEngine authzEngine, AsyncSupplier resolvedIndicesAsyncSupplier, - AsyncSupplier> authorizedIndicesSupplier, Metadata metadata, String requestId, ActionListener listener) { final Authentication authentication = requestInfo.getAuthentication(); final BulkShardRequest request = (BulkShardRequest) requestInfo.getRequest(); @@ -519,44 +510,43 @@ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authz final Map> actionToIndicesMap = new HashMap<>(); final AuditTrail auditTrail = auditTrailService.get(); - authorizedIndicesSupplier.getAsync(ActionListener.wrap(authorizedIndices -> { - resolvedIndicesAsyncSupplier.getAsync(ActionListener.wrap(overallResolvedIndices -> { - final Set localIndices = new HashSet<>(overallResolvedIndices.getLocal()); - for (BulkItemRequest item : request.items()) { - final String itemAction = getAction(item); - String resolvedIndex = resolvedIndexNames.computeIfAbsent(item.index(), key -> { - final ResolvedIndices resolvedIndices = - indicesAndAliasesResolver.resolveIndicesAndAliases(itemAction, item.request(), metadata, authorizedIndices); - if (resolvedIndices.getRemote().size() != 0) { - throw illegalArgument("Bulk item should not write to remote indices, but request writes to " + resolvedIndicesAsyncSupplier.getAsync(ActionListener.wrap(overallResolvedIndices -> { + final Set localIndices = new HashSet<>(overallResolvedIndices.getLocal()); + for (BulkItemRequest item : request.items()) { + final String itemAction = getAction(item); + String resolvedIndex = resolvedIndexNames.computeIfAbsent(item.index(), key -> { + final ResolvedIndices resolvedIndices = + indicesAndAliasesResolver.resolveIndicesAndAliases(itemAction, item.request(), metadata, localIndices); + if (resolvedIndices.getRemote().size() != 0) { + throw illegalArgument("Bulk item should not write to remote indices, but request writes to " + String.join(",", resolvedIndices.getRemote())); - } - if (resolvedIndices.getLocal().size() != 1) { - throw illegalArgument("Bulk item should write to exactly 1 index, but request writes to " + } + if (resolvedIndices.getLocal().size() != 1) { + throw illegalArgument("Bulk item should write to exactly 1 index, but request writes to " + String.join(",", resolvedIndices.getLocal())); - } - final String resolved = resolvedIndices.getLocal().get(0); - if (localIndices.contains(resolved) == false) { - throw illegalArgument("Found bulk item that writes to index " + resolved + " but the request writes to " + + } + final String resolved = resolvedIndices.getLocal().get(0); + if (localIndices.contains(resolved) == false) { + throw illegalArgument("Found bulk item that writes to index [" + resolved + "] but the request writes to " + localIndices); - } - return resolved; - }); - - actionToIndicesMap.compute(itemAction, (key, resolvedIndicesSet) -> { - final Set localSet = resolvedIndicesSet != null ? resolvedIndicesSet : new HashSet<>(); - localSet.add(resolvedIndex); - return localSet; - }); - } + } + return resolved; + }); - final ActionListener>> bulkAuthzListener = + actionToIndicesMap.compute(itemAction, (key, resolvedIndicesSet) -> { + final Set localSet = resolvedIndicesSet != null ? resolvedIndicesSet : new HashSet<>(); + localSet.add(resolvedIndex); + return localSet; + }); + } + + final ActionListener>> bulkAuthzListener = ActionListener.wrap(collection -> { final Map actionToIndicesAccessControl = new HashMap<>(); final AtomicBoolean audit = new AtomicBoolean(false); collection.forEach(tuple -> { final IndicesAccessControl existing = - actionToIndicesAccessControl.putIfAbsent(tuple.v1(), tuple.v2().getIndicesAccessControl()); + actionToIndicesAccessControl.putIfAbsent(tuple.v1(), tuple.v2().getIndicesAccessControl()); if (existing != null) { throw new IllegalStateException("a value already exists for action " + tuple.v1()); } @@ -570,12 +560,12 @@ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authz final String itemAction = getAction(item); final IndicesAccessControl indicesAccessControl = actionToIndicesAccessControl.get(itemAction); final IndicesAccessControl.IndexAccessControl indexAccessControl - = indicesAccessControl.getIndexPermissions(resolvedIndex); + = indicesAccessControl.getIndexPermissions(resolvedIndex); if (indexAccessControl == null || indexAccessControl.isGranted() == false) { auditTrail.explicitIndexAccessEvent(requestId, AuditLevel.ACCESS_DENIED, authentication, itemAction, resolvedIndex, item.getClass().getSimpleName(), request.remoteAddress(), authzInfo); item.abort(resolvedIndex, denialException(authentication, itemAction, request, - AuthorizationEngine.IndexAuthorizationResult.getFailureDescription(List.of(resolvedIndex)), null)); + AuthorizationEngine.IndexAuthorizationResult.getFailureDescription(List.of(resolvedIndex)), null)); } else if (audit.get()) { auditTrail.explicitIndexAccessEvent(requestId, AuditLevel.ACCESS_GRANTED, authentication, itemAction, resolvedIndex, item.getClass().getSimpleName(), request.remoteAddress(), authzInfo); @@ -583,19 +573,18 @@ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authz } listener.onResponse(null); }, listener::onFailure); - final ActionListener> groupedActionListener = wrapPreservingContext( + final ActionListener> groupedActionListener = wrapPreservingContext( new GroupedActionListener<>(bulkAuthzListener, actionToIndicesMap.size()), threadContext); - actionToIndicesMap.forEach((bulkItemAction, indices) -> { - final RequestInfo bulkItemInfo = + actionToIndicesMap.forEach((bulkItemAction, indices) -> { + final RequestInfo bulkItemInfo = new RequestInfo(requestInfo.getAuthentication(), requestInfo.getRequest(), bulkItemAction); - authzEngine.authorizeIndexAction(bulkItemInfo, authzInfo, + authzEngine.authorizeIndexAction(bulkItemInfo, authzInfo, ril -> ril.onResponse(new ResolvedIndices(new ArrayList<>(indices), Collections.emptyList())), metadata.getIndicesLookup(), ActionListener.wrap(indexAuthorizationResult -> - groupedActionListener.onResponse(new Tuple<>(bulkItemAction, indexAuthorizationResult)), - groupedActionListener::onFailure)); - }); - }, listener::onFailure)); + groupedActionListener.onResponse(new Tuple<>(bulkItemAction, indexAuthorizationResult)), + groupedActionListener::onFailure)); + }); }, listener::onFailure)); } 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 261decc281834..8f44055e9ebb1 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 @@ -22,9 +22,12 @@ import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.bulk.BulkShardRequest; +import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesAction; import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest; import org.elasticsearch.action.get.MultiGetRequest; +import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.search.MultiSearchAction; import org.elasticsearch.action.search.MultiSearchRequest; import org.elasticsearch.action.search.SearchAction; @@ -32,6 +35,7 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.termvectors.MultiTermVectorsRequest; +import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.cluster.metadata.AliasMetadata; import org.elasticsearch.cluster.metadata.DataStream; import org.elasticsearch.cluster.metadata.DataStreamTestHelper; @@ -306,47 +310,54 @@ public void testDashIndicesAreAllowedInShardLevelRequests() { } public void testWildcardsAreNotAllowedInShardLevelRequests() { - ShardSearchRequest request = mock(ShardSearchRequest.class); - when(request.indices()).thenReturn(new String[]{"index*"}); - IllegalArgumentException exception = expectThrows( - IllegalArgumentException.class, - () -> resolveIndices(SearchAction.NAME + "[s]", request, buildAuthorizedIndices(userDashIndices, SearchAction.NAME)).getLocal() - ); - assertThat( - exception, - throwableWithMessage( - "the action indices:data/read/search[s] does not support wildcards;" - + " the provided index expression(s) [index*] are not allowed" - ) - ); + for (IndicesRequest request : List.of(mock(DeleteRequest.class), mock(IndexRequest.class), + mock(UpdateRequest.class), mock(BulkShardRequest.class), mock(ShardSearchRequest.class))) { + when(request.indices()).thenReturn(new String[]{"index*"}); + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> resolveIndices(SearchAction.NAME + "[s]", (TransportRequest) request, + buildAuthorizedIndices(userDashIndices, SearchAction.NAME)).getLocal() + ); + assertThat( + exception, + throwableWithMessage( + "the action indices:data/read/search[s] does not support wildcards;" + + " the provided index expression(s) [index*] are not allowed" + ) + ); + } } public void testAllIsNotAllowedInShardLevelRequests() { - ShardSearchRequest request = mock(ShardSearchRequest.class); - final boolean literalAll = randomBoolean(); - if (literalAll) { - when(request.indices()).thenReturn(new String[]{"_all"}); - } else { - if (randomBoolean()) { - when(request.indices()).thenReturn(Strings.EMPTY_ARRAY); + for (IndicesRequest request : List.of(mock(DeleteRequest.class), mock(IndexRequest.class), + mock(UpdateRequest.class), mock(BulkShardRequest.class), mock(ShardSearchRequest.class))) { + final boolean literalAll = randomBoolean(); + if (literalAll) { + when(request.indices()).thenReturn(new String[]{"_all"}); } else { - when(request.indices()).thenReturn(null); + if (randomBoolean()) { + when(request.indices()).thenReturn(Strings.EMPTY_ARRAY); + } else { + when(request.indices()).thenReturn(null); + } } + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> resolveIndices(SearchAction.NAME + "[s]", (TransportRequest) request, + buildAuthorizedIndices(userDashIndices, SearchAction.NAME)).getLocal() + ); + assertThat( + exception, + literalAll + ? throwableWithMessage( + "the action indices:data/read/search[s] does not support accessing all indices;" + + " the provided index expression [_all] is not allowed" + ) + : + throwableWithMessage("the action indices:data/read/search[s] requires explicit index names, " + + "but none were provided") + ); } - IllegalArgumentException exception = expectThrows( - IllegalArgumentException.class, - () -> resolveIndices(SearchAction.NAME + "[s]", request, buildAuthorizedIndices(userDashIndices, SearchAction.NAME)).getLocal() - ); - - assertThat( - exception, - literalAll - ? throwableWithMessage( - "the action indices:data/read/search[s] does not support accessing all indices;" - + " the provided index expression [_all] is not allowed" - ) - : throwableWithMessage("the action indices:data/read/search[s] requires explicit index names, but none were provided") - ); } public void testExplicitDashIndices() {