Skip to content

Commit d0363fb

Browse files
slobodanadamovicelasticsearchmachine
andauthored
[8.x] Improve error handling during index expressions resolving (elastic#126018) (elastic#126373)
* Improve error handling during index expressions resolving (elastic#126018) The `InvalidIndexNameException` exception was wrapped in a `ElasticsearchSecurityException`, which returns HTTP `403` status. This exception (along with newly introduced `InvalidSelectorException` and `UnsupportedSelectorException`) can be raised during index expression resolving due to an invalid user input and should result in HTTP `400` response instead. This PR changes exception handling to avoid wrapping them in the `ElasticsearchSecurityException`. (cherry picked from commit 0b09506) # Conflicts: # x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent ff3699b commit d0363fb

File tree

9 files changed

+85
-18
lines changed

9 files changed

+85
-18
lines changed

server/src/main/java/org/elasticsearch/action/support/IndexComponentSelector.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public static IndexComponentSelector getByKeyOrThrow(@Nullable String key) {
8383
}
8484
IndexComponentSelector selector = getByKey(key);
8585
if (selector == null) {
86-
throw new IllegalArgumentException(
86+
throw new InvalidSelectorException(
8787
"Unknown key of index component selector [" + key + "], available options are: " + KEY_REGISTRY.keySet()
8888
);
8989
}
@@ -105,7 +105,7 @@ public static IndexComponentSelector read(StreamInput in) throws IOException {
105105
static IndexComponentSelector getById(byte id) {
106106
IndexComponentSelector indexComponentSelector = ID_REGISTRY.get(id);
107107
if (indexComponentSelector == null) {
108-
throw new IllegalArgumentException(
108+
throw new InvalidSelectorException(
109109
"Unknown id of index component selector [" + id + "], available options are: " + ID_REGISTRY
110110
);
111111
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.action.support;
11+
12+
/**
13+
* Exception thrown when a :: selector is invalid.
14+
*/
15+
public class InvalidSelectorException extends IllegalArgumentException {
16+
17+
public InvalidSelectorException(String message) {
18+
super(message);
19+
}
20+
21+
public InvalidSelectorException(String message, Throwable cause) {
22+
super(message, cause);
23+
}
24+
25+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.action.support;
11+
12+
/**
13+
* Exception thrown when a :: selector is not supported.
14+
*/
15+
public class UnsupportedSelectorException extends IllegalArgumentException {
16+
17+
public UnsupportedSelectorException(String expression) {
18+
super("Index component selectors are not supported in this context but found selector in expression [" + expression + "]");
19+
}
20+
21+
}

server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import org.elasticsearch.action.support.IndexComponentSelector;
1313
import org.elasticsearch.action.support.IndicesOptions;
14+
import org.elasticsearch.action.support.UnsupportedSelectorException;
1415
import org.elasticsearch.common.regex.Regex;
1516
import org.elasticsearch.core.Nullable;
1617
import org.elasticsearch.core.Tuple;
@@ -57,11 +58,7 @@ public List<String> resolveIndexAbstractions(
5758
Tuple<String, String> expressionAndSelector = IndexNameExpressionResolver.splitSelectorExpression(indexAbstraction);
5859
String selectorString = expressionAndSelector.v2();
5960
if (indicesOptions.allowSelectors() == false && selectorString != null) {
60-
throw new IllegalArgumentException(
61-
"Index component selectors are not supported in this context but found selector in expression ["
62-
+ indexAbstraction
63-
+ "]"
64-
);
61+
throw new UnsupportedSelectorException(indexAbstraction);
6562
}
6663
indexAbstraction = expressionAndSelector.v1();
6764
IndexComponentSelector selector = IndexComponentSelector.getByKeyOrThrow(selectorString);

server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.action.IndicesRequest;
1616
import org.elasticsearch.action.support.IndexComponentSelector;
1717
import org.elasticsearch.action.support.IndicesOptions;
18+
import org.elasticsearch.action.support.UnsupportedSelectorException;
1819
import org.elasticsearch.cluster.ClusterState;
1920
import org.elasticsearch.cluster.metadata.IndexAbstraction.Type;
2021
import org.elasticsearch.common.Strings;
@@ -2156,13 +2157,11 @@ private static IndexComponentSelector resolveAndValidateSelectorString(Supplier<
21562157
* Checks the selectors that have been returned from splitting an expression and throws an exception if any were present.
21572158
* @param expression Original expression
21582159
* @param selector Selectors to validate
2159-
* @throws IllegalArgumentException if selectors are present
2160+
* @throws UnsupportedSelectorException if selectors are present
21602161
*/
21612162
private static void ensureNoSelectorsProvided(String expression, IndexComponentSelector selector) {
21622163
if (selector != null) {
2163-
throw new IllegalArgumentException(
2164-
"Index component selectors are not supported in this context but found selector in expression [" + expression + "]"
2165-
);
2164+
throw new UnsupportedSelectorException(expression);
21662165
}
21672166
}
21682167

x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ protected Tuple<String, String> getSingleDataAndFailureIndices(String dataStream
170170
}
171171

172172
protected static void assertSelectorsNotSupported(ResponseException exception) {
173-
assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403));
173+
assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(400));
174174
assertThat(exception.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns"));
175175
}
176176

x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1817,7 +1817,11 @@ public void testWriteOperations() throws IOException {
18171817
expectThrows(() -> deleteDataStream(MANAGE_FAILURE_STORE_ACCESS, dataIndexName), 403);
18181818

18191819
expectThrows(() -> deleteDataStream(MANAGE_FAILURE_STORE_ACCESS, "test1"), 403);
1820-
expectThrows(() -> deleteDataStream(MANAGE_FAILURE_STORE_ACCESS, "test1::failures"), 403);
1820+
// selectors aren't supported for deletes so we get a 403
1821+
expectThrowsBadRequest(
1822+
() -> deleteDataStream(MANAGE_FAILURE_STORE_ACCESS, "test1::failures"),
1823+
containsString("Index component selectors are not supported in this context but found selector in expression [test1::failures]")
1824+
);
18211825

18221826
// manage user can delete data stream
18231827
deleteDataStream(MANAGE_ACCESS, "test1");
@@ -2271,6 +2275,12 @@ private static void expectThrows(ThrowingRunnable runnable, int statusCode) {
22712275
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(statusCode));
22722276
}
22732277

2278+
private void expectThrowsBadRequest(ThrowingRunnable runnable, Matcher<String> errorMatcher) {
2279+
ResponseException ex = expectThrows(ResponseException.class, runnable);
2280+
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(400));
2281+
assertThat(ex.getMessage(), errorMatcher);
2282+
}
2283+
22742284
private void expectThrowsUnauthorized(String user, Search search, Matcher<String> errorMatcher) {
22752285
ResponseException ex = expectThrows(ResponseException.class, () -> performRequestMaybeUsingApiKey(user, search.toSearchRequest()));
22762286
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403));

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.elasticsearch.action.index.TransportIndexAction;
2929
import org.elasticsearch.action.search.SearchRequest;
3030
import org.elasticsearch.action.support.GroupedActionListener;
31+
import org.elasticsearch.action.support.InvalidSelectorException;
32+
import org.elasticsearch.action.support.UnsupportedSelectorException;
3133
import org.elasticsearch.action.support.replication.TransportReplicationAction.ConcreteShardRequest;
3234
import org.elasticsearch.action.update.TransportUpdateAction;
3335
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
@@ -42,6 +44,7 @@
4244
import org.elasticsearch.core.Nullable;
4345
import org.elasticsearch.core.Tuple;
4446
import org.elasticsearch.index.IndexNotFoundException;
47+
import org.elasticsearch.indices.InvalidIndexNameException;
4548
import org.elasticsearch.license.XPackLicenseState;
4649
import org.elasticsearch.threadpool.ThreadPool;
4750
import org.elasticsearch.transport.TransportActionProxy;
@@ -496,6 +499,21 @@ private void authorizeAction(
496499
indicesAndAliasesResolver.resolve(action, request, metadata, authorizedIndices)
497500
),
498501
e -> {
502+
if (e instanceof InvalidIndexNameException
503+
|| e instanceof InvalidSelectorException
504+
|| e instanceof UnsupportedSelectorException) {
505+
logger.debug(
506+
() -> Strings.format(
507+
"failed [%s] action authorization for [%s] due to [%s] exception",
508+
action,
509+
authentication,
510+
e.getClass().getSimpleName()
511+
),
512+
e
513+
);
514+
listener.onFailure(e);
515+
return;
516+
}
499517
auditTrail.accessDenied(requestId, authentication, action, request, authzInfo);
500518
if (e instanceof IndexNotFoundException) {
501519
listener.onFailure(e);

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.action.search.SearchRequest;
1616
import org.elasticsearch.action.support.IndexComponentSelector;
1717
import org.elasticsearch.action.support.IndicesOptions;
18+
import org.elasticsearch.action.support.UnsupportedSelectorException;
1819
import org.elasticsearch.cluster.metadata.AliasMetadata;
1920
import org.elasticsearch.cluster.metadata.IndexAbstraction;
2021
import org.elasticsearch.cluster.metadata.IndexAbstractionResolver;
@@ -314,11 +315,7 @@ ResolvedIndices resolveIndicesAndAliases(
314315
// First, if a selector is present, check to make sure that selectors are even allowed here
315316
if (indicesOptions.allowSelectors() == false && allIndicesPatternSelector != null) {
316317
String originalIndexExpression = indicesRequest.indices()[0];
317-
throw new IllegalArgumentException(
318-
"Index component selectors are not supported in this context but found selector in expression ["
319-
+ originalIndexExpression
320-
+ "]"
321-
);
318+
throw new UnsupportedSelectorException(originalIndexExpression);
322319
}
323320
if (indicesOptions.expandWildcardExpressions()) {
324321
IndexComponentSelector selector = IndexComponentSelector.getByKeyOrThrow(allIndicesPatternSelector);

0 commit comments

Comments
 (0)