From 3bc21dc7b0378603d176212334f993de8e5cc450 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Sun, 30 Aug 2020 12:43:04 +0200 Subject: [PATCH 1/2] Add `sendDefaultPii` flag to `SentryOptions`. Filters out user information and sensitive request headers in Spring Boot integration if this flag is set to false. --- .../java/io/sentry/core/SentryOptions.java | 11 ++++ .../samples/spring/SecurityConfiguration.java | 9 +++- .../src/main/resources/application.properties | 1 + .../spring/boot/SentryAutoConfiguration.java | 4 +- .../spring/boot/SentryRequestFilter.java | 7 ++- ...tryRequestHttpServletRequestProcessor.java | 26 ++++++--- .../spring/boot/SentrySecurityFilter.java | 7 ++- ...SentryUserHttpServletRequestProcessor.java | 25 +++++---- ...yRequestHttpServletRequestProcessorTest.kt | 54 +++++++++++++++++-- .../boot/SentrySpringIntegrationTest.kt | 10 ++-- ...ntryUserHttpServletRequestProcessorTest.kt | 25 ++++++++- 11 files changed, 147 insertions(+), 32 deletions(-) diff --git a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java index 101a5f49c..7c10b1702 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java @@ -210,6 +210,9 @@ public class SentryOptions { /** SdkVersion object that contains the Sentry Client Name and its version */ private @Nullable SdkVersion sdkVersion; + /** whether to send personal identifiable information along with events */ + private boolean sendDefaultPii = false; + /** * Adds an event processor * @@ -996,6 +999,14 @@ public void setSdkVersion(final @Nullable SdkVersion sdkVersion) { this.sdkVersion = sdkVersion; } + public boolean isSendDefaultPii() { + return sendDefaultPii; + } + + public void setSendDefaultPii(boolean sendDefaultPii) { + this.sendDefaultPii = sendDefaultPii; + } + /** The BeforeSend callback */ public interface BeforeSendCallback { diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java index d5527cdbb..5d8173881 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/SecurityConfiguration.java @@ -1,6 +1,7 @@ package io.sentry.samples.spring; import io.sentry.core.IHub; +import io.sentry.core.SentryOptions; import io.sentry.spring.boot.SentrySecurityFilter; import org.jetbrains.annotations.NotNull; import org.springframework.context.annotation.Bean; @@ -19,15 +20,19 @@ public class SecurityConfiguration extends WebSecurityConfigurerAdapter { private final @NotNull IHub hub; + private final @NotNull SentryOptions options; - public SecurityConfiguration(final @NotNull IHub hub) { + public SecurityConfiguration(final @NotNull IHub hub, final @NotNull SentryOptions options) { this.hub = hub; + this.options = options; } + // this API is meant to be consumed by non-browser clients thus the CSRF protection is not needed. @Override + @SuppressWarnings("lgtm[java/spring-disabled-csrf-protection]") protected void configure(final @NotNull HttpSecurity http) throws Exception { // register SentrySecurityFilter to attach user information to SentryEvents - http.addFilterAfter(new SentrySecurityFilter(hub), AnonymousAuthenticationFilter.class) + http.addFilterAfter(new SentrySecurityFilter(hub, options), AnonymousAuthenticationFilter.class) .csrf() .disable() .authorizeRequests() diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties index 3d8acbbc1..9f28892b0 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot/src/main/resources/application.properties @@ -1,2 +1,3 @@ # NOTE: Replace the test DSN below with YOUR OWN DSN to see the events from this app in your Sentry project/dashboard sentry.dsn=https://f7f320d5c3a54709be7b28e0f2ca7081@sentry.io/1808954 +sentry.send-default-pii=true diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index ffda4a04a..4f768765d 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -80,9 +80,9 @@ static class SentryWebMvcConfiguration { @Bean public @NotNull FilterRegistrationBean sentryRequestFilter( - final @NotNull IHub sentryHub) { + final @NotNull IHub sentryHub, final @NotNull SentryOptions sentryOptions) { FilterRegistrationBean filterRegistrationBean = - new FilterRegistrationBean<>(new SentryRequestFilter(sentryHub)); + new FilterRegistrationBean<>(new SentryRequestFilter(sentryHub, sentryOptions)); filterRegistrationBean.setOrder(Ordered.HIGHEST_PRECEDENCE); return filterRegistrationBean; } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java index 31e72d9a3..fe0fb81aa 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestFilter.java @@ -2,6 +2,7 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.core.IHub; +import io.sentry.core.SentryOptions; import java.io.IOException; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -14,9 +15,11 @@ @Open public class SentryRequestFilter extends OncePerRequestFilter { private final @NotNull IHub hub; + private final @NotNull SentryOptions options; - public SentryRequestFilter(final @NotNull IHub hub) { + public SentryRequestFilter(final @NotNull IHub hub, final @NotNull SentryOptions options) { this.hub = hub; + this.options = options; } @Override @@ -29,7 +32,7 @@ protected void doFilterInternal( hub.configureScope( scope -> { - scope.addEventProcessor(new SentryRequestHttpServletRequestProcessor(request)); + scope.addEventProcessor(new SentryRequestHttpServletRequestProcessor(request, options)); }); filterChain.doFilter(request, response); } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java index af8c78e9f..7b718377f 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessor.java @@ -3,10 +3,13 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.core.EventProcessor; import io.sentry.core.SentryEvent; +import io.sentry.core.SentryOptions; import io.sentry.core.protocol.Request; +import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.servlet.http.HttpServletRequest; import org.jetbrains.annotations.NotNull; @@ -15,10 +18,16 @@ /** Attaches information about HTTP request to {@link SentryEvent}. */ @Open public class SentryRequestHttpServletRequestProcessor implements EventProcessor { + private static final List SENSITIVE_HEADERS = + Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIES"); + private final @NotNull HttpServletRequest request; + private final @NotNull SentryOptions options; - public SentryRequestHttpServletRequestProcessor(final @NotNull HttpServletRequest request) { + public SentryRequestHttpServletRequestProcessor( + final @NotNull HttpServletRequest request, final @NotNull SentryOptions options) { this.request = request; + this.options = options; } @Override @@ -30,22 +39,27 @@ public SentryRequestHttpServletRequestProcessor(final @NotNull HttpServletReques // httpRequest.getRequestURL() returns StringBuffer which is considered an obsolete class. @SuppressWarnings("JdkObsolete") - private static @NotNull Request resolveSentryRequest( - final @NotNull HttpServletRequest httpRequest) { + private @NotNull Request resolveSentryRequest(final @NotNull HttpServletRequest httpRequest) { final Request sentryRequest = new Request(); sentryRequest.setMethod(httpRequest.getMethod()); sentryRequest.setQueryString(httpRequest.getQueryString()); sentryRequest.setUrl(httpRequest.getRequestURL().toString()); sentryRequest.setHeaders(resolveHeadersMap(httpRequest)); - sentryRequest.setCookies(toString(httpRequest.getHeaders("Cookie"))); + + if (options.isSendDefaultPii()) { + sentryRequest.setCookies(toString(httpRequest.getHeaders("Cookie"))); + } return sentryRequest; } - private static @NotNull Map resolveHeadersMap( + private @NotNull Map resolveHeadersMap( final @NotNull HttpServletRequest request) { final Map headersMap = new HashMap<>(); for (String headerName : Collections.list(request.getHeaderNames())) { - headersMap.put(headerName, toString(request.getHeaders(headerName))); + // do not copy personal information identifiable headers + if (options.isSendDefaultPii() || !SENSITIVE_HEADERS.contains(headerName.toUpperCase())) { + headersMap.put(headerName, toString(request.getHeaders(headerName))); + } } return headersMap; } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java index 2525b4c7f..546e8dd79 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySecurityFilter.java @@ -2,6 +2,7 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.core.IHub; +import io.sentry.core.SentryOptions; import java.io.IOException; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -17,9 +18,11 @@ @Open public class SentrySecurityFilter extends OncePerRequestFilter { private final @NotNull IHub hub; + private final @NotNull SentryOptions options; - public SentrySecurityFilter(final @NotNull IHub hub) { + public SentrySecurityFilter(final @NotNull IHub hub, final @NotNull SentryOptions options) { this.hub = hub; + this.options = options; } @Override @@ -32,7 +35,7 @@ protected void doFilterInternal( scope -> scope.addEventProcessor( new SentryUserHttpServletRequestProcessor( - request.getUserPrincipal(), toIpAddress(request)))); + request.getUserPrincipal(), toIpAddress(request), options))); filterChain.doFilter(request, response); } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java index c3bf8fe0a..e7923ff54 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java @@ -3,6 +3,7 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.core.EventProcessor; import io.sentry.core.SentryEvent; +import io.sentry.core.SentryOptions; import io.sentry.core.protocol.User; import java.security.Principal; import java.util.Optional; @@ -14,25 +15,31 @@ public class SentryUserHttpServletRequestProcessor implements EventProcessor { private final @Nullable Principal principal; private final @Nullable String ipAddress; + private final @Nullable SentryOptions options; public SentryUserHttpServletRequestProcessor( - final @Nullable Principal principal, final @Nullable String ipAddress) { + final @Nullable Principal principal, + final @Nullable String ipAddress, + final @Nullable SentryOptions options) { this.principal = principal; this.ipAddress = ipAddress; + this.options = options; } @Override public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { - final User user = Optional.ofNullable(event.getUser()).orElseGet(User::new); + if (options.isSendDefaultPii()) { + final User user = Optional.ofNullable(event.getUser()).orElseGet(User::new); - if (ipAddress != null) { - user.setIpAddress(ipAddress); - } - if (principal != null) { - user.setUsername(principal.getName()); - } + if (ipAddress != null) { + user.setIpAddress(ipAddress); + } + if (principal != null) { + user.setUsername(principal.getName()); + } - event.setUser(user); + event.setUser(user); + } return event; } } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt index 7579e5384..ba709d6c9 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryRequestHttpServletRequestProcessorTest.kt @@ -1,9 +1,13 @@ package io.sentry.spring.boot import io.sentry.core.SentryEvent +import io.sentry.core.SentryOptions import java.net.URI import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue import org.springframework.http.MediaType import org.springframework.mock.web.MockServletContext import org.springframework.test.web.servlet.request.MockMvcRequestBuilders @@ -17,7 +21,7 @@ class SentryRequestHttpServletRequestProcessorTest { .header("some-header", "some-header value") .accept(MediaType.APPLICATION_JSON) .buildRequest(MockServletContext()) - val eventProcessor = SentryRequestHttpServletRequestProcessor(request) + val eventProcessor = SentryRequestHttpServletRequestProcessor(request, SentryOptions()) val event = SentryEvent() eventProcessor.process(event, null) @@ -38,7 +42,7 @@ class SentryRequestHttpServletRequestProcessorTest { .header("another-header", "another value") .header("another-header", "another value2") .buildRequest(MockServletContext()) - val eventProcessor = SentryRequestHttpServletRequestProcessor(request) + val eventProcessor = SentryRequestHttpServletRequestProcessor(request, SentryOptions()) val event = SentryEvent() eventProcessor.process(event, null) @@ -49,17 +53,59 @@ class SentryRequestHttpServletRequestProcessorTest { } @Test - fun `attaches cookies information`() { + fun `when sendDefaultPii is set to true, attaches cookies information`() { val request = MockMvcRequestBuilders .get(URI.create("http://example.com?param1=xyz")) .header("Cookie", "name=value") .header("Cookie", "name2=value2") .buildRequest(MockServletContext()) - val eventProcessor = SentryRequestHttpServletRequestProcessor(request) + val sentryOptions = SentryOptions() + sentryOptions.isSendDefaultPii = true + val eventProcessor = SentryRequestHttpServletRequestProcessor(request, sentryOptions) val event = SentryEvent() eventProcessor.process(event, null) assertEquals("name=value,name2=value2", event.request.cookies) } + + @Test + fun `when sendDefaultPii is set to false, does not attach cookies`() { + val request = MockMvcRequestBuilders + .get(URI.create("http://example.com?param1=xyz")) + .header("Cookie", "name=value") + .buildRequest(MockServletContext()) + val sentryOptions = SentryOptions() + sentryOptions.isSendDefaultPii = false + val eventProcessor = SentryRequestHttpServletRequestProcessor(request, sentryOptions) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertNull(event.request.cookies) + } + + @Test + fun `when sendDefaultPii is set to false, does not attach sensitive headers`() { + val request = MockMvcRequestBuilders + .get(URI.create("http://example.com?param1=xyz")) + .header("some-header", "some-header value") + .header("X-FORWARDED-FOR", "192.168.0.1") + .header("authorization", "Token") + .header("Authorization", "Token") + .header("Cookies", "some cookies") + .buildRequest(MockServletContext()) + val sentryOptions = SentryOptions() + sentryOptions.isSendDefaultPii = false + val eventProcessor = SentryRequestHttpServletRequestProcessor(request, sentryOptions) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertFalse(event.request.headers.containsKey("X-FORWARDED-FOR")) + assertFalse(event.request.headers.containsKey("Authorization")) + assertFalse(event.request.headers.containsKey("authorization")) + assertFalse(event.request.headers.containsKey("Cookies")) + assertTrue(event.request.headers.containsKey("some-header")) + } } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt index 0f6dcbe87..e5d0cd1ee 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpringIntegrationTest.kt @@ -5,6 +5,7 @@ import com.nhaarman.mockitokotlin2.verify import io.sentry.core.IHub import io.sentry.core.Sentry import io.sentry.core.SentryEvent +import io.sentry.core.SentryOptions import io.sentry.core.transport.ITransport import org.assertj.core.api.Assertions.assertThat import org.awaitility.kotlin.await @@ -37,7 +38,7 @@ import org.springframework.web.bind.annotation.RestController @SpringBootTest( classes = [App::class], webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT, - properties = ["sentry.dsn=http://key@localhost/proj"] + properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true"] ) class SentrySpringIntegrationTest { @@ -97,11 +98,14 @@ class HelloController { } @Configuration -open class SecurityConfiguration(private val hub: IHub) : WebSecurityConfigurerAdapter() { +open class SecurityConfiguration( + private val hub: IHub, + private val options: SentryOptions +) : WebSecurityConfigurerAdapter() { override fun configure(http: HttpSecurity) { http - .addFilterAfter(SentrySecurityFilter(hub), AnonymousAuthenticationFilter::class.java) + .addFilterAfter(SentrySecurityFilter(hub, options), AnonymousAuthenticationFilter::class.java) .csrf().disable() .authorizeRequests().anyRequest().authenticated() .and() diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt index d26907783..9fa3fe46e 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryUserHttpServletRequestProcessorTest.kt @@ -3,15 +3,19 @@ package io.sentry.spring.boot import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import io.sentry.core.SentryEvent +import io.sentry.core.SentryOptions import java.security.Principal import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull class SentryUserHttpServletRequestProcessorTest { @Test fun `attaches user's IP address to Sentry Event`() { - val eventProcessor = SentryUserHttpServletRequestProcessor(null, "192.168.0.1") + val options = SentryOptions() + options.isSendDefaultPii = true + val eventProcessor = SentryUserHttpServletRequestProcessor(null, "192.168.0.1", options) val event = SentryEvent() eventProcessor.process(event, null) @@ -24,11 +28,28 @@ class SentryUserHttpServletRequestProcessorTest { val principal = mock() whenever(principal.name).thenReturn("janesmith") - val eventProcessor = SentryUserHttpServletRequestProcessor(principal, null) + val options = SentryOptions() + options.isSendDefaultPii = true + val eventProcessor = SentryUserHttpServletRequestProcessor(principal, null, options) val event = SentryEvent() eventProcessor.process(event, null) assertEquals("janesmith", event.user.username) } + + @Test + fun `when sendDefaultPii is set to false, does not attach user data Sentry Event`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + + val options = SentryOptions() + options.isSendDefaultPii = false + val eventProcessor = SentryUserHttpServletRequestProcessor(principal, null, options) + val event = SentryEvent() + + eventProcessor.process(event, null) + + assertNull(event.user) + } } From efc9e4988d3537690aee131f562e2094f2576fa2 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Sun, 30 Aug 2020 13:01:47 +0200 Subject: [PATCH 2/2] Polish. --- .../spring/boot/SentryUserHttpServletRequestProcessor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java index e7923ff54..c9bfff93e 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryUserHttpServletRequestProcessor.java @@ -15,12 +15,12 @@ public class SentryUserHttpServletRequestProcessor implements EventProcessor { private final @Nullable Principal principal; private final @Nullable String ipAddress; - private final @Nullable SentryOptions options; + private final @NotNull SentryOptions options; public SentryUserHttpServletRequestProcessor( final @Nullable Principal principal, final @Nullable String ipAddress, - final @Nullable SentryOptions options) { + final @NotNull SentryOptions options) { this.principal = principal; this.ipAddress = ipAddress; this.options = options;