Add more context to index access denied errors #60357
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Access denied messages for indices were overly brief and missed two pieces of useful information: 1. The names of the indices for which access was denied 2. The privileges that could be used to grant that access This change improves the access denied messages for index based actions by adding the index and privilege names. Privilege names are listed in order from least-privilege to most-privileged so that the first recommended path to resolution is also the lowest privilege change. Relates: elastic#42166
tvernum
added
>enhancement
:Security/Authorization
|
Pinging @elastic/es-security (:Security/Authorization) |
|
An example of the new failure message
|
albertzaharovits
approved these changes
Aug 3, 2020
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Aug 5, 2020
Access denied messages for indices were overly brief and missed two pieces of useful information: 1. The names of the indices for which access was denied 2. The privileges that could be used to grant that access This change improves the access denied messages for index based actions by adding the index and privilege names. Privilege names are listed in order from least-privilege to most-privileged so that the first recommended path to resolution is also the lowest privilege change. Relates: elastic#42166 Backport of: elastic#60357
61 tasks
|
This change failed to be backported to |
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Dec 31, 2020
In elastic#60357 we improved the error message when access to perform an action on an index was denied by including the index name and the privileges that would grant the action. This commit extends the second part of that change (the list of privileges that would resolve the problem) to situations when a cluster action is denied. This implementation for cluster privileges is slightly more complex than that of index privileges because cluster privileges can be dependent on parameters in the request, not just the action name. For example, "manage_own_api_key" should be suggested as a matching privilege when a user attempts to create an API key, or delete their own API key, but should not be suggested when that same user attempts to delete another user's API key. Relates: elastic#42166
tvernum
added a commit
that referenced
this pull request
Dec 31, 2020
Access denied messages for indices were overly brief and missed two pieces of useful information: 1. The names of the indices for which access was denied 2. The privileges that could be used to grant that access This change improves the access denied messages for index based actions by adding the index and privilege names. Privilege names are listed in order from least-privilege to most-privileged so that the first recommended path to resolution is also the lowest privilege change. Backport of: #60357
tvernum
added a commit
that referenced
this pull request
Jan 12, 2021
In #60357 we improved the error message when access to perform an action on an index was denied by including the index name and the privileges that would grant the action. This commit extends the second part of that change (the list of privileges that would resolve the problem) to situations when a cluster action is denied. This implementation for cluster privileges is slightly more complex than that of index privileges because cluster privileges can be dependent on parameters in the request, not just the action name. For example, "manage_own_api_key" should be suggested as a matching privilege when a user attempts to create an API key, or delete their own API key, but should not be suggested when that same user attempts to delete another user's API key. Relates: #42166
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Jan 31, 2021
Some actions that start with "indices:" are actually handled by cluster privileges in ES security (e.g. indices:admin/template/*) In elastic#60357 and elastic#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: elastic#68144
This was referenced Jan 31, 2021
Closed
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Feb 1, 2021
In elastic#60357 we improved the error message when access to perform an action on an index was denied by including the index name and the privileges that would grant the action. This commit extends the second part of that change (the list of privileges that would resolve the problem) to situations when a cluster action is denied. This implementation for cluster privileges is slightly more complex than that of index privileges because cluster privileges can be dependent on parameters in the request, not just the action name. For example, "manage_own_api_key" should be suggested as a matching privilege when a user attempts to create an API key, or delete their own API key, but should not be suggested when that same user attempts to delete another user's API key. Backport of: elastic#66900
tvernum
added a commit
that referenced
this pull request
Feb 3, 2021
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
tvernum
added a commit
that referenced
this pull request
Feb 3, 2021
In #60357 we improved the error message when access to perform an action on an index was denied by including the index name and the privileges that would grant the action. This commit extends the second part of that change (the list of privileges that would resolve the problem) to situations when a cluster action is denied. This implementation for cluster privileges is slightly more complex than that of index privileges because cluster privileges can be dependent on parameters in the request, not just the action name. For example, "manage_own_api_key" should be suggested as a matching privilege when a user attempts to create an API key, or delete their own API key, but should not be suggested when that same user attempts to delete another user's API key. Backport of: #66900
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
Feb 3, 2021
Some actions that start with "indices:" are actually handled by cluster privileges in ES security (e.g. indices:admin/template/*) In elastic#60357 and elastic#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. Backport of: elastic#68260
tvernum
added a commit
that referenced
this pull request
Feb 3, 2021
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. Backport of: #68260
tvernum
added a commit
to tvernum/elasticsearch
that referenced
this pull request
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 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
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 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: #42166, #60357
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
This was referenced Jul 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Access denied messages for indices were overly brief and missed two
pieces of useful information:
This change improves the access denied messages for index based
actions by adding the index and privilege names.
Privilege names are listed in order from least-privilege to
most-privileged so that the first recommended path to resolution is
also the lowest privilege change.
Relates: #42166