[Failure Store] Add more test coverage #126891
Open
+332
−0
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
This PR adds more test coverage for: - pattern exclusions - adding/removing a failure store backing index - data stream APIs - alias based access
slobodanadamovic
added
>test
…re-store-it # Conflicts: # x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java
gmarouli
added a commit
that referenced
this pull request
Apr 29, 2025
…7458) While expanding our tests in #126891, we discovered that there was a difference in behaviour when an alias was used during search. Expected behaviour When a user uses an alias to access data, we pass this alias also to the node requests because that ensures that the user's authorisation will be evaluated based on the same premise. Observed behaviour When a user would use the alias and the ::failures selector, we noticed that the request was resolved to the failure indices and the alias was not used further which sometimes resulted in an unauthorised error. The reason was that when a node different than the coordinating node would try to evaluate if the user has permissions, they would evaluate that against the failure indices themselves and not the alias with the ::failures selector. Solution In this PR we ensure that a resolved alias is retrieved for failure indices too. The indexAliases method is used in two ways (in production code): To retrieve all the resolved aliases that match an index To retrieve the filtered aliases that match the index (when there is no unfiltered reference to this index) Because failure indices are not supported by filtered aliases, the code would return null when a failure index was encountered. That works well for the second case and not for the first. In order to address that we moved the check for the failure index to the data stream alias predicate and we let the code match failure indices against resolved failure expressions and backing indices against resolved data expressions. Furthermore, we added methods capturing these two common cases so the users of these methods for not need to know the details of how to call them.
…re-store-it # Conflicts: # x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java
gmarouli
added a commit
to gmarouli/elasticsearch
that referenced
this pull request
Apr 29, 2025
…stic#127458) While expanding our tests in elastic#126891, we discovered that there was a difference in behaviour when an alias was used during search. Expected behaviour When a user uses an alias to access data, we pass this alias also to the node requests because that ensures that the user's authorisation will be evaluated based on the same premise. Observed behaviour When a user would use the alias and the ::failures selector, we noticed that the request was resolved to the failure indices and the alias was not used further which sometimes resulted in an unauthorised error. The reason was that when a node different than the coordinating node would try to evaluate if the user has permissions, they would evaluate that against the failure indices themselves and not the alias with the ::failures selector. Solution In this PR we ensure that a resolved alias is retrieved for failure indices too. The indexAliases method is used in two ways (in production code): To retrieve all the resolved aliases that match an index To retrieve the filtered aliases that match the index (when there is no unfiltered reference to this index) Because failure indices are not supported by filtered aliases, the code would return null when a failure index was encountered. That works well for the second case and not for the first. In order to address that we moved the check for the failure index to the data stream alias predicate and we let the code match failure indices against resolved failure expressions and backing indices against resolved data expressions. Furthermore, we added methods capturing these two common cases so the users of these methods for not need to know the details of how to call them. (cherry picked from commit f209db6)
|
Pinging @elastic/es-security (Team:Security) |
gmarouli
added a commit
that referenced
this pull request
Apr 30, 2025
…7458) (#127498) While expanding our tests in #126891, we discovered that there was a difference in behaviour when an alias was used during search. Expected behaviour When a user uses an alias to access data, we pass this alias also to the node requests because that ensures that the user's authorisation will be evaluated based on the same premise. Observed behaviour When a user would use the alias and the ::failures selector, we noticed that the request was resolved to the failure indices and the alias was not used further which sometimes resulted in an unauthorised error. The reason was that when a node different than the coordinating node would try to evaluate if the user has permissions, they would evaluate that against the failure indices themselves and not the alias with the ::failures selector. Solution In this PR we ensure that a resolved alias is retrieved for failure indices too. The indexAliases method is used in two ways (in production code): To retrieve all the resolved aliases that match an index To retrieve the filtered aliases that match the index (when there is no unfiltered reference to this index) Because failure indices are not supported by filtered aliases, the code would return null when a failure index was encountered. That works well for the second case and not for the first. In order to address that we moved the check for the failure index to the data stream alias predicate and we let the code match failure indices against resolved failure expressions and backing indices against resolved data expressions. Furthermore, we added methods capturing these two common cases so the users of these methods for not need to know the details of how to call them. (cherry picked from commit f209db6)
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 PR adds more test coverage for: