-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Fix security origin for TokenService#findActiveTokensFor... #47418
Fix security origin for TokenService#findActiveTokensFor... #47418
Conversation
Pinging @elastic/es-security (:Security/Authorization) |
@elasticmachine run elasticsearch-ci/bwc |
@elasticmachine run elasticsearch-ci/default-distro |
The CI failures had |
@elasticmachine update branch |
Can we add an x-pack test (e.g. a rest |
@elasticmachine test this please |
1 similar comment
@elasticmachine test this please |
Hi @albertzaharovits, Thank you for addressing the review comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@elasticmachine run elasticsearch-ci/2 |
@bizybot I have pushed the change as you recommended. |
@elasticmachine run elasticsearch-ci/bwc |
@elasticmachine run elasticsearch-ci/2 |
Previous failure (https://gradle-enterprise.elastic.co/s/aulxbxfqrnjxc) tracked under #48258 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thank you.
…48280) All internal searches (triggered by APIs) across the .security index must be performed while "under the security origin". Otherwise, the search is performed in the context of the caller which most likely does not have privileges to search .security (hopefully). This commit fixes this in the case of two methods in the TokenService and corrects an overly done such context switch in the ApiKeyService. In addition, this makes all tests from the client/rest-high-level module execute as an all mighty administrator, but not a literal superuser. Closes #47151
"username": "api_key_user_1" | ||
} | ||
- length: { "invalidated_api_keys" : 1 } | ||
- match: { "invalidated_api_keys.0" : "${user1_key_id}" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "${user1_key_id}"
looks incorrect — all the other "stash" matchers have only $user1_key_id
. This breaks for me on the Go client, I'm wondering what the Java runner is doing, so this syntax passes?
All internal searches (triggered by APIs) across the
.security
index must be performed while "under thesecurity
origin". Otherwise the search is performed in the context of the caller which most likely does not have privileges to search.security
(hopefully).This commit fixes this in the case of two methods in the
TokenService
and corrects an overly done such context switch in theApiKeyService
.In addition, this makes all tests from the
client/rest-high-level
module execute as an all mighty administrator, but not a literalsuperuser
.Closes #47151