Skip to content

Commit

Permalink
In access deny msg, only show indices if resolved (#71806)
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 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.

Backport of: #71715
  • Loading branch information
tvernum committed Apr 19, 2021
1 parent f4d1c1e commit ceb191a
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 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,7 +679,7 @@ public void testUnknownRoleCausesDenial() throws IOException {
ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class,
() -> authorize(authentication, action, request));
assertThat(securityException,
throwableWithMessage(containsString("[" + action + "] is unauthorized for user [test user] on indices [")));
throwableWithMessage(containsString("[" + action + "] is unauthorized for user [test user],")));
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 @@ -718,7 +718,7 @@ public void testThatRoleWithNoIndicesIsDenied() throws IOException {
ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class,
() -> authorize(authentication, action, request));
assertThat(securityException,
throwableWithMessage(containsString("[" + action + "] is unauthorized for user [test user] on indices [")));
throwableWithMessage(containsString("[" + action + "] is unauthorized for user [test user],")));
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 ceb191a

Please sign in to comment.