Fix issue with Get mappings on a Closed index

[cwperks]

Description

This PR fixes an issue filtering out fields on a GET Mappings request on a closed index. This PR updates the indices options used when getting concrete indices.

• Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Bug fix

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR
• API changes companion pull request created
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov]

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 65.20%. Comparing base (6735b5c) to head (e998d92).

Files with missing lines	Patch %	Lines
...pensearch/security/securityconf/ConfigModelV6.java	42.85%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4685      +/-   ##
==========================================
- Coverage   65.20%   65.20%   -0.01%     
==========================================
  Files         318      318              
  Lines       22327    22328       +1     
  Branches     3591     3593       +2     
==========================================
  Hits        14558    14558              
  Misses       5971     5971              
- Partials     1798     1799       +1     

Files with missing lines	Coverage Δ
...pensearch/security/securityconf/ConfigModelV7.java	67.55% <100.00%> (ø)
...pensearch/security/securityconf/ConfigModelV6.java	33.00% <42.85%> (-0.06%)	⬇️

[cwperks]

Don't merge this PR yet, I am planning to push a commit to pass a boolean param in this method to includeClosed with default value of false. I plan to pass this param as true when computing the EvaluatedDlsFlsConfig.

[cwperks]

Pushed a change to pass includeClosed as a param to ConfigV7.getResolvedIndexPattern so that the caller can determine how they want the resolution to proceed.

It makes sense to include Closed when computing the EvaluatedFlsDlsConfig, but should not break other use-cases.

For instance if you have 3 indices:

• logs-1 - open
• logs-2 - open
• logs-3 - closed

When you search on logs-* the current behavior (need to double check) will only resolve to logs-1 and logs-2 when ultimately performing the search so logs-3 is excluded. If this expansion mode is changed, then it might include logs-3 in the resolution which it currently does not. I will write an integ test to confirm this search scenario.

[cwperks]

Its a bit more complicated then this. The security plugin has a setting to preserve the indices options from the original request, but it is disabled by default. See this section of IndexResolverReplacer.

There's a few places in the security plugin that resolve patterns -> concrete indices:

1. On the original request, if the request contains index patterns then these will get resolved to concrete indices for authorization
2. When looping through roles, it will extract all patterns associated with index_permissions and resolve these to concrete indices

I think the logic I added into this PR is the right way to go. Consider the closed indices when computing the EvaluatedFlsDlsConfig, but keep the existing IndicesOptions.lenientExpandOpen() otherwise.

[shikharj05]

When you search on logs-* the current behavior (need to double check) will only resolve to logs-1 and logs-2 when ultimately performing the search so logs-3 is excluded. If this expansion mode is changed, then it includes logs-3 in the resolution which it currently does not. I will write an integ test to confirm this search scenario.

Thanks, good catch! This is an important scenario to test, DLS/FLS should not run search on closed indices.

[cwperks]

The security plugin does index pattern -> concrete resolution in 2 places.

1. When expanding out the index_permissions from roles definitions. It expands to concrete indices to determine DLS/FLS/Field Masking rules per concrete index. See ConfigV7.getDlsFls
2. When determining the indices from a request.

For the second use-case, I actually found a new dynamic setting from the security plugin that I wasn't aware of yet. config.dynamic.respect_request_indices_options which has a default value of false. If this value is set to true it will use the original request's options when expanding concrete indices. For example, if a search request includes expand_wildcards=all then security would copy this. The default value for security is here.

IndicesOptions.fromOptions(false, true, true, false, true);

For reference, this is the default for a SearchRequest

See the full method here for determining IndicesOptions: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java#L810-L823

[shikharj05]

You might need to remove the commits from a different rebase?

[cwperks]

You might need to remove the commits from a different rebase?

Done