Skip to content

Only update PrivilegesEvaluator configuration when the base config objects have actually changed. #6037

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.

Sign up for GitHub

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

Draft
nibix wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Only update PrivilegesEvaluator configuration when the base config objects have actually changed. #6037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df9aaf3
Only update PrivilegesEvaluator configuration when the base config ob…
nibix Mar 26, 2026
004a9ca
Changelog
nibix Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Enable basic authentication for gRPC transport ([#6005](https://github.com/opensearch-project/security/pull/6005))
- Allow specifying parentType and parentIdField in ResourceProvider ([#5735](https://github.com/opensearch-project/security/pull/5735))
- [Resource Sharing] Allow specifying default access level in resource access levels yml file ([#6018](https://github.com/opensearch-project/security/pull/6018))
- Only update internal compiled privileges configuration when the base config objects have actually changed ([#6037](https://github.com/opensearch-project/security/pull/6037))

### Bug Fixes
- Fix audit log writing errors for rollover-enabled alias indices ([#5878](https://github.com/opensearch-project/security/pull/5878)
Expand Down
179 changes: 109 additions & 70 deletions src/main/java/org/opensearch/security/privileges/PrivilegesConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;

Expand Down Expand Up @@ -61,8 +62,7 @@ public class PrivilegesConfiguration {

private final AtomicReference<TenantPrivileges> tenantPrivileges = new AtomicReference<>(TenantPrivileges.EMPTY);
private final AtomicReference<PrivilegesEvaluator> privilegesEvaluator;
private final AtomicReference<FlattenedActionGroups> actionGroups = new AtomicReference<>(FlattenedActionGroups.EMPTY);
private final AtomicReference<CompiledRoles> compiledRoles = new AtomicReference<>();
private final AtomicReference<RawConfiguration> rawConfiguration = new AtomicReference<>();
private final Map<String, RoleV7> pluginIdToRolePrivileges = new HashMap<>();
private final AtomicReference<DashboardsMultiTenancyConfiguration> multiTenancyConfiguration = new AtomicReference<>(
DashboardsMultiTenancyConfiguration.DEFAULT
Expand Down Expand Up @@ -121,16 +121,14 @@ public PrivilegesConfiguration(
.withStaticConfig();
ConfigV7 generalConfiguration = configurationRepository.getConfiguration(CType.CONFIG).getCEntry(CType.CONFIG.name());

FlattenedActionGroups flattenedActionGroups = new FlattenedActionGroups(actionGroupsConfiguration.withStaticConfig());
this.actionGroups.set(flattenedActionGroups);

CompiledRoles newCompiledRoles = new CompiledRoles(
rolesConfiguration.withStaticConfig(),
flattenedActionGroups,
namedXContentRegistry,
fieldMaskingConfig
RawConfiguration rawConfiguration = new RawConfiguration(
actionGroupsConfiguration,
rolesConfiguration,
tenantConfiguration,
PrivilegesEvaluator.GlobalDynamicSettings.fromConfigV7(generalConfiguration)
);
this.compiledRoles.set(newCompiledRoles);

RawConfiguration oldRawConfiguration = this.rawConfiguration.getAndSet(rawConfiguration);

PrivilegesEvaluator currentPrivilegesEvaluator = privilegesEvaluator.get();

Expand All @@ -139,58 +137,81 @@ public PrivilegesConfiguration(
PrivilegesEvaluator.PrivilegesEvaluatorType targetType = PrivilegesEvaluator.PrivilegesEvaluatorType.STANDARD;
PrivilegesEvaluator.PrivilegesEvaluatorType currentType = currentPrivilegesEvaluator.type();

if (currentType != targetType) {
PrivilegesEvaluator oldInstance = privilegesEvaluator.getAndSet(
new org.opensearch.security.privileges.PrivilegesEvaluatorImpl(
clusterService,
clusterStateSupplier,
roleMapper,
threadPool,
threadPool.getThreadContext(),
resolver,
auditLog,
settings,
privilegesInterceptor,
indexResolverReplacer,
flattenedActionGroups,
staticActionGroups,
newCompiledRoles,
generalConfiguration,
pluginIdToRolePrivileges
)
boolean privilegesChanged = oldRawConfiguration == null
|| !oldRawConfiguration.equals(rawConfiguration)
|| currentType != targetType;
if (privilegesChanged) {
log.debug("Privileges for PrivilegesEvaluator and DLS/FLS did not change; updating.");

FlattenedActionGroups flattenedActionGroups = new FlattenedActionGroups(actionGroupsConfiguration.withStaticConfig());

CompiledRoles newCompiledRoles = new CompiledRoles(
rolesConfiguration.withStaticConfig(),
flattenedActionGroups,
namedXContentRegistry,
fieldMaskingConfig
);
if (oldInstance != null) {
oldInstance.shutdown();

if (currentType != targetType) {
PrivilegesEvaluator oldInstance = privilegesEvaluator.getAndSet(
new org.opensearch.security.privileges.PrivilegesEvaluatorImpl(
clusterService,
clusterStateSupplier,
roleMapper,
threadPool,
threadPool.getThreadContext(),
resolver,
auditLog,
settings,
privilegesInterceptor,
indexResolverReplacer,
flattenedActionGroups,
staticActionGroups,
newCompiledRoles,
rawConfiguration.privilegesEvaluatorGlobalSettings,
pluginIdToRolePrivileges
)
);
if (oldInstance != null) {
oldInstance.shutdown();
}
} else {
privilegesEvaluator.get()
.updateConfiguration(
flattenedActionGroups,
newCompiledRoles,
rawConfiguration.privilegesEvaluatorGlobalSettings
);
}
} else {
privilegesEvaluator.get().updateConfiguration(flattenedActionGroups, newCompiledRoles, generalConfiguration);
}

try {
this.dlsFlsProcessedConfig.set(
new DlsFlsProcessedConfig(
newCompiledRoles,
clusterStateSupplier.get().metadata().getIndicesLookup(),
namedXContentRegistry,
settings,
fieldMaskingConfig
)
);
} catch (Exception e) {
log.error("Error while updating DlsFlsProcessedConfig", e);
try {
this.dlsFlsProcessedConfig.set(
new DlsFlsProcessedConfig(
newCompiledRoles,
clusterStateSupplier.get().metadata().getIndicesLookup(),
namedXContentRegistry,
settings,
fieldMaskingConfig
)
);
} catch (Exception e) {
log.error("Error while updating DlsFlsProcessedConfig", e);
}

try {
this.tenantPrivileges.set(new TenantPrivileges(rolesConfiguration, tenantConfiguration, flattenedActionGroups));
} catch (Exception e) {
log.error("Error while updating TenantPrivileges", e);
}
} else {
log.debug("Privileges for PrivilegesEvaluator and DLS/FLS did not change.");
}

try {
this.multiTenancyConfiguration.set(new DashboardsMultiTenancyConfiguration(generalConfiguration));
} catch (Exception e) {
log.error("Error while updating DashboardsMultiTenancyConfiguration", e);
}

try {
this.tenantPrivileges.set(new TenantPrivileges(rolesConfiguration, tenantConfiguration, flattenedActionGroups));
} catch (Exception e) {
log.error("Error while updating TenantPrivileges", e);
}
});
}

Expand Down Expand Up @@ -227,23 +248,6 @@ public PrivilegesEvaluator privilegesEvaluator() {
return this.privilegesEvaluator.get();
}

/**
* Returns the current action groups configuration. Important: Do not store the references to the instances returned here; these will change
* after configuration updates.
*/
public FlattenedActionGroups actionGroups() {
return this.actionGroups.get();
}

/**
* Returns the current compiled roles. These are built once per configuration update and shared between the
* action privilege layer and the DLS/FLS layer to avoid redundant pattern compilation.
* Important: Do not store the references to the instances returned here; these will change after configuration updates.
*/
public CompiledRoles compiledRoles() {
return this.compiledRoles.get();
}

Copy link
Collaborator Author

@nibix nibix Mar 26, 2026

Choose a reason for hiding this comment

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

Code was already unused before, removed to facilitate changes.

/**
* Returns the current Dashboards multi tenancy configuration. Important: Do not store the references to the instances returned here; these will change
* after configuration updates.
Expand All @@ -264,4 +268,39 @@ private static FlattenedActionGroups buildStaticActionGroups() {
return new FlattenedActionGroups(DynamicConfigFactory.addStatics(SecurityDynamicConfiguration.empty(CType.ACTIONGROUPS)));
}

private static class RawConfiguration {
final SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsConfiguration;
final SecurityDynamicConfiguration<RoleV7> rolesConfiguration;
final SecurityDynamicConfiguration<TenantV7> tenantConfiguration;
final PrivilegesEvaluator.GlobalDynamicSettings privilegesEvaluatorGlobalSettings;

RawConfiguration(
SecurityDynamicConfiguration<ActionGroupsV7> actionGroupsConfiguration,
SecurityDynamicConfiguration<RoleV7> rolesConfiguration,
SecurityDynamicConfiguration<TenantV7> tenantConfiguration,
PrivilegesEvaluator.GlobalDynamicSettings privilegesEvaluatorGlobalSettings
) {
this.actionGroupsConfiguration = actionGroupsConfiguration;
this.rolesConfiguration = rolesConfiguration;
this.tenantConfiguration = tenantConfiguration;
this.privilegesEvaluatorGlobalSettings = privilegesEvaluatorGlobalSettings;
}

@Override
public boolean equals(Object o) {
if (!(o instanceof RawConfiguration that)) {
return false;
}
return Objects.equals(actionGroupsConfiguration, that.actionGroupsConfiguration)
&& Objects.equals(rolesConfiguration, that.rolesConfiguration)
&& Objects.equals(tenantConfiguration, that.tenantConfiguration)
&& Objects.equals(privilegesEvaluatorGlobalSettings, that.privilegesEvaluatorGlobalSettings);
}

@Override
public int hashCode() {
return Objects.hash(actionGroupsConfiguration, rolesConfiguration, tenantConfiguration, privilegesEvaluatorGlobalSettings);
}
}

}
55 changes: 53 additions & 2 deletions src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

package org.opensearch.security.privileges;

import java.util.Objects;
import java.util.function.Supplier;

import org.opensearch.OpenSearchSecurityException;
Expand Down Expand Up @@ -59,7 +60,11 @@ PrivilegesEvaluationContext createContext(

boolean isClusterPermission(String action);

void updateConfiguration(FlattenedActionGroups actionGroups, CompiledRoles rolesConfiguration, ConfigV7 generalConfiguration);
void updateConfiguration(
FlattenedActionGroups actionGroups,
CompiledRoles rolesConfiguration,
GlobalDynamicSettings globalDynamicSettings
);

void updateClusterStateMetadata(ClusterService clusterService);

Expand Down Expand Up @@ -118,7 +123,7 @@ public boolean isClusterPermission(String action) {
public void updateConfiguration(
FlattenedActionGroups actionGroups,
CompiledRoles rolesConfiguration,
ConfigV7 generalConfiguration
GlobalDynamicSettings globalDynamicSettings
) {

}
Expand Down Expand Up @@ -152,4 +157,50 @@ private OpenSearchSecurityException exception() {
}
};

/**
* Configuration that is sourced from the "general purpose mixed bag" configuration type called config.
* The purpose of this class is to provide a focused view to the needed settings.
*/
class GlobalDynamicSettings {
final boolean dnfofEnabled;
final boolean dnfofForEmptyResultsEnabled;
final String filteredAliasMode;

GlobalDynamicSettings(boolean dnfofEnabled, boolean dnfofForEmptyResultsEnabled, String filteredAliasMode) {
this.dnfofEnabled = dnfofEnabled;
this.dnfofForEmptyResultsEnabled = dnfofForEmptyResultsEnabled;
this.filteredAliasMode = filteredAliasMode;
}

public static GlobalDynamicSettings fromConfigV7(ConfigV7 configV7) {
return new GlobalDynamicSettings(isDnfofEnabled(configV7), isDnfofEmptyEnabled(configV7), getFilteredAliasMode(configV7));
}

private static boolean isDnfofEnabled(ConfigV7 generalConfiguration) {
return generalConfiguration.dynamic != null && generalConfiguration.dynamic.do_not_fail_on_forbidden;
}

private static boolean isDnfofEmptyEnabled(ConfigV7 generalConfiguration) {
return generalConfiguration.dynamic != null && generalConfiguration.dynamic.do_not_fail_on_forbidden_empty;
}

private static String getFilteredAliasMode(ConfigV7 generalConfiguration) {
return generalConfiguration.dynamic != null ? generalConfiguration.dynamic.filtered_alias_mode : "none";
}

@Override
public boolean equals(Object o) {
if (!(o instanceof GlobalDynamicSettings that)) {
return false;
}
return dnfofEnabled == that.dnfofEnabled
&& dnfofForEmptyResultsEnabled == that.dnfofForEmptyResultsEnabled
&& Objects.equals(filteredAliasMode, that.filteredAliasMode);
}

@Override
public int hashCode() {
return Objects.hash(dnfofEnabled, dnfofForEmptyResultsEnabled, filteredAliasMode);
}
}
}
28 changes: 10 additions & 18 deletions src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
import org.opensearch.security.securityconf.FlattenedActionGroups;
import org.opensearch.security.securityconf.impl.v7.ConfigV7;
import org.opensearch.security.securityconf.impl.v7.RoleV7;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.WildcardMatcher;
Expand Down Expand Up @@ -166,7 +165,7 @@ public PrivilegesEvaluatorImpl(
FlattenedActionGroups actionGroups,
FlattenedActionGroups staticActionGroups,
CompiledRoles rolesConfiguration,
ConfigV7 generalConfiguration,
GlobalDynamicSettings globalDynamicSettings,
Map<String, RoleV7> pluginIdToRolePrivileges
) {

Expand Down Expand Up @@ -198,7 +197,7 @@ public PrivilegesEvaluatorImpl(
pitPrivilegesEvaluator = new PitPrivilegesEvaluator();

this.pluginIdToActionPrivileges = createActionPrivileges(pluginIdToRolePrivileges, staticActionGroups);
this.updateConfiguration(actionGroups, rolesConfiguration, generalConfiguration);
this.updateConfiguration(actionGroups, rolesConfiguration, globalDynamicSettings);
}

@Override
Expand All @@ -207,10 +206,14 @@ public PrivilegesEvaluatorType type() {
}

@Override
public void updateConfiguration(FlattenedActionGroups flattenedActionGroups, CompiledRoles roles, ConfigV7 generalConfiguration) {
this.dnfofEnabled = isDnfofEnabled(generalConfiguration);
this.dnfofForEmptyResultsEnabled = isDnfofEmptyEnabled(generalConfiguration);
this.filteredAliasMode = getFilteredAliasMode(generalConfiguration);
public void updateConfiguration(
FlattenedActionGroups flattenedActionGroups,
CompiledRoles roles,
GlobalDynamicSettings globalDynamicSettings
) {
this.dnfofEnabled = globalDynamicSettings.dnfofEnabled;
this.dnfofForEmptyResultsEnabled = globalDynamicSettings.dnfofForEmptyResultsEnabled;
this.filteredAliasMode = globalDynamicSettings.filteredAliasMode;

try {
RoleBasedActionPrivileges actionPrivileges = new RoleBasedActionPrivileges(roles, settings);
Expand Down Expand Up @@ -735,15 +738,4 @@ private static ImmutableMap<String, ActionPrivileges> createActionPrivileges(
return ImmutableMap.copyOf(result);
}

private static boolean isDnfofEnabled(ConfigV7 generalConfiguration) {
return generalConfiguration.dynamic != null && generalConfiguration.dynamic.do_not_fail_on_forbidden;
}

private static boolean isDnfofEmptyEnabled(ConfigV7 generalConfiguration) {
return generalConfiguration.dynamic != null && generalConfiguration.dynamic.do_not_fail_on_forbidden_empty;
}

private static String getFilteredAliasMode(ConfigV7 generalConfiguration) {
return generalConfiguration.dynamic != null ? generalConfiguration.dynamic.filtered_alias_mode : "none";
}
}
Loading
Loading