PI-2597 Disable logging to telemetry for sensitive event types (#4322)

ministryofjustice · Oct 16, 2024 · 6cff799 · 6cff799
1 parent 8dd4912
commit 6cff799
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 19 deletions.
diff --git a/...essaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt b/...essaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt
@@ -9,6 +9,7 @@ import io.opentelemetry.api.trace.Span
 import io.opentelemetry.api.trace.SpanKind
 import io.sentry.Sentry
 import io.sentry.spring.jakarta.tracing.SentryTransaction
+import org.springframework.beans.factory.annotation.Value
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression
 import org.springframework.context.annotation.Conditional
 import org.springframework.dao.CannotAcquireLockException
@@ -31,16 +32,25 @@ import java.util.concurrent.CompletionException
 @ConditionalOnExpression("\${messaging.consumer.enabled:true} and '\${messaging.consumer.queue:}' != ''")
 class AwsNotificationListener(
     private val handler: NotificationHandler<*>,
-    private val objectMapper: ObjectMapper
+    private val objectMapper: ObjectMapper,
+    @Value("\${messaging.consumer.sensitive-event-types:[]}") private val sensitiveEventTypes: List<String>,
+    @Value("\${messaging.consumer.queue}") private val queueName: String
 ) {
     @SentryTransaction(operation = "messaging")
     @SqsListener("\${messaging.consumer.queue}")
     fun receive(message: String) {
         val notification = objectMapper.readValue(message, jacksonTypeRef<Notification<String>>())
         notification.attributes
             .extractTelemetryContext()
-            .withSpan(this::class.java.simpleName, "RECEIVE ${notification.eventType}", SpanKind.CONSUMER) {
-                Span.current().setAttribute("message", message)
+            .withSpan(
+                this::class.java.simpleName,
+                "RECEIVE ${notification.eventType ?: "unknown event type"}",
+                SpanKind.CONSUMER
+            ) {
+                Span.current().setAttribute("queue", queueName)
+                if (notification.eventType != null && notification.eventType !in sensitiveEventTypes) {
+                    Span.current().setAttribute("message", message)
+                }
                 try {
                     retry(3, RETRYABLE_EXCEPTIONS) { handler.handle(message) }
                 } catch (e: Throwable) {

diff --git a/...ging/src/test/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListenerTest.kt b/...ging/src/test/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListenerTest.kt
@@ -1,60 +1,56 @@
 package uk.gov.justice.digital.hmpps.listener
 
-import com.fasterxml.jackson.core.type.TypeReference
-import com.fasterxml.jackson.databind.ObjectMapper
 import io.awspring.cloud.sqs.listener.AsyncAdapterBlockingExecutionFailedException
 import io.awspring.cloud.sqs.listener.ListenerExecutionFailedException
+import io.opentelemetry.api.trace.Span
 import io.sentry.Sentry
 import org.hamcrest.CoreMatchers.equalTo
 import org.hamcrest.MatcherAssert.assertThat
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
 import org.junit.jupiter.api.extension.ExtendWith
-import org.mockito.InjectMocks
 import org.mockito.Mock
-import org.mockito.Mockito.mockStatic
+import org.mockito.Mockito.*
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.kotlin.any
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import org.springframework.messaging.support.GenericMessage
+import uk.gov.justice.digital.hmpps.message.MessageAttributes
 import uk.gov.justice.digital.hmpps.message.Notification
 import uk.gov.justice.digital.hmpps.messaging.NotificationHandler
+import uk.gov.justice.digital.hmpps.test.MockMvcExtensions.objectMapper
 import java.util.concurrent.CompletionException
 
 @ExtendWith(MockitoExtension::class)
 class AwsNotificationListenerTest {
     @Mock
     lateinit var handler: NotificationHandler<Any>
 
-    @Mock
-    lateinit var objectMapper: ObjectMapper
-
-    @InjectMocks
     lateinit var listener: AwsNotificationListener
 
     @BeforeEach
     fun setUp() {
-        whenever(objectMapper.readValue(any<String>(), any<TypeReference<Notification<String>>>()))
-            .thenReturn(Notification("message"))
+        listener = AwsNotificationListener(handler, objectMapper, listOf("my-sensitive-event-type"), "my-queue")
     }
 
     @Test
     fun `messages are dispatched to handler`() {
-        listener.receive("message")
-        verify(handler).handle("message")
+        val notification = objectMapper.writeValueAsString(Notification("message"))
+        listener.receive(notification)
+        verify(handler).handle(notification)
     }
 
     @Test
     fun `errors are captured and rethrown`() {
         mockStatic(Sentry::class.java).use {
             val exception = RuntimeException("error")
-            whenever(handler.handle("message")).thenThrow(exception)
+            whenever(handler.handle(any<String>())).thenThrow(exception)
 
             assertThat(
                 assertThrows<RuntimeException> {
-                    listener.receive("message")
+                    listener.receive(objectMapper.writeValueAsString(Notification("message")))
                 },
                 equalTo(exception)
             )
@@ -73,16 +69,50 @@ class AwsNotificationListenerTest {
                     ListenerExecutionFailedException("listener failure", meaningfulException, GenericMessage("test"))
                 )
             )
-            whenever(handler.handle("message")).thenThrow(wrappedException)
+            whenever(handler.handle(any<String>())).thenThrow(wrappedException)
 
             assertThat(
                 assertThrows<CompletionException> {
-                    listener.receive("message")
+                    listener.receive(objectMapper.writeValueAsString(Notification("message")))
                 },
                 equalTo(wrappedException)
             )
 
             it.verify { Sentry.captureException(meaningfulException) }
         }
     }
+
+    @Test
+    fun `sensitive messages are not logged`() {
+        val span = mock(Span::class.java, CALLS_REAL_METHODS)
+        mockStatic(Span::class.java, CALLS_REAL_METHODS).use {
+            it.`when`<Span> { Span.current() }.thenReturn(span)
+            listener.receive(
+                objectMapper.writeValueAsString(
+                    Notification(
+                        "my message",
+                        MessageAttributes("my-sensitive-event-type")
+                    )
+                )
+            )
+            verify(span, never()).setAttribute(eq("message"), any<String>())
+        }
+    }
+
+    @Test
+    fun `non-sensitive messages are logged`() {
+        val span = mock(Span::class.java, CALLS_REAL_METHODS)
+        mockStatic(Span::class.java, CALLS_REAL_METHODS).use {
+            it.`when`<Span> { Span.current() }.thenReturn(span)
+            listener.receive(
+                objectMapper.writeValueAsString(
+                    Notification(
+                        "my message",
+                        MessageAttributes("some-other-event-type")
+                    )
+                )
+            )
+            verify(span).setAttribute(eq("message"), any<String>())
+        }
+    }
 }
diff --git a/projects/common-platform-and-delius/src/main/resources/application.yml b/projects/common-platform-and-delius/src/main/resources/application.yml
@@ -30,6 +30,9 @@ spring:
 
 delius.db.username: CommonPlatformAndDelius # Should match value in [deploy/database/access.yml].
 
+messaging.consumer.sensitive-event-types:
+  - COMMON_PLATFORM_HEARING
+
 management:
   endpoints.web:
     base-path: /