Fix incorrect assumptions for api_key authentication type #81564
Conversation
This file contains hidden or 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
Still need sweep through test code Also add new tests
ywangd
added
>non-issue
:Security/Authorization
|
Pinging @elastic/es-security (Team:Security) |
|
@tvernum I was going to use |
|
@elasticmachine update branch |
|
@elasticmachine run elasticsearch-ci/part-1-fips |
ywangd
added a commit
to ywangd/elasticsearch
that referenced
this pull request
Feb 24, 2022
API Key can run-as since elastic#79809. There are places in the code where we assume API key cannot run-as. Most of them are corrected in elastic#81564. But there are still a few things got missed. This PR fixes the methods for checking owner user realm for API key. Note the resource sharing check (canAccessResourcesOf) also needs to be fixed, this will be handled by elastic#84277
ywangd
added a commit
that referenced
this pull request
Feb 28, 2022
API Key can run-as since #79809. There are places in the code where we assume API key cannot run-as. Most of them are corrected in #81564. But there are still a few things got missed. This PR fixes the methods for checking owner user realm for API key. This means, when API Keys "running-as" (impersonating other users), we do not expose the authenticating key ID and name to the end-user such as the Authenticate API and the SetSecurityUseringest processor. Only the effective user is revealed, just like in the regular case of a realm user run as. For audit logging, the key's ID and name are not exposed either. But this is mainly because there are no existing fields suitable for these information. We do intend to add them later (#84394) because auditing logging is to consumed by system admin instead of end-users. Note the resource sharing check (canAccessResourcesOf) also needs to be fixed, this will be handled by #84277
ywangd
added a commit
to ywangd/elasticsearch
that referenced
this pull request
Feb 28, 2022
API Key can run-as since elastic#79809. There are places in the code where we assume API key cannot run-as. Most of them are corrected in elastic#81564. But there are still a few things got missed. This PR fixes the methods for checking owner user realm for API key. This means, when API Keys "running-as" (impersonating other users), we do not expose the authenticating key ID and name to the end-user such as the Authenticate API and the SetSecurityUseringest processor. Only the effective user is revealed, just like in the regular case of a realm user run as. For audit logging, the key's ID and name are not exposed either. But this is mainly because there are no existing fields suitable for these information. We do intend to add them later (elastic#84394) because auditing logging is to consumed by system admin instead of end-users. Note the resource sharing check (canAccessResourcesOf) also needs to be fixed, this will be handled by elastic#84277
elasticsearchmachine
pushed a commit
that referenced
this pull request
Feb 28, 2022
* Fix owner user realm check for API key authentication (#84325) API Key can run-as since #79809. There are places in the code where we assume API key cannot run-as. Most of them are corrected in #81564. But there are still a few things got missed. This PR fixes the methods for checking owner user realm for API key. This means, when API Keys "running-as" (impersonating other users), we do not expose the authenticating key ID and name to the end-user such as the Authenticate API and the SetSecurityUseringest processor. Only the effective user is revealed, just like in the regular case of a realm user run as. For audit logging, the key's ID and name are not exposed either. But this is mainly because there are no existing fields suitable for these information. We do intend to add them later (#84394) because auditing logging is to consumed by system admin instead of end-users. Note the resource sharing check (canAccessResourcesOf) also needs to be fixed, this will be handled by #84277 * fix test
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.
This is essentially a follow-up PR for #79809 which enables run-as for API keys. The code still has a few places where it assumes API keys cannot perform run-as. This PR fixes the incorrect assumptions and adds relevant tests.
Resolves: #81425