Skip to content
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.

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

[2.16] Revert change to use System Index Registry from core to determine if request matches system indices #4616

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 0 additions & 2 deletions release-notes/opensearch-security.release-notes-2.16.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.16.0

### Enhancements
* Add support for PBKDF2 for password hashing & add support for configuring BCrypt and PBKDF2 ([#4524](https://github.com/opensearch-project/security/pull/4524))
* Use SystemIndexRegistry from core to determine if request contains system indices ([#4471](https://github.com/opensearch-project/security/pull/4471))
* Separated DLS/FLS privilege evaluation from action privilege evaluation ([#4490](https://github.com/opensearch-project/security/pull/4490))
* Update PULL_REQUEST_TEMPLATE to include an API spec change in the checklist. ([#4533](https://github.com/opensearch-project/security/pull/4533))
* Update PATCH API to fail validation if nothing changes ([#4530](https://github.com/opensearch-project/security/pull/4530))
Expand All @@ -26,7 +25,6 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.16.0
### Maintenance
* Remove unused dependancy Apache CXF ([#4580](https://github.com/opensearch-project/security/pull/4580))
* Remove unnecessary return statements ([#4558](https://github.com/opensearch-project/security/pull/4558))
* Pass set to SystemIndexRegistry.matchesSystemIndexPattern ([#4569](https://github.com/opensearch-project/security/pull/4569))
* Refactor and update existing ml roles ([#4151](https://github.com/opensearch-project/security/pull/4151))
* Replace JUnit assertEquals() with Hamcrest matchers assertThat() ([#4544](https://github.com/opensearch-project/security/pull/4544))
* Update Gradle to 8.9 ([#4553](https://github.com/opensearch-project/security/pull/4553))
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public class PrivilegesEvaluator {
private ConfigModel configModel;
private final IndexResolverReplacer irr;
private final SnapshotRestoreEvaluator snapshotRestoreEvaluator;
private final SystemIndexAccessEvaluator systemIndexAccessEvaluator;
private final SecurityIndexAccessEvaluator securityIndexAccessEvaluator;
private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator;
private final TermsAggregationEvaluator termsAggregationEvaluator;
private final PitPrivilegesEvaluator pitPrivilegesEvaluator;
Expand Down Expand Up @@ -172,7 +172,7 @@ public PrivilegesEvaluator(
this.clusterInfoHolder = clusterInfoHolder;
this.irr = irr;
snapshotRestoreEvaluator = new SnapshotRestoreEvaluator(settings, auditLog);
systemIndexAccessEvaluator = new SystemIndexAccessEvaluator(settings, auditLog, irr);
securityIndexAccessEvaluator = new SecurityIndexAccessEvaluator(settings, auditLog, irr);
protectedIndexAccessEvaluator = new ProtectedIndexAccessEvaluator(settings, auditLog);
termsAggregationEvaluator = new TermsAggregationEvaluator();
pitPrivilegesEvaluator = new PitPrivilegesEvaluator();
Expand Down Expand Up @@ -328,7 +328,7 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
}

// Security index access
if (systemIndexAccessEvaluator.evaluate(
if (securityIndexAccessEvaluator.evaluate(
request,
task,
action0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.indices.SystemIndexRegistry;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
Expand All @@ -57,7 +56,7 @@
* - The term `protected system indices` used here translates to system indices
* which have an added layer of security and cannot be accessed by anyone except Super Admin
*/
public class SystemIndexAccessEvaluator {
public class SecurityIndexAccessEvaluator {

Logger log = LogManager.getLogger(this.getClass());

Expand All @@ -73,7 +72,7 @@ public class SystemIndexAccessEvaluator {
private final boolean isSystemIndexEnabled;
private final boolean isSystemIndexPermissionEnabled;

public SystemIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) {
public SecurityIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) {
this.securityIndex = settings.get(
ConfigConstants.SECURITY_CONFIG_INDEX_NAME,
ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX
Expand All @@ -84,7 +83,6 @@ public SystemIndexAccessEvaluator(final Settings settings, AuditLog auditLog, In
this.systemIndexMatcher = WildcardMatcher.from(
settings.getAsList(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY, ConfigConstants.SECURITY_SYSTEM_INDICES_DEFAULT)
);

this.superAdminAccessOnlyIndexMatcher = WildcardMatcher.from(this.securityIndex);
this.isSystemIndexEnabled = settings.getAsBoolean(
ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY,
Expand Down Expand Up @@ -170,16 +168,15 @@ private boolean requestContainsAnySystemIndices(final Resolved requestedResolved
* It will always return security index if it is present in the request, as security index is protected regardless
* of feature being enabled or disabled
* @param requestedResolved request which contains indices to be matched against system indices
* @return the set of protected system indices present in the request
* @return the list of protected system indices present in the request
*/
private Set<String> getAllSystemIndices(final Resolved requestedResolved) {
final Set<String> systemIndices = requestedResolved.getAllIndices()
private List<String> getAllSystemIndices(final Resolved requestedResolved) {
final List<String> systemIndices = requestedResolved.getAllIndices()
.stream()
.filter(securityIndex::equals)
.collect(Collectors.toSet());
.collect(Collectors.toList());
if (isSystemIndexEnabled) {
systemIndices.addAll(systemIndexMatcher.getMatchAny(requestedResolved.getAllIndices(), Collectors.toList()));
systemIndices.addAll(SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices()));
}
return systemIndices;
}
Expand Down Expand Up @@ -213,7 +210,7 @@ private List<String> getAllProtectedSystemIndices(final Resolved requestedResolv
private boolean requestContainsAnyRegularIndices(final Resolved requestedResolved) {
Set<String> allIndices = requestedResolved.getAllIndices();

Set<String> allSystemIndices = getAllSystemIndices(requestedResolved);
List<String> allSystemIndices = getAllSystemIndices(requestedResolved);
List<String> allProtectedSystemIndices = getAllProtectedSystemIndices(requestedResolved);

return allIndices.stream().anyMatch(index -> !allSystemIndices.contains(index) && !allProtectedSystemIndices.contains(index));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
import static org.mockito.Mockito.when;

@RunWith(MockitoJUnitRunner.class)
public class SystemIndexAccessEvaluatorTest {
public class SecurityIndexAccessEvaluatorTest {

@Mock
private AuditLog auditLog;
Expand All @@ -73,7 +73,7 @@ public class SystemIndexAccessEvaluatorTest {
@Mock
ClusterService cs;

private SystemIndexAccessEvaluator evaluator;
private SecurityIndexAccessEvaluator evaluator;
private static final String UNPROTECTED_ACTION = "indices:data/read";
private static final String PROTECTED_ACTION = "indices:data/write";

Expand Down Expand Up @@ -137,7 +137,7 @@ public void setup(

// when trying to resolve Index Names

evaluator = new SystemIndexAccessEvaluator(
evaluator = new SecurityIndexAccessEvaluator(
Settings.builder()
.put(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY, TEST_SYSTEM_INDEX)
.put(ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY, isSystemIndexEnabled)
Expand Down
Loading