Skip to content

Conversation

@slobodanadamovic
Copy link
Contributor

@slobodanadamovic slobodanadamovic commented Apr 16, 2025

This PR adds more test coverage for:

  • adding/removing a failure store backing index
  • data stream APIs
  • watcher

This PR adds more test coverage for:

- pattern exclusions
- adding/removing a failure store backing index
- data stream APIs
- alias based access
@slobodanadamovic slobodanadamovic added >test Issues or PRs that are addressing/adding tests :Security/Security Security issues without another label Team:Security Meta label for security team auto-backport Automatically create backport pull requests when merged v8.19.0 v9.1.0 labels Apr 16, 2025
@slobodanadamovic slobodanadamovic self-assigned this Apr 16, 2025
@gmarouli
Copy link
Contributor

gmarouli commented Apr 28, 2025

#127458 is fixing (hopefully) the data stream alias test. 7922694 copied the test case from here.

…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)
@slobodanadamovic slobodanadamovic requested a review from n1v0lg April 29, 2025 15:21
@slobodanadamovic slobodanadamovic marked this pull request as ready for review April 29, 2025 15:21
@elasticsearchmachine
Copy link
Collaborator

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)
Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

LGTM!

@slobodanadamovic slobodanadamovic added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label May 7, 2025
@elasticsearchmachine elasticsearchmachine merged commit 72d2c92 into elastic:main May 8, 2025
22 checks passed
@slobodanadamovic slobodanadamovic deleted the sa-failure-store-it branch May 8, 2025 06:51
slobodanadamovic added a commit to slobodanadamovic/elasticsearch that referenced this pull request May 8, 2025
This PR adds more test coverage for:

- adding/removing a failure store backing index
- data stream APIs
- watcher
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.19

elasticsearchmachine pushed a commit that referenced this pull request May 8, 2025
This PR adds more test coverage for:

- adding/removing a failure store backing index
- data stream APIs
- watcher
ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request May 9, 2025
This PR adds more test coverage for:

- adding/removing a failure store backing index
- data stream APIs
- watcher
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request May 12, 2025
This PR adds more test coverage for:

- adding/removing a failure store backing index
- data stream APIs
- watcher
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Security/Security Security issues without another label Team:Security Meta label for security team >test Issues or PRs that are addressing/adding tests v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants