Skip to content

Commit 5e75b5a

Browse files
n1v0lgn1v0lg
committed
Tweak and tests
1 parent 5d29d1c commit 5e75b5a

File tree

3 files changed

+147
-14
lines changed

3 files changed

+147
-14
lines changed

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.elasticsearch.xcontent.json.JsonXContent;
3333
import org.elasticsearch.xpack.core.security.user.User;
3434
import org.elasticsearch.xpack.security.SecurityOnTrialLicenseRestTestCase;
35+
import org.hamcrest.Matcher;
3536
import org.junit.Before;
3637
import org.junit.ClassRule;
3738

@@ -47,6 +48,7 @@
4748
import java.util.stream.Collectors;
4849

4950
import static org.hamcrest.Matchers.containsInAnyOrder;
51+
import static org.hamcrest.Matchers.containsString;
5052
import static org.hamcrest.Matchers.equalTo;
5153
import static org.hamcrest.Matchers.hasItem;
5254
import static org.hamcrest.Matchers.is;
@@ -1096,6 +1098,12 @@ public void testFailureStoreAccess() throws Exception {
10961098
case FAILURE_STORE_ACCESS, BACKING_INDEX_DATA_ACCESS, BACKING_INDEX_FAILURE_ACCESS, FAILURE_INDEX_DATA_ACCESS,
10971099
FAILURE_INDEX_FAILURE_ACCESS:
10981100
expectThrows(user, request, 403);
1101+
// also check authz message
1102+
expectThrowsUnauthorized(
1103+
user,
1104+
request,
1105+
containsString("this action is granted by the index privileges [read,all]")
1106+
);
10991107
break;
11001108
default:
11011109
fail("must cover user: " + user);
@@ -1281,6 +1289,15 @@ public void testFailureStoreAccess() throws Exception {
12811289
case DATA_ACCESS, STAR_READ_ONLY_ACCESS, BACKING_INDEX_DATA_ACCESS, BACKING_INDEX_FAILURE_ACCESS,
12821290
FAILURE_INDEX_FAILURE_ACCESS, FAILURE_INDEX_DATA_ACCESS:
12831291
expectThrows(user, request, 403);
1292+
// also check authz message
1293+
expectThrowsUnauthorized(
1294+
user,
1295+
request,
1296+
containsString(
1297+
"this action is granted by the index privileges [read,all] for data access, "
1298+
+ "or by [read_failure_store] for access with the [failures] selector"
1299+
)
1300+
);
12841301
break;
12851302
case ADMIN_USER, FAILURE_STORE_ACCESS, BOTH_ACCESS:
12861303
expectSearch(user, request, failuresDocId);
@@ -2167,6 +2184,12 @@ private static void expectThrows(ThrowingRunnable runnable, int statusCode) {
21672184
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(statusCode));
21682185
}
21692186

2187+
private void expectThrowsUnauthorized(String user, Search search, Matcher<String> errorMatcher) {
2188+
ResponseException ex = expectThrows(ResponseException.class, () -> performRequest(user, search.toSearchRequest()));
2189+
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403));
2190+
assertThat(ex.getMessage(), errorMatcher);
2191+
}
2192+
21702193
private void expectThrows(String user, Search search, int statusCode) {
21712194
expectThrows(() -> performRequest(user, search.toSearchRequest()), statusCode);
21722195
expectThrows(() -> performRequest(user, search.toAsyncSearchRequest()), statusCode);

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

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package org.elasticsearch.xpack.security.authz;
99

1010
import org.elasticsearch.action.support.IndexComponentSelector;
11+
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1112
import org.elasticsearch.common.Strings;
1213
import org.elasticsearch.common.util.set.Sets;
1314
import org.elasticsearch.core.Nullable;
@@ -103,27 +104,31 @@ public String actionDenied(
103104
action,
104105
p -> p.getSelectorPredicate().test(IndexComponentSelector.DATA)
105106
);
106-
final Collection<String> privilegesForFailuresSelector = findIndexPrivilegesThatGrant(
107-
action,
108-
p -> p.getSelectorPredicate().test(IndexComponentSelector.FAILURES)
109-
&& false == p.getSelectorPredicate().test(IndexComponentSelector.DATA)
110-
);
111-
if (privileges != null && false == privileges.isEmpty()) {
107+
assert false == privileges.isEmpty()
108+
|| findIndexPrivilegesThatGrant(
109+
action,
110+
p -> p.getSelectorPredicate().test(IndexComponentSelector.FAILURES)
111+
&& false == p.getSelectorPredicate().test(IndexComponentSelector.DATA)
112+
).isEmpty()
113+
: "action [" + action + "] is not covered by any regular index privilege, only by failures-selector privileges";
114+
115+
if (false == privileges.isEmpty()) {
112116
message = message
113117
+ ", this action is granted by the index privileges ["
114118
+ collectionToCommaDelimitedString(privileges)
115119
+ "]";
116-
}
117-
if (privilegesForFailuresSelector != null && false == privilegesForFailuresSelector.isEmpty()) {
118-
if (privileges != null && false == privileges.isEmpty()) {
120+
121+
final Collection<String> privilegesForFailuresOnly = findIndexPrivilegesThatGrant(
122+
action,
123+
p -> p.getSelectorPredicate().test(IndexComponentSelector.FAILURES)
124+
&& false == p.getSelectorPredicate().test(IndexComponentSelector.DATA)
125+
);
126+
if (false == privilegesForFailuresOnly.isEmpty() && hasIndicesWithFailuresSelector(request)) {
119127
message = message
120-
+ ", or by ["
121-
+ collectionToCommaDelimitedString(privilegesForFailuresSelector)
128+
+ " for data access, or by ["
129+
+ collectionToCommaDelimitedString(privilegesForFailuresOnly)
122130
+ "] for access with the [failures] selector";
123-
} else {
124-
assert false : "action covered by failures selector privileges but no regular privileges [" + action + "]";
125131
}
126-
127132
}
128133
}
129134
return message;
@@ -160,6 +165,19 @@ private String remoteClusterText(@Nullable String clusterAlias) {
160165
return Strings.format("towards remote cluster%s ", clusterAlias == null ? "" : " [" + clusterAlias + "]");
161166
}
162167

168+
private boolean hasIndicesWithFailuresSelector(TransportRequest request) {
169+
String[] indices = AuthorizationEngine.RequestInfo.indices(request);
170+
if (indices == null) {
171+
return false;
172+
}
173+
for (String index : indices) {
174+
if (IndexNameExpressionResolver.hasSelector(index, IndexComponentSelector.FAILURES)) {
175+
return true;
176+
}
177+
}
178+
return false;
179+
}
180+
163181
private String authenticatedUserDescription(Authentication authentication) {
164182
String userText = (authentication.isServiceAccount() ? "service account" : "user")
165183
+ " ["

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

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@
77

88
package org.elasticsearch.xpack.security.authz;
99

10+
import org.elasticsearch.action.search.SearchRequest;
11+
import org.elasticsearch.cluster.metadata.DataStream;
1012
import org.elasticsearch.common.Strings;
1113
import org.elasticsearch.core.Tuple;
1214
import org.elasticsearch.test.ESTestCase;
15+
import org.elasticsearch.transport.TransportRequest;
1316
import org.elasticsearch.xpack.core.security.authc.Authentication;
1417
import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
1518
import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper;
@@ -23,6 +26,7 @@
2326
import java.util.stream.IntStream;
2427
import java.util.stream.Stream;
2528

29+
import static org.hamcrest.Matchers.endsWith;
2630
import static org.hamcrest.Matchers.equalTo;
2731
import static org.mockito.Mockito.mock;
2832
import static org.mockito.Mockito.when;
@@ -233,6 +237,94 @@ public void testActionDeniedForCrossClusterAccessAuthentication() {
233237
);
234238
}
235239

240+
public void testActionDeniedWithFailuresAndCorrectActionIncludesFailuresMessage() {
241+
assumeTrue("failure store required", DataStream.isFailureStoreFeatureFlagEnabled());
242+
243+
Authentication authentication = AuthenticationTestHelper.builder().build();
244+
245+
final String action = "indices:data/read/" + randomAlphaOfLengthBetween(0, 8);
246+
SearchRequest request = mock(SearchRequest.class);
247+
for (List<String> requestedIndices : List.of(
248+
List.of(randomAlphaOfLength(5) + "::failures"),
249+
List.of(randomAlphaOfLength(5) + "::failures", randomAlphaOfLength(5)),
250+
List.of(randomAlphaOfLength(5), randomAlphaOfLength(5) + "::failures")
251+
)) {
252+
when(request.indices()).thenReturn(requestedIndices.toArray(new String[0]));
253+
assertThat(
254+
"for [" + Strings.collectionToCommaDelimitedString(requestedIndices) + "]",
255+
denialMessages.actionDenied(authentication, null, action, request, null),
256+
endsWith(
257+
"this action is granted by the index privileges [read,all] for data access, "
258+
+ "or by [read_failure_store] for access with the [failures] selector"
259+
)
260+
);
261+
}
262+
}
263+
264+
public void testActionDeniedWithNonMatchingActionFailuresOmitsFailuresMessage() {
265+
assumeTrue("failure store required", DataStream.isFailureStoreFeatureFlagEnabled());
266+
267+
Authentication authentication = AuthenticationTestHelper.builder().build();
268+
269+
// granted only by all, so selector message is omitted
270+
final String action = "indices:/some/action/" + randomAlphaOfLengthBetween(0, 8);
271+
SearchRequest request = mock(SearchRequest.class);
272+
for (List<String> requestedIndices : List.of(
273+
List.of(randomAlphaOfLength(5) + "::failures"),
274+
List.of(randomAlphaOfLength(5) + "::failures", randomAlphaOfLength(5)),
275+
List.of(randomAlphaOfLength(5), randomAlphaOfLength(5) + "::failures")
276+
)) {
277+
when(request.indices()).thenReturn(requestedIndices.toArray(new String[0]));
278+
assertThat(
279+
"for [" + Strings.collectionToCommaDelimitedString(requestedIndices) + "]",
280+
denialMessages.actionDenied(authentication, null, action, request, null),
281+
endsWith("this action is granted by the index privileges [all]")
282+
);
283+
}
284+
}
285+
286+
public void testActionDeniedWithoutFailuresOmitsFailuresMessage() {
287+
assumeTrue("failure store required", DataStream.isFailureStoreFeatureFlagEnabled());
288+
289+
Authentication authentication = AuthenticationTestHelper.builder().build();
290+
291+
final String action = "indices:data/read/" + randomAlphaOfLengthBetween(0, 8);
292+
SearchRequest request = mock(SearchRequest.class);
293+
for (List<String> requestedIndices : List.of(
294+
List.<String>of(),
295+
List.of(randomAlphaOfLength(5)),
296+
List.of(randomAlphaOfLength(5), randomAlphaOfLength(5))
297+
)) {
298+
when(request.indices()).thenReturn(requestedIndices.toArray(new String[0]));
299+
assertThat(
300+
"for [" + Strings.collectionToCommaDelimitedString(requestedIndices) + "]",
301+
denialMessages.actionDenied(authentication, null, action, request, null),
302+
endsWith("this action is granted by the index privileges [read,all]")
303+
);
304+
}
305+
}
306+
307+
public void testActionDeniedWithoutIndicesOmitsFailuresMessage() {
308+
assumeTrue("failure store required", DataStream.isFailureStoreFeatureFlagEnabled());
309+
310+
Authentication authentication = AuthenticationTestHelper.builder().build();
311+
312+
final String action = "indices:data/read/" + randomAlphaOfLengthBetween(0, 8);
313+
// not an IndicesRequest
314+
TransportRequest request = mock(TransportRequest.class);
315+
for (List<String> requestedIndices : List.of(
316+
List.<String>of(),
317+
List.of(randomAlphaOfLength(5)),
318+
List.of(randomAlphaOfLength(5), randomAlphaOfLength(5))
319+
)) {
320+
assertThat(
321+
"for [" + Strings.collectionToCommaDelimitedString(requestedIndices) + "]",
322+
denialMessages.actionDenied(authentication, null, action, request, null),
323+
endsWith("this action is granted by the index privileges [read,all]")
324+
);
325+
}
326+
}
327+
236328
public void testSuccessfulAuthenticationDescription() {
237329
final Authentication authentication1 = AuthenticationTestHelper.builder().realm().build(false);
238330
assertThat(

0 commit comments

Comments
 (0)