Skip to content

Commit

Permalink
In access deny msg, only show indices if resolved
Browse files Browse the repository at this point in the history
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 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 IndiceAccessControl
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
  • Loading branch information
tvernum committed Apr 15, 2021
1 parent a642cb3 commit 27bc0d3
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ public String getFailureContext() {
}

public static String getFailureDescription(Collection<?> deniedIndices) {
if (deniedIndices.isEmpty()) {
return null;
}
return "on indices [" + Strings.collectionToCommaDelimitedString(deniedIndices) + "]";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -679,8 +679,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()));
Expand Down Expand Up @@ -721,8 +720,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),
Expand Down

0 comments on commit 27bc0d3

Please sign in to comment.