Skip to content

Commit

Permalink
Update missing RFC parts for user event tacking
Browse files Browse the repository at this point in the history
  • Loading branch information
@manuel-alvarez-alvarez
manuel-alvarez-alvarez committed Jun 25, 2024
1 parent 2d36ca7 commit b9fdde2
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 98 deletions.
33 changes: 23 additions & 10 deletions ...main/java/datadog/trace/bootstrap/instrumentation/decorator/AppSecUserEventDecorator.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.bootstrap.instrumentation.decorator;

import static datadog.trace.api.UserEventTrackingMode.DISABLED;
import static datadog.trace.api.UserEventTrackingMode.SAFE;

import datadog.trace.api.Config;
import datadog.trace.api.UserEventTrackingMode;
Expand All @@ -9,10 +10,17 @@
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import java.util.Map;
import java.util.regex.Pattern;
import javax.annotation.Nonnull;

public class AppSecUserEventDecorator {

private static final String NUMBER_PATTERN = "[0-9]+";
private static final String UUID_PATTERN =
"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}";
private static final Pattern SAFE_USER_ID_PATTERN =
Pattern.compile(NUMBER_PATTERN + "|" + UUID_PATTERN, Pattern.CASE_INSENSITIVE);

public boolean isEnabled() {
if (!ActiveSubsystems.APPSEC_ACTIVE) {
return false;
Expand Down Expand Up @@ -41,9 +49,7 @@ public void onLoginSuccess(String userId, Map<String, String> metadata) {
return;
}

if (userId != null) {
segment.setTagTop("usr.id", userId);
}
onUserId(segment, "usr.id", userId);
onEvent(segment, "users.login.success", metadata);
}

Expand All @@ -53,10 +59,7 @@ public void onLoginFailure(String userId, Map<String, String> metadata) {
return;
}

if (userId != null) {
segment.setTagTop("appsec.events.users.login.failure.usr.id", userId);
}

onUserId(segment, "appsec.events.users.login.failure.usr.id", userId);
onEvent(segment, "users.login.failure", metadata);
}

Expand All @@ -66,9 +69,7 @@ public void onSignup(String userId, Map<String, String> metadata) {
return;
}

if (userId != null) {
segment.setTagTop("usr.id", userId);
}
onUserId(segment, "usr.id", userId);
onEvent(segment, "users.signup", metadata);
}

Expand All @@ -87,6 +88,18 @@ private void onEvent(@Nonnull TraceSegment segment, String eventName, Map<String
}
}

private void onUserId(final TraceSegment segment, final String tag, final String userId) {
if (userId == null) {
return;
}
UserEventTrackingMode mode = Config.get().getAppSecUserEventsTrackingMode();
if (mode == SAFE && !SAFE_USER_ID_PATTERN.matcher(userId).matches()) {
// do not set the user id if not numeric or UUID
return;
}
segment.setTagTop(tag, userId);
}

protected TraceSegment getSegment() {
return AgentTracer.get().getTraceSegment();
}
Expand Down
101 changes: 63 additions & 38 deletions ...ovy/datadog/trace/bootstrap/instrumentation/decorator/AppSecUserEventDecoratorTest.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package datadog.trace.bootstrap.instrumentation.decorator

import datadog.trace.api.Config
import datadog.trace.api.internal.TraceSegment
import datadog.trace.bootstrap.ActiveSubsystems
import datadog.trace.test.util.DDSpecification

import static datadog.trace.api.UserEventTrackingMode.DISABLED
import static datadog.trace.api.UserEventTrackingMode.EXTENDED
import static datadog.trace.api.UserEventTrackingMode.SAFE
import static datadog.trace.api.config.AppSecConfig.APPSEC_AUTOMATED_USER_EVENTS_TRACKING

class AppSecUserEventDecoratorTest extends DDSpecification {
Expand All @@ -27,20 +31,26 @@ class AppSecUserEventDecoratorTest extends DDSpecification {
def decorator = newDecorator()

when:
decorator.onSignup('user', ['key1': 'value1', 'key2': 'value2'])
decorator.onSignup(user, ['key1': 'value1', 'key2': 'value2'])

then:
1 * traceSegment.setTagTop('_dd.appsec.events.users.signup.auto.mode', modeTag)
1 * traceSegment.setTagTop('_dd.appsec.events.users.signup.auto.mode', mode)
1 * traceSegment.setTagTop('appsec.events.users.signup.track', true, true)
1 * traceSegment.setTagTop('asm.keep', true)
1 * traceSegment.setTagTop('usr.id', 'user')
1 * traceSegment.setTagTop('appsec.events.users.signup', ['key1':'value1', 'key2':'value2'])
if (setUser) {
1 * traceSegment.setTagTop('usr.id', user)
}
1 * traceSegment.setTagTop('appsec.events.users.signup', ['key1': 'value1', 'key2': 'value2'])
0 * _

where:
mode | modeTag
'safe' | 'SAFE'
'extended' | 'EXTENDED'
mode | user | setUser
'safe' | 'user' | false
'safe' | '1234' | true
'safe' | '591dc126-8431-4d0f-9509-b23318d3dce4' | true
'extended' | 'user' | true
'extended' | '1234' | true
'extended' | '591dc126-8431-4d0f-9509-b23318d3dce4' | true
}

def "test onLoginSuccess [#mode]"() {
Expand All @@ -49,44 +59,54 @@ class AppSecUserEventDecoratorTest extends DDSpecification {
def decorator = newDecorator()

when:
decorator.onLoginSuccess('user', ['key1': 'value1', 'key2': 'value2'])
decorator.onLoginSuccess(user, ['key1': 'value1', 'key2': 'value2'])

then:
1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', modeTag)
1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', mode)
1 * traceSegment.setTagTop('appsec.events.users.login.success.track', true, true)
1 * traceSegment.setTagTop('asm.keep', true)
1 * traceSegment.setTagTop('usr.id', 'user')
1 * traceSegment.setTagTop('appsec.events.users.login.success', ['key1':'value1', 'key2':'value2'])
if (setUser) {
1 * traceSegment.setTagTop('usr.id', user)
}
1 * traceSegment.setTagTop('appsec.events.users.login.success', ['key1': 'value1', 'key2': 'value2'])
0 * _

where:
mode | modeTag
'safe' | 'SAFE'
'extended' | 'EXTENDED'
mode | user | setUser
'safe' | 'user' | false
'safe' | '1234' | true
'safe' | '591dc126-8431-4d0f-9509-b23318d3dce4' | true
'extended' | 'user' | true
'extended' | '1234' | true
'extended' | '591dc126-8431-4d0f-9509-b23318d3dce4' | true
}

def "test onLoginFailed #description [#mode]"() {
def "test onLoginFailed [#mode]"() {
setup:
injectSysConfig(APPSEC_AUTOMATED_USER_EVENTS_TRACKING, mode)
def decorator = newDecorator()

when:
decorator.onLoginFailure('user', ['key1': 'value1', 'key2': 'value2'])
decorator.onLoginFailure(user, ['key1': 'value1', 'key2': 'value2'])

then:
1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.auto.mode', modeTag)
1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.auto.mode', mode)
1 * traceSegment.setTagTop('appsec.events.users.login.failure.track', true, true)
1 * traceSegment.setTagTop('asm.keep', true)
1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.id', 'user')
1 * traceSegment.setTagTop('appsec.events.users.login.failure', ['key1':'value1', 'key2':'value2'])
if (setUser) {
1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.id', user)
}
1 * traceSegment.setTagTop('appsec.events.users.login.failure', ['key1': 'value1', 'key2': 'value2'])
0 * _

where:
mode | modeTag | description
'safe' | 'SAFE' | 'with existing user'
'safe' | 'SAFE' | 'user doesn\'t exist'
'extended' | 'EXTENDED' | 'with existing user'
'extended' | 'EXTENDED' | 'user doesn\'t exist'
mode | user | setUser
'safe' | 'user' | false
'safe' | '1234' | true
'safe' | '591dc126-8431-4d0f-9509-b23318d3dce4' | true
'extended' | 'user' | true
'extended' | '1234' | true
'extended' | '591dc126-8431-4d0f-9509-b23318d3dce4' | true
}

def "test onUserNotFound [#mode]"() {
Expand All @@ -102,9 +122,7 @@ class AppSecUserEventDecoratorTest extends DDSpecification {
0 * _

where:
mode | modeTag
'safe' | 'SAFE'
'extended' | 'EXTENDED'
mode << ['safe', 'extended']
}

def "test isEnabled (appsec = #appsec, mode = #mode)"() {
Expand All @@ -119,22 +137,29 @@ class AppSecUserEventDecoratorTest extends DDSpecification {
then:
enabled == result
and:
Config.get().getAppSecUserEventsTrackingMode() == expectedMode
where:
appsec | mode | result
false | "disabled" | false
false | "safe" | false
false | "extended" | false
true | "disabled" | false
true | "safe" | true
true | "extended" | true
appsec | mode | result | expectedMode
false | "disabled" | false | DISABLED
false | "safe" | false | SAFE
false | "1" | false | SAFE
false | "true" | false | SAFE
false | "extended" | false | EXTENDED
true | "disabled" | false | DISABLED
true | "safe" | true | SAFE
true | "1" | true | SAFE
true | "true" | true | SAFE
true | "extended" | true | EXTENDED
}
def newDecorator() {
return new AppSecUserEventDecorator() {
@Override
protected TraceSegment getSegment() {
return traceSegment
}
@Override
protected TraceSegment getSegment() {
return traceSegment
}
}
}
}
72 changes: 31 additions & 41 deletions ...ava17/datadog/trace/instrumentation/springsecurity5/SpringSecurityUserEventDecorator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@ public void onSignup(UserDetails user, Throwable throwable) {
}

UserEventTrackingMode mode = Config.get().getAppSecUserEventsTrackingMode();
String userId = null;
String userId = user.getUsername();
Map<String, String> metadata = null;

if (mode == EXTENDED && user != null) {
userId = user.getUsername();

if (mode == EXTENDED) {
metadata = new HashMap<>();
metadata.put("enabled", String.valueOf(user.isEnabled()));
metadata.put(
Expand All @@ -71,44 +69,36 @@ public void onLogin(Authentication authentication, Throwable throwable, Authenti
return;
}

String userId = null;

if (mode == EXTENDED) {
userId = authentication.getName();
}

if (mode != DISABLED) {
if (throwable == null && result != null && result.isAuthenticated()) {
Map<String, String> metadata = null;
Object principal = result.getPrincipal();
if (principal instanceof User) {
User user = (User) principal;
metadata = new HashMap<>();
metadata.put("enabled", String.valueOf(user.isEnabled()));
metadata.put(
"authorities",
user.getAuthorities().stream()
.map(Object::toString)
.collect(Collectors.joining(",")));
metadata.put("accountNonExpired", String.valueOf(user.isAccountNonExpired()));
metadata.put("accountNonLocked", String.valueOf(user.isAccountNonLocked()));
metadata.put("credentialsNonExpired", String.valueOf(user.isCredentialsNonExpired()));
}

onLoginSuccess(userId, metadata);
} else if (throwable != null) {
// On bad password, throwable would be
// org.springframework.security.authentication.BadCredentialsException,
// on user not found, throwable can be BadCredentials or
// org.springframework.security.core.userdetails.UsernameNotFoundException depending on the
// internal setting
// hideUserNotFoundExceptions (or a custom AuthenticationProvider implementation overriding
// this).
// This would be the ideal place to check whether the user exists or not, but we cannot do
// so reliably yet.
// See UsernameNotFoundExceptionInstrumentation for more details.
onLoginFailure(userId, null);
String userId = result != null ? result.getName() : authentication.getName();

if (throwable == null && result != null && result.isAuthenticated()) {
Map<String, String> metadata = null;
Object principal = result.getPrincipal();
if (principal instanceof User) {
User user = (User) principal;
metadata = new HashMap<>();
metadata.put("enabled", String.valueOf(user.isEnabled()));
metadata.put(
"authorities",
user.getAuthorities().stream().map(Object::toString).collect(Collectors.joining(",")));
metadata.put("accountNonExpired", String.valueOf(user.isAccountNonExpired()));
metadata.put("accountNonLocked", String.valueOf(user.isAccountNonLocked()));
metadata.put("credentialsNonExpired", String.valueOf(user.isCredentialsNonExpired()));
}

onLoginSuccess(userId, metadata);
} else if (throwable != null) {
// On bad password, throwable would be
// org.springframework.security.authentication.BadCredentialsException,
// on user not found, throwable can be BadCredentials or
// org.springframework.security.core.userdetails.UsernameNotFoundException depending on the
// internal setting
// hideUserNotFoundExceptions (or a custom AuthenticationProvider implementation overriding
// this).
// This would be the ideal place to check whether the user exists or not, but we cannot do
// so reliably yet.
// See UsernameNotFoundExceptionInstrumentation for more details.
onLoginFailure(userId, null);
}
}

Expand Down
8 changes: 4 additions & 4 deletions .../src/test/groovy/datadog/trace/instrumentation/springsecurity5/SpringBootBasedTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
response.body().string() == REGISTER.body
!span.getTags().isEmpty()
span.getTag("appsec.events.users.signup.track") == true
span.getTag("_dd.appsec.events.users.signup.auto.mode") == 'EXTENDED'
span.getTag("_dd.appsec.events.users.signup.auto.mode") == 'extended'
span.getTag("usr.id") == 'admin'
span.getTag("appsec.events.users.signup")['enabled'] == 'true'
span.getTag("appsec.events.users.signup")['authorities'] == 'ROLE_USER'
Expand All @@ -150,7 +150,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
response.body().string() == LOGIN.body
!span.getTags().isEmpty()
span.getTag("appsec.events.users.login.failure.track") == true
span.getTag("_dd.appsec.events.users.login.failure.auto.mode") == 'EXTENDED'
span.getTag("_dd.appsec.events.users.login.failure.auto.mode") == 'extended'
span.getTag("appsec.events.users.login.failure.usr.exists") == false
span.getTag("appsec.events.users.login.failure.usr.id") == 'not_existing_user'
}
Expand All @@ -174,7 +174,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
response.body().string() == LOGIN.body
!span.getTags().isEmpty()
span.getTag("appsec.events.users.login.failure.track") == true
span.getTag("_dd.appsec.events.users.login.failure.auto.mode") == 'EXTENDED'
span.getTag("_dd.appsec.events.users.login.failure.auto.mode") == 'extended'
// TODO: Ideally should be `false` but we have no reliable method to detect it it is just absent. See APPSEC-12765.
span.getTag("appsec.events.users.login.failure.usr.exists") == null
span.getTag("appsec.events.users.login.failure.usr.id") == 'admin'
Expand All @@ -200,7 +200,7 @@ class SpringBootBasedTest extends AppSecHttpServerTest<ConfigurableApplicationCo
response.body().string() == LOGIN.body
!span.getTags().isEmpty()
span.getTag("appsec.events.users.login.success.track") == true
span.getTag("_dd.appsec.events.users.login.success.auto.mode") == 'EXTENDED'
span.getTag("_dd.appsec.events.users.login.success.auto.mode") == 'extended'
span.getTag("usr.id") == 'admin'
span.getTag("appsec.events.users.login.success")['credentialsNonExpired'] == 'true'
span.getTag("appsec.events.users.login.success")['accountNonExpired'] == 'true'
Expand Down
Loading

0 comments on commit b9fdde2

Please sign in to comment.