Skip to content

Commit acaae3e

Browse files
committed
Fix IllegalArgumentException when resolved indices are empty
Signed-off-by: Maxim Muzafarov <[email protected]>
1 parent 17522f3 commit acaae3e

File tree

3 files changed

+72
-4
lines changed

3 files changed

+72
-4
lines changed

src/integrationTest/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivilegesTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.junit.runners.Suite;
3131

3232
import org.opensearch.action.support.IndicesOptions;
33+
import org.opensearch.cluster.ClusterState;
3334
import org.opensearch.cluster.metadata.IndexAbstraction;
3435
import org.opensearch.cluster.metadata.IndexMetadata;
3536
import org.opensearch.cluster.metadata.Metadata;
@@ -1034,6 +1035,30 @@ public void statefulDisabled() throws Exception {
10341035
subject.updateStatefulIndexPrivileges(metadata, 1);
10351036
assertEquals(0, subject.getEstimatedStatefulIndexByteSize());
10361037
}
1038+
1039+
/**
1040+
* Tests the behavior of hasIndexPrivilege when the resolved indices are empty.
1041+
* @throws Exception If failed.
1042+
*/
1043+
@Test
1044+
public void hasIndexPrivilegeEmptyResolvedIndices() throws Exception {
1045+
SecurityDynamicConfiguration<RoleV7> roles = SecurityDynamicConfiguration.fromYaml(
1046+
"test_role:\n"
1047+
+ " index_permissions:\n"
1048+
+ " - index_patterns: ['*']\n"
1049+
+ " allowed_actions: ['indices:monitor/recovery']",
1050+
CType.ROLES
1051+
);
1052+
1053+
RoleBasedActionPrivileges subject = new RoleBasedActionPrivileges(roles, FlattenedActionGroups.EMPTY, Settings.EMPTY);
1054+
1055+
PrivilegesEvaluatorResponse result = subject.hasIndexPrivilege(
1056+
ctx().clusterState(ClusterState.EMPTY_STATE).roles("test_role").get(),
1057+
ImmutableSet.of("indices:monitor/recovery"),
1058+
IndexResolverReplacer.Resolved._LOCAL_ALL
1059+
);
1060+
assertThat(result, isAllowed());
1061+
}
10371062
}
10381063

10391064
/**

src/integrationTest/java/org/opensearch/security/privileges/int_tests/IndexAuthorizationReadOnlyIntTests.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,20 @@ public class IndexAuthorizationReadOnlyIntTests {
361361
.reference(READ, unlimitedIncludingOpenSearchSecurityIndex())//
362362
.reference(GET_ALIAS, unlimitedIncludingOpenSearchSecurityIndex());
363363

364+
/**
365+
* A user with indices:monitor/recovery permission on all indices to verify that it succeeds in case all indices are closed.
366+
*/
367+
static final TestSecurityConfig.User LIMITED_USER_RECOVERY = new TestSecurityConfig.User("limited_user_recovery")//
368+
.description("indices:monitor/recovery on *")//
369+
.roles(
370+
new TestSecurityConfig.Role("r1")//
371+
.clusterPermissions("cluster_composite_ops_ro", "cluster_monitor")
372+
.indexPermissions("indices:monitor/recovery")
373+
.on("*")
374+
)//
375+
.reference(READ, limitedToNone())//
376+
.reference(GET_ALIAS, limitedToNone());
377+
364378
static final List<TestSecurityConfig.User> USERS = ImmutableList.of(
365379
LIMITED_USER_A,
366380
LIMITED_USER_B,
@@ -372,6 +386,7 @@ public class IndexAuthorizationReadOnlyIntTests {
372386
LIMITED_USER_C_WITH_SYSTEM_INDICES,
373387
LIMITED_USER_OTHER_PRIVILEGES,
374388
LIMITED_USER_NONE,
389+
LIMITED_USER_RECOVERY,
375390
UNLIMITED_USER,
376391
SUPER_UNLIMITED_USER
377392
);
@@ -1864,6 +1879,27 @@ public void pit_catSegments_all() throws Exception {
18641879
}
18651880
}
18661881

1882+
@Test
1883+
public void cat_recovery_allIndicesClosed() throws Exception {
1884+
try (TestRestClient adminClient = cluster.getAdminCertRestClient()) {
1885+
TestRestClient.HttpResponse closeResponse = adminClient.post("*/_close");
1886+
assertThat(closeResponse, isOk());
1887+
}
1888+
1889+
// We use /_cat/recovery test to verify that RuntimeOptimizedActionPrivileges
1890+
// work as intended when all indices are closed. This request should still be allowed.
1891+
// When all indices are closed, getAllIndicesResolved() returns empty, and
1892+
// RuntimeOptimizedActionPrivileges should grant the request permission.
1893+
try (TestRestClient restClient = cluster.getRestClient(user)) {
1894+
TestRestClient.HttpResponse httpResponse = restClient.get("_cat/recovery");
1895+
if (user == LIMITED_USER_RECOVERY || user == UNLIMITED_USER || user == SUPER_UNLIMITED_USER) {
1896+
assertThat(httpResponse, isOk());
1897+
} else {
1898+
assertThat(httpResponse, isForbidden("/error/root_cause/0/reason", "no permissions for [indices:monitor/recovery]"));
1899+
}
1900+
}
1901+
}
1902+
18671903
@ParametersFactory(shuffle = false, argumentFormatting = "%1$s, %3$s")
18681904
public static Collection<Object[]> params() {
18691905
List<Object[]> result = new ArrayList<>();

src/main/java/org/opensearch/security/privileges/actionlevel/RuntimeOptimizedActionPrivileges.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,19 @@ public PrivilegesEvaluatorResponse hasIndexPrivilege(
9898
return PrivilegesEvaluatorResponse.ok();
9999
}
100100

101+
Set<String> allIndicesResolved = resolvedIndices.getAllIndicesResolved(
102+
context.getClusterStateSupplier(),
103+
context.getIndexNameExpressionResolver()
104+
);
105+
106+
if (allIndicesResolved.isEmpty()) {
107+
log.debug("No resolved indices; grant the request");
108+
return PrivilegesEvaluatorResponse.ok();
109+
}
110+
101111
// TODO one might want to consider to create a semantic wrapper for action in order to be better tell apart
102112
// what's the action and what's the index in the generic parameters of CheckTable.
103-
CheckTable<String, String> checkTable = CheckTable.create(
104-
resolvedIndices.getAllIndicesResolved(context.getClusterStateSupplier(), context.getIndexNameExpressionResolver()),
105-
actions
106-
);
113+
CheckTable<String, String> checkTable = CheckTable.create(allIndicesResolved, actions);
107114

108115
StatefulIndexPrivileges statefulIndex = this.currentStatefulIndexPrivileges();
109116
PrivilegesEvaluatorResponse resultFromStatefulIndex = null;

0 commit comments

Comments
 (0)