From e0e41df2b2e36c187edd3c45e214451208b10309 Mon Sep 17 00:00:00 2001 From: Sriram <59816283+skkosuri-amzn@users.noreply.github.com> Date: Fri, 15 Oct 2021 14:30:40 -0700 Subject: [PATCH] Remove user from the responses (#201) * Add valid search filters. * Remove user from the responses --- DEVELOPER_GUIDE.md | 4 +-- .../org/opensearch/alerting/AlertService.kt | 9 +++-- .../opensearch/alerting/alerts/AlertMover.kt | 3 +- .../org/opensearch/alerting/model/Alert.kt | 15 ++++++-- .../org/opensearch/alerting/model/Monitor.kt | 20 +++++++---- .../alerting/model/destination/Destination.kt | 17 +++++++--- .../TransportIndexDestinationAction.kt | 4 +-- .../transport/TransportIndexMonitorAction.kt | 4 +-- .../alerting/AlertingRestTestCase.kt | 34 +++++++++++++++---- .../opensearch/alerting/MonitorRunnerIT.kt | 2 +- .../org/opensearch/alerting/TestHelpers.kt | 23 ++++++++++++- .../alerting/action/GetAlertsResponseTests.kt | 6 ++-- .../alerting/model/XContentTests.kt | 7 ++-- .../alerting/resthandler/MonitorRestApiIT.kt | 23 ++++++++++--- .../resthandler/SecureMonitorRestApiIT.kt | 19 ++++------- 15 files changed, 131 insertions(+), 59 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 5dadfb35c..93f36331b 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -51,9 +51,9 @@ When launching a cluster using one of the above commands, logs are placed in `al 1. Setup a local opensearch cluster with security plugin. - - `./gradlew :alerting:integTestRunner -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername=es-integrationtest -Dhttps=true -Dsecurity=true -Duser=admin -Dpassword=admin` + - `./gradlew :alerting:integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername=opensearch -Dhttps=true -Dsecurity=true -Duser=admin -Dpassword=admin` - - `./gradlew :alerting:integTestRunner -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername=es-integrationtest -Dhttps=true -Dsecurity=true -Duser=admin -Dpassword=admin --tests "org.opensearch.alerting.MonitorRunnerIT.test execute monitor returns search result"` + - `./gradlew :alerting:integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername=opensearch -Dhttps=true -Dsecurity=true -Duser=admin -Dpassword=admin --tests "org.opensearch.alerting.MonitorRunnerIT.test execute monitor returns search result"` #### Building from the IDE diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/AlertService.kt b/alerting/src/main/kotlin/org/opensearch/alerting/AlertService.kt index 197c5892c..d825eeb12 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/AlertService.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/AlertService.kt @@ -42,7 +42,6 @@ import org.opensearch.client.Client import org.opensearch.common.bytes.BytesReference import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry -import org.opensearch.common.xcontent.ToXContent import org.opensearch.common.xcontent.XContentFactory import org.opensearch.common.xcontent.XContentHelper import org.opensearch.common.xcontent.XContentParser @@ -276,7 +275,7 @@ class AlertService( listOf>( IndexRequest(AlertIndices.ALERT_INDEX) .routing(alert.monitorId) - .source(alert.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .source(alert.toXContentWithUser(XContentFactory.jsonBuilder())) .id(if (alert.id != Alert.NO_ID) alert.id else null) ) } @@ -287,7 +286,7 @@ class AlertService( listOf>( IndexRequest(AlertIndices.ALERT_INDEX) .routing(alert.monitorId) - .source(alert.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .source(alert.toXContentWithUser(XContentFactory.jsonBuilder())) .id(if (alert.id != Alert.NO_ID) alert.id else null) ) } else { @@ -305,7 +304,7 @@ class AlertService( if (alertIndices.isHistoryEnabled()) { IndexRequest(AlertIndices.HISTORY_WRITE_INDEX) .routing(alert.monitorId) - .source(alert.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .source(alert.toXContentWithUser(XContentFactory.jsonBuilder())) .id(alert.id) } else null ) @@ -349,7 +348,7 @@ class AlertService( } IndexRequest(AlertIndices.ALERT_INDEX) .routing(alert.monitorId) - .source(alert.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .source(alert.toXContentWithUser(XContentFactory.jsonBuilder())) }.toMutableList() if (requestsToRetry.isEmpty()) return listOf() diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/alerts/AlertMover.kt b/alerting/src/main/kotlin/org/opensearch/alerting/alerts/AlertMover.kt index 467b46755..45e09e09e 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/alerts/AlertMover.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/alerts/AlertMover.kt @@ -41,7 +41,6 @@ import org.opensearch.client.Client import org.opensearch.common.bytes.BytesReference import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry -import org.opensearch.common.xcontent.ToXContent import org.opensearch.common.xcontent.XContentFactory import org.opensearch.common.xcontent.XContentHelper import org.opensearch.common.xcontent.XContentParser @@ -88,7 +87,7 @@ suspend fun moveAlerts(client: Client, monitorId: String, monitor: Monitor? = nu .source( Alert.parse(alertContentParser(hit.sourceRef), hit.id, hit.version) .copy(state = Alert.State.DELETED) - .toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS) + .toXContentWithUser(XContentFactory.jsonBuilder()) ) .version(hit.version) .versionType(VersionType.EXTERNAL_GTE) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/model/Alert.kt b/alerting/src/main/kotlin/org/opensearch/alerting/model/Alert.kt index 22ba715e7..527d03c92 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/model/Alert.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/model/Alert.kt @@ -297,6 +297,13 @@ data class Alert( } override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { + return createXContentBuilder(builder, true) + } + + fun toXContentWithUser(builder: XContentBuilder): XContentBuilder { + return createXContentBuilder(builder, false) + } + private fun createXContentBuilder(builder: XContentBuilder, secure: Boolean): XContentBuilder { builder.startObject() .field(ALERT_ID_FIELD, id) .field(ALERT_VERSION_FIELD, version) @@ -304,8 +311,12 @@ data class Alert( .field(SCHEMA_VERSION_FIELD, schemaVersion) .field(MONITOR_VERSION_FIELD, monitorVersion) .field(MONITOR_NAME_FIELD, monitorName) - .optionalUserField(MONITOR_USER_FIELD, monitorUser) - .field(TRIGGER_ID_FIELD, triggerId) + + if (!secure) { + builder.optionalUserField(MONITOR_USER_FIELD, monitorUser) + } + + builder.field(TRIGGER_ID_FIELD, triggerId) .field(TRIGGER_NAME_FIELD, triggerName) .field(STATE_FIELD, state) .field(ERROR_MESSAGE_FIELD, errorMessage) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/model/Monitor.kt b/alerting/src/main/kotlin/org/opensearch/alerting/model/Monitor.kt index 8fe3e8ce5..a96bcbacd 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/model/Monitor.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/model/Monitor.kt @@ -141,24 +141,32 @@ data class Monitor( } } - fun toXContent(builder: XContentBuilder): XContentBuilder { - return toXContent(builder, ToXContent.EMPTY_PARAMS) - } - /** Returns a representation of the monitor suitable for passing into painless and mustache scripts. */ fun asTemplateArg(): Map { return mapOf(_ID to id, _VERSION to version, NAME_FIELD to name, ENABLED_FIELD to enabled) } + fun toXContentWithUser(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { + return createXContentBuilder(builder, params, false) + } + override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { + return createXContentBuilder(builder, params, true) + } + + private fun createXContentBuilder(builder: XContentBuilder, params: ToXContent.Params, secure: Boolean): XContentBuilder { builder.startObject() if (params.paramAsBoolean("with_type", false)) builder.startObject(type) builder.field(TYPE_FIELD, type) .field(SCHEMA_VERSION_FIELD, schemaVersion) .field(NAME_FIELD, name) .field(MONITOR_TYPE_FIELD, monitorType) - .optionalUserField(USER_FIELD, user) - .field(ENABLED_FIELD, enabled) + + if (!secure) { + builder.optionalUserField(USER_FIELD, user) + } + + builder.field(ENABLED_FIELD, enabled) .optionalTimeField(ENABLED_TIME_FIELD, enabledTime) .field(SCHEDULE_FIELD, schedule) .field(INPUTS_FIELD, inputs.toTypedArray()) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/model/destination/Destination.kt b/alerting/src/main/kotlin/org/opensearch/alerting/model/destination/Destination.kt index 5aeb15693..9cbd471fe 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/model/destination/Destination.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/model/destination/Destination.kt @@ -33,7 +33,6 @@ import org.opensearch.alerting.destination.message.ChimeMessage import org.opensearch.alerting.destination.message.CustomWebhookMessage import org.opensearch.alerting.destination.message.EmailMessage import org.opensearch.alerting.destination.message.SlackMessage -import org.opensearch.alerting.destination.response.DestinationResponse import org.opensearch.alerting.elasticapi.convertToMap import org.opensearch.alerting.elasticapi.instant import org.opensearch.alerting.elasticapi.optionalTimeField @@ -74,13 +73,24 @@ data class Destination( ) : ToXContent { override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { + return createXContentBuilder(builder, params, true) + } + + fun toXContentWithUser(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder { + return createXContentBuilder(builder, params, false) + } + private fun createXContentBuilder(builder: XContentBuilder, params: ToXContent.Params, secure: Boolean): XContentBuilder { builder.startObject() if (params.paramAsBoolean("with_type", false)) builder.startObject(DESTINATION) builder.field(ID_FIELD, id) .field(TYPE_FIELD, type.value) .field(NAME_FIELD, name) - .optionalUserField(USER_FIELD, user) - .field(SCHEMA_VERSION, schemaVersion) + + if (!secure) { + builder.optionalUserField(USER_FIELD, user) + } + + builder.field(SCHEMA_VERSION, schemaVersion) .field(SEQ_NO_FIELD, seqNo) .field(PRIMARY_TERM_FIELD, primaryTerm) .optionalTimeField(LAST_UPDATE_TIME_FIELD, lastUpdateTime) @@ -88,7 +98,6 @@ data class Destination( if (params.paramAsBoolean("with_type", false)) builder.endObject() return builder.endObject() } - fun toXContent(builder: XContentBuilder): XContentBuilder { return toXContent(builder, ToXContent.EMPTY_PARAMS) } diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt index 5299dd0af..fd7f72fec 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt @@ -164,7 +164,7 @@ class TransportIndexDestinationAction @Inject constructor( val destination = request.destination.copy(schemaVersion = IndexUtils.scheduledJobIndexSchemaVersion) val indexRequest = IndexRequest(ScheduledJob.SCHEDULED_JOBS_INDEX) .setRefreshPolicy(request.refreshPolicy) - .source(destination.toXContent(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) + .source(destination.toXContentWithUser(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) .setIfSeqNo(request.seqNo) .setIfPrimaryTerm(request.primaryTerm) .timeout(indexTimeout) @@ -282,7 +282,7 @@ class TransportIndexDestinationAction @Inject constructor( val indexDestination = request.destination.copy(schemaVersion = IndexUtils.scheduledJobIndexSchemaVersion) val indexRequest = IndexRequest(ScheduledJob.SCHEDULED_JOBS_INDEX) .setRefreshPolicy(request.refreshPolicy) - .source(indexDestination.toXContent(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) + .source(indexDestination.toXContentWithUser(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) .id(request.destinationId) .setIfSeqNo(request.seqNo) .setIfPrimaryTerm(request.primaryTerm) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt index 1075fa6a6..3602534f7 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt @@ -400,7 +400,7 @@ class TransportIndexMonitorAction @Inject constructor( request.monitor = request.monitor.copy(schemaVersion = IndexUtils.scheduledJobIndexSchemaVersion) val indexRequest = IndexRequest(SCHEDULED_JOBS_INDEX) .setRefreshPolicy(request.refreshPolicy) - .source(request.monitor.toXContent(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) + .source(request.monitor.toXContentWithUser(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) .setIfSeqNo(request.seqNo) .setIfPrimaryTerm(request.primaryTerm) .timeout(indexTimeout) @@ -470,7 +470,7 @@ class TransportIndexMonitorAction @Inject constructor( request.monitor = request.monitor.copy(schemaVersion = IndexUtils.scheduledJobIndexSchemaVersion) val indexRequest = IndexRequest(SCHEDULED_JOBS_INDEX) .setRefreshPolicy(request.refreshPolicy) - .source(request.monitor.toXContent(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) + .source(request.monitor.toXContentWithUser(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true")))) .id(request.monitorId) .setIfSeqNo(request.seqNo) .setIfPrimaryTerm(request.primaryTerm) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt index d39542a02..83a0efc11 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt @@ -115,6 +115,7 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, response.entity.content ).map() + assertUserNull(monitorJson as HashMap) return monitor.copy(id = monitorJson["_id"] as String, version = (monitorJson["_version"] as Int).toLong()) } @@ -140,6 +141,8 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, response.entity.content ).map() + assertUserNull(destinationJson as HashMap) + return destination.copy( id = destinationJson["_id"] as String, version = (destinationJson["_version"] as Int).toLong(), @@ -171,6 +174,8 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, response.entity.content ).map() + assertUserNull(destinationJson as HashMap) + return destination.copy(id = destinationJson["_id"] as String, version = (destinationJson["_version"] as Int).toLong()) } @@ -311,6 +316,7 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, response.entity.content ).map() + assertUserNull(destinationJson as HashMap) return (destinationJson["destinations"] as List)[0] as Map } @@ -392,7 +398,7 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { protected fun createAlert(alert: Alert): Alert { val response = adminClient().makeRequest( "POST", "/${AlertIndices.ALERT_INDEX}/_doc?refresh=true&routing=${alert.monitorId}", - emptyMap(), alert.toHttpEntity() + emptyMap(), alert.toHttpEntityWithUser() ) assertEquals("Unable to create a new alert", RestStatus.CREATED, response.restStatus()) @@ -400,6 +406,8 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, response.entity.content ).map() + + assertNull(alertJson["monitor_user"]) return alert.copy(id = alertJson["_id"] as String, version = (alertJson["_version"] as Int).toLong()) } @@ -418,6 +426,7 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { emptyMap(), monitor.toHttpEntity() ) assertEquals("Unable to update a monitor", RestStatus.OK, response.restStatus()) + assertUserNull(response.asMap()["monitor"] as Map) return getMonitor(monitorId = monitor.id) } @@ -441,6 +450,8 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { "monitor" -> monitor = Monitor.parse(parser) } } + + assertUserNull(monitor) return monitor.copy(id = id, version = version) } @@ -526,7 +537,7 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { } protected fun executeMonitor(client: RestClient, monitor: Monitor, params: Map = mapOf()): Response = - client.makeRequest("POST", "$ALERTING_BASE_URI/_execute", params, monitor.toHttpEntity()) + client.makeRequest("POST", "$ALERTING_BASE_URI/_execute", params, monitor.toHttpEntityWithUser()) protected fun indexDoc(index: String, id: String, doc: String, refresh: Boolean = true): Response { val requestBody = StringEntity(doc, APPLICATION_JSON) @@ -630,7 +641,16 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { private fun Monitor.toJsonString(): String { val builder = XContentFactory.jsonBuilder() - return shuffleXContent(toXContent(builder)).string() + return shuffleXContent(toXContent(builder, ToXContent.EMPTY_PARAMS)).string() + } + + protected fun Monitor.toHttpEntityWithUser(): HttpEntity { + return StringEntity(toJsonStringWithUser(), APPLICATION_JSON) + } + + private fun Monitor.toJsonStringWithUser(): String { + val builder = XContentFactory.jsonBuilder() + return shuffleXContent(toXContentWithUser(builder, ToXContent.EMPTY_PARAMS)).string() } protected fun Destination.toHttpEntity(): HttpEntity { @@ -660,13 +680,13 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { return shuffleXContent(toXContent(builder)).string() } - private fun Alert.toHttpEntity(): HttpEntity { - return StringEntity(toJsonString(), APPLICATION_JSON) + private fun Alert.toHttpEntityWithUser(): HttpEntity { + return StringEntity(toJsonStringWithUser(), APPLICATION_JSON) } - private fun Alert.toJsonString(): String { + private fun Alert.toJsonStringWithUser(): String { val builder = XContentFactory.jsonBuilder() - return shuffleXContent(toXContent(builder, ToXContent.EMPTY_PARAMS)).string() + return shuffleXContent(toXContentWithUser(builder)).string() } protected fun Monitor.relativeUrl() = "$ALERTING_BASE_URI/$id" diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerIT.kt index 6194191e6..0ad93db90 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/MonitorRunnerIT.kt @@ -861,7 +861,7 @@ class MonitorRunnerIT : AlertingRestTestCase() { assertEquals("Alert not saved", 1, alerts.size) verifyAlert(alerts.single(), monitor, ERROR) - Assert.assertTrue(alerts.single().errorMessage?.contains("The destination address is invalid") as Boolean) + Assert.assertNotNull(alerts.single().errorMessage) } } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt b/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt index c576ba104..60d632dc9 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/TestHelpers.kt @@ -25,6 +25,7 @@ */ package org.opensearch.alerting +import junit.framework.TestCase.assertNull import org.apache.http.Header import org.apache.http.HttpEntity import org.opensearch.alerting.aggregation.bucketselectorext.BucketSelectorExtAggregationBuilder @@ -67,6 +68,7 @@ import org.opensearch.common.settings.SecureString import org.opensearch.common.settings.Settings import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry +import org.opensearch.common.xcontent.ToXContent import org.opensearch.common.xcontent.XContentBuilder import org.opensearch.common.xcontent.XContentFactory import org.opensearch.common.xcontent.XContentParser @@ -398,7 +400,17 @@ fun randomActionRunResult(): ActionRunResult { fun Monitor.toJsonString(): String { val builder = XContentFactory.jsonBuilder() - return this.toXContent(builder).string() + return this.toXContent(builder, ToXContent.EMPTY_PARAMS).string() +} + +fun Monitor.toJsonStringWithUser(): String { + val builder = XContentFactory.jsonBuilder() + return this.toXContentWithUser(builder, ToXContent.EMPTY_PARAMS).string() +} + +fun Alert.toJsonString(): String { + val builder = XContentFactory.jsonBuilder() + return this.toXContent(builder, ToXContent.EMPTY_PARAMS).string() } fun randomUser(): User { @@ -494,3 +506,12 @@ fun xContentRegistry(): NamedXContentRegistry { ) + SearchModule(Settings.EMPTY, false, emptyList()).namedXContents ) } + +fun assertUserNull(map: Map) { + val user = map["user"] + assertNull("User is not null", user) +} + +fun assertUserNull(monitor: Monitor) { + assertNull("User is not null", monitor.user) +} diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/action/GetAlertsResponseTests.kt b/alerting/src/test/kotlin/org/opensearch/alerting/action/GetAlertsResponseTests.kt index 66931a415..8e111bf3e 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/action/GetAlertsResponseTests.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/action/GetAlertsResponseTests.kt @@ -34,7 +34,6 @@ import org.opensearch.alerting.randomUser import org.opensearch.common.io.stream.BytesStreamOutput import org.opensearch.common.io.stream.StreamInput import org.opensearch.common.xcontent.ToXContent -import org.opensearch.commons.authuser.User import org.opensearch.test.OpenSearchTestCase import java.time.Instant import java.util.Collections @@ -97,7 +96,7 @@ class GetAlertsResponseTests : OpenSearchTestCase() { "monitorId", "monitorName", 0L, - User("admin", listOf(), listOf(), listOf()), + null, "triggerId", "triggerName", Alert.State.ACKNOWLEDGED, @@ -115,8 +114,7 @@ class GetAlertsResponseTests : OpenSearchTestCase() { var actualXContentString = req.toXContent(builder(), ToXContent.EMPTY_PARAMS).string() val expectedXContentString = "{\"alerts\":[{\"id\":\"id\",\"version\":0,\"monitor_id\":\"monitorId\"," + "\"schema_version\":0,\"monitor_version\":0,\"monitor_name\":\"monitorName\"," + - "\"monitor_user\":{\"name\":\"admin\",\"backend_roles\":[],\"roles\":[]," + - "\"custom_attribute_names\":[],\"user_requested_tenant\":null},\"trigger_id\":\"triggerId\"," + + "\"trigger_id\":\"triggerId\"," + "\"trigger_name\":\"triggerName\",\"state\":\"ACKNOWLEDGED\",\"error_message\":null,\"alert_history\":[]," + "\"severity\":\"severity\",\"action_execution_results\":[],\"start_time\":" + now.toEpochMilli() + ",\"last_notification_time\":null,\"end_time\":null,\"acknowledged_time\":null}],\"totalAlerts\":1}" diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/model/XContentTests.kt b/alerting/src/test/kotlin/org/opensearch/alerting/model/XContentTests.kt index 3268309d1..f515806d0 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/model/XContentTests.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/model/XContentTests.kt @@ -52,6 +52,7 @@ import org.opensearch.alerting.randomThrottle import org.opensearch.alerting.randomUser import org.opensearch.alerting.randomUserEmpty import org.opensearch.alerting.toJsonString +import org.opensearch.alerting.toJsonStringWithUser import org.opensearch.common.xcontent.ToXContent import org.opensearch.commons.authuser.User import org.opensearch.index.query.QueryBuilders @@ -93,7 +94,7 @@ class XContentTests : OpenSearchTestCase() { fun `test action with per execution scope does not support throttling`() { try { - val action = randomActionWithPolicy().copy( + randomActionWithPolicy().copy( throttleEnabled = true, throttle = Throttle(value = 5, unit = ChronoUnit.MINUTES), actionExecutionPolicy = ActionExecutionPolicy(PerExecutionActionScope()) @@ -128,7 +129,7 @@ class XContentTests : OpenSearchTestCase() { fun `test query-level monitor parsing`() { val monitor = randomQueryLevelMonitor() - val monitorString = monitor.toJsonString() + val monitorString = monitor.toJsonStringWithUser() val parsedMonitor = Monitor.parse(parser(monitorString)) assertEquals("Round tripping QueryLevelMonitor doesn't work", monitor, parsedMonitor) } @@ -154,7 +155,7 @@ class XContentTests : OpenSearchTestCase() { fun `test alert parsing`() { val alert = randomAlert() - val alertString = alert.toXContent(builder(), ToXContent.EMPTY_PARAMS).string() + val alertString = alert.toXContentWithUser(builder()).string() val parsedAlert = Alert.parse(parser(alertString)) assertEquals("Round tripping alert doesn't work", alert, parsedAlert) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt index ccc23f894..25ac5b4a4 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/MonitorRestApiIT.kt @@ -56,6 +56,7 @@ import org.opensearch.alerting.randomQueryLevelTrigger import org.opensearch.alerting.randomThrottle import org.opensearch.alerting.randomUser import org.opensearch.alerting.settings.AlertingSettings +import org.opensearch.alerting.toJsonString import org.opensearch.alerting.util.DestinationType import org.opensearch.client.ResponseException import org.opensearch.client.WarningFailureException @@ -99,7 +100,7 @@ class MonitorRestApiIT : AlertingRestTestCase() { fun `test parsing monitor as a scheduled job`() { val monitor = createRandomMonitor() - val builder = monitor.toXContent(XContentBuilder.builder(XContentType.JSON.xContent()), USE_TYPED_KEYS) + val builder = monitor.toXContentWithUser(XContentBuilder.builder(XContentType.JSON.xContent()), USE_TYPED_KEYS) val string = BytesReference.bytes(builder).utf8ToString() val xcp = createParser(XContentType.JSON.xContent(), string) val scheduledJob = ScheduledJob.parse(xcp, monitor.id, monitor.version) @@ -794,7 +795,11 @@ class MonitorRestApiIT : AlertingRestTestCase() { val historyAlerts = searchAlerts(monitor, AlertIndices.HISTORY_WRITE_INDEX) assertEquals("Alert was not moved to history", 1, historyAlerts.size) - assertEquals("Alert data incorrect", alert.copy(state = Alert.State.DELETED), historyAlerts.single()) + assertEquals( + "Alert data incorrect", + alert.copy(state = Alert.State.DELETED).toJsonString(), + historyAlerts.single().toJsonString() + ) } fun `test delete trigger moves alerts`() { @@ -819,7 +824,11 @@ class MonitorRestApiIT : AlertingRestTestCase() { val historyAlerts = searchAlerts(monitor, AlertIndices.HISTORY_WRITE_INDEX) assertEquals("Alert was not moved to history", 1, historyAlerts.size) - assertEquals("Alert data incorrect", alert.copy(state = Alert.State.DELETED), historyAlerts.single()) + assertEquals( + "Alert data incorrect", + alert.copy(state = Alert.State.DELETED).toJsonString(), + historyAlerts.single().toJsonString() + ) } fun `test delete trigger moves alerts only for deleted trigger`() { @@ -844,12 +853,16 @@ class MonitorRestApiIT : AlertingRestTestCase() { val alerts = searchAlerts(monitor) // We have two alerts from above, 1 for each trigger, there should be only 1 left in active index assertEquals("One alert should be in active index", 1, alerts.size) - assertEquals("Wrong alert in active index", alertKeep, alerts.single()) + assertEquals("Wrong alert in active index", alertKeep.toJsonString(), alerts.single().toJsonString()) val historyAlerts = searchAlerts(monitor, AlertIndices.HISTORY_WRITE_INDEX) // Only alertDelete should of been moved to history index assertEquals("One alert should be in history index", 1, historyAlerts.size) - assertEquals("Alert data incorrect", alertDelete.copy(state = Alert.State.DELETED), historyAlerts.single()) + assertEquals( + "Alert data incorrect", + alertDelete.copy(state = Alert.State.DELETED).toJsonString(), + historyAlerts.single().toJsonString() + ) } fun `test update monitor with wrong version`() { 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 254659b65..d613d9719 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt @@ -19,6 +19,7 @@ import org.opensearch.alerting.ALERTING_BASE_URI import org.opensearch.alerting.ALWAYS_RUN import org.opensearch.alerting.AlertingRestTestCase import org.opensearch.alerting.DRYRUN_MONITOR +import org.opensearch.alerting.assertUserNull import org.opensearch.alerting.core.model.SearchInput import org.opensearch.alerting.makeRequest import org.opensearch.alerting.model.Alert @@ -82,18 +83,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { val createResponse = userClient?.makeRequest("POST", ALERTING_BASE_URI, emptyMap(), monitor.toHttpEntity()) assertEquals("Create monitor failed", RestStatus.CREATED, createResponse?.restStatus()) - // val newMonitor = getMonitor(userClient as RestClient, monitor.id) - val responseBody = createResponse?.asMap() - val monitorMap = responseBody!!["monitor"] as HashMap - val userMap = monitorMap["user"] as HashMap - assertEquals("User is not present", user, userMap["name"]) - - val brolesArray = userMap["backend_roles"] as ArrayList - assertTrue(brolesArray.contains("HR")) - - val rolesArray = userMap["roles"] as ArrayList - assertTrue(rolesArray.contains("hr_role")) - assertTrue(rolesArray.contains("alerting_full_access")) + assertUserNull(createResponse?.asMap()!!["monitor"] as HashMap) } finally { deleteRoleMapping("hr_role") deleteRole("hr_role") @@ -153,6 +143,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { val monitor = randomQueryLevelMonitor() 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) } fun `test create monitor with enable filter by`() { @@ -163,6 +154,7 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { // 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) } else { // when security is disable. Must return Forbidden. try { @@ -419,7 +411,8 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { enableFilterBy() putAlertMappings() - val monitor = createRandomMonitor(refresh = true) + val adminUser = User("admin", listOf("admin"), listOf("all_access"), listOf()) + var monitor = createRandomMonitor(refresh = true).copy(user = adminUser) createAlert(randomAlert(monitor).copy(state = Alert.State.ACKNOWLEDGED)) createAlert(randomAlert(monitor).copy(state = Alert.State.COMPLETED)) createAlert(randomAlert(monitor).copy(state = Alert.State.ERROR))