Skip to content

Commit e36bb81

Browse files
Pull cross-project authorization forward (#136164)
Fixing the wrong order of listener calls. Note: This fix is only relevant when project authorization is used and cross-project enabled. Labeling as `>refactoring` since the feature is under development and not affecting production code. Kudos to @ywangd for tracking the issue down.
1 parent 40c76bc commit e36bb81

File tree

3 files changed

+264
-72
lines changed

3 files changed

+264
-72
lines changed

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
import org.elasticsearch.rest.RestRequest;
104104
import org.elasticsearch.rest.RestStatus;
105105
import org.elasticsearch.script.ScriptService;
106+
import org.elasticsearch.search.crossproject.CrossProjectModeDecider;
106107
import org.elasticsearch.search.internal.ShardSearchRequest;
107108
import org.elasticsearch.telemetry.TelemetryProvider;
108109
import org.elasticsearch.threadpool.ExecutorBuilder;
@@ -1162,7 +1163,8 @@ Collection<Object> createComponents(
11621163
authorizationDenialMessages.get(),
11631164
linkedProjectConfigService,
11641165
projectResolver,
1165-
getCustomAuthorizedProjectsResolverOrDefault(extensionComponents)
1166+
getCustomAuthorizedProjectsResolverOrDefault(extensionComponents),
1167+
new CrossProjectModeDecider(settings)
11661168
);
11671169

11681170
components.add(nativeRolesStore); // used by roles actions

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

Lines changed: 87 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ public AuthorizationService(
172172
AuthorizationDenialMessages authorizationDenialMessages,
173173
LinkedProjectConfigService linkedProjectConfigService,
174174
ProjectResolver projectResolver,
175-
AuthorizedProjectsResolver authorizedProjectsResolver
175+
AuthorizedProjectsResolver authorizedProjectsResolver,
176+
CrossProjectModeDecider crossProjectModeDecider
176177
) {
177178
this.clusterService = clusterService;
178179
this.auditTrailService = auditTrailService;
@@ -181,7 +182,7 @@ public AuthorizationService(
181182
settings,
182183
linkedProjectConfigService,
183184
resolver,
184-
new CrossProjectModeDecider(settings)
185+
crossProjectModeDecider
185186
);
186187
this.authcFailureHandler = authcFailureHandler;
187188
this.threadContext = threadPool.getThreadContext();
@@ -500,82 +501,102 @@ private void authorizeAction(
500501
} else if (isIndexAction(action)) {
501502
final ProjectMetadata projectMetadata = projectResolver.getProjectMetadata(clusterService.state());
502503
assert projectMetadata != null;
503-
final AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>(() -> {
504-
if (request instanceof SearchRequest searchRequest && searchRequest.pointInTimeBuilder() != null) {
505-
var resolvedIndices = indicesAndAliasesResolver.resolvePITIndices(searchRequest);
506-
return SubscribableListener.newSucceeded(resolvedIndices);
507-
}
508-
final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request);
509-
if (resolvedIndices != null) {
510-
return SubscribableListener.newSucceeded(resolvedIndices);
511-
} else {
512-
final SubscribableListener<ResolvedIndices> resolvedIndicesListener = new SubscribableListener<>();
513-
final var authorizedIndicesListener = new SubscribableListener<AuthorizationEngine.AuthorizedIndices>();
514-
authorizedIndicesListener.<Tuple<AuthorizationEngine.AuthorizedIndices, TargetProjects>>andThen(
515-
(l, authorizedIndices) -> {
516-
if (indicesAndAliasesResolver.resolvesCrossProject(request)) {
517-
authorizedProjectsResolver.resolveAuthorizedProjects(
518-
l.map(targetProjects -> new Tuple<>(authorizedIndices, targetProjects))
519-
);
520-
} else {
521-
l.onResponse(new Tuple<>(authorizedIndices, TargetProjects.NOT_CROSS_PROJECT));
522-
}
523-
}
524-
)
504+
final SubscribableListener<TargetProjects> targetProjectListener;
505+
if (indicesAndAliasesResolver.resolvesCrossProject(request)) {
506+
targetProjectListener = new SubscribableListener<>();
507+
authorizedProjectsResolver.resolveAuthorizedProjects(targetProjectListener);
508+
} else {
509+
targetProjectListener = SubscribableListener.newSucceeded(TargetProjects.NOT_CROSS_PROJECT);
510+
}
511+
512+
targetProjectListener.addListener(ActionListener.wrap(targetProjects -> {
513+
final AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier = makeResolvedIndicesAsyncSupplier(
514+
targetProjects,
515+
requestInfo,
516+
requestId,
517+
request,
518+
action,
519+
projectMetadata,
520+
authzInfo,
521+
authzEngine,
522+
auditTrail,
523+
listener
524+
);
525+
526+
// Wrapping here in order to have exceptions thrown from the {@code authorizeIndexAction} method
527+
// get handled directly by the listener and not go through {@code onAuthorizedResourceLoadFailure}
528+
// which wraps them in security exception. This is in order to maintain the same behavior as before.
529+
ActionListener.run(
530+
listener,
531+
l -> authzEngine.authorizeIndexAction(requestInfo, authzInfo, resolvedIndicesAsyncSupplier, projectMetadata)
525532
.addListener(
526-
ActionListener.wrap(
527-
authorizedIndicesAndProjects -> resolvedIndicesListener.onResponse(
528-
indicesAndAliasesResolver.resolve(
529-
action,
530-
request,
533+
wrapPreservingContext(
534+
new AuthorizationResultListener<>(
535+
result -> handleIndexActionAuthorizationResult(
536+
result,
537+
requestInfo,
538+
requestId,
539+
authzInfo,
540+
authzEngine,
541+
resolvedIndicesAsyncSupplier,
531542
projectMetadata,
532-
authorizedIndicesAndProjects.v1(),
533-
authorizedIndicesAndProjects.v2()
534-
)
543+
l
544+
),
545+
l::onFailure,
546+
requestInfo,
547+
requestId,
548+
authzInfo
535549
),
536-
e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e)
550+
threadContext
537551
)
538-
);
539-
540-
authzEngine.loadAuthorizedIndices(
541-
requestInfo,
542-
authzInfo,
543-
projectMetadata.getIndicesLookup(),
544-
authorizedIndicesListener
545-
);
546-
547-
return resolvedIndicesListener;
548-
}
549-
});
550-
authzEngine.authorizeIndexAction(requestInfo, authzInfo, resolvedIndicesAsyncSupplier, projectMetadata)
551-
.addListener(
552-
wrapPreservingContext(
553-
new AuthorizationResultListener<>(
554-
result -> handleIndexActionAuthorizationResult(
555-
result,
556-
requestInfo,
557-
requestId,
558-
authzInfo,
559-
authzEngine,
560-
resolvedIndicesAsyncSupplier,
561-
projectMetadata,
562-
listener
563-
),
564-
listener::onFailure,
565-
requestInfo,
566-
requestId,
567-
authzInfo
568-
),
569-
threadContext
570-
)
552+
)
571553
);
554+
}, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e)));
572555
} else {
573556
logger.warn("denying access for [{}] as action [{}] is not an index or cluster action", authentication, action);
574557
auditTrail.accessDenied(requestId, authentication, action, request, authzInfo);
575558
listener.onFailure(actionDenied(authentication, authzInfo, action, request));
576559
}
577560
}
578561

562+
private AsyncSupplier<ResolvedIndices> makeResolvedIndicesAsyncSupplier(
563+
TargetProjects targetProjects,
564+
RequestInfo requestInfo,
565+
String requestId,
566+
TransportRequest request,
567+
String action,
568+
ProjectMetadata projectMetadata,
569+
AuthorizationInfo authzInfo,
570+
AuthorizationEngine authzEngine,
571+
AuditTrail auditTrail,
572+
ActionListener<Void> listener
573+
) {
574+
return new CachingAsyncSupplier<>(() -> {
575+
if (request instanceof SearchRequest searchRequest && searchRequest.pointInTimeBuilder() != null) {
576+
var resolvedIndices = indicesAndAliasesResolver.resolvePITIndices(searchRequest);
577+
return SubscribableListener.newSucceeded(resolvedIndices);
578+
}
579+
final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request);
580+
if (resolvedIndices != null) {
581+
return SubscribableListener.newSucceeded(resolvedIndices);
582+
} else {
583+
final SubscribableListener<ResolvedIndices> resolvedIndicesListener = new SubscribableListener<>();
584+
authzEngine.loadAuthorizedIndices(
585+
requestInfo,
586+
authzInfo,
587+
projectMetadata.getIndicesLookup(),
588+
ActionListener.wrap(authorizedIndices -> {
589+
resolvedIndicesListener.onResponse(
590+
indicesAndAliasesResolver.resolve(action, request, projectMetadata, authorizedIndices, targetProjects)
591+
);
592+
}, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e))
593+
);
594+
595+
return resolvedIndicesListener;
596+
}
597+
});
598+
}
599+
579600
private void onAuthorizedResourceLoadFailure(
580601
String requestId,
581602
RequestInfo requestInfo,

0 commit comments

Comments
 (0)