Skip to content

Commit 752dbff

Browse files
Fix Security Tests After Changes to Permissions Requirements (#1308) (#1310)
This PR addresses errors in security tests caused by recent changes in opensearch-project/security#4719. Previously, users needed both AD full access and source index permissions to fully utilize anomaly detection (AD). AD full access has already included all alias and mapping permissions. it was inconsistent not to include index search permission, which would otherwise force users to create an additional role. The change in the referenced PR aimed to simplify user management. Due to this change, existing security tests that relied on a user having AD full access but lacking data search permission would no longer trigger the expected search permission exception. This PR addresses that issue by creating a new user role with only AD read permission (note we didn't change ad read access permission in the referenced PR) and without source index search permission, ensuring the tests correctly validate the lack of search permissions. Testing Done: * Verified that previously failing security tests now pass (cherry picked from commit 0aebc6d) Signed-off-by: Kaituo Li <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 47d159c commit 752dbff

File tree

1 file changed

+17
-11
lines changed

1 file changed

+17
-11
lines changed

src/test/java/org/opensearch/ad/rest/SecureADRestIT.java

+17-11
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ public class SecureADRestIT extends AnomalyDetectorRestTestCase {
6161
RestClient lionClient;
6262
private String indexAllAccessRole = "index_all_access";
6363
private String indexSearchAccessRole = "index_all_search";
64+
String oceanUser = "ocean";
65+
RestClient oceanClient;
6466

6567
/**
6668
* Create an unguessable password. Simple password are weak due to https://tinyurl.com/383em9zk
@@ -156,7 +158,13 @@ public void setupSecureTests() throws IOException {
156158
.setSocketTimeout(60000)
157159
.build();
158160

159-
createRoleMapping("anomaly_read_access", new ArrayList<>(Arrays.asList(bobUser)));
161+
String oceanPassword = generatePassword(oceanUser);
162+
createUser(oceanUser, elkPassword, new ArrayList<>(Arrays.asList("odfe")));
163+
oceanClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), oceanUser, oceanPassword)
164+
.setSocketTimeout(60000)
165+
.build();
166+
167+
createRoleMapping("anomaly_read_access", new ArrayList<>(Arrays.asList(bobUser, oceanUser)));
160168
createRoleMapping("anomaly_full_access", new ArrayList<>(Arrays.asList(aliceUser, catUser, dogUser, elkUser, fishUser, goatUser)));
161169
createRoleMapping(indexAllAccessRole, new ArrayList<>(Arrays.asList(aliceUser, bobUser, catUser, dogUser, fishUser, lionUser)));
162170
createRoleMapping(indexSearchAccessRole, new ArrayList<>(Arrays.asList(goatUser)));
@@ -172,6 +180,7 @@ public void deleteUserSetup() throws IOException {
172180
fishClient.close();
173181
goatClient.close();
174182
lionClient.close();
183+
oceanClient.close();
175184
deleteUser(aliceUser);
176185
deleteUser(bobUser);
177186
deleteUser(catUser);
@@ -180,6 +189,7 @@ public void deleteUserSetup() throws IOException {
180189
deleteUser(fishUser);
181190
deleteUser(goatUser);
182191
deleteUser(lionUser);
192+
deleteUser(oceanUser);
183193
}
184194

185195
public void testCreateAnomalyDetectorWithWriteAccess() throws IOException {
@@ -414,8 +424,8 @@ public void testCreateAnomalyDetectorWithNoReadPermissionOfIndex() throws IOExce
414424
AnomalyDetector anomalyDetector = createRandomAnomalyDetector(false, false, aliceClient);
415425
// User elk has AD full access, but has no read permission of index
416426
String indexName = anomalyDetector.getIndices().get(0);
417-
Exception exception = expectThrows(IOException.class, () -> { createRandomAnomalyDetector(false, false, indexName, elkClient); });
418-
Assert.assertTrue(exception.getMessage().contains("no permissions for [indices:data/read/search]"));
427+
Exception exception = expectThrows(IOException.class, () -> { createRandomAnomalyDetector(false, false, indexName, oceanClient); });
428+
Assert.assertTrue("actual: " + exception.getMessage(), exception.getMessage().contains("Unauthorized"));
419429
}
420430

421431
public void testCreateAnomalyDetectorWithCustomResultIndex() throws IOException {
@@ -494,12 +504,8 @@ public void testPreviewAnomalyDetectorWithNoReadPermissionOfIndex() throws IOExc
494504
);
495505
enableFilterBy();
496506
// User elk has no read permission of index
497-
Exception exception = expectThrows(Exception.class, () -> { previewAnomalyDetector(aliceDetector.getId(), elkClient, input); });
498-
Assert
499-
.assertTrue(
500-
"actual msg: " + exception.getMessage(),
501-
exception.getMessage().contains("no permissions for [indices:data/read/search]")
502-
);
507+
Exception exception = expectThrows(Exception.class, () -> { previewAnomalyDetector(aliceDetector.getId(), oceanClient, input); });
508+
Assert.assertTrue("actual msg: " + exception.getMessage(), exception.getMessage().contains("Unauthorized"));
503509
}
504510

505511
public void testValidateAnomalyDetectorWithWriteAccess() throws IOException {
@@ -528,8 +534,8 @@ public void testValidateAnomalyDetectorWithNoReadPermissionOfIndex() throws IOEx
528534
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(null, Instant.now());
529535
enableFilterBy();
530536
// User elk has no read permission of index, can't validate detector
531-
Exception exception = expectThrows(Exception.class, () -> { validateAnomalyDetector(detector, elkClient); });
532-
Assert.assertTrue(exception.getMessage().contains("no permissions for [indices:data/read/search]"));
537+
Exception exception = expectThrows(Exception.class, () -> { validateAnomalyDetector(detector, oceanClient); });
538+
Assert.assertTrue("actual: " + exception.getMessage(), exception.getMessage().contains("Unauthorized"));
533539
}
534540

535541
public void testValidateAnomalyDetectorWithNoBackendRole() throws IOException {

0 commit comments

Comments
 (0)