Skip to content

Commit bc1cee5

Browse files
committed
Lazily resolve project in authorization
We have optimizations to avoid doing unnecessary authorization work on child actions, and resolving the project can be unnecssary work. This moves project resolution to be lazy and cached so we only do it once but only when actually necessary.
1 parent d9c0ef1 commit bc1cee5

File tree

6 files changed

+28
-14
lines changed

6 files changed

+28
-14
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
* can actually impersonate the user running the request.</li>
7676
* <li>{@link #authorizeClusterAction(RequestInfo, AuthorizationInfo, ActionListener)} if the
7777
* request is a cluster level operation.</li>
78-
* <li>{@link #authorizeIndexAction(RequestInfo, AuthorizationInfo, AsyncSupplier, ProjectMetadata, ActionListener)} if
78+
* <li>{@link #authorizeIndexAction(RequestInfo, AuthorizationInfo, AsyncSupplier, Supplier, ActionListener)} if
7979
* the request is a an index action. This method may be called multiple times for a single
8080
* request as the request may be made up of sub-requests that also need to be authorized. The async supplier
8181
* for resolved indices will invoke the
@@ -85,7 +85,7 @@
8585
* <br><p>
8686
* <em>NOTE:</em> the {@link #loadAuthorizedIndices(RequestInfo, AuthorizationInfo, Map, ActionListener)}
8787
* method may be called prior to
88-
* {@link #authorizeIndexAction(RequestInfo, AuthorizationInfo, AsyncSupplier, ProjectMetadata, ActionListener)}
88+
* {@link #authorizeIndexAction(RequestInfo, AuthorizationInfo, AsyncSupplier, Supplier, ActionListener)}
8989
* in cases where wildcards need to be expanded.
9090
* </p><br>
9191
* Authorization engines can be called from various threads including network threads that should
@@ -167,7 +167,7 @@ void authorizeIndexAction(
167167
RequestInfo requestInfo,
168168
AuthorizationInfo authorizationInfo,
169169
AsyncSupplier<ResolvedIndices> indicesAsyncSupplier,
170-
ProjectMetadata metadata,
170+
Supplier<ProjectMetadata> metadata,
171171
ActionListener<IndexAuthorizationResult> listener
172172
);
173173

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/profile/ProfileCancellationIntegTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import java.util.concurrent.atomic.AtomicBoolean;
5959
import java.util.concurrent.atomic.AtomicLong;
6060
import java.util.concurrent.atomic.AtomicReference;
61+
import java.util.function.Supplier;
6162

6263
import static org.elasticsearch.test.SecuritySettingsSource.TEST_USER_NAME;
6364
import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING;
@@ -410,7 +411,7 @@ public void authorizeIndexAction(
410411
RequestInfo requestInfo,
411412
AuthorizationInfo authorizationInfo,
412413
AsyncSupplier<ResolvedIndices> indicesAsyncSupplier,
413-
ProjectMetadata metadata,
414+
Supplier<ProjectMetadata> metadata,
414415
ActionListener<IndexAuthorizationResult> listener
415416
) {
416417
listener.onResponse(IndexAuthorizationResult.ALLOW_NO_INDICES);

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.elasticsearch.common.settings.Setting;
3939
import org.elasticsearch.common.settings.Setting.Property;
4040
import org.elasticsearch.common.settings.Settings;
41+
import org.elasticsearch.common.util.CachedSupplier;
4142
import org.elasticsearch.common.util.concurrent.ListenableFuture;
4243
import org.elasticsearch.common.util.concurrent.ThreadContext;
4344
import org.elasticsearch.core.Nullable;
@@ -480,8 +481,14 @@ private void authorizeAction(
480481
l.onResponse(result);
481482
}));
482483
} else if (isIndexAction(action)) {
483-
final ProjectMetadata projectMetadata = projectResolver.getProjectMetadata(clusterService.state());
484-
assert projectMetadata != null;
484+
// We use a supplier here to avoid resolving the project in cases where it's not going to be used
485+
// The most common case is when a child action is authorized based on its parent, in that case we never need
486+
// to lookup indices, so we never need the project metadata.
487+
// But we do want to optimize by only looking up the project once for each action, so we use a cached supplier.
488+
final Supplier<ProjectMetadata> projectMetadataSupplier = CachedSupplier.wrap(
489+
() -> projectResolver.getProjectMetadata(clusterService.state())
490+
);
491+
assert projectMetadataSupplier != null;
485492
final AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>(resolvedIndicesListener -> {
486493
if (request instanceof SearchRequest searchRequest && searchRequest.pointInTimeBuilder() != null) {
487494
var resolvedIndices = indicesAndAliasesResolver.resolvePITIndices(searchRequest);
@@ -492,6 +499,7 @@ private void authorizeAction(
492499
if (resolvedIndices != null) {
493500
resolvedIndicesListener.onResponse(resolvedIndices);
494501
} else {
502+
final ProjectMetadata projectMetadata = projectMetadataSupplier.get();
495503
authzEngine.loadAuthorizedIndices(
496504
requestInfo,
497505
authzInfo,
@@ -516,7 +524,7 @@ private void authorizeAction(
516524
requestInfo,
517525
authzInfo,
518526
resolvedIndicesAsyncSupplier,
519-
projectMetadata,
527+
projectMetadataSupplier,
520528
wrapPreservingContext(
521529
new AuthorizationResultListener<>(
522530
result -> handleIndexActionAuthorizationResult(
@@ -526,7 +534,7 @@ private void authorizeAction(
526534
authzInfo,
527535
authzEngine,
528536
resolvedIndicesAsyncSupplier,
529-
projectMetadata,
537+
projectMetadataSupplier,
530538
listener
531539
),
532540
listener::onFailure,
@@ -551,7 +559,7 @@ private void handleIndexActionAuthorizationResult(
551559
final AuthorizationInfo authzInfo,
552560
final AuthorizationEngine authzEngine,
553561
final AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier,
554-
final ProjectMetadata projectMetadata,
562+
final Supplier<ProjectMetadata> projectMetadata,
555563
final ActionListener<Void> listener
556564
) {
557565
final IndicesAccessControl indicesAccessControl = indicesAccessControlWrapper.wrap(result.getIndicesAccessControl());
@@ -763,7 +771,7 @@ private void authorizeBulkItems(
763771
AuthorizationContext bulkAuthzContext,
764772
AuthorizationEngine authzEngine,
765773
AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier,
766-
ProjectMetadata projectMetadata,
774+
Supplier<ProjectMetadata> projectMetadata,
767775
String requestId,
768776
ActionListener<Void> listener
769777
) {

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public void authorizeIndexAction(
318318
RequestInfo requestInfo,
319319
AuthorizationInfo authorizationInfo,
320320
AsyncSupplier<ResolvedIndices> indicesAsyncSupplier,
321-
ProjectMetadata metadata,
321+
Supplier<ProjectMetadata> projectMetadataSupplier,
322322
ActionListener<IndexAuthorizationResult> listener
323323
) {
324324
final String action = requestInfo.getAction();
@@ -423,7 +423,12 @@ public void authorizeIndexAction(
423423
.allMatch(IndicesAliasesRequest.AliasActions::expandAliasesWildcards))
424424
: "expanded wildcards for local indices OR the request should not expand wildcards at all";
425425

426-
IndexAuthorizationResult result = buildIndicesAccessControl(action, role, resolvedIndices, metadata);
426+
IndexAuthorizationResult result = buildIndicesAccessControl(
427+
action,
428+
role,
429+
resolvedIndices,
430+
projectMetadataSupplier.get()
431+
);
427432
if (requestInfo.getAuthentication().isCrossClusterAccess()
428433
&& request instanceof IndicesRequest.RemoteClusterShardRequest shardsRequest
429434
&& shardsRequest.shards() != null) {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3456,7 +3456,7 @@ public void authorizeIndexAction(
34563456
RequestInfo requestInfo,
34573457
AuthorizationInfo authorizationInfo,
34583458
AsyncSupplier<ResolvedIndices> indicesAsyncSupplier,
3459-
ProjectMetadata metadata,
3459+
Supplier<ProjectMetadata> metadata,
34603460
ActionListener<IndexAuthorizationResult> listener
34613461
) {
34623462
throw new UnsupportedOperationException("not implemented");

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1981,7 +1981,7 @@ private void authorizeIndicesAction(
19811981
)
19821982
);
19831983

1984-
engine.authorizeIndexAction(requestInfo, authzInfo, indicesAsyncSupplier, metadata.build().getProject(), listener);
1984+
engine.authorizeIndexAction(requestInfo, authzInfo, indicesAsyncSupplier, metadata.build()::getProject, listener);
19851985
}
19861986

19871987
private static RequestInfo createRequestInfo(TransportRequest request, String action, ParentActionAuthorization parentAuthorization) {

0 commit comments

Comments
 (0)