Skip to content

Commit

Permalink
Avoid spans with both login success and failure events
Browse files Browse the repository at this point in the history
  • Loading branch information
manuel-alvarez-alvarez committed Nov 11, 2024
1 parent 9e8d0dc commit bdaf40a
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@
import org.springframework.security.core.AuthenticationException;

@AutoService(InstrumenterModule.class)
public class AuthenticationProviderInstrumentation extends InstrumenterModule.AppSec
public class AuthenticationManagerInstrumentation extends InstrumenterModule.AppSec
implements Instrumenter.ForTypeHierarchy {

public AuthenticationProviderInstrumentation() {
public AuthenticationManagerInstrumentation() {
super("spring-security");
}

@Override
public String hierarchyMarkerType() {
return "org.springframework.security.authentication.AuthenticationProvider";
return "org.springframework.security.authentication.AuthenticationManager";
}

@Override
Expand All @@ -50,10 +50,10 @@ public void methodAdvice(MethodTransformer transformer) {
.and(takesArgument(0, named("org.springframework.security.core.Authentication")))
.and(returns(named("org.springframework.security.core.Authentication")))
.and(isPublic()),
getClass().getName() + "$AuthenticationProviderAdvice");
getClass().getName() + "$AuthenticationManagerAdvice");
}

public static class AuthenticationProviderAdvice {
public static class AuthenticationManagerAdvice {

@Advice.OnMethodExit(onThrowable = AuthenticationException.class, suppress = Throwable.class)
public static void onExit(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ package datadog.trace.instrumentation.springsecurity5

import custom.CustomAuthenticationFilter
import custom.CustomAuthenticationProvider
import custom.FailingAuthenticationProvider
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.security.authentication.AuthenticationManager
import org.springframework.security.authentication.AuthenticationProvider
import org.springframework.security.authentication.ProviderManager
import org.springframework.security.config.annotation.ObjectPostProcessor
import org.springframework.security.config.annotation.web.builders.HttpSecurity
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer
Expand Down Expand Up @@ -49,6 +53,7 @@ class SecurityConfig {
@Override
void configure(HttpSecurity http) throws Exception {
AuthenticationManager authenticationManager = http.getSharedObject(AuthenticationManager)
http.authenticationProvider(new FailingAuthenticationProvider())
http.authenticationProvider(new CustomAuthenticationProvider())
http.addFilterBefore(new CustomAuthenticationFilter(authenticationManager), UsernamePasswordAuthenticationFilter)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
span.getTag("appsec.events.users.login.success")['enabled'] == 'true'
span.getTag("appsec.events.users.login.success")['authorities'] == 'ROLE_USER'
span.getTag("appsec.events.users.login.success")['accountNonLocked'] == 'true'

span.getTag("appsec.events.users.login.failure.track") == null
}

void 'test failed signup'() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package custom;

import org.springframework.security.authentication.AuthenticationProvider;
import org.springframework.security.authentication.AuthenticationServiceException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;

public class FailingAuthenticationProvider implements AuthenticationProvider {

@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
throw new AuthenticationServiceException("I'm dumb");
}

@Override
public boolean supports(Class<?> authentication) {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package datadog.trace.instrumentation.springsecurity6

import custom.CustomAuthenticationFilter
import custom.CustomAuthenticationProvider
import custom.FailingAuthenticationProvider
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.security.authentication.AuthenticationManager
Expand Down Expand Up @@ -47,6 +48,7 @@ class SecurityConfig {
@Override
void configure(HttpSecurity http) throws Exception {
AuthenticationManager authenticationManager = http.getSharedObject(AuthenticationManager)
http.authenticationProvider(new FailingAuthenticationProvider())
http.authenticationProvider(new CustomAuthenticationProvider())
http.addFilterBefore(new CustomAuthenticationFilter(authenticationManager), UsernamePasswordAuthenticationFilter)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
span.getTag("appsec.events.users.login.success")['enabled'] == 'true'
span.getTag("appsec.events.users.login.success")['authorities'] == 'ROLE_USER'
span.getTag("appsec.events.users.login.success")['accountNonLocked'] == 'true'

span.getTag("appsec.events.users.login.failure.track") == null
}

void 'test failed signup'() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package custom;

import org.springframework.security.authentication.AuthenticationProvider;
import org.springframework.security.authentication.AuthenticationServiceException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;

public class FailingAuthenticationProvider implements AuthenticationProvider {

@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
throw new AuthenticationServiceException("I'm dumb");
}

@Override
public boolean supports(Class<?> authentication) {
return true;
}
}

0 comments on commit bdaf40a

Please sign in to comment.