Skip to content

Commit c7a85ae

Browse files
n1v0lgn1v0lg
authored
[Failure Store] Authorization denial messages (elastic#125757) (elastic#126033)
This PR makes authorization denial messages account for privileges that grant access to the failure store. This is a minimal implementation that only displays information around failure store privileges for requests that include concrete names with the `::failures` selector. This avoids including irrelevant information in regular non-failures requests. We can improve on this in follow ups. Closes: ES-11158
1 parent 69f4500 commit c7a85ae

File tree

5 files changed

+177
-8
lines changed

5 files changed

+177
-8
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,10 +482,13 @@ public static Set<String> names() {
482482
* @see Privilege#sortByAccessLevel
483483
*/
484484
public static Collection<String> findPrivilegesThatGrant(String action) {
485+
return findPrivilegesThatGrant(action, p -> p.getSelectorPredicate().test(IndexComponentSelector.DATA));
486+
}
487+
488+
public static Collection<String> findPrivilegesThatGrant(String action, Predicate<IndexPrivilege> preCondition) {
485489
return VALUES.entrySet()
486490
.stream()
487-
// Only include privileges that grant data access; failures access is handled separately in authorization failure messages
488-
.filter(e -> e.getValue().selectorPredicate.test(IndexComponentSelector.DATA))
491+
.filter(e -> preCondition.test(e.getValue()))
489492
.filter(e -> e.getValue().predicate.test(action))
490493
.map(Map.Entry::getKey)
491494
.toList();

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilegeTests.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.Collection;
2424
import java.util.List;
2525
import java.util.Set;
26+
import java.util.function.Predicate;
2627

2728
import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.findPrivilegesThatGrant;
2829
import static org.hamcrest.Matchers.containsInAnyOrder;
@@ -68,6 +69,16 @@ public void testFindPrivilegesThatGrant() {
6869
equalTo(List.of("monitor", "cross_cluster_replication", "manage", "all"))
6970
);
7071
assertThat(findPrivilegesThatGrant(RefreshAction.NAME), equalTo(List.of("maintenance", "manage", "all")));
72+
73+
if (DataStream.isFailureStoreFeatureFlagEnabled()) {
74+
Predicate<IndexPrivilege> failuresOnly = p -> p.getSelectorPredicate() == IndexComponentSelectorPredicate.FAILURES;
75+
assertThat(findPrivilegesThatGrant(TransportSearchAction.TYPE.name(), failuresOnly), equalTo(List.of("read_failure_store")));
76+
assertThat(findPrivilegesThatGrant(TransportIndexAction.NAME, failuresOnly), equalTo(List.of()));
77+
assertThat(findPrivilegesThatGrant(TransportUpdateAction.NAME, failuresOnly), equalTo(List.of()));
78+
assertThat(findPrivilegesThatGrant(TransportDeleteAction.NAME, failuresOnly), equalTo(List.of()));
79+
assertThat(findPrivilegesThatGrant(IndicesStatsAction.NAME, failuresOnly), equalTo(List.of("manage_failure_store")));
80+
assertThat(findPrivilegesThatGrant(RefreshAction.NAME, failuresOnly), equalTo(List.of("manage_failure_store")));
81+
}
7182
}
7283

7384
public void testGet() {

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

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

@@ -1092,6 +1093,12 @@ public void testFailureStoreAccess() throws Exception {
10921093
case FAILURE_STORE_ACCESS, BACKING_INDEX_DATA_ACCESS, BACKING_INDEX_FAILURE_ACCESS, FAILURE_INDEX_DATA_ACCESS,
10931094
FAILURE_INDEX_FAILURE_ACCESS:
10941095
expectThrows(user, request, 403);
1096+
// also check authz message
1097+
expectThrowsUnauthorized(
1098+
user,
1099+
request,
1100+
containsString("this action is granted by the index privileges [read,all]")
1101+
);
10951102
break;
10961103
default:
10971104
fail("must cover user: " + user);
@@ -1277,6 +1284,15 @@ public void testFailureStoreAccess() throws Exception {
12771284
case DATA_ACCESS, STAR_READ_ONLY_ACCESS, BACKING_INDEX_DATA_ACCESS, BACKING_INDEX_FAILURE_ACCESS,
12781285
FAILURE_INDEX_FAILURE_ACCESS, FAILURE_INDEX_DATA_ACCESS:
12791286
expectThrows(user, request, 403);
1287+
// also check authz message
1288+
expectThrowsUnauthorized(
1289+
user,
1290+
request,
1291+
containsString(
1292+
"this action is granted by the index privileges [read,all] for data access, "
1293+
+ "or by [read_failure_store] for access with the [::failures] selector"
1294+
)
1295+
);
12801296
break;
12811297
case ADMIN_USER, FAILURE_STORE_ACCESS, BOTH_ACCESS:
12821298
expectSearch(user, request, failuresDocId);
@@ -2255,6 +2271,12 @@ private static void expectThrows(ThrowingRunnable runnable, int statusCode) {
22552271
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(statusCode));
22562272
}
22572273

2274+
private void expectThrowsUnauthorized(String user, Search search, Matcher<String> errorMatcher) {
2275+
ResponseException ex = expectThrows(ResponseException.class, () -> performRequestMaybeUsingApiKey(user, search.toSearchRequest()));
2276+
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403));
2277+
assertThat(ex.getMessage(), errorMatcher);
2278+
}
2279+
22582280
private void expectThrows(String user, Search search, int statusCode) {
22592281
expectThrows(() -> performRequest(user, search.toSearchRequest()), statusCode);
22602282
expectThrows(() -> performRequest(user, search.toAsyncSearchRequest()), statusCode);

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

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

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

10+
import org.elasticsearch.action.support.IndexComponentSelector;
11+
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1012
import org.elasticsearch.common.Strings;
1113
import org.elasticsearch.common.util.set.Sets;
1214
import org.elasticsearch.core.Nullable;
@@ -24,6 +26,7 @@
2426
import java.util.Map;
2527
import java.util.Set;
2628
import java.util.SortedSet;
29+
import java.util.function.Predicate;
2730

2831
import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString;
2932
import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME;
@@ -90,22 +93,47 @@ public String actionDenied(
9093

9194
if (ClusterPrivilegeResolver.isClusterAction(action)) {
9295
final Collection<String> privileges = findClusterPrivilegesThatGrant(authentication, action, request);
93-
if (privileges != null && privileges.size() > 0) {
96+
if (privileges != null && false == privileges.isEmpty()) {
9497
message = message
9598
+ ", this action is granted by the cluster privileges ["
9699
+ collectionToCommaDelimitedString(privileges)
97100
+ "]";
98101
}
99102
} else if (isIndexAction(action)) {
100-
final Collection<String> privileges = findIndexPrivilegesThatGrant(action);
101-
if (privileges != null && privileges.size() > 0) {
103+
// this includes `all`
104+
final Collection<String> privileges = findIndexPrivilegesThatGrant(
105+
action,
106+
p -> p.getSelectorPredicate().test(IndexComponentSelector.DATA)
107+
);
108+
// this is an invariant since `all` is included in the above so the only way
109+
// we can get an empty result here is a bogus action, which will never be covered by a failures privilege
110+
assert false == privileges.isEmpty()
111+
|| findIndexPrivilegesThatGrant(
112+
action,
113+
p -> p.getSelectorPredicate().test(IndexComponentSelector.FAILURES)
114+
&& false == p.getSelectorPredicate().test(IndexComponentSelector.DATA)
115+
).isEmpty()
116+
: "action [" + action + "] is not covered by any regular index privilege, only by failures-selector privileges";
117+
118+
if (false == privileges.isEmpty()) {
102119
message = message
103120
+ ", this action is granted by the index privileges ["
104121
+ collectionToCommaDelimitedString(privileges)
105122
+ "]";
123+
124+
final Collection<String> privilegesForFailuresOnly = findIndexPrivilegesThatGrant(
125+
action,
126+
p -> p.getSelectorPredicate().test(IndexComponentSelector.FAILURES)
127+
&& false == p.getSelectorPredicate().test(IndexComponentSelector.DATA)
128+
);
129+
if (false == privilegesForFailuresOnly.isEmpty() && hasIndicesWithFailuresSelector(request)) {
130+
message = message
131+
+ " for data access, or by ["
132+
+ collectionToCommaDelimitedString(privilegesForFailuresOnly)
133+
+ "] for access with the [::failures] selector";
134+
}
106135
}
107136
}
108-
109137
return message;
110138
}
111139

@@ -132,14 +160,27 @@ protected Collection<String> findClusterPrivilegesThatGrant(
132160
return ClusterPrivilegeResolver.findPrivilegesThatGrant(action, request, authentication);
133161
}
134162

135-
protected Collection<String> findIndexPrivilegesThatGrant(String action) {
136-
return IndexPrivilege.findPrivilegesThatGrant(action);
163+
protected Collection<String> findIndexPrivilegesThatGrant(String action, Predicate<IndexPrivilege> preCondition) {
164+
return IndexPrivilege.findPrivilegesThatGrant(action, preCondition);
137165
}
138166

139167
private String remoteClusterText(@Nullable String clusterAlias) {
140168
return Strings.format("towards remote cluster%s ", clusterAlias == null ? "" : " [" + clusterAlias + "]");
141169
}
142170

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

‎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)