Skip to content

Commit

Permalink
Refactor Security Integration Tests (opensearch-project#297)
Browse files Browse the repository at this point in the history 
Signed-off-by: Aditya Jindal <[email protected]>
  • Loading branch information
@adityaj1107 @qreshi
adityaj1107 authored and qreshi committed Feb 14, 2022
1 parent 23eb3c4 commit 214f177
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 @@ -26,6 +26,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 @@ -34,7 +35,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 @@ -43,6 +43,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 @@ -76,29 +85,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 @@ -130,11 +123,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 @@ -194,11 +182,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 @@ -257,11 +240,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 @@ -15,6 +15,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 @@ -60,12 +61,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 @@ -75,7 +84,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {

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

userClient?.close()
deleteUser(user)
Expand All @@ -84,7 +92,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 @@ -107,7 +114,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 @@ -129,7 +135,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 @@ -159,26 +164,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 @@ -287,7 +272,6 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
}

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

disableFilterBy()

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

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

enableFilterBy()

Expand Down Expand Up @@ -381,7 +364,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 @@ -418,9 +400,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 @@ -460,7 +439,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 @@ -491,8 +469,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 @@ -531,7 +507,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 @@ -583,7 +558,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 214f177

Please sign in to comment.