From 752dbffd6aab3685779dbca990d4d4892147d72e Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 11 Sep 2024 16:44:39 -0700 Subject: [PATCH] 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 0aebc6d4cb2652786e17a5fb579060543ec6c028) Signed-off-by: Kaituo Li Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../opensearch/ad/rest/SecureADRestIT.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java b/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java index a63917f0f..3f38e8913 100644 --- a/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java +++ b/src/test/java/org/opensearch/ad/rest/SecureADRestIT.java @@ -61,6 +61,8 @@ public class SecureADRestIT extends AnomalyDetectorRestTestCase { RestClient lionClient; private String indexAllAccessRole = "index_all_access"; private String indexSearchAccessRole = "index_all_search"; + String oceanUser = "ocean"; + RestClient oceanClient; /** * Create an unguessable password. Simple password are weak due to https://tinyurl.com/383em9zk @@ -156,7 +158,13 @@ public void setupSecureTests() throws IOException { .setSocketTimeout(60000) .build(); - createRoleMapping("anomaly_read_access", new ArrayList<>(Arrays.asList(bobUser))); + String oceanPassword = generatePassword(oceanUser); + createUser(oceanUser, elkPassword, new ArrayList<>(Arrays.asList("odfe"))); + oceanClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), oceanUser, oceanPassword) + .setSocketTimeout(60000) + .build(); + + createRoleMapping("anomaly_read_access", new ArrayList<>(Arrays.asList(bobUser, oceanUser))); createRoleMapping("anomaly_full_access", new ArrayList<>(Arrays.asList(aliceUser, catUser, dogUser, elkUser, fishUser, goatUser))); createRoleMapping(indexAllAccessRole, new ArrayList<>(Arrays.asList(aliceUser, bobUser, catUser, dogUser, fishUser, lionUser))); createRoleMapping(indexSearchAccessRole, new ArrayList<>(Arrays.asList(goatUser))); @@ -172,6 +180,7 @@ public void deleteUserSetup() throws IOException { fishClient.close(); goatClient.close(); lionClient.close(); + oceanClient.close(); deleteUser(aliceUser); deleteUser(bobUser); deleteUser(catUser); @@ -180,6 +189,7 @@ public void deleteUserSetup() throws IOException { deleteUser(fishUser); deleteUser(goatUser); deleteUser(lionUser); + deleteUser(oceanUser); } public void testCreateAnomalyDetectorWithWriteAccess() throws IOException { @@ -414,8 +424,8 @@ public void testCreateAnomalyDetectorWithNoReadPermissionOfIndex() throws IOExce AnomalyDetector anomalyDetector = createRandomAnomalyDetector(false, false, aliceClient); // User elk has AD full access, but has no read permission of index String indexName = anomalyDetector.getIndices().get(0); - Exception exception = expectThrows(IOException.class, () -> { createRandomAnomalyDetector(false, false, indexName, elkClient); }); - Assert.assertTrue(exception.getMessage().contains("no permissions for [indices:data/read/search]")); + Exception exception = expectThrows(IOException.class, () -> { createRandomAnomalyDetector(false, false, indexName, oceanClient); }); + Assert.assertTrue("actual: " + exception.getMessage(), exception.getMessage().contains("Unauthorized")); } public void testCreateAnomalyDetectorWithCustomResultIndex() throws IOException { @@ -494,12 +504,8 @@ public void testPreviewAnomalyDetectorWithNoReadPermissionOfIndex() throws IOExc ); enableFilterBy(); // User elk has no read permission of index - Exception exception = expectThrows(Exception.class, () -> { previewAnomalyDetector(aliceDetector.getId(), elkClient, input); }); - Assert - .assertTrue( - "actual msg: " + exception.getMessage(), - exception.getMessage().contains("no permissions for [indices:data/read/search]") - ); + Exception exception = expectThrows(Exception.class, () -> { previewAnomalyDetector(aliceDetector.getId(), oceanClient, input); }); + Assert.assertTrue("actual msg: " + exception.getMessage(), exception.getMessage().contains("Unauthorized")); } public void testValidateAnomalyDetectorWithWriteAccess() throws IOException { @@ -528,8 +534,8 @@ public void testValidateAnomalyDetectorWithNoReadPermissionOfIndex() throws IOEx AnomalyDetector detector = TestHelpers.randomAnomalyDetector(null, Instant.now()); enableFilterBy(); // User elk has no read permission of index, can't validate detector - Exception exception = expectThrows(Exception.class, () -> { validateAnomalyDetector(detector, elkClient); }); - Assert.assertTrue(exception.getMessage().contains("no permissions for [indices:data/read/search]")); + Exception exception = expectThrows(Exception.class, () -> { validateAnomalyDetector(detector, oceanClient); }); + Assert.assertTrue("actual: " + exception.getMessage(), exception.getMessage().contains("Unauthorized")); } public void testValidateAnomalyDetectorWithNoBackendRole() throws IOException {