From 26547201a43266ce9f280992a10601f970a0495e Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 8 Oct 2025 10:46:54 +0200 Subject: [PATCH 1/6] Pull cross-project authorization forward --- .../xpack/security/Security.java | 4 +- .../security/authz/AuthorizationService.java | 159 +++++++++------- .../authz/AuthorizationServiceTests.java | 179 +++++++++++++++++- 3 files changed, 269 insertions(+), 73 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 37e4ea024a858..b449144cb1d98 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -103,6 +103,7 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.search.crossproject.CrossProjectModeDecider; import org.elasticsearch.search.internal.ShardSearchRequest; import org.elasticsearch.telemetry.TelemetryProvider; import org.elasticsearch.threadpool.ExecutorBuilder; @@ -1162,7 +1163,8 @@ Collection createComponents( authorizationDenialMessages.get(), linkedProjectConfigService, projectResolver, - getCustomAuthorizedProjectsResolverOrDefault(extensionComponents) + getCustomAuthorizedProjectsResolverOrDefault(extensionComponents), + new CrossProjectModeDecider(settings) ); components.add(nativeRolesStore); // used by roles actions 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 9297fce4326ca..ebfb51cec4a98 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 @@ -172,7 +172,8 @@ public AuthorizationService( AuthorizationDenialMessages authorizationDenialMessages, LinkedProjectConfigService linkedProjectConfigService, ProjectResolver projectResolver, - AuthorizedProjectsResolver authorizedProjectsResolver + AuthorizedProjectsResolver authorizedProjectsResolver, + CrossProjectModeDecider crossProjectModeDecider ) { this.clusterService = clusterService; this.auditTrailService = auditTrailService; @@ -181,7 +182,7 @@ public AuthorizationService( settings, linkedProjectConfigService, resolver, - new CrossProjectModeDecider(settings) + crossProjectModeDecider ); this.authcFailureHandler = authcFailureHandler; this.threadContext = threadPool.getThreadContext(); @@ -500,75 +501,49 @@ private void authorizeAction( } else if (isIndexAction(action)) { final ProjectMetadata projectMetadata = projectResolver.getProjectMetadata(clusterService.state()); assert projectMetadata != null; - final AsyncSupplier resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>(() -> { - if (request instanceof SearchRequest searchRequest && searchRequest.pointInTimeBuilder() != null) { - var resolvedIndices = indicesAndAliasesResolver.resolvePITIndices(searchRequest); - return SubscribableListener.newSucceeded(resolvedIndices); - } - final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request); - if (resolvedIndices != null) { - return SubscribableListener.newSucceeded(resolvedIndices); - } else { - final SubscribableListener resolvedIndicesListener = new SubscribableListener<>(); - final var authorizedIndicesListener = new SubscribableListener(); - authorizedIndicesListener.>andThen( - (l, authorizedIndices) -> { - if (indicesAndAliasesResolver.resolvesCrossProject(request)) { - authorizedProjectsResolver.resolveAuthorizedProjects( - l.map(targetProjects -> new Tuple<>(authorizedIndices, targetProjects)) - ); - } else { - l.onResponse(new Tuple<>(authorizedIndices, TargetProjects.NOT_CROSS_PROJECT)); - } - } - ) - .addListener( - ActionListener.wrap( - authorizedIndicesAndProjects -> resolvedIndicesListener.onResponse( - indicesAndAliasesResolver.resolve( - action, - request, - projectMetadata, - authorizedIndicesAndProjects.v1(), - authorizedIndicesAndProjects.v2() - ) - ), - e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e) - ) - ); - - authzEngine.loadAuthorizedIndices( - requestInfo, - authzInfo, - projectMetadata.getIndicesLookup(), - authorizedIndicesListener - ); + final var targetProjectListener = new SubscribableListener(); + if (indicesAndAliasesResolver.resolvesCrossProject(request)) { + authorizedProjectsResolver.resolveAuthorizedProjects(targetProjectListener); + } else { + targetProjectListener.onResponse(TargetProjects.NOT_CROSS_PROJECT); + } - return resolvedIndicesListener; - } - }); - authzEngine.authorizeIndexAction(requestInfo, authzInfo, resolvedIndicesAsyncSupplier, projectMetadata) - .addListener( - wrapPreservingContext( - new AuthorizationResultListener<>( - result -> handleIndexActionAuthorizationResult( - result, + targetProjectListener.addListener(ActionListener.wrap(targetProjects -> { + final AsyncSupplier resolvedIndicesAsyncSupplier = makeResolvedIndicesAsyncSupplier( + targetProjects, + requestInfo, + requestId, + request, + action, + projectMetadata, + authzInfo, + authzEngine, + auditTrail, + listener + ); + authzEngine.authorizeIndexAction(requestInfo, authzInfo, resolvedIndicesAsyncSupplier, projectMetadata) + .addListener( + wrapPreservingContext( + new AuthorizationResultListener<>( + result -> handleIndexActionAuthorizationResult( + result, + requestInfo, + requestId, + authzInfo, + authzEngine, + resolvedIndicesAsyncSupplier, + projectMetadata, + listener + ), + listener::onFailure, requestInfo, requestId, - authzInfo, - authzEngine, - resolvedIndicesAsyncSupplier, - projectMetadata, - listener + authzInfo ), - listener::onFailure, - requestInfo, - requestId, - authzInfo - ), - threadContext - ) - ); + threadContext + ) + ); + }, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e))); } else { logger.warn("denying access for [{}] as action [{}] is not an index or cluster action", authentication, action); auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); @@ -576,6 +551,51 @@ private void authorizeAction( } } + private AsyncSupplier makeResolvedIndicesAsyncSupplier( + TargetProjects targetProjects, + RequestInfo requestInfo, + String requestId, + TransportRequest request, + String action, + ProjectMetadata projectMetadata, + AuthorizationInfo authzInfo, + AuthorizationEngine authzEngine, + AuditTrail auditTrail, + ActionListener listener + ) { + return new CachingAsyncSupplier<>(() -> { + if (request instanceof SearchRequest searchRequest && searchRequest.pointInTimeBuilder() != null) { + var resolvedIndices = indicesAndAliasesResolver.resolvePITIndices(searchRequest); + return SubscribableListener.newSucceeded(resolvedIndices); + } + final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request); + if (resolvedIndices != null) { + return SubscribableListener.newSucceeded(resolvedIndices); + } else { + final SubscribableListener resolvedIndicesListener = new SubscribableListener<>(); + authzEngine.loadAuthorizedIndices( + requestInfo, + authzInfo, + projectMetadata.getIndicesLookup(), + ActionListener.wrap(authorizedIndices -> { + resolvedIndicesListener.onResponse( + indicesAndAliasesResolver.resolve( + action, + request, + projectMetadata, + authorizedIndices, + targetProjects + + ) + ); + }, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e)) + ); + + return resolvedIndicesListener; + } + }); + } + private void onAuthorizedResourceLoadFailure( String requestId, RequestInfo requestInfo, @@ -603,6 +623,11 @@ private void onAuthorizedResourceLoadFailure( listener.onFailure(ex); return; } + if (ex instanceof IllegalStateException || ex instanceof IllegalArgumentException) { + logger.warn(() -> Strings.format("failed [%s] action authorization for [%s]", action, authentication), ex); + listener.onFailure(ex); + return; + } auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); if (ex instanceof IndexNotFoundException || ex instanceof NoMatchingProjectException) { listener.onFailure(ex); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index cb11095a5aad8..c2653e3d683e7 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction; import org.elasticsearch.action.admin.indices.recovery.RecoveryAction; import org.elasticsearch.action.admin.indices.recovery.RecoveryRequest; +import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction; import org.elasticsearch.action.admin.indices.segments.IndicesSegmentsAction; import org.elasticsearch.action.admin.indices.segments.IndicesSegmentsRequest; import org.elasticsearch.action.admin.indices.settings.get.GetSettingsAction; @@ -126,6 +127,10 @@ import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.builder.PointInTimeBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.crossproject.CrossProjectModeDecider; +import org.elasticsearch.search.crossproject.ProjectRoutingInfo; +import org.elasticsearch.search.crossproject.ProjectTags; +import org.elasticsearch.search.crossproject.TargetProjects; import org.elasticsearch.search.internal.AliasFilter; import org.elasticsearch.search.internal.ShardSearchContextId; import org.elasticsearch.search.internal.ShardSearchRequest; @@ -271,6 +276,8 @@ public class AuthorizationServiceTests extends ESTestCase { private ProjectResolver projectResolver; private IndexNameExpressionResolver indexNameExpressionResolver; private LinkedProjectConfigService linkedProjectConfigService; + private AuthorizedProjectsResolver authorizedProjectsResolver; + private CrossProjectModeDecider crossProjectModeDecider; @SuppressWarnings("unchecked") @Before @@ -325,6 +332,15 @@ public void setup() { projectResolver = TestProjectResolvers.singleProject(projectId); indexNameExpressionResolver = TestIndexNameExpressionResolver.newInstance(projectResolver); linkedProjectConfigService = new ClusterSettingsLinkedProjectConfigService(settings, clusterSettings, projectResolver); + authorizedProjectsResolver = mock(AuthorizedProjectsResolver.class); + doAnswer(invocation -> { + ActionListener callback = (ActionListener) invocation.getArguments()[0]; + callback.onResponse(TargetProjects.NOT_CROSS_PROJECT); + return null; + }).when(authorizedProjectsResolver).resolveAuthorizedProjects(anyActionListener()); + crossProjectModeDecider = mock(CrossProjectModeDecider.class); + when(crossProjectModeDecider.crossProjectEnabled()).thenReturn(false); + when(crossProjectModeDecider.resolvesCrossProject(any())).thenReturn(false); authorizationService = new AuthorizationService( settings, rolesStore, @@ -343,7 +359,8 @@ public void setup() { new AuthorizationDenialMessages.Default(), linkedProjectConfigService, projectResolver, - new AuthorizedProjectsResolver.Default() + authorizedProjectsResolver, + crossProjectModeDecider ); } @@ -1270,6 +1287,145 @@ public void testSearchAgainstIndex() { verifyNoMoreInteractions(auditTrail); } + public void testResolveIndexActionWithProjectAuthorization() { + final String randomTransientHeader = randomAlphanumericOfLength(11); + final String randomTransientHeaderValue = randomAlphaOfLengthBetween(1, 5); + final ProjectRoutingInfo originProject = createRandomProjectWithAlias(randomAlphaOfLengthBetween(6, 10)); + final ProjectRoutingInfo linkedProject = createRandomProjectWithAlias(randomAlphaOfLengthBetween(1, 5)); + + doAnswer(invocation -> { + @SuppressWarnings("unchecked") + ActionListener callback = (ActionListener) invocation.getArguments()[0]; + threadContext.putTransient(randomTransientHeader, randomTransientHeaderValue); + callback.onResponse(new TargetProjects(originProject, List.of(linkedProject))); + return null; + }).when(authorizedProjectsResolver).resolveAuthorizedProjects(anyActionListener()); + + // signals that cross-project authorization should be invoked + when(crossProjectModeDecider.crossProjectEnabled()).thenReturn(true); + when(crossProjectModeDecider.resolvesCrossProject(any())).thenReturn(true); + final Settings settings = Settings.builder().put("serverless.cross_project.enabled", "true").build(); + + authorizationService = new AuthorizationService( + settings, + rolesStore, + fieldPermissionsCache, + clusterService, + auditTrailService, + new DefaultAuthenticationFailureHandler(Collections.emptyMap()), + threadPool, + new AnonymousUser(settings), + null, + Collections.emptySet(), + new XPackLicenseState(() -> 0), + indexNameExpressionResolver, + operatorPrivilegesService, + RESTRICTED_INDICES, + new AuthorizationDenialMessages.Default(), + linkedProjectConfigService, + projectResolver, + authorizedProjectsResolver, + crossProjectModeDecider + ); + + RoleDescriptor role = new RoleDescriptor( + "resolve_index", + null, + new IndicesPrivileges[] { IndicesPrivileges.builder().indices("index-*").privileges("read").build() }, + null + ); + roleMap.put(role.getName(), role); + final Authentication authentication = createAuthentication(new User("test_resolve_index_user", role.getName())); + final String requestId = AuditUtil.getOrGenerateRequestId(threadContext); + final ResolveIndexAction.Request resolveIndexRequest = new ResolveIndexAction.Request( + new String[] { randomAlphanumericOfLength(8) } + ); + authorize(authentication, ResolveIndexAction.NAME, resolveIndexRequest, true, () -> { + verify(rolesStore).getRoles(Mockito.same(authentication), any()); + assertThat(securityContext.getParentAuthorization(), nullValue()); + assertThat(threadContext.getTransient(randomTransientHeader), sameInstance(randomTransientHeaderValue)); + }); + verify(auditTrail).accessGranted( + eq(requestId), + eq(authentication), + eq(ResolveIndexAction.NAME), + eq(resolveIndexRequest), + authzInfoRoles(new String[] { role.getName() }) + ); + verifyNoMoreInteractions(auditTrail); + } + + public void testResolveIndexActionWithProjectAuthorizationFailure() { + final Exception authzFailure = new ElasticsearchSecurityException("project authz failure"); + doAnswer(invocation -> { + @SuppressWarnings("unchecked") + ActionListener callback = (ActionListener) invocation.getArguments()[0]; + callback.onFailure(authzFailure); + return null; + }).when(authorizedProjectsResolver).resolveAuthorizedProjects(anyActionListener()); + + // signals that cross-project authorization should be invoked + when(crossProjectModeDecider.crossProjectEnabled()).thenReturn(true); + when(crossProjectModeDecider.resolvesCrossProject(any())).thenReturn(true); + final Settings settings = Settings.builder().put("serverless.cross_project.enabled", "true").build(); + + authorizationService = new AuthorizationService( + settings, + rolesStore, + fieldPermissionsCache, + clusterService, + auditTrailService, + new DefaultAuthenticationFailureHandler(Collections.emptyMap()), + threadPool, + new AnonymousUser(settings), + null, + Collections.emptySet(), + new XPackLicenseState(() -> 0), + indexNameExpressionResolver, + operatorPrivilegesService, + RESTRICTED_INDICES, + new AuthorizationDenialMessages.Default(), + linkedProjectConfigService, + projectResolver, + authorizedProjectsResolver, + crossProjectModeDecider + ); + + RoleDescriptor role = new RoleDescriptor( + "resolve_index", + null, + new IndicesPrivileges[] { IndicesPrivileges.builder().indices("index-*").privileges("read").build() }, + null + ); + roleMap.put(role.getName(), role); + final Authentication authentication = createAuthentication(new User("test_resolve_index_user", role.getName())); + final String requestId = AuditUtil.getOrGenerateRequestId(threadContext); + final ResolveIndexAction.Request resolveIndexRequest = new ResolveIndexAction.Request( + new String[] { randomAlphanumericOfLength(8) } + ); + var e = expectThrows( + ElasticsearchSecurityException.class, + () -> authorize(authentication, ResolveIndexAction.NAME, resolveIndexRequest, true, null) + ); + assertThat( + e.getMessage(), + containsString( + "action [indices:admin/resolve/index] is unauthorized for user " + + "[test_resolve_index_user] with effective roles [resolve_index]" + ) + ); + assertThat(e.getCause().getMessage(), containsString("project authz failure")); + + verify(auditTrail).accessDenied( + eq(requestId), + eq(authentication), + eq(ResolveIndexAction.NAME), + eq(resolveIndexRequest), + authzInfoRoles(new String[] { role.getName() }) + ); + verifyNoMoreInteractions(auditTrail); + } + public void testSearchPITAgainstIndex() { RoleDescriptor role = new RoleDescriptor( "search_index", @@ -1778,7 +1934,8 @@ public void testDenialForAnonymousUser() { new AuthorizationDenialMessages.Default(), linkedProjectConfigService, projectResolver, - new AuthorizedProjectsResolver.Default() + new AuthorizedProjectsResolver.Default(), + new CrossProjectModeDecider(settings) ); RoleDescriptor role = new RoleDescriptor( @@ -1830,7 +1987,8 @@ public void testDenialForAnonymousUserAuthorizationExceptionDisabled() { new AuthorizationDenialMessages.Default(), linkedProjectConfigService, projectResolver, - new AuthorizedProjectsResolver.Default() + new AuthorizedProjectsResolver.Default(), + new CrossProjectModeDecider(settings) ); RoleDescriptor role = new RoleDescriptor( @@ -3370,7 +3528,8 @@ public void testAuthorizationEngineSelectionForCheckPrivileges() throws Exceptio new AuthorizationDenialMessages.Default(), linkedProjectConfigService, projectResolver, - new AuthorizedProjectsResolver.Default() + new AuthorizedProjectsResolver.Default(), + new CrossProjectModeDecider(Settings.EMPTY) ); Subject subject = new Subject(new User("test", "a role"), mock(RealmRef.class)); @@ -3528,7 +3687,8 @@ public void getUserPrivileges(AuthorizationInfo authorizationInfo, ActionListene new AuthorizationDenialMessages.Default(), linkedProjectConfigService, projectResolver, - new AuthorizedProjectsResolver.Default() + new AuthorizedProjectsResolver.Default(), + new CrossProjectModeDecider(Settings.EMPTY) ); Authentication authentication; try (StoredContext ignore = threadContext.stashContext()) { @@ -3714,6 +3874,15 @@ private static BytesReference createEncodedPIT(Index index) { return SearchContextId.encode(results, Collections.emptyMap(), TransportVersion.current(), ShardSearchFailure.EMPTY_ARRAY); } + private static ProjectRoutingInfo createRandomProjectWithAlias(String alias) { + ProjectId projectId = randomUniqueProjectId(); + String type = randomFrom("elasticsearch", "security", "observability"); + String org = randomAlphaOfLength(10); + Map tags = Map.of("_id", projectId.id(), "_type", type, "_organization", org, "_alias", alias); + ProjectTags projectTags = new ProjectTags(tags); + return new ProjectRoutingInfo(projectId, type, alias, org, projectTags); + } + private static class RBACAuthorizationInfoRoleMatcher implements ArgumentMatcher { private final String[] wanted; From 46f9173bb76b3858de7f6d2865c794024d6c1ccd Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 8 Oct 2025 08:55:30 +0000 Subject: [PATCH 2/6] [CI] Auto commit changes from spotless --- .../xpack/security/authz/AuthorizationServiceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index c2653e3d683e7..da5b0f412b7fb 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -1415,7 +1415,7 @@ public void testResolveIndexActionWithProjectAuthorizationFailure() { ) ); assertThat(e.getCause().getMessage(), containsString("project authz failure")); - + verify(auditTrail).accessDenied( eq(requestId), eq(authentication), From d5639b641bd15bcd9c91c4c12c9b184745e3ba0f Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 8 Oct 2025 11:12:52 +0200 Subject: [PATCH 3/6] special error handling only for IllegalStateException --- .../xpack/security/authz/AuthorizationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ebfb51cec4a98..1e6cb7e43bda5 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 @@ -623,7 +623,7 @@ private void onAuthorizedResourceLoadFailure( listener.onFailure(ex); return; } - if (ex instanceof IllegalStateException || ex instanceof IllegalArgumentException) { + if (ex instanceof IllegalStateException) { logger.warn(() -> Strings.format("failed [%s] action authorization for [%s]", action, authentication), ex); listener.onFailure(ex); return; From 5f6b2568c8d90555a49962f8ff0ef2dfacda5d78 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 8 Oct 2025 11:43:34 +0200 Subject: [PATCH 4/6] special handling for `IllegalArgumentException` to let 400 result in 400 --- .../xpack/security/authz/AuthorizationService.java | 8 ++++++++ 1 file changed, 8 insertions(+) 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 1e6cb7e43bda5..8c98b105996cd 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 @@ -628,6 +628,14 @@ private void onAuthorizedResourceLoadFailure( listener.onFailure(ex); return; } + if (ex instanceof IllegalArgumentException) { + logger.debug( + () -> Strings.format("failed [%s] action authorization for [%s]. Reason [%s]", action, authentication, ex.getMessage()), + ex + ); + listener.onFailure(ex); + return; + } auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); if (ex instanceof IndexNotFoundException || ex instanceof NoMatchingProjectException) { listener.onFailure(ex); From 7ff77b535670b8969515194584fe1201e3efd8f9 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Wed, 8 Oct 2025 16:51:44 +0200 Subject: [PATCH 5/6] add new ActionListener implementation to maintain bwc behaviour in regards to the error handling --- .../security/authz/AuthorizationService.java | 80 +++++++++++++------ 1 file changed, 57 insertions(+), 23 deletions(-) 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 8c98b105996cd..98b05e9c41882 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 @@ -43,6 +43,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ListenableFuture; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.core.CheckedConsumer; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.IndexNotFoundException; @@ -508,7 +509,7 @@ private void authorizeAction( targetProjectListener.onResponse(TargetProjects.NOT_CROSS_PROJECT); } - targetProjectListener.addListener(ActionListener.wrap(targetProjects -> { + targetProjectListener.addListener(actionListenerWithErrorHandling(targetProjects -> { final AsyncSupplier resolvedIndicesAsyncSupplier = makeResolvedIndicesAsyncSupplier( targetProjects, requestInfo, @@ -543,7 +544,7 @@ private void authorizeAction( threadContext ) ); - }, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e))); + }, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e), listener::onFailure)); } else { logger.warn("denying access for [{}] as action [{}] is not an index or cluster action", authentication, action); auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); @@ -579,14 +580,7 @@ private AsyncSupplier makeResolvedIndicesAsyncSupplier( projectMetadata.getIndicesLookup(), ActionListener.wrap(authorizedIndices -> { resolvedIndicesListener.onResponse( - indicesAndAliasesResolver.resolve( - action, - request, - projectMetadata, - authorizedIndices, - targetProjects - - ) + indicesAndAliasesResolver.resolve(action, request, projectMetadata, authorizedIndices, targetProjects) ); }, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e)) ); @@ -623,19 +617,6 @@ private void onAuthorizedResourceLoadFailure( listener.onFailure(ex); return; } - if (ex instanceof IllegalStateException) { - logger.warn(() -> Strings.format("failed [%s] action authorization for [%s]", action, authentication), ex); - listener.onFailure(ex); - return; - } - if (ex instanceof IllegalArgumentException) { - logger.debug( - () -> Strings.format("failed [%s] action authorization for [%s]. Reason [%s]", action, authentication, ex.getMessage()), - ex - ); - listener.onFailure(ex); - return; - } auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); if (ex instanceof IndexNotFoundException || ex instanceof NoMatchingProjectException) { listener.onFailure(ex); @@ -1192,6 +1173,59 @@ public SubscribableListener getAsync() { } } + /** + * Creates a listener that executes the appropriate consumer when the response (or failure) is received. + * Any exceptions thrown from the {@code onResponse} consumer is passed into the {@code errorHandler} consumer. + * This is the main difference from {@link ActionListener#wrap(CheckedConsumer, Consumer)}. + * + * @param onResponse the checked consumer of the response, executed when the listener is completed successfully. If it throws an + * exception, the exception is passed to the {@code errorHandler} consumer. + * @param onFailure the consumer of the failure, executed when the listener is completed with a failure + * @param errorHandler the consumer of exceptions thrown by the {@code onResponse} consumer + * @param the type of the response + * @return a listener that executes the appropriate consumer when the response (or failure) is received. + */ + private static ActionListener actionListenerWithErrorHandling( + CheckedConsumer onResponse, + Consumer onFailure, + Consumer errorHandler + ) { + return new ActionListener<>() { + @Override + public void onResponse(Response response) { + try { + onResponse.accept(response); + } catch (Exception e) { + safeAcceptException(errorHandler, e); + } + } + + @Override + public void onFailure(Exception e) { + safeAcceptException(onFailure, e); + } + + @Override + public String toString() { + return "ActionListenerWithErrorHandler{" + onResponse + "}{" + onFailure + "}{" + errorHandler + "}"; + } + }; + } + + private static void safeAcceptException(Consumer consumer, Exception e) { + assert e != null; + try { + consumer.accept(e); + } catch (RuntimeException ex) { + // noinspection ConstantConditions + if (e != null && ex != e) { + ex.addSuppressed(e); + } + assert false : ex; + throw ex; + } + } + public static void addSettings(List> settings) { settings.add(ANONYMOUS_AUTHORIZATION_EXCEPTION_SETTING); settings.addAll(LoadAuthorizedIndicesTimeChecker.Factory.getSettings()); From ee4f6eba6063124ccb09e1c3b34c4d061ee116e6 Mon Sep 17 00:00:00 2001 From: Slobodan Adamovic Date: Thu, 9 Oct 2025 10:51:39 +0200 Subject: [PATCH 6/6] apply review suggestions --- .../security/authz/AuthorizationService.java | 106 +++++------------- 1 file changed, 30 insertions(+), 76 deletions(-) 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 98b05e9c41882..4869dffc1f6d4 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 @@ -43,7 +43,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ListenableFuture; import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.core.CheckedConsumer; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.IndexNotFoundException; @@ -502,14 +501,15 @@ private void authorizeAction( } else if (isIndexAction(action)) { final ProjectMetadata projectMetadata = projectResolver.getProjectMetadata(clusterService.state()); assert projectMetadata != null; - final var targetProjectListener = new SubscribableListener(); + final SubscribableListener targetProjectListener; if (indicesAndAliasesResolver.resolvesCrossProject(request)) { + targetProjectListener = new SubscribableListener<>(); authorizedProjectsResolver.resolveAuthorizedProjects(targetProjectListener); } else { - targetProjectListener.onResponse(TargetProjects.NOT_CROSS_PROJECT); + targetProjectListener = SubscribableListener.newSucceeded(TargetProjects.NOT_CROSS_PROJECT); } - targetProjectListener.addListener(actionListenerWithErrorHandling(targetProjects -> { + targetProjectListener.addListener(ActionListener.wrap(targetProjects -> { final AsyncSupplier resolvedIndicesAsyncSupplier = makeResolvedIndicesAsyncSupplier( targetProjects, requestInfo, @@ -522,29 +522,36 @@ private void authorizeAction( auditTrail, listener ); - authzEngine.authorizeIndexAction(requestInfo, authzInfo, resolvedIndicesAsyncSupplier, projectMetadata) - .addListener( - wrapPreservingContext( - new AuthorizationResultListener<>( - result -> handleIndexActionAuthorizationResult( - result, + + // Wrapping here in order to have exceptions thrown from the {@code authorizeIndexAction} method + // get handled directly by the listener and not go through {@code onAuthorizedResourceLoadFailure} + // which wraps them in security exception. This is in order to maintain the same behavior as before. + ActionListener.run( + listener, + l -> authzEngine.authorizeIndexAction(requestInfo, authzInfo, resolvedIndicesAsyncSupplier, projectMetadata) + .addListener( + wrapPreservingContext( + new AuthorizationResultListener<>( + result -> handleIndexActionAuthorizationResult( + result, + requestInfo, + requestId, + authzInfo, + authzEngine, + resolvedIndicesAsyncSupplier, + projectMetadata, + l + ), + l::onFailure, requestInfo, requestId, - authzInfo, - authzEngine, - resolvedIndicesAsyncSupplier, - projectMetadata, - listener + authzInfo ), - listener::onFailure, - requestInfo, - requestId, - authzInfo - ), - threadContext + threadContext + ) ) - ); - }, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e), listener::onFailure)); + ); + }, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e))); } else { logger.warn("denying access for [{}] as action [{}] is not an index or cluster action", authentication, action); auditTrail.accessDenied(requestId, authentication, action, request, authzInfo); @@ -1173,59 +1180,6 @@ public SubscribableListener getAsync() { } } - /** - * Creates a listener that executes the appropriate consumer when the response (or failure) is received. - * Any exceptions thrown from the {@code onResponse} consumer is passed into the {@code errorHandler} consumer. - * This is the main difference from {@link ActionListener#wrap(CheckedConsumer, Consumer)}. - * - * @param onResponse the checked consumer of the response, executed when the listener is completed successfully. If it throws an - * exception, the exception is passed to the {@code errorHandler} consumer. - * @param onFailure the consumer of the failure, executed when the listener is completed with a failure - * @param errorHandler the consumer of exceptions thrown by the {@code onResponse} consumer - * @param the type of the response - * @return a listener that executes the appropriate consumer when the response (or failure) is received. - */ - private static ActionListener actionListenerWithErrorHandling( - CheckedConsumer onResponse, - Consumer onFailure, - Consumer errorHandler - ) { - return new ActionListener<>() { - @Override - public void onResponse(Response response) { - try { - onResponse.accept(response); - } catch (Exception e) { - safeAcceptException(errorHandler, e); - } - } - - @Override - public void onFailure(Exception e) { - safeAcceptException(onFailure, e); - } - - @Override - public String toString() { - return "ActionListenerWithErrorHandler{" + onResponse + "}{" + onFailure + "}{" + errorHandler + "}"; - } - }; - } - - private static void safeAcceptException(Consumer consumer, Exception e) { - assert e != null; - try { - consumer.accept(e); - } catch (RuntimeException ex) { - // noinspection ConstantConditions - if (e != null && ex != e) { - ex.addSuppressed(e); - } - assert false : ex; - throw ex; - } - } - public static void addSettings(List> settings) { settings.add(ANONYMOUS_AUTHORIZATION_EXCEPTION_SETTING); settings.addAll(LoadAuthorizedIndicesTimeChecker.Factory.getSettings());