From 3531a995c8840cc7668203ab5ecbdaf25d6dde62 Mon Sep 17 00:00:00 2001 From: RAJ CHAKRAVARTHI Date: Tue, 14 Feb 2023 12:28:50 -0500 Subject: [PATCH] fixed security tests (#484) (#794) * fixed security tests Signed-off-by: Raj Chakravarthi (cherry picked from commit c51940f5e2d14a00262f7f4675bbba2d930d2ede) --- .../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 94c6fd7a4..f5c2236d7 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 35b9cad43..a95bdc9fe 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 b27b6285a..5cf047681 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 @@ -126,7 +128,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, @@ -135,9 +137,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { listOf(TEST_HR_BACKEND_ROLE), getClusterPermissionsFromCustomRole(ALERTING_NO_ACCESS_ROLE) ) - val emailAccount = createRandomEmailAccountWithGivenName(true, randomAlphaOfLength(5)) - try { userClient?.makeRequest( "GET", @@ -155,9 +155,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, @@ -165,9 +163,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() { listOf(TEST_HR_BACKEND_ROLE), getClusterPermissionsFromCustomRole(ALERTING_NO_ACCESS_ROLE) ) - createRandomEmailAccountWithGivenName(true, randomAlphaOfLength(5)) - try { userClient?.makeRequest( "POST", @@ -182,6 +178,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 13e51e62d..2e52514a4 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 4b6f18f50..ddaaa55b5 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 @@ -86,11 +95,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( @@ -111,6 +124,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { /* TODO: https://github.com/opensearch-project/alerting/issues/300 + */ fun `test create monitor with an user without alerting role`() { createUserWithTestDataAndCustomRole( @@ -139,13 +153,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( @@ -160,9 +170,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`() { @@ -187,10 +197,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( @@ -216,7 +228,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ fun `test create monitor with an user without index read role`() { @@ -279,6 +290,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, @@ -304,7 +316,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ fun getDocs(response: Response?): Any? { val hits = createParser( @@ -927,8 +938,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. @@ -953,12 +964,17 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { NStringEntity(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", @@ -969,7 +985,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) } } @@ -999,8 +1015,8 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { NStringEntity(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 @@ -1019,8 +1035,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { } } - */ - fun `test execute monitor with an user with execute monitor access`() { createUserWithTestDataAndCustomRole( user, @@ -1046,6 +1060,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, @@ -1071,7 +1086,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ fun `test delete monitor with an user with delete monitor access`() { createUserWithTestDataAndCustomRole( @@ -1100,6 +1114,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, @@ -1149,8 +1164,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 @@ -1186,10 +1201,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 { @@ -1200,8 +1214,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { } } - */ - fun `test get alerts with an user with get alerts role`() { putAlertMappings() @@ -1322,21 +1334,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( @@ -1344,7 +1359,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { """ { "test_field": "a", - "accessible": true + "accessible": true } """.trimIndent() ) @@ -1363,7 +1378,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()) @@ -1383,14 +1398,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( @@ -1450,5 +1466,4 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleAndRoleMapping(TEST_HR_ROLE) } } - */ }