Skip to content

Commit fa9c4bb

Browse files
authored
Fix identification of action type in error message (#68260)
Some actions that start with "indices:" are actually handled by cluster privileges in ES security (e.g. indices:admin/template/*) In #60357 and #66900 we added better context information for the error messages that are generated when an action is denied, but the generation of that message did not correctly classify actions between cluster and index level privileges. This change does 2 things: 1. It fixes the code that determines whether an action is handled by a cluster privilege or an index privilege 2. Includes the words "cluster" and "index" in the error message so that classification is clear to the reader The latter change is not directly related to the issue being resolved, but in the course of fixing the issue it became evident that the message lacked clarity because it did not tell the reader what type of privilege would be needed to resolve the access denied issue. Resolves: #68144
1 parent a92a647 commit fa9c4bb

File tree

5 files changed

+157
-126
lines changed

5 files changed

+157
-126
lines changed

x-pack/plugin/ilm/qa/with-security/src/javaRestTest/java/org/elasticsearch/xpack/security/PermissionsIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public void testCanManageIndexWithNoPermissions() throws Exception {
147147
assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized" +
148148
" for user [test_ilm]" +
149149
" on indices [not-ilm]," +
150-
" this action is granted by the privileges [monitor,manage,all]"));
150+
" this action is granted by the index privileges [monitor,manage,all]"));
151151
}
152152
});
153153
}

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,14 +1218,14 @@ private Client getClientForRunAsUser() {
12181218
private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName, String apiKeyId) {
12191219
assertThat(ese, throwableWithMessage(
12201220
containsString("action [" + action + "] is unauthorized for API key id [" + apiKeyId + "] of user [" + userName + "]")));
1221-
assertThat(ese, throwableWithMessage(containsString(", this action is granted by the privileges [")));
1221+
assertThat(ese, throwableWithMessage(containsString(", this action is granted by the cluster privileges [")));
12221222
assertThat(ese, throwableWithMessage(containsString("manage_api_key,manage_security,all]")));
12231223
}
12241224

12251225
private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName) {
12261226
assertThat(ese, throwableWithMessage(
12271227
containsString("action [" + action + "] is unauthorized for user [" + userName + "]")));
1228-
assertThat(ese, throwableWithMessage(containsString(", this action is granted by the privileges [")));
1228+
assertThat(ese, throwableWithMessage(containsString(", this action is granted by the cluster privileges [")));
12291229
assertThat(ese, throwableWithMessage(containsString("manage_api_key,manage_security,all]")));
12301230
}
12311231
}

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -636,15 +636,17 @@ private ElasticsearchSecurityException denialException(Authentication authentica
636636
message = message + " " + context;
637637
}
638638

639-
if (isIndexAction(action)) {
640-
final Collection<String> privileges = IndexPrivilege.findPrivilegesThatGrant(action);
639+
if (ClusterPrivilegeResolver.isClusterAction(action)) {
640+
final Collection<String> privileges = ClusterPrivilegeResolver.findPrivilegesThatGrant(action, request, authentication);
641641
if (privileges != null && privileges.size() > 0) {
642-
message = message + ", this action is granted by the privileges [" + collectionToCommaDelimitedString(privileges) + "]";
642+
message = message + ", this action is granted by the cluster privileges ["
643+
+ collectionToCommaDelimitedString(privileges) + "]";
643644
}
644-
} else if (ClusterPrivilegeResolver.isClusterAction(action)) {
645-
final Collection<String> privileges = ClusterPrivilegeResolver.findPrivilegesThatGrant(action, request, authentication);
645+
} else if (isIndexAction(action)) {
646+
final Collection<String> privileges = IndexPrivilege.findPrivilegesThatGrant(action);
646647
if (privileges != null && privileges.size() > 0) {
647-
message = message + ", this action is granted by the privileges [" + collectionToCommaDelimitedString(privileges) + "]";
648+
message = message + ", this action is granted by the index privileges ["
649+
+ collectionToCommaDelimitedString(privileges) + "]";
648650
}
649651
}
650652

0 commit comments

Comments
 (0)