Skip to content

Commit bdecbde

Browse files
nibixnibix
committed
Introduced more tests targeting system index privilege behavior
Signed-off-by: Nils Bandener <[email protected]>
1 parent ce90d22 commit bdecbde

File tree

1 file changed

+126
-7
lines changed

1 file changed

+126
-7
lines changed

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

Lines changed: 126 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,14 +212,20 @@ public class IndexAuthorizationReadOnlyIntTests {
212212
.roles(
213213
new TestSecurityConfig.Role("r1")//
214214
.clusterPermissions("cluster_composite_ops_ro", "cluster_monitor")
215-
.indexPermissions("read", "indices_monitor", "indices:admin/analyze")
216-
.on("index_c*")//
217-
.indexPermissions("read", "indices_monitor", "indices:admin/analyze", "system:admin/system_index")
215+
.indexPermissions("read", "indices_monitor", "indices:admin/analyze", "indices:admin/aliases/get")
216+
.on("index_c*", "alias_c1")//
217+
.indexPermissions(
218+
"read",
219+
"indices_monitor",
220+
"indices:admin/analyze",
221+
"indices:admin/aliases/get",
222+
"system:admin/system_index"
223+
)
218224
.on(".system_index_plugin")
219225
)//
220226
.indexMatcher("read", limitedTo(index_c1, alias_c1, system_index_plugin))//
221227
.indexMatcher("search", limitedTo(index_c1, alias_c1, system_index_plugin))//
222-
.indexMatcher("get_alias", limitedToNone());
228+
.indexMatcher("get_alias", limitedTo(index_c1, alias_c1, system_index_plugin));
223229

224230
/**
225231
* This user has no privileges for indices that are used in this test. But they have privileges for other indices.
@@ -725,6 +731,59 @@ public void search_indexPatternAndStatic_negation() throws Exception {
725731
}
726732
}
727733

734+
@Test
735+
public void search_indexPattern_includeHidden() throws Exception {
736+
try (TestRestClient restClient = cluster.getRestClient(user)) {
737+
TestRestClient.HttpResponse httpResponse = restClient.get("*index*/_search?size=1000&expand_wildcards=all");
738+
739+
if (user == SUPER_UNLIMITED_USER) {
740+
// The super admin sees everything
741+
assertThat(
742+
httpResponse,
743+
containsExactly(
744+
index_a1,
745+
index_a2,
746+
index_a3,
747+
index_b1,
748+
index_b2,
749+
index_b3,
750+
index_c1,
751+
index_hidden,
752+
index_hidden_dot,
753+
system_index_plugin
754+
).at("hits.hits[*]._index")
755+
.reducedBy(user.indexMatcher("search"))
756+
.whenEmpty(clusterConfig.allowsEmptyResultSets ? isOk() : isForbidden())
757+
);
758+
} else if (!clusterConfig.systemIndexPrivilegeEnabled) {
759+
// Without system index privileges, the system_index_plugin will be never included
760+
assertThat(
761+
httpResponse,
762+
containsExactly(index_a1, index_a2, index_a3, index_b1, index_b2, index_b3, index_c1, index_hidden, index_hidden_dot)
763+
.at("hits.hits[*]._index")
764+
.reducedBy(user.indexMatcher("search"))
765+
.whenEmpty(clusterConfig.allowsEmptyResultSets ? isOk() : isForbidden())
766+
);
767+
} else {
768+
// Things get buggy here; basically all requests fail with a 403
769+
if (user == LIMITED_USER_C_WITH_SYSTEM_INDICES) {
770+
// This user is supposed to have the system index privilege for the index .system_index_plugin
771+
// However, the system index privilege evaluation code only works correct when the system index is the
772+
// only requested index. If also non system indices are requested in the same request, it will require
773+
// the presence of the system index privilege for all indices. As this is not the case, the request
774+
// will be denied with a 403 error.
775+
assertThat(httpResponse, isForbidden());
776+
} else {
777+
// The other users do not have privileges for the system index. The dnfof feature promises to filter
778+
// out indices without authorization from eligible requests. However, the SystemIndexAccessEvaluator
779+
// is not aware of this and just denies all these requests
780+
// See also https://github.com/opensearch-project/security/issues/5546
781+
assertThat(httpResponse, isForbidden());
782+
}
783+
}
784+
}
785+
}
786+
728787
@Test
729788
public void search_alias() throws Exception {
730789
try (TestRestClient restClient = cluster.getRestClient(user)) {
@@ -789,7 +848,6 @@ public void search_aliasAndIndex_ignoreUnavailable() throws Exception {
789848
public void search_nonExisting_static() throws Exception {
790849
try (TestRestClient restClient = cluster.getRestClient(user)) {
791850
TestRestClient.HttpResponse httpResponse = restClient.get("x_does_not_exist/_search?size=1000");
792-
// TODO adapt name to match privs for some others
793851
if (user == UNLIMITED_USER || user == SUPER_UNLIMITED_USER) {
794852
assertThat(httpResponse, isNotFound());
795853
} else {
@@ -1228,7 +1286,7 @@ public void index_stats_pattern() throws Exception {
12281286
public void getAlias_all() throws Exception {
12291287
try (TestRestClient restClient = cluster.getRestClient(user)) {
12301288
TestRestClient.HttpResponse httpResponse = restClient.get("_alias");
1231-
if (clusterConfig.legacyPrivilegeEvaluation && user == UNLIMITED_USER) {
1289+
if (user == UNLIMITED_USER) {
12321290
// The legacy privilege evaluation also allows regular users access to metadata of the security index
12331291
// This is not a security issue, as the metadata are not really security relevant
12341292
assertThat(httpResponse, containsExactly(ALL_INDICES).at("$.keys()"));
@@ -1283,15 +1341,76 @@ public void getAlias_aliasPattern() throws Exception {
12831341
assertThat(httpResponse, isOk());
12841342
assertThat(httpResponse, containsExactly(alias_ab1).at("$.*.aliases.keys()").reducedBy(user.indexMatcher("get_alias")));
12851343
assertThat(httpResponse, containsExactly(index_a1, index_a2, index_a3, index_b1).at("$.keys()"));
1286-
} else if (user == LIMITED_USER_ALIAS_C1) {
1344+
} else if (user == LIMITED_USER_ALIAS_C1 || user == LIMITED_USER_C_WITH_SYSTEM_INDICES) {
12871345
// This is also a kind of anomaly in the legacy privilege evaluation: Even though we do not have permissions
12881346
// we get a 200 response with an empty result
12891347
assertThat(httpResponse, isOk());
12901348
assertTrue(httpResponse.getBody(), httpResponse.bodyAsMap().isEmpty());
12911349
} else {
12921350
assertThat(httpResponse, isForbidden("/error/root_cause/0/reason", "no permissions for [indices:admin/aliases/get]"));
12931351
}
1352+
}
1353+
}
12941354

1355+
@Test
1356+
public void getAlias_indexPattern_includeHidden() throws Exception {
1357+
try (TestRestClient restClient = cluster.getRestClient(user)) {
1358+
TestRestClient.HttpResponse httpResponse = restClient.get("*index*/_alias?expand_wildcards=all");
1359+
if (user == SUPER_UNLIMITED_USER) {
1360+
// The super admin sees everything
1361+
assertThat(httpResponse, isOk());
1362+
assertThat(httpResponse, containsExactly(alias_ab1, alias_c1).at("$.*.aliases.keys()"));
1363+
assertThat(
1364+
httpResponse,
1365+
containsExactly(
1366+
index_a1,
1367+
index_a2,
1368+
index_a3,
1369+
index_b1,
1370+
index_b2,
1371+
index_b3,
1372+
index_c1,
1373+
index_hidden,
1374+
index_hidden_dot,
1375+
system_index_plugin
1376+
).at("$.keys()")
1377+
);
1378+
} else if (!clusterConfig.systemIndexPrivilegeEnabled) {
1379+
if (user == UNLIMITED_USER) {
1380+
assertThat(
1381+
httpResponse,
1382+
containsExactly(
1383+
index_a1,
1384+
index_a2,
1385+
index_a3,
1386+
index_b1,
1387+
index_b2,
1388+
index_b3,
1389+
index_c1,
1390+
index_hidden,
1391+
index_hidden_dot,
1392+
system_index_plugin
1393+
).at("$.keys()")
1394+
);
1395+
} else {
1396+
assertThat(
1397+
httpResponse,
1398+
containsExactly(alias_ab1, alias_c1).at("$.*.aliases.keys()")
1399+
.reducedBy(user.indexMatcher("get_alias"))
1400+
.whenEmpty(clusterConfig.allowsEmptyResultSets ? isOk() : isForbidden())
1401+
);
1402+
assertThat(
1403+
httpResponse,
1404+
containsExactly(ALL_INDICES).at("$.keys()")
1405+
.reducedBy(user.indexMatcher("get_alias"))
1406+
.whenEmpty(clusterConfig.allowsEmptyResultSets ? isOk() : isForbidden())
1407+
);
1408+
}
1409+
} else {
1410+
// If the system index privilege is enabled, we only get 403 errors, as SystemIndexPrivilegeEvaluator
1411+
// is not aware of dnfof; see https://github.com/opensearch-project/security/issues/5546
1412+
assertThat(httpResponse, isForbidden());
1413+
}
12951414
}
12961415
}
12971416

0 commit comments

Comments
 (0)