Skip to content

In access deny msg, only show indices if resolved #71715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Apr 15, 2021

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 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: #42166, #60357

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
@tvernum tvernum requested a review from ywangd April 15, 2021 01:32
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Apr 15, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tvernum tvernum merged commit 852db21 into elastic:master Apr 19, 2021
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Apr 19, 2021
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 added a commit that referenced this pull request Apr 19, 2021
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
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Apr 19, 2021
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: elastic#71715
tvernum added a commit that referenced this pull request Apr 19, 2021
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v7.12.2 v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants