Skip to content

Commit

Permalink
Refactor Security Integration Tests (#297)
Browse files Browse the repository at this point in the history 
Signed-off-by: Aditya Jindal <[email protected]>
  • Loading branch information
@adityaj1107
adityaj1107 authored Feb 1, 2022
1 parent 36c6edd commit 4801c33
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 75 deletions.
57 changes: 18 additions & 39 deletions alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package org.opensearch.alerting.resthandler

import org.junit.BeforeClass
import org.opensearch.alerting.AlertingRestTestCase
import org.opensearch.alerting.DESTINATION_BASE_URI
import org.opensearch.alerting.makeRequest
Expand All @@ -13,7 +14,6 @@ import org.opensearch.alerting.model.destination.Destination
import org.opensearch.alerting.model.destination.Slack
import org.opensearch.alerting.randomUser
import org.opensearch.alerting.util.DestinationType
import org.opensearch.client.ResponseException
import org.opensearch.rest.RestStatus
import org.opensearch.test.junit.annotations.TestLogging
import java.time.Instant
Expand All @@ -22,6 +22,15 @@ import java.time.Instant
@Suppress("UNCHECKED_CAST")
class SecureDestinationRestApiIT : AlertingRestTestCase() {

companion object {

@BeforeClass
@JvmStatic fun setup() {
// things to execute once and keep around for the class
org.junit.Assume.assumeTrue(System.getProperty("security", "false")!!.toBoolean())
}
}

fun `test create destination with disable filter by`() {
disableFilterBy()

Expand Down Expand Up @@ -55,29 +64,13 @@ class SecureDestinationRestApiIT : AlertingRestTestCase() {
email = null
)

if (securityEnabled()) {
// when security is enabled. No errors, must succeed.
val response = client().makeRequest(
"POST",
"$DESTINATION_BASE_URI?refresh=true",
emptyMap(),
destination.toHttpEntity()
)
assertEquals("Create monitor failed", RestStatus.CREATED, response.restStatus())
} else {
// when security is disable. Must return Forbidden.
try {
client().makeRequest(
"POST",
"$DESTINATION_BASE_URI?refresh=true",
emptyMap(),
destination.toHttpEntity()
)
fail("Expected 403 FORBIDDEN response")
} catch (e: ResponseException) {
assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus())
}
}
val response = client().makeRequest(
"POST",
"$DESTINATION_BASE_URI?refresh=true",
emptyMap(),
destination.toHttpEntity()
)
assertEquals("Create monitor failed", RestStatus.CREATED, response.restStatus())
}

fun `test update destination with disable filter by`() {
Expand Down Expand Up @@ -109,11 +102,6 @@ class SecureDestinationRestApiIT : AlertingRestTestCase() {

fun `test update destination with enable filter by`() {
enableFilterBy()
if (!isHttps()) {
// if security is disabled and filter by is enabled, we can't create monitor
// refer: `test create destination with enable filter by`
return
}

val chime = Chime("http://abc.com")
val destination = Destination(
Expand Down Expand Up @@ -173,11 +161,6 @@ class SecureDestinationRestApiIT : AlertingRestTestCase() {

fun `test delete destination with enable filter by`() {
enableFilterBy()
if (!isHttps()) {
// if security is disabled and filter by is enabled, we can't create monitor
// refer: `test create destination with enable filter by`
return
}

val chime = Chime("http://abc.com")
val destination = Destination(
Expand Down Expand Up @@ -236,11 +219,7 @@ class SecureDestinationRestApiIT : AlertingRestTestCase() {

fun `test get destinations with a destination type and filter by`() {
enableFilterBy()
if (!securityEnabled()) {
// if security is disabled and filter by is enabled, we can't create monitor
// refer: `test create destination with enable filter by`
return
}

val slack = Slack("url")
val destination = Destination(
type = DestinationType.SLACK,
Expand Down
46 changes: 10 additions & 36 deletions alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import org.apache.http.entity.ContentType
import org.apache.http.nio.entity.NStringEntity
import org.junit.After
import org.junit.Before
import org.junit.BeforeClass
import org.opensearch.alerting.ADMIN
import org.opensearch.alerting.ALERTING_BASE_URI
import org.opensearch.alerting.ALERTING_FULL_ACCESS_ROLE
Expand Down Expand Up @@ -54,12 +55,20 @@ import org.opensearch.test.junit.annotations.TestLogging
@Suppress("UNCHECKED_CAST")
class SecureMonitorRestApiIT : AlertingRestTestCase() {

companion object {

@BeforeClass
@JvmStatic fun setup() {
// things to execute once and keep around for the class
org.junit.Assume.assumeTrue(System.getProperty("security", "false")!!.toBoolean())
}
}

val user = "userOne"
var userClient: RestClient? = null

@Before
fun create() {
if (!securityEnabled()) return

if (userClient == null) {
createUser(user, user, arrayOf())
Expand All @@ -69,7 +78,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {

@After
fun cleanup() {
if (!securityEnabled()) return

userClient?.close()
deleteUser(user)
Expand All @@ -78,7 +86,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
// Create Monitor related security tests

fun `test create monitor with an user with alerting role`() {
if (!securityEnabled()) return

createUserWithTestData(user, TEST_HR_INDEX, TEST_HR_ROLE, TEST_HR_BACKEND_ROLE)
createUserRolesMapping(ALERTING_FULL_ACCESS_ROLE, arrayOf(user))
Expand All @@ -101,7 +108,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test create monitor with an user without alerting role`() {
if (!securityEnabled()) return

createUserWithTestData(user, TEST_HR_INDEX, TEST_HR_ROLE, TEST_HR_BACKEND_ROLE)
try {
Expand All @@ -123,7 +129,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test create monitor with an user without index read role`() {
if (!securityEnabled()) return

createUserWithTestData(user, TEST_HR_INDEX, TEST_HR_ROLE, TEST_HR_BACKEND_ROLE)
createUserRolesMapping(ALERTING_FULL_ACCESS_ROLE, arrayOf(user))
Expand Down Expand Up @@ -153,26 +158,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
assertUserNull(createResponse.asMap()["monitor"] as HashMap<String, Any>)
}

fun `test create monitor with enable filter by`() {
enableFilterBy()
val monitor = randomQueryLevelMonitor()

if (securityEnabled()) {
// when security is enabled. No errors, must succeed.
val createResponse = client().makeRequest("POST", ALERTING_BASE_URI, emptyMap(), monitor.toHttpEntity())
assertEquals("Create monitor failed", RestStatus.CREATED, createResponse.restStatus())
assertUserNull(createResponse.asMap()["monitor"] as HashMap<String, Any>)
} else {
// when security is disable. Must return Forbidden.
try {
client().makeRequest("POST", ALERTING_BASE_URI, emptyMap(), monitor.toHttpEntity())
fail("Expected 403 FORBIDDEN response")
} catch (e: ResponseException) {
assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus())
}
}
}

fun getDocs(response: Response?): Any? {
val hits = createParser(
XContentType.JSON.xContent(),
Expand Down Expand Up @@ -281,7 +266,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test query monitors with disable filter by`() {
if (!securityEnabled()) return

disableFilterBy()

Expand Down Expand Up @@ -328,7 +312,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test query monitors with enable filter by`() {
if (!securityEnabled()) return

enableFilterBy()

Expand Down Expand Up @@ -375,7 +358,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test query all alerts in all states with disabled filter by`() {
if (!securityEnabled()) return

disableFilterBy()
putAlertMappings()
Expand Down Expand Up @@ -412,9 +394,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test query all alerts in all states with filter by`() {
// if security is disabled and filter by is enabled, we can't create monitor
// refer: `test create monitor with enable filter by`
if (!securityEnabled()) return

enableFilterBy()
putAlertMappings()
Expand Down Expand Up @@ -454,7 +433,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
// Execute Monitor related security tests

fun `test execute monitor with elevate permissions`() {
if (!securityEnabled()) return

val action = randomAction(template = randomTemplateScript("Hello {{ctx.monitor.name}}"), destinationId = createDestination().id)
val inputs = listOf(
Expand Down Expand Up @@ -485,8 +463,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test admin all access with enable filter by`() {
if (!securityEnabled())
return

enableFilterBy()
createUserWithTestData(user, TEST_HR_INDEX, TEST_HR_ROLE, TEST_HR_BACKEND_ROLE)
Expand Down Expand Up @@ -544,7 +520,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test execute query-level monitor with user having partial index permissions`() {
if (!securityEnabled()) return

createUserWithDocLevelSecurityTestData(
user,
Expand Down Expand Up @@ -596,7 +571,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

fun `test execute bucket-level monitor with user having partial index permissions`() {
if (!securityEnabled()) return

createUserWithDocLevelSecurityTestData(
user,
Expand Down

0 comments on commit 4801c33

Please sign in to comment.