Skip to content

Commit c64178f

Browse files
Avoid spans with both login success and failure events
1 parent 59388f1 commit c64178f

File tree

7 files changed

+54
-5
lines changed

7 files changed

+54
-5
lines changed
Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@
1818
import org.springframework.security.core.AuthenticationException;
1919

2020
@AutoService(InstrumenterModule.class)
21-
public class AuthenticationProviderInstrumentation extends InstrumenterModule.AppSec
21+
public class AuthenticationManagerInstrumentation extends InstrumenterModule.AppSec
2222
implements Instrumenter.ForTypeHierarchy {
2323

24-
public AuthenticationProviderInstrumentation() {
24+
public AuthenticationManagerInstrumentation() {
2525
super("spring-security");
2626
}
2727

2828
@Override
2929
public String hierarchyMarkerType() {
30-
return "org.springframework.security.authentication.AuthenticationProvider";
30+
return "org.springframework.security.authentication.AuthenticationManager";
3131
}
3232

3333
@Override
@@ -50,10 +50,10 @@ public void methodAdvice(MethodTransformer transformer) {
5050
.and(takesArgument(0, named("org.springframework.security.core.Authentication")))
5151
.and(returns(named("org.springframework.security.core.Authentication")))
5252
.and(isPublic()),
53-
getClass().getName() + "$AuthenticationProviderAdvice");
53+
getClass().getName() + "$AuthenticationManagerAdvice");
5454
}
5555

56-
public static class AuthenticationProviderAdvice {
56+
public static class AuthenticationManagerAdvice {
5757

5858
@Advice.OnMethodExit(onThrowable = AuthenticationException.class, suppress = Throwable.class)
5959
public static void onExit(

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ package datadog.trace.instrumentation.springsecurity5
22

33
import custom.CustomAuthenticationFilter
44
import custom.CustomAuthenticationProvider
5+
import custom.FailingAuthenticationProvider
56
import org.springframework.context.annotation.Bean
67
import org.springframework.context.annotation.Configuration
78
import org.springframework.security.authentication.AuthenticationManager
9+
import org.springframework.security.authentication.AuthenticationProvider
10+
import org.springframework.security.authentication.ProviderManager
11+
import org.springframework.security.config.annotation.ObjectPostProcessor
812
import org.springframework.security.config.annotation.web.builders.HttpSecurity
913
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity
1014
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer
@@ -49,6 +53,7 @@ class SecurityConfig {
4953
@Override
5054
void configure(HttpSecurity http) throws Exception {
5155
AuthenticationManager authenticationManager = http.getSharedObject(AuthenticationManager)
56+
http.authenticationProvider(new FailingAuthenticationProvider())
5257
http.authenticationProvider(new CustomAuthenticationProvider())
5358
http.addFilterBefore(new CustomAuthenticationFilter(authenticationManager), UsernamePasswordAuthenticationFilter)
5459
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,8 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
235235
span.getTag("appsec.events.users.login.success")['enabled'] == 'true'
236236
span.getTag("appsec.events.users.login.success")['authorities'] == 'ROLE_USER'
237237
span.getTag("appsec.events.users.login.success")['accountNonLocked'] == 'true'
238+
239+
span.getTag("appsec.events.users.login.failure.track") == null
238240
}
239241

240242
void 'test failed signup'() {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package custom;
2+
3+
import org.springframework.security.authentication.AuthenticationProvider;
4+
import org.springframework.security.authentication.AuthenticationServiceException;
5+
import org.springframework.security.core.Authentication;
6+
import org.springframework.security.core.AuthenticationException;
7+
8+
public class FailingAuthenticationProvider implements AuthenticationProvider {
9+
10+
@Override
11+
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
12+
throw new AuthenticationServiceException("I'm dumb");
13+
}
14+
15+
@Override
16+
public boolean supports(Class<?> authentication) {
17+
return true;
18+
}
19+
}

dd-java-agent/instrumentation/spring-security-6/src/test/groovy/datadog/trace/instrumentation/springsecurity6/SecurityConfig.groovy

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package datadog.trace.instrumentation.springsecurity6
22

33
import custom.CustomAuthenticationFilter
44
import custom.CustomAuthenticationProvider
5+
import custom.FailingAuthenticationProvider
56
import org.springframework.context.annotation.Bean
67
import org.springframework.context.annotation.Configuration
78
import org.springframework.security.authentication.AuthenticationManager
@@ -47,6 +48,7 @@ class SecurityConfig {
4748
@Override
4849
void configure(HttpSecurity http) throws Exception {
4950
AuthenticationManager authenticationManager = http.getSharedObject(AuthenticationManager)
51+
http.authenticationProvider(new FailingAuthenticationProvider())
5052
http.authenticationProvider(new CustomAuthenticationProvider())
5153
http.addFilterBefore(new CustomAuthenticationFilter(authenticationManager), UsernamePasswordAuthenticationFilter)
5254
}

dd-java-agent/instrumentation/spring-security-6/src/test/groovy/datadog/trace/instrumentation/springsecurity6/SpringBootBasedTest.groovy

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
211211
span.getTag("appsec.events.users.login.success")['enabled'] == 'true'
212212
span.getTag("appsec.events.users.login.success")['authorities'] == 'ROLE_USER'
213213
span.getTag("appsec.events.users.login.success")['accountNonLocked'] == 'true'
214+
215+
span.getTag("appsec.events.users.login.failure.track") == null
214216
}
215217

216218
void 'test failed signup'() {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package custom;
2+
3+
import org.springframework.security.authentication.AuthenticationProvider;
4+
import org.springframework.security.authentication.AuthenticationServiceException;
5+
import org.springframework.security.core.Authentication;
6+
import org.springframework.security.core.AuthenticationException;
7+
8+
public class FailingAuthenticationProvider implements AuthenticationProvider {
9+
10+
@Override
11+
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
12+
throw new AuthenticationServiceException("I'm dumb");
13+
}
14+
15+
@Override
16+
public boolean supports(Class<?> authentication) {
17+
return true;
18+
}
19+
}

0 commit comments

Comments
 (0)