Remove user from the responses (#201)

* Add valid search filters.

* Remove user from the responses

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

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<DocWriteRequest<*>>(
                         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<DocWriteRequest<*>>(
                             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()

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)

alerting/src/main/kotlin/org/opensearch/alerting/model/Alert.kt

@@ -297,15 +297,26 @@ 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)
             .field(MONITOR_ID_FIELD, monitorId)
             .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)

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<String, Any> {
         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())

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,21 +73,31 @@ 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)
             .field(type.value, constructResponseForDestinationType(type))
         if (params.paramAsBoolean("with_type", false)) builder.endObject()
         return builder.endObject()
     }
-
     fun toXContent(builder: XContentBuilder): XContentBuilder {
         return toXContent(builder, ToXContent.EMPTY_PARAMS)
     }

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)

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)

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<String, Any>)
         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<String, Any>)
+
         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<String, Any>)
+
         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<String, Any>)
         return (destinationJson["destinations"] as List<Any?>)[0] as Map<String, Any>
     }
 
@@ -392,14 +398,16 @@ 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())
 
         val alertJson = jsonXContent.createParser(
             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<String, Any>)
         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<String, String> = 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"

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)
         }
     }
 

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<String, Any?>) {
+    val user = map["user"]
+    assertNull("User is not null", user)
+}
+
+fun assertUserNull(monitor: Monitor) {
+    assertNull("User is not null", monitor.user)
+}