Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
eba4f92
record security exceptions in resolved index expressions
richard-dennehy Sep 29, 2025
e5e3e12
[CI] Auto commit changes from spotless
Sep 29, 2025
d2f5563
override equality in LocalExpressions
richard-dennehy Sep 29, 2025
c236591
merge main
richard-dennehy Oct 7, 2025
e7e8c16
[CI] Auto commit changes from spotless
Oct 7, 2025
6c302bd
record exception for unresolved index when ignore_unavailable is true
richard-dennehy Oct 9, 2025
359fca7
clean up
richard-dennehy Oct 9, 2025
1fdb0a8
spotless
richard-dennehy Oct 9, 2025
4a3ffd4
Merge remote-tracking branch 'upstream/main' into resolved-index-expr…
richard-dennehy Oct 10, 2025
98a50b9
enable CPS mode in test
richard-dennehy Oct 10, 2025
9d89502
Merge remote-tracking branch 'upstream/main' into resolved-index-expr…
richard-dennehy Oct 13, 2025
f143685
include index name in exception message
richard-dennehy Oct 13, 2025
6109ecb
[CI] Auto commit changes from spotless
Oct 13, 2025
b81d263
merge main
richard-dennehy Oct 24, 2025
69e46f3
address review comments
richard-dennehy Oct 24, 2025
a0e594f
[CI] Auto commit changes from spotless
Oct 24, 2025
d99f487
assert exception is not set twice
richard-dennehy Oct 24, 2025
dfb3081
use recorded exception in CrossProjectIndexResolutionValidator
richard-dennehy Oct 27, 2025
38115ad
[CI] Auto commit changes from spotless
Oct 27, 2025
88f63a3
actually only set exception if unset
richard-dennehy Oct 28, 2025
9aef86f
Merge branch 'main' into resolved-index-expression-record-exceptions
richard-dennehy Oct 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@

package org.elasticsearch.action;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.core.Nullable;

import java.io.IOException;
import java.util.Objects;
import java.util.Set;

/**
Expand Down Expand Up @@ -49,6 +52,8 @@ public record ResolvedIndexExpression(String original, LocalExpressions localExp
implements
Writeable {

private static final Logger logger = LogManager.getLogger(ResolvedIndexExpression.class);

public ResolvedIndexExpression(StreamInput in) throws IOException {
this(in.readString(), new LocalExpressions(in), in.readCollectionAsImmutableSet(StreamInput::readString));
}
Expand Down Expand Up @@ -76,17 +81,83 @@ public enum LocalIndexResolutionResult {

/**
* Represents local (non-remote) resolution results, including expanded indices, and a {@link LocalIndexResolutionResult}.
*
* @param indices represents the resolved concrete indices backing the expression
*/
public record LocalExpressions(
Set<String> indices,
LocalIndexResolutionResult localIndexResolutionResult,
@Nullable ElasticsearchException exception
) implements Writeable {
public LocalExpressions {
public static final class LocalExpressions implements Writeable {
private final Set<String> indices;
private final LocalIndexResolutionResult localIndexResolutionResult;
@Nullable
private ElasticsearchException exception;

/**
* @param indices represents the resolved concrete indices backing the expression
*/
public LocalExpressions(
Set<String> indices,
LocalIndexResolutionResult localIndexResolutionResult,
@Nullable ElasticsearchException exception
) {
assert localIndexResolutionResult != LocalIndexResolutionResult.SUCCESS || exception == null
: "If the local resolution result is SUCCESS, exception must be null";
this.indices = indices;
this.localIndexResolutionResult = localIndexResolutionResult;
this.exception = exception;
}

public Set<String> indices() {
return indices;
}

public LocalIndexResolutionResult localIndexResolutionResult() {
return localIndexResolutionResult;
}

@Nullable
public ElasticsearchException exception() {
return exception;
}

public void setExceptionIfUnset(ElasticsearchException exception) {
assert localIndexResolutionResult != LocalIndexResolutionResult.SUCCESS
: "If the local resolution result is SUCCESS, exception must be null";
Objects.requireNonNull(exception);

if (this.exception == null) {
this.exception = exception;
} else if (Objects.equals(this.exception.getMessage(), exception.getMessage()) == false) {
// see https://github.com/elastic/elasticsearch/issues/135799
var message = "Exception is already set: " + exception.getMessage();
logger.debug(message);
assert false : message;
}
}

@Override
public boolean equals(Object obj) {
if (obj == this) return true;
if (obj == null || obj.getClass() != this.getClass()) return false;
var that = (LocalExpressions) obj;
return Objects.equals(this.indices, that.indices)
&& Objects.equals(this.localIndexResolutionResult, that.localIndexResolutionResult)
&& Objects.equals(this.exception, that.exception);
}

@Override
public int hashCode() {
return Objects.hash(indices, localIndexResolutionResult, exception);
}

@Override
public String toString() {
return "LocalExpressions["
+ "indices="
+ indices
+ ", "
+ "localIndexResolutionResult="
+ localIndexResolutionResult
+ ", "
+ "exception="
+ exception
+ ']';
}

// Singleton for the case where all expressions in a ResolvedIndexExpression instance are remote
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public static ElasticsearchException validate(
ResolvedIndexExpression.LocalExpressions localExpressions = localResolvedIndices.localExpressions();
ResolvedIndexExpression.LocalIndexResolutionResult result = localExpressions.localIndexResolutionResult();
if (isQualifiedExpression) {
ElasticsearchException e = checkResolutionFailure(localExpressions.indices(), result, originalExpression, indicesOptions);
ElasticsearchException e = checkResolutionFailure(localExpressions, result, originalExpression, indicesOptions);
if (e != null) {
return e;
}
Expand All @@ -118,7 +118,7 @@ public static ElasticsearchException validate(
}
} else {
ElasticsearchException localException = checkResolutionFailure(
localExpressions.indices(),
localExpressions,
result,
originalExpression,
indicesOptions
Expand Down Expand Up @@ -152,7 +152,7 @@ public static ElasticsearchException validate(
continue;
}
if (isUnauthorized) {
return securityException(originalExpression);
return localException;
}
return new IndexNotFoundException(originalExpression);
}
Expand Down Expand Up @@ -191,7 +191,7 @@ private static ElasticsearchException checkSingleRemoteExpression(
}

return checkResolutionFailure(
matchingExpression.indices(),
matchingExpression,
matchingExpression.localIndexResolutionResult(),
remoteExpression,
indicesOptions
Expand Down Expand Up @@ -223,7 +223,7 @@ private static ResolvedIndexExpression.LocalExpressions findMatchingExpression(
}

private static ElasticsearchException checkResolutionFailure(
Set<String> localExpressions,
ResolvedIndexExpression.LocalExpressions localExpressions,
ResolvedIndexExpression.LocalIndexResolutionResult result,
String expression,
IndicesOptions indicesOptions
Expand All @@ -235,11 +235,14 @@ private static ElasticsearchException checkResolutionFailure(
if (result == CONCRETE_RESOURCE_NOT_VISIBLE) {
return new IndexNotFoundException(expression);
} else if (result == CONCRETE_RESOURCE_UNAUTHORIZED) {
return securityException(expression);
assert localExpressions.exception() != null
: "ResolvedIndexExpression should have exception set when concrete index is unauthorized";

return localExpressions.exception();
}
}

if (indicesOptions.allowNoIndices() == false && result == SUCCESS && localExpressions.isEmpty()) {
if (indicesOptions.allowNoIndices() == false && result == SUCCESS && localExpressions.indices().isEmpty()) {
return new IndexNotFoundException(expression);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;

public class CrossProjectIndexResolutionValidatorTests extends ESTestCase {

Expand Down Expand Up @@ -124,14 +125,15 @@ public void testMissingFlatExpressionWithStrictIgnoreUnavailable() {
}

public void testUnauthorizedFlatExpressionWithStrictIgnoreUnavailable() {
final var exception = new ElasticsearchSecurityException("authorization errors while resolving [logs]");
ResolvedIndexExpressions local = new ResolvedIndexExpressions(
List.of(
new ResolvedIndexExpression(
"logs",
new ResolvedIndexExpression.LocalExpressions(
Set.of(),
ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED,
new ElasticsearchSecurityException("authorization errors while resolving [logs]")
exception
),
Set.of("P1:logs")
)
Expand All @@ -157,8 +159,7 @@ public void testUnauthorizedFlatExpressionWithStrictIgnoreUnavailable() {

var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), local, remote);
assertNotNull(e);
assertThat(e, instanceOf(ElasticsearchSecurityException.class));
assertThat(e.getMessage(), containsString("user cannot access [logs]"));
assertThat(e, is(exception));
}

public void testQualifiedExpressionWithStrictIgnoreUnavailableMatchingInOriginProject() {
Expand Down Expand Up @@ -270,6 +271,7 @@ public void testUnauthorizedQualifiedExpressionWithStrictIgnoreUnavailable() {
List.of(new ResolvedIndexExpression("P1:logs", ResolvedIndexExpression.LocalExpressions.NONE, Set.of("P1:logs")))
);

final var exception = new ElasticsearchException("action is unauthorized for indices [logs]");
var remote = Map.of(
"P1",
new ResolvedIndexExpressions(
Expand All @@ -279,7 +281,7 @@ public void testUnauthorizedQualifiedExpressionWithStrictIgnoreUnavailable() {
new ResolvedIndexExpression.LocalExpressions(
Set.of(),
ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED,
new ElasticsearchException("logs")
exception
),
Set.of()
)
Expand All @@ -289,8 +291,7 @@ public void testUnauthorizedQualifiedExpressionWithStrictIgnoreUnavailable() {

var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), local, remote);
assertNotNull(e);
assertThat(e, instanceOf(ElasticsearchSecurityException.class));
assertThat(e.getMessage(), containsString("user cannot access [P1:logs]"));
assertThat(e, is(exception));
}

public void testFlatExpressionWithStrictAllowNoIndicesMatchingInOriginProject() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.DelegatingActionListener;
import org.elasticsearch.action.DocWriteRequest;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.admin.indices.alias.TransportIndicesAliasesAction;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
Expand Down Expand Up @@ -108,6 +109,7 @@
import java.util.function.Consumer;
import java.util.function.Supplier;

import static org.elasticsearch.action.ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED;
import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
import static org.elasticsearch.xpack.core.security.SecurityField.setting;
import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.ACTION_SCOPE_AUTHORIZATION_KEYS;
Expand Down Expand Up @@ -529,27 +531,19 @@ private void authorizeAction(
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
),
threadContext
)
)
.addListener(wrapPreservingContext(new AuthorizationResultListener<>(result -> {
setIndexResolutionException(authzInfo, request, authentication, action);
handleIndexActionAuthorizationResult(
result,
requestInfo,
requestId,
authzInfo,
authzEngine,
resolvedIndicesAsyncSupplier,
projectMetadata,
l
);
}, l::onFailure, requestInfo, requestId, authzInfo), threadContext))
);
}, e -> onAuthorizedResourceLoadFailure(requestId, requestInfo, authzInfo, auditTrail, listener, e)));
} else {
Expand Down Expand Up @@ -714,6 +708,34 @@ private void handleIndexActionAuthorizationResult(
}
}

private void setIndexResolutionException(
AuthorizationInfo authzInfo,
TransportRequest request,
Authentication authentication,
String action
) {
if (request instanceof IndicesRequest.Replaceable replaceable) {
var indexExpressions = replaceable.getResolvedIndexExpressions();
if (indexExpressions != null) {
indexExpressions.expressions().forEach(resolved -> {
if (resolved.localExpressions().localIndexResolutionResult() == CONCRETE_RESOURCE_UNAUTHORIZED) {
resolved.localExpressions()
.setExceptionIfUnset(
actionDenied(
authentication,
authzInfo,
action,
request,
IndexAuthorizationResult.getFailureDescription(List.of(resolved.original()), restrictedIndices),
null
)
);
}
});
}
}
}

private void runRequestInterceptors(
RequestInfo requestInfo,
AuthorizationInfo authorizationInfo,
Expand Down
Loading