From a239ba5cc18ebcc292b5f38b5a5712d5e8cb1812 Mon Sep 17 00:00:00 2001 From: Marcus Aspin Date: Wed, 16 Oct 2024 13:21:20 +0000 Subject: [PATCH] PI-2597 Disable logging to telemetry for sensitive event types --- .../hmpps/listener/AwsNotificationListener.kt | 16 ++++- .../listener/AwsNotificationListenerTest.kt | 62 ++++++++++++++----- .../src/main/resources/application.yml | 3 + 3 files changed, 62 insertions(+), 19 deletions(-) diff --git a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt index 5095399b28..f6a30e0a2b 100644 --- a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt +++ b/libs/messaging/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,7 +32,9 @@ 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, + @Value("\${messaging.consumer.queue}") private val queueName: String ) { @SentryTransaction(operation = "messaging") @SqsListener("\${messaging.consumer.queue}") @@ -39,8 +42,15 @@ class AwsNotificationListener( val notification = objectMapper.readValue(message, jacksonTypeRef>()) 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/libs/messaging/src/test/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListenerTest.kt b/libs/messaging/src/test/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListenerTest.kt index d438afbce6..e5f3a8cc67 100644 --- a/libs/messaging/src/test/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListenerTest.kt +++ b/libs/messaging/src/test/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListenerTest.kt @@ -1,9 +1,8 @@ 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 @@ -11,16 +10,17 @@ 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) @@ -28,33 +28,29 @@ class AwsNotificationListenerTest { @Mock lateinit var handler: NotificationHandler - @Mock - lateinit var objectMapper: ObjectMapper - - @InjectMocks lateinit var listener: AwsNotificationListener @BeforeEach fun setUp() { - whenever(objectMapper.readValue(any(), any>>())) - .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())).thenThrow(exception) assertThat( assertThrows { - listener.receive("message") + listener.receive(objectMapper.writeValueAsString(Notification("message"))) }, equalTo(exception) ) @@ -73,11 +69,11 @@ class AwsNotificationListenerTest { ListenerExecutionFailedException("listener failure", meaningfulException, GenericMessage("test")) ) ) - whenever(handler.handle("message")).thenThrow(wrappedException) + whenever(handler.handle(any())).thenThrow(wrappedException) assertThat( assertThrows { - listener.receive("message") + listener.receive(objectMapper.writeValueAsString(Notification("message"))) }, equalTo(wrappedException) ) @@ -85,4 +81,38 @@ class AwsNotificationListenerTest { 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.current() }.thenReturn(span) + listener.receive( + objectMapper.writeValueAsString( + Notification( + "my message", + MessageAttributes("my-sensitive-event-type") + ) + ) + ) + verify(span, never()).setAttribute(eq("message"), any()) + } + } + + @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.current() }.thenReturn(span) + listener.receive( + objectMapper.writeValueAsString( + Notification( + "my message", + MessageAttributes("some-other-event-type") + ) + ) + ) + verify(span).setAttribute(eq("message"), any()) + } + } } diff --git a/projects/common-platform-and-delius/src/main/resources/application.yml b/projects/common-platform-and-delius/src/main/resources/application.yml index 7a2d427f19..38684787bb 100644 --- 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: /