Skip to content

Conversation

@nibix
Copy link
Collaborator

@nibix nibix commented Nov 7, 2025

Description

This fixes the issue observed in opensearch-project/observability#1951 and #5753 (comment) :

Update operations did not work on the "virtual" .kibana index that is being provided by the multi tenancy implementation in PrivilegesInterceptorImpl. This is because since OpenSearch 3, we have a "deny-by-default" model for DLS and FLS. As the PrivilegesInterceptorImpl overrides normal privileges and redirects access to indices for which no privileges are necessarily present, the .kibana index appeared to be protected by DLS/FLS. Due to the implementation of DlsFlsValveImpl, this only affected update operations and no other operations.

For handling this, the PrivilegesInterceptorImpl already utilizes the DocumentAllowList:

// The new DLS/FLS implementation defaults to a "deny all" pattern in case no roles are configured
// for an index. As the PrivilegeInterceptor grants access to indices bypassing index privileges,
// we need to allow-list these indices.
applyDocumentAllowList(tenantIndexName);
return newAccessGrantedReplaceResult(replaceIndex(request, dashboardsIndexName, tenantIndexName, action));
} else if (!user.getName().equals(dashboardsServerUsername)) {
if (isTraceEnabled) {
log.trace("not a request to only the .kibana index");
log.trace(user.getName() + "/" + dashboardsServerUsername);
log.trace(requestedResolved + " does not contain only " + dashboardsIndexName);
}
}
return CONTINUE_EVALUATION_REPLACE_RESULT;
}
private void applyDocumentAllowList(String indexName) {
DocumentAllowList documentAllowList = new DocumentAllowList();
documentAllowList.add(indexName, "*");
IndexAbstraction indexAbstraction = clusterService.state().getMetadata().getIndicesLookup().get(indexName);
if (indexAbstraction instanceof IndexAbstraction.Alias) {
for (IndexMetadata index : ((IndexAbstraction.Alias) indexAbstraction).getIndices()) {
documentAllowList.add(index.getIndex().getName(), "*");
}
}
documentAllowList.applyTo(threadPool.getThreadContext());
}

However, this was not yet evaluated in DlsFlsValve.invoke().

Note: This PR requires #5753 to be merged first, as it utilizes the tests introduced there. Thus, this is a draft PR so far.

  • Category: Bug fix
  • Why these changes are required?: Update operations on the .kibana index do not work correctly
  • What is the old behavior before changes and new behavior after changes? Update operations now work correctly.

Issues Resolved

Testing

  • integration test

Check List

  • New functionality includes testing
  • New functionality has been documented
  • 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.

…king is active" in Dashboards multi tenancy

Signed-off-by: Nils Bandener <[email protected]>
@codecov
Copy link

codecov bot commented Nov 11, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 52.42%. Comparing base (062ea71) to head (d3b1494).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...search/security/configuration/DlsFlsValveImpl.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5778      +/-   ##
==========================================
- Coverage   52.46%   52.42%   -0.04%     
==========================================
  Files         434      434              
  Lines       26617    26621       +4     
  Branches     3963     3964       +1     
==========================================
- Hits        13964    13957       -7     
- Misses      10966    10973       +7     
- Partials     1687     1691       +4     
Files with missing lines Coverage Δ
...search/security/configuration/DlsFlsValveImpl.java 55.97% <75.00%> (-1.36%) ⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cwperks cwperks merged commit e444495 into opensearch-project:main Nov 19, 2025
74 of 75 checks passed
@cwperks cwperks added the v3.4.0 label Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants