From 7b50e44b609606b8def5544303485289237b7039 Mon Sep 17 00:00:00 2001 From: Kaituo Li Date: Wed, 11 Sep 2024 15:00:20 -0700 Subject: [PATCH] Fix Security Tests After Changes to Permissions Requirements 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 Signed-off-by: Kaituo Li --- .../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 2b227bd9c..ddfa00662 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 { @@ -416,8 +426,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 { @@ -496,12 +506,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 { @@ -530,8 +536,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 {