In access deny msg, only show indices if resolved

Our authorization engine has a short-circuit check for the intended action the takes place before resolving index names (wildcards). That is, a requests like GET /_search GET /logs-*/_search GET /logs-20210414/_search will fail fast if the user does not have read permission on any indices, and we will never resolve the list of indices that the request targets. Consequently, it is impossible to provide the list of denied indices in the error message because that list does not exist (and, in the case of wildards would be empty even if we did resolve it). This change updates the access denied message so that it does not attempt to include the list of indices if the IndicesAccessControl object has an empty list of denied indices. Prior to this, we would generate messages such as action [indices:data/read/search] is unauthorized for user [test] with roles [test] on indices [], That "indices []" section is never useful since it does not name any indices, so it has now been dropped from the message if it is empty. Relates: elastic#42166, elastic#60357 Backport of: elastic#71715
tvernum · Apr 19, 2021 · 2202d79 · 2202d79
1 parent 8076001
commit 2202d79
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 4 deletions.
diff --git a/...n/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java b/...n/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java
@@ -342,6 +342,9 @@ public String getFailureContext() {
         }
 
         public static String getFailureDescription(Collection<?> deniedIndices) {
+            if (deniedIndices.isEmpty()) {
+                return null;
+            }
             return "on indices [" + Strings.collectionToCommaDelimitedString(deniedIndices) + "]";
         }
 

diff --git a/...urity/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/...urity/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java
@@ -680,8 +680,7 @@ public void testUnknownRoleCausesDenial() throws IOException {
         assertThat(securityException, throwableWithMessage(containsString(
             "[" + action + "] is unauthorized" +
                 " for user [test user]" +
-                " with roles [non-existent-role]" +
-                " on indices [")));
+                " with roles [non-existent-role],")));
         assertThat(securityException, throwableWithMessage(containsString("this action is granted by the index privileges [read,all]")));
 
         verify(auditTrail).accessDenied(eq(requestId), eq(authentication), eq(action), eq(request), authzInfoRoles(Role.EMPTY.names()));
@@ -722,8 +721,7 @@ public void testThatRoleWithNoIndicesIsDenied() throws IOException {
         assertThat(securityException, throwableWithMessage(containsString(
             "[" + action + "] is unauthorized" +
                 " for user [test user]" +
-                " with roles [no_indices]" +
-                " on indices [")));
+                " with roles [no_indices],")));
         assertThat(securityException, throwableWithMessage(containsString("this action is granted by the index privileges [read,all]")));
 
         verify(auditTrail).accessDenied(eq(requestId), eq(authentication), eq(action), eq(request),