From c51940f5e2d14a00262f7f4675bbba2d930d2ede Mon Sep 17 00:00:00 2001 From: RAJ CHAKRAVARTHI <49325334+raj-chak@users.noreply.github.com> Date: Fri, 10 Feb 2023 13:25:53 -0500 Subject: [PATCH] fixed security tests (#484) * fixed security tests Signed-off-by: Raj Chakravarthi --- .../alerting/AlertingRestTestCase.kt | 3 +- .../resthandler/SecureDestinationRestApiIT.kt | 2 +- .../SecureEmailAccountRestApiIT.kt | 14 +-- .../resthandler/SecureEmailGroupsRestApiIT.kt | 2 +- .../resthandler/SecureMonitorRestApiIT.kt | 105 ++++++++++-------- 5 files changed, 68 insertions(+), 58 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt index a4bb7404f..d6cd4047c 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt @@ -1182,10 +1182,11 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { client().performRequest(request) } - fun createIndexRoleWithDocLevelSecurity(name: String, index: String, dlsQuery: String) { + fun createIndexRoleWithDocLevelSecurity(name: String, index: String, dlsQuery: String, clusterPermissions: String? = "") { val request = Request("PUT", "/_plugins/_security/api/roles/$name") var entity = "{\n" + "\"cluster_permissions\": [\n" + + "\"$clusterPermissions\"\n" + "],\n" + "\"index_permissions\": [\n" + "{\n" + diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt index 554f946c3..065e2d71d 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt @@ -41,7 +41,7 @@ class SecureDestinationRestApiIT : AlertingRestTestCase() { } } - val user = "userOne" + val user = "userA" var userClient: RestClient? = null @Before diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt index 782e2b1c9..16ee4fd96 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt @@ -13,6 +13,7 @@ import org.junit.After import org.junit.Before import org.junit.BeforeClass import org.opensearch.alerting.ALERTING_GET_EMAIL_ACCOUNT_ACCESS +import org.opensearch.alerting.ALERTING_NO_ACCESS_ROLE import org.opensearch.alerting.ALERTING_SEARCH_EMAIL_ACCOUNT_ACCESS import org.opensearch.alerting.AlertingPlugin import org.opensearch.alerting.AlertingRestTestCase @@ -20,6 +21,7 @@ import org.opensearch.alerting.TEST_HR_BACKEND_ROLE import org.opensearch.alerting.TEST_HR_INDEX import org.opensearch.alerting.TEST_HR_ROLE import org.opensearch.alerting.makeRequest +import org.opensearch.client.ResponseException import org.opensearch.client.RestClient import org.opensearch.commons.rest.SecureRestClientBuilder import org.opensearch.rest.RestStatus @@ -50,7 +52,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { } } - val user = "userOne" + val user = "userB" var userClient: RestClient? = null @Before @@ -129,7 +131,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { /* TODO: https://github.com/opensearch-project/alerting/issues/300 - + */ fun `test get email accounts with an user without get email account role`() { createUserWithTestDataAndCustomRole( user, @@ -138,9 +140,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { listOf(TEST_HR_BACKEND_ROLE), getClusterPermissionsFromCustomRole(ALERTING_NO_ACCESS_ROLE) ) - val emailAccount = createRandomEmailAccountWithGivenName(true, randomAlphaOfLength(5)) - try { userClient?.makeRequest( "GET", @@ -158,9 +158,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - fun `test search email accounts with an user without search email account role`() { - createUserWithTestDataAndCustomRole( user, TEST_HR_INDEX, @@ -168,9 +166,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { listOf(TEST_HR_BACKEND_ROLE), getClusterPermissionsFromCustomRole(ALERTING_NO_ACCESS_ROLE) ) - createRandomEmailAccountWithGivenName(true, randomAlphaOfLength(5)) - try { userClient?.makeRequest( "POST", @@ -185,6 +181,4 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - - */ } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt index 0b02d1e29..9dc8738c0 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt @@ -52,7 +52,7 @@ class SecureEmailGroupsRestApiIT : AlertingRestTestCase() { } } - val user = "userOne" + val user = "userC" var userClient: RestClient? = null @Before diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt index 7396b2fb4..32c58e77c 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt @@ -20,12 +20,15 @@ import org.opensearch.alerting.ALERTING_FULL_ACCESS_ROLE import org.opensearch.alerting.ALERTING_GET_ALERTS_ACCESS import org.opensearch.alerting.ALERTING_GET_MONITOR_ACCESS import org.opensearch.alerting.ALERTING_INDEX_MONITOR_ACCESS +import org.opensearch.alerting.ALERTING_NO_ACCESS_ROLE +import org.opensearch.alerting.ALERTING_READ_ONLY_ACCESS import org.opensearch.alerting.ALERTING_SEARCH_MONITOR_ONLY_ACCESS import org.opensearch.alerting.ALL_ACCESS_ROLE import org.opensearch.alerting.ALWAYS_RUN import org.opensearch.alerting.AlertingRestTestCase import org.opensearch.alerting.DRYRUN_MONITOR import org.opensearch.alerting.READALL_AND_MONITOR_ROLE +import org.opensearch.alerting.TERM_DLS_QUERY import org.opensearch.alerting.TEST_HR_BACKEND_ROLE import org.opensearch.alerting.TEST_HR_INDEX import org.opensearch.alerting.TEST_HR_ROLE @@ -34,6 +37,8 @@ import org.opensearch.alerting.assertUserNull import org.opensearch.alerting.makeRequest import org.opensearch.alerting.randomAction import org.opensearch.alerting.randomAlert +import org.opensearch.alerting.randomBucketLevelMonitor +import org.opensearch.alerting.randomBucketLevelTrigger import org.opensearch.alerting.randomQueryLevelMonitor import org.opensearch.alerting.randomQueryLevelTrigger import org.opensearch.alerting.randomTemplateScript @@ -44,12 +49,16 @@ import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry import org.opensearch.common.xcontent.XContentType import org.opensearch.common.xcontent.json.JsonXContent +import org.opensearch.commons.alerting.aggregation.bucketselectorext.BucketSelectorExtAggregationBuilder import org.opensearch.commons.alerting.model.Alert import org.opensearch.commons.alerting.model.SearchInput import org.opensearch.commons.authuser.User import org.opensearch.commons.rest.SecureRestClientBuilder import org.opensearch.index.query.QueryBuilders import org.opensearch.rest.RestStatus +import org.opensearch.script.Script +import org.opensearch.search.aggregations.bucket.composite.CompositeAggregationBuilder +import org.opensearch.search.aggregations.bucket.composite.TermsValuesSourceBuilder import org.opensearch.search.builder.SearchSourceBuilder import org.opensearch.test.junit.annotations.TestLogging @@ -66,7 +75,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { } } - val user = "userOne" + val user = "userD" var userClient: RestClient? = null @Before @@ -89,11 +98,15 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { } // Create Monitor related security tests - fun `test create monitor with an user with alerting role`() { - createUserWithTestData(user, TEST_HR_INDEX, TEST_HR_ROLE, TEST_HR_BACKEND_ROLE) - createUserRolesMapping(ALERTING_FULL_ACCESS_ROLE, arrayOf(user)) + createUserWithTestDataAndCustomRole( + user, + TEST_HR_INDEX, + TEST_HR_ROLE, + listOf(TEST_HR_BACKEND_ROLE), + getClusterPermissionsFromCustomRole(ALERTING_INDEX_MONITOR_ACCESS) + ) try { // randomMonitor has a dummy user, api ignores the User passed as part of monitor, it picks user info from the logged-in user. val monitor = randomQueryLevelMonitor().copy( @@ -114,6 +127,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test create monitor with an user without alerting role`() { createUserWithTestDataAndCustomRole( @@ -142,13 +156,9 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { fun `test create monitor with an user with read-only role`() { - createUserWithTestDataAndCustomRole( - user, - TEST_HR_INDEX, - TEST_HR_ROLE, - listOf(TEST_HR_BACKEND_ROLE), - getClusterPermissionsFromCustomRole(ALERTING_READ_ONLY_ACCESS) - ) + createUserWithTestData(user, TEST_HR_INDEX, TEST_HR_ROLE, TEST_HR_BACKEND_ROLE) + createUserRolesMapping(ALERTING_READ_ONLY_ACCESS, arrayOf(user)) + try { val monitor = randomQueryLevelMonitor().copy( inputs = listOf( @@ -163,9 +173,9 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) } finally { deleteRoleAndRoleMapping(TEST_HR_ROLE) + deleteRoleMapping(ALERTING_READ_ONLY_ACCESS) } } - */ fun `test query monitors with an user with only search monitor cluster permission`() { @@ -190,10 +200,12 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { val hits = xcp.map()["hits"]!! as Map> val numberDocsFound = hits["total"]?.get("value") assertEquals("Monitor not found during search", 1, numberDocsFound) + deleteRoleAndRoleMapping(TEST_HR_ROLE) } /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test query monitors with an user without search monitor cluster permission`() { createUserWithTestDataAndCustomRole( @@ -219,7 +231,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ fun `test create monitor with an user without index read role`() { @@ -282,6 +293,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test get monitor with an user without get monitor role`() { createUserWithTestDataAndCustomRole( user, @@ -307,7 +319,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ fun getDocs(response: Response?): Any? { val hits = createParser( @@ -940,8 +951,8 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test query monitors with disable filter by`() { - disableFilterBy() // creates monitor as "admin" user. @@ -966,12 +977,17 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { StringEntity(search, ContentType.APPLICATION_JSON) ) fail("Expected 403 FORBIDDEN response") - } catch (e: AssertionError) { - assertEquals("Unexpected status", "Expected 403 FORBIDDEN response", e.message) + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) } - // add alerting roles and search as userOne - must return 1 docs - createUserRolesMapping(ALERTING_FULL_ACCESS_ROLE, arrayOf(user)) + createUserWithTestDataAndCustomRole( + user, + TEST_HR_INDEX, + TEST_HR_ROLE, + listOf(TEST_HR_BACKEND_ROLE), + getClusterPermissionsFromCustomRole(ALERTING_SEARCH_MONITOR_ONLY_ACCESS) + ) try { val userOneSearchResponse = userClient?.makeRequest( "POST", @@ -982,7 +998,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { assertEquals("Search monitor failed", RestStatus.OK, userOneSearchResponse?.restStatus()) assertEquals("Monitor not found during search", 1, getDocs(userOneSearchResponse)) } finally { - deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + deleteRoleAndRoleMapping(TEST_HR_ROLE) } } @@ -1012,8 +1028,8 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { StringEntity(search, ContentType.APPLICATION_JSON) ) fail("Expected 403 FORBIDDEN response") - } catch (e: AssertionError) { - assertEquals("Unexpected status", "Expected 403 FORBIDDEN response", e.message) + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) } // add alerting roles and search as userOne - must return 0 docs @@ -1032,8 +1048,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { } } - */ - fun `test execute monitor with an user with execute monitor access`() { createUserWithTestDataAndCustomRole( user, @@ -1059,6 +1073,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test execute monitor with an user without execute monitor access`() { createUserWithTestDataAndCustomRole( user, @@ -1084,7 +1099,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ fun `test delete monitor with an user with delete monitor access`() { createUserWithTestDataAndCustomRole( @@ -1113,6 +1127,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test delete monitor with an user without delete monitor access`() { createUserWithTestDataAndCustomRole( user, @@ -1162,8 +1177,8 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { try { getAlerts(userClient as RestClient, inputMap).asMap() fail("Expected 403 FORBIDDEN response") - } catch (e: AssertionError) { - assertEquals("Unexpected status", "Expected 403 FORBIDDEN response", e.message) + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) } // add alerting roles and search as userOne - must return 0 docs @@ -1199,10 +1214,9 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { try { getAlerts(userClient as RestClient, inputMap).asMap() fail("Expected 403 FORBIDDEN response") - } catch (e: AssertionError) { - assertEquals("Unexpected status", "Expected 403 FORBIDDEN response", e.message) + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) } - // add alerting roles and search as userOne - must return 0 docs createUserRolesMapping(ALERTING_FULL_ACCESS_ROLE, arrayOf(user)) try { @@ -1213,8 +1227,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { } } - */ - fun `test get alerts with an user with get alerts role`() { putAlertMappings() @@ -1335,21 +1347,24 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { assertEquals("Delete monitor failed", RestStatus.OK, adminDeleteResponse.restStatus()) } finally { deleteRoleAndRoleMapping(TEST_HR_ROLE) + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) } } /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test execute query-level monitor with user having partial index permissions`() { - createUserWithDocLevelSecurityTestDataAndCustomRole( - user, - TEST_HR_INDEX, + createUser(user, user, arrayOf(TEST_HR_BACKEND_ROLE)) + createTestIndex(TEST_HR_INDEX) + createIndexRoleWithDocLevelSecurity( TEST_HR_ROLE, - listOf(TEST_HR_BACKEND_ROLE), + TEST_HR_INDEX, TERM_DLS_QUERY, - getClusterPermissionsFromCustomRole(ALERTING_FULL_ACCESS_ROLE) + getClusterPermissionsFromCustomRole(ALERTING_INDEX_MONITOR_ACCESS) ) + createUserRolesMapping(TEST_HR_ROLE, arrayOf(user)) // Add a doc that is accessible to the user indexDoc( @@ -1357,7 +1372,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { """ { "test_field": "a", - "accessible": true + "accessible": true } """.trimIndent() ) @@ -1376,7 +1391,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { val input = SearchInput(indices = listOf(TEST_HR_INDEX), query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery())) val triggerScript = """ // make sure there is exactly one hit - return ctx.results[0].hits.hits.size() == 1 + return ctx.results[0].hits.hits.size() == 1 """.trimIndent() val trigger = randomQueryLevelTrigger(condition = Script(triggerScript)).copy(actions = listOf()) @@ -1396,14 +1411,15 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { fun `test execute bucket-level monitor with user having partial index permissions`() { - createUserWithDocLevelSecurityTestDataAndCustomRole( - user, - TEST_HR_INDEX, + createUser(user, user, arrayOf(TEST_HR_BACKEND_ROLE)) + createTestIndex(TEST_HR_INDEX) + createIndexRoleWithDocLevelSecurity( TEST_HR_ROLE, - listOf(TEST_HR_BACKEND_ROLE), + TEST_HR_INDEX, TERM_DLS_QUERY, - getClusterPermissionsFromCustomRole(ALERTING_FULL_ACCESS_ROLE) + getClusterPermissionsFromCustomRole(ALERTING_INDEX_MONITOR_ACCESS) ) + createUserRolesMapping(TEST_HR_ROLE, arrayOf(user)) // Add a doc that is accessible to the user indexDoc( @@ -1463,5 +1479,4 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ }