Merge branch 'main' into gh-1421

CHANGELOG.md

@@ -13,6 +13,7 @@
 * Fix: Run event processors and enrich transactions with contexts (#1430)
 * Bump: sentry-native to 0.4.9 (#1431)
 * Fix: Set Span status for OkHttp integration (#1447)
+* Fix: Set user on transaction in Spring & Spring Boot integrations (#1443)
 
 # 4.4.0-alpha.2
 

sentry-samples/sentry-samples-spring/src/main/java/io/sentry/samples/spring/AppConfig.java

@@ -1,8 +1,20 @@
 package io.sentry.samples.spring;
 
+import io.sentry.IHub;
+import io.sentry.spring.SentryUserFilter;
+import io.sentry.spring.SentryUserProvider;
+import java.util.List;
+import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 
 @Configuration
 @Import(SentryConfig.class)
-public class AppConfig {}
+public class AppConfig {
+
+  @Bean
+  SentryUserFilter sentryUserFilter(
+      final IHub hub, final List<SentryUserProvider> sentryUserProviders) {
+    return new SentryUserFilter(hub, sentryUserProviders);
+  }
+}

sentry-samples/sentry-samples-spring/src/main/java/io/sentry/samples/spring/AppInitializer.java

@@ -3,6 +3,7 @@
 import io.sentry.spring.tracing.SentryTracingFilter;
 import javax.servlet.Filter;
 import org.springframework.web.filter.DelegatingFilterProxy;
+import org.springframework.web.filter.RequestContextFilter;
 import org.springframework.web.servlet.support.AbstractAnnotationConfigDispatcherServletInitializer;
 
 public class AppInitializer extends AbstractAnnotationConfigDispatcherServletInitializer {
@@ -30,6 +31,15 @@ protected Filter[] getServletFilters() {
     // filter required by Spring Security
     DelegatingFilterProxy springSecurityFilterChain = new DelegatingFilterProxy();
     springSecurityFilterChain.setTargetBeanName("springSecurityFilterChain");
-    return new Filter[] {sentryTracingFilter, springSecurityFilterChain};
+    // sets request on RequestContextHolder
+    RequestContextFilter requestContextFilter = new RequestContextFilter();
+
+    // sets Sentry user on the scope
+    DelegatingFilterProxy sentryUserFilterProxy = new DelegatingFilterProxy();
+    sentryUserFilterProxy.setTargetBeanName("sentryUserFilter");
+
+    return new Filter[] {
+      sentryTracingFilter, springSecurityFilterChain, requestContextFilter, sentryUserFilterProxy
+    };
   }
 }

sentry-spring-boot-starter/api/sentry-spring-boot-starter.api

@@ -22,12 +22,14 @@ public class io/sentry/spring/boot/SentryProperties : io/sentry/SentryOptions {
 	public fun <init> ()V
 	public fun getExceptionResolverOrder ()I
 	public fun getLogging ()Lio/sentry/spring/boot/SentryProperties$Logging;
+	public fun getUserFilterOrder ()Ljava/lang/Integer;
 	public fun isEnableTracing ()Z
 	public fun isUseGitCommitIdAsRelease ()Z
 	public fun setEnableTracing (Z)V
 	public fun setExceptionResolverOrder (I)V
 	public fun setLogging (Lio/sentry/spring/boot/SentryProperties$Logging;)V
 	public fun setUseGitCommitIdAsRelease (Z)V
+	public fun setUserFilterOrder (Ljava/lang/Integer;)V
 }
 
 public class io/sentry/spring/boot/SentryProperties$Logging {

sentry-spring-boot-starter/build.gradle.kts

@@ -39,6 +39,7 @@ dependencies {
     compileOnly(Config.Libs.springWeb)
     compileOnly(Config.Libs.servletApi)
     compileOnly(Config.Libs.springBootStarterAop)
+    compileOnly(Config.Libs.springBootStarterSecurity)
 
     annotationProcessor(Config.AnnotationProcessors.springBootAutoConfigure)
     annotationProcessor(Config.AnnotationProcessors.springBootConfiguration)

sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java

@@ -12,16 +12,19 @@
 import io.sentry.spring.SentryExceptionResolver;
 import io.sentry.spring.SentryRequestResolver;
 import io.sentry.spring.SentrySpringRequestListener;
+import io.sentry.spring.SentryUserFilter;
 import io.sentry.spring.SentryUserProvider;
-import io.sentry.spring.SentryUserProviderEventProcessor;
 import io.sentry.spring.SentryWebConfiguration;
+import io.sentry.spring.SpringSecuritySentryUserProvider;
 import io.sentry.spring.tracing.SentryAdviceConfiguration;
 import io.sentry.spring.tracing.SentrySpanPointcutConfiguration;
 import io.sentry.spring.tracing.SentryTracingFilter;
 import io.sentry.spring.tracing.SentryTransactionPointcutConfiguration;
 import io.sentry.transport.ITransportGate;
 import io.sentry.transport.apache.ApacheHttpClientTransportFactory;
 import java.util.List;
+import java.util.Optional;
+import javax.servlet.http.HttpServletRequest;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.jetbrains.annotations.NotNull;
 import org.springframework.beans.factory.ObjectProvider;
@@ -42,6 +45,7 @@
 import org.springframework.context.annotation.Import;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
+import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.web.client.RestTemplate;
 
 @Configuration(proxyBeanMethods = false)
@@ -66,7 +70,6 @@ static class HubConfiguration {
         final @NotNull List<EventProcessor> eventProcessors,
         final @NotNull List<Integration> integrations,
         final @NotNull ObjectProvider<ITransportGate> transportGate,
-        final @NotNull List<SentryUserProvider> sentryUserProviders,
         final @NotNull ObjectProvider<ITransportFactory> transportFactory,
         final @NotNull InAppIncludesResolver inAppPackagesResolver) {
       return options -> {
@@ -75,10 +78,6 @@ static class HubConfiguration {
         tracesSamplerCallback.ifAvailable(options::setTracesSampler);
         eventProcessors.forEach(options::addEventProcessor);
         integrations.forEach(options::addIntegration);
-        sentryUserProviders.forEach(
-            sentryUserProvider ->
-                options.addEventProcessor(
-                    new SentryUserProviderEventProcessor(options, sentryUserProvider)));
         transportGate.ifAvailable(options::setTransportGate);
         transportFactory.ifAvailable(options::setTransportFactory);
         inAppPackagesResolver.resolveInAppIncludes().forEach(options::addInAppInclude);
@@ -125,6 +124,51 @@ static class HubConfiguration {
     @Open
     static class SentryWebMvcConfiguration {
 
+      /**
+       * Configures {@link SpringSecuritySentryUserProvider} only if Spring Security is on the
+       * classpath. Its order is set to be higher than {@link
+       * SentryWebConfiguration#httpServletRequestSentryUserProvider(SentryOptions)}
+       *
+       * @param sentryOptions the Sentry options
+       * @return {@link SpringSecuritySentryUserProvider}
+       */
+      @Bean
+      @ConditionalOnClass(SecurityContextHolder.class)
+      @Order(1)
+      public @NotNull SpringSecuritySentryUserProvider springSecuritySentryUserProvider(
+          final @NotNull SentryOptions sentryOptions) {
+        return new SpringSecuritySentryUserProvider(sentryOptions);
+      }
+
+      /**
+       * Configures {@link SentryUserFilter}. By default it runs as the last filter in order to make
+       * sure that all potential authentication information is propagated to {@link
+       * HttpServletRequest#getUserPrincipal()}. If Spring Security is auto-configured, its order is
+       * set to run after Spring Security.
+       *
+       * @param hub the Sentry hub
+       * @param sentryProperties the Sentry properties
+       * @param sentryUserProvider the user provider
+       * @return {@link SentryUserFilter} registration bean
+       */
+      @Bean
+      @ConditionalOnBean(SentryUserProvider.class)
+      public @NotNull FilterRegistrationBean<SentryUserFilter> sentryUserFilter(
+          final @NotNull IHub hub,
+          final @NotNull SentryProperties sentryProperties,
+          final @NotNull List<SentryUserProvider> sentryUserProvider) {
+        final FilterRegistrationBean<SentryUserFilter> filter = new FilterRegistrationBean<>();
+        filter.setFilter(new SentryUserFilter(hub, sentryUserProvider));
+        filter.setOrder(resolveUserFilterOrder(sentryProperties));
+        return filter;
+      }
+
+      private @NotNull Integer resolveUserFilterOrder(
+          final @NotNull SentryProperties sentryProperties) {
+        return Optional.ofNullable(sentryProperties.getUserFilterOrder())
+            .orElse(Ordered.LOWEST_PRECEDENCE);
+      }
+
       @Bean
       public @NotNull SentryRequestResolver sentryRequestResolver(final @NotNull IHub hub) {
         return new SentryRequestResolver(hub);

sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java

@@ -26,6 +26,13 @@ public class SentryProperties extends SentryOptions {
   /** Report all or only uncaught web exceptions. */
   private int exceptionResolverOrder = 1;
 
+  /**
+   * Defines the {@link io.sentry.spring.SentryUserFilter} order. The default value is {@link
+   * org.springframework.core.Ordered.LOWEST_PRECEDENCE}, if Spring Security is auto-configured, its
+   * guaranteed to run after Spring Security filter chain.
+   */
+  private @Nullable Integer userFilterOrder;
+
   /** Logging framework integration properties. */
   private @NotNull Logging logging = new Logging();
 
@@ -67,7 +74,15 @@ public void setExceptionResolverOrder(int exceptionResolverOrder) {
     this.exceptionResolverOrder = exceptionResolverOrder;
   }
 
-  public Logging getLogging() {
+  public @Nullable Integer getUserFilterOrder() {
+    return userFilterOrder;
+  }
+
+  public void setUserFilterOrder(final @Nullable Integer userFilterOrder) {
+    this.userFilterOrder = userFilterOrder;
+  }
+
+  public @NotNull Logging getLogging() {
     return logging;
   }
 

sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt

@@ -20,8 +20,9 @@ import io.sentry.SentryLevel
 import io.sentry.SentryOptions
 import io.sentry.protocol.User
 import io.sentry.spring.HttpServletRequestSentryUserProvider
+import io.sentry.spring.SentryUserFilter
 import io.sentry.spring.SentryUserProvider
-import io.sentry.spring.SentryUserProviderEventProcessor
+import io.sentry.spring.SpringSecuritySentryUserProvider
 import io.sentry.spring.tracing.SentryTracingFilter
 import io.sentry.test.checkEvent
 import io.sentry.transport.ITransport
@@ -319,11 +320,11 @@ class SentryAutoConfigurationTest {
         contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true")
             .withConfiguration(UserConfigurations.of(SentryUserProviderConfiguration::class.java))
             .run {
-                val options = it.getBean(SentryProperties::class.java)
-                val userProviderEventProcessors = options.eventProcessors.filterIsInstance<SentryUserProviderEventProcessor>()
-                assertEquals(2, userProviderEventProcessors.size)
-                assertTrue(userProviderEventProcessors[0].sentryUserProvider is HttpServletRequestSentryUserProvider)
-                assertTrue(userProviderEventProcessors[1].sentryUserProvider is CustomSentryUserProvider)
+                val userProviders = it.getSentryUserProviders()
+                assertEquals(3, userProviders.size)
+                assertTrue(userProviders[0] is HttpServletRequestSentryUserProvider)
+                assertTrue(userProviders[1] is SpringSecuritySentryUserProvider)
+                assertTrue(userProviders[2] is CustomSentryUserProvider)
             }
     }
 
@@ -332,11 +333,11 @@ class SentryAutoConfigurationTest {
         contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true")
             .withConfiguration(UserConfigurations.of(SentryHighestOrderUserProviderConfiguration::class.java))
             .run {
-                val options = it.getBean(SentryProperties::class.java)
-                val userProviderEventProcessors = options.eventProcessors.filterIsInstance<SentryUserProviderEventProcessor>()
-                assertEquals(2, userProviderEventProcessors.size)
-                assertTrue(userProviderEventProcessors[0].sentryUserProvider is CustomSentryUserProvider)
-                assertTrue(userProviderEventProcessors[1].sentryUserProvider is HttpServletRequestSentryUserProvider)
+                val userProviders = it.getSentryUserProviders()
+                assertEquals(3, userProviders.size)
+                assertTrue(userProviders[0] is CustomSentryUserProvider)
+                assertTrue(userProviders[1] is HttpServletRequestSentryUserProvider)
+                assertTrue(userProviders[2] is SpringSecuritySentryUserProvider)
             }
     }
 
@@ -753,4 +754,9 @@ class SentryAutoConfigurationTest {
         this.doesNotHaveBean("sentrySpanAdvisor")
         return this
     }
+
+    private fun ApplicationContext.getSentryUserProviders(): List<SentryUserProvider> {
+        val userFilter = this.getBean("sentryUserFilter", FilterRegistrationBean::class.java).filter as SentryUserFilter
+        return userFilter.sentryUserProviders
+    }
 }

sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt

@@ -12,6 +12,7 @@ import io.sentry.ITransportFactory
 import io.sentry.Sentry
 import io.sentry.spring.tracing.SentrySpan
 import io.sentry.test.checkEvent
+import io.sentry.test.checkTransaction
 import io.sentry.transport.ITransport
 import java.lang.RuntimeException
 import org.assertj.core.api.Assertions.assertThat
@@ -52,7 +53,7 @@ import org.springframework.web.bind.annotation.RestController
 @SpringBootTest(
     classes = [App::class],
     webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
-    properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.enable-tracing=true"]
+    properties = ["sentry.dsn=http://key@localhost/proj", "sentry.send-default-pii=true", "sentry.enable-tracing=true", "sentry.traces-sample-rate=1.0"]
 )
 class SentrySpringIntegrationTest {
 
@@ -165,6 +166,20 @@ class SentrySpringIntegrationTest {
 
         verify(hub, never()).captureEvent(any())
     }
+
+    @Test
+    fun `sets user on transaction`() {
+        val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
+
+        restTemplate.getForEntity("http://localhost:$port/performance", String::class.java)
+
+        await.untilAsserted {
+            verify(transport).send(checkTransaction { transaction ->
+                assertThat(transaction.user).isNotNull()
+                assertThat(transaction.user!!.username).isEqualTo("user")
+            }, anyOrNull())
+        }
+    }
 }
 
 @SpringBootApplication

sentry-spring/api/sentry-spring.api

@@ -54,6 +54,12 @@ public class io/sentry/spring/SentrySpringServletContainerInitializer : javax/se
 	public fun onStartup (Ljava/util/Set;Ljavax/servlet/ServletContext;)V
 }
 
+public class io/sentry/spring/SentryUserFilter : javax/servlet/Filter {
+	public fun <init> (Lio/sentry/IHub;Ljava/util/List;)V
+	public fun doFilter (Ljavax/servlet/ServletRequest;Ljavax/servlet/ServletResponse;Ljavax/servlet/FilterChain;)V
+	public fun getSentryUserProviders ()Ljava/util/List;
+}
+
 public abstract interface class io/sentry/spring/SentryUserProvider {
 	public abstract fun provideUser ()Lio/sentry/protocol/User;
 }
@@ -70,6 +76,11 @@ public class io/sentry/spring/SentryWebConfiguration {
 	public fun httpServletRequestSentryUserProvider (Lio/sentry/SentryOptions;)Lio/sentry/spring/HttpServletRequestSentryUserProvider;
 }
 
+public final class io/sentry/spring/SpringSecuritySentryUserProvider : io/sentry/spring/SentryUserProvider {
+	public fun <init> (Lio/sentry/SentryOptions;)V
+	public fun provideUser ()Lio/sentry/protocol/User;
+}
+
 public class io/sentry/spring/tracing/SentryAdviceConfiguration {
 	public fun <init> ()V
 	public fun sentrySpanAdvice (Lio/sentry/IHub;)Lorg/aopalliance/aop/Advice;

sentry-spring/build.gradle.kts

@@ -34,6 +34,7 @@ dependencies {
     api(project(":sentry"))
     compileOnly(Config.Libs.springWeb)
     compileOnly(Config.Libs.springAop)
+    compileOnly(Config.Libs.springSecurityWeb)
     compileOnly(Config.Libs.aspectj)
     compileOnly(Config.Libs.servletApi)
 

sentry-spring/src/main/java/io/sentry/spring/SentryInitBeanPostProcessor.java

@@ -20,7 +20,7 @@ public class SentryInitBeanPostProcessor implements BeanPostProcessor, Applicati
   private @Nullable ApplicationContext applicationContext;
 
   @Override
-  @SuppressWarnings("unchecked")
+  @SuppressWarnings({"unchecked", "deprecation"})
   public Object postProcessAfterInitialization(
       final @NotNull Object bean, @NotNull final String beanName) throws BeansException {
     if (bean instanceof SentryOptions) {