Skip auto user events when sign-up logic fails (#6964)

Author: manuel-alvarez-alvarez

dd-java-agent/instrumentation/spring-security-5/src/main/java17/datadog/trace/instrumentation/springsecurity5/SpringSecurityUserEventDecorator.java

@@ -18,7 +18,13 @@ public class SpringSecurityUserEventDecorator extends AppSecUserEventDecorator {
   public static final SpringSecurityUserEventDecorator DECORATE =
       new SpringSecurityUserEventDecorator();
 
-  public void onSignup(UserDetails user) {
+  public void onSignup(UserDetails user, Throwable throwable) {
+    // skip failures while signing up a user, later on, we might want to generate a separate event
+    // for this case
+    if (throwable != null) {
+      return;
+    }
+
     UserEventTrackingMode mode = Config.get().getAppSecUserEventsTrackingMode();
     String userId = null;
     Map<String, String> metadata = null;

dd-java-agent/instrumentation/spring-security-5/src/main/java17/datadog/trace/instrumentation/springsecurity5/UserDetailsManagerAdvice.java

@@ -7,9 +7,11 @@
 public class UserDetailsManagerAdvice {
 
   @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
-  public static void onExit(@Advice.Argument(value = 0, readOnly = false) UserDetails user) {
+  public static void onExit(
+      @Advice.Argument(value = 0, readOnly = false) UserDetails user,
+      @Advice.Thrown Throwable throwable) {
     if (ActiveSubsystems.APPSEC_ACTIVE) {
-      SpringSecurityUserEventDecorator.DECORATE.onSignup(user);
+      SpringSecurityUserEventDecorator.DECORATE.onSignup(user, throwable);
     }
   }
 }

dd-java-agent/instrumentation/spring-security-5/src/test/groovy/datadog/trace/instrumentation/springsecurity5/SpringBootBasedTest.groovy

@@ -83,15 +83,6 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
     injectSysConfig('dd.appsec.automated-user-events-tracking', 'extended')
   }
 
-  def setupSpec() {
-    server = server()
-    server.start()
-    address = server.address()
-    assert address.port > 0
-    assert address.path.endsWith("/")
-    println "$server started at: $address"
-  }
-
   Request.Builder request(TestEndpoint uri, String method, RequestBody body) {
     def url = HttpUrl.get(uri.resolve(address)).newBuilder()
       .encodedQuery(uri.rawQuery)
@@ -214,4 +205,30 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
     span.getTag("appsec.events.users.login.success")['authorities'] == 'ROLE_USER'
     span.getTag("appsec.events.users.login.success")['accountNonLocked'] == 'true'
   }
+
+  void 'test failed signup'() {
+    setup:
+    final formBody = new FormBody.Builder()
+      .add('username', randomString(1_000))
+      .add('password', 'cant_create_me')
+      .build()
+
+    final request = request(REGISTER, 'POST', formBody).build()
+
+    when:
+    final response = client.newCall(request).execute()
+    TEST_WRITER.waitForTraces(1)
+    final span = TEST_WRITER.flatten().first() as DDSpan
+
+    then:
+    response.code() == 500
+    span.getTags().findAll { it.key.startsWith('appsec.events.users.signup') }.isEmpty()
+  }
+
+  @SuppressWarnings('GroovyAssignabilityCheck')
+  private static String randomString(int length) {
+    return new Random().with { random ->
+      (1..length).collect { Character.valueOf((char) (random.nextInt(26) + (char)'a')) }
+    }.join()
+  }
 }