diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java index f8476e2ced1..a5d23687f5f 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/GatewayBridge.java @@ -234,10 +234,6 @@ private Flow onLoginEvent( if (mode == DISABLED) { return NoopFlow.INSTANCE; } - final String user = anonymizeUser(mode, originalUser); - if (user == null) { - return NoopFlow.INSTANCE; - } final AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); if (ctx == null) { return NoopFlow.INSTANCE; @@ -248,29 +244,38 @@ private Flow onLoginEvent( segment.setTagTop(Tags.ASM_KEEP, true); segment.setTagTop(Tags.PROPAGATED_APPSEC, true); + // update span tags + segment.setTagTop("appsec.events." + eventName + ".track", true, true); + if (exists != null) { + segment.setTagTop("appsec.events." + eventName + ".usr.exists", exists, true); + } + if (metadata != null && !metadata.isEmpty()) { + segment.setTagTop("appsec.events." + eventName, metadata, true); + } + if (mode == SDK) { + segment.setTagTop("_dd.appsec.events." + eventName + ".sdk", true, true); + } else { + segment.setTagTop("_dd.appsec.events." + eventName + ".auto.mode", mode.fullName(), true); + } + + final String user = anonymizeUser(mode, originalUser); + if (user == null) { + // can happen in custom events + return NoopFlow.INSTANCE; + } + // skip event if we have an SDK one if (mode != SDK) { segment.setTagTop("_dd.appsec.usr.login", user); segment.setTagTop("_dd.appsec.usr.id", user); - segment.setTagTop( - "_dd.appsec.events.users." + eventName + ".auto.mode", mode.fullName(), true); if (ctx.getUserLoginSource() == SDK) { return NoopFlow.INSTANCE; } - } else { - segment.setTagTop("_dd.appsec.events.users." + eventName + ".sdk", true, true); } - // update span tags - segment.setTagTop("appsec.events.users." + eventName + ".usr.login", user, true); - segment.setTagTop("appsec.events.users." + eventName + ".usr.id", user, true); - segment.setTagTop("appsec.events.users." + eventName + ".track", true, true); - if (exists != null) { - segment.setTagTop("appsec.events.users." + eventName + ".usr.exists", exists, true); - } - if (metadata != null && !metadata.isEmpty()) { - segment.setTagTop("appsec.events.users." + eventName, metadata, true); - } + // update user span tags + segment.setTagTop("appsec.events." + eventName + ".usr.login", user, true); + segment.setTagTop("appsec.events." + eventName + ".usr.id", user, true); // update current context with new user login ctx.setUserLoginSource(mode); @@ -284,9 +289,9 @@ private Flow onLoginEvent( final List> addresses = new ArrayList<>(3); addresses.add(KnownAddresses.USER_LOGIN); addresses.add(KnownAddresses.USER_ID); - if (KnownAddresses.LOGIN_SUCCESS.getKey().endsWith(eventName)) { + if (eventName.endsWith("login.success")) { addresses.add(KnownAddresses.LOGIN_SUCCESS); - } else if (KnownAddresses.LOGIN_FAILURE.getKey().endsWith(eventName)) { + } else if (eventName.endsWith("login.failure")) { addresses.add(KnownAddresses.LOGIN_FAILURE); } final MapDataBundle.Builder bundleBuilder = diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy index a39cd81af0b..97c5589faaf 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/gateway/GatewayBridgeSpecification.groovy @@ -1092,7 +1092,7 @@ class GatewayBridgeSpecification extends DDSpecification { eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo when: - loginEventCB.apply(ctx, mode, 'signup', null, USER_ID, metadata) + loginEventCB.apply(ctx, mode, 'users.signup', null, USER_ID, metadata) then: if (mode == DISABLED) { @@ -1130,7 +1130,7 @@ class GatewayBridgeSpecification extends DDSpecification { eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo when: - loginEventCB.apply(ctx, mode, 'login.success', null, USER_ID, metadata) + loginEventCB.apply(ctx, mode, 'users.login.success', null, USER_ID, metadata) then: if (mode == DISABLED) { @@ -1169,7 +1169,7 @@ class GatewayBridgeSpecification extends DDSpecification { eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo when: - loginEventCB.apply(ctx, mode, 'login.failure', false, USER_ID, metadata) + loginEventCB.apply(ctx, mode, 'users.login.failure', false, USER_ID, metadata) then: if (mode == DISABLED) { @@ -1202,6 +1202,23 @@ class GatewayBridgeSpecification extends DDSpecification { mode << UserIdCollectionMode.values() } + void "test onCustomEvent (#mode)"() { + setup: + final metadata = ['key1': 'value1', 'key2': 'value2'] + eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo + + when: + loginEventCB.apply(ctx, SDK, 'my.event', null, null, metadata) + + then: + 1 * traceSegment.setTagTop('_dd.appsec.events.my.event.sdk', true, true) + 1 * traceSegment.setTagTop('appsec.events.my.event.track', true, true) + 1 * traceSegment.setTagTop('appsec.events.my.event', ['key1': 'value1', 'key2': 'value2'], true) + 1 * traceSegment.setTagTop('asm.keep', true) + 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 0 * eventDispatcher.publishDataEvent + } + void "test onUserEvent (automated login events should not overwrite SDK)"() { setup: final firstUser = 'first-user' @@ -1234,7 +1251,7 @@ class GatewayBridgeSpecification extends DDSpecification { eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo when: - loginEventCB.apply(ctx, SDK, 'login.success', null, firstUser, null) + loginEventCB.apply(ctx, SDK, 'users.login.success', null, firstUser, null) then: 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', firstUser, true) @@ -1245,7 +1262,7 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> NoopFlow.INSTANCE when: - loginEventCB.apply(ctx, IDENTIFICATION, 'login.success', null, secondUser, null) + loginEventCB.apply(ctx, IDENTIFICATION, 'users.login.success', null, secondUser, null) then: 0 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', _, _) diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/user/AppSecEventTrackerSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/user/AppSecEventTrackerSpecification.groovy index d41f4bd0751..890c8cf7bb1 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/user/AppSecEventTrackerSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/user/AppSecEventTrackerSpecification.groovy @@ -76,7 +76,7 @@ class AppSecEventTrackerSpecification extends DDSpecification { GlobalTracer.getEventTracker().trackLoginSuccessEvent('user1', ['key1': 'value1', 'key2': 'value2']) then: - 1 * loginEvent.apply(_ as RequestContext, SDK, 'login.success', null, 'user1', ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + 1 * loginEvent.apply(_ as RequestContext, SDK, 'users.login.success', null, 'user1', ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE 0 * _ } @@ -85,7 +85,7 @@ class AppSecEventTrackerSpecification extends DDSpecification { GlobalTracer.getEventTracker().trackLoginFailureEvent('user1', true, ['key1': 'value1', 'key2': 'value2']) then: - 1 * loginEvent.apply(_ as RequestContext, SDK, 'login.failure', true, 'user1', ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + 1 * loginEvent.apply(_ as RequestContext, SDK, 'users.login.failure', true, 'user1', ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE 0 * _ } @@ -142,7 +142,7 @@ class AppSecEventTrackerSpecification extends DDSpecification { then: if (mode != DISABLED) { - 1 * loginEvent.apply(_ as RequestContext, mode, 'signup', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + 1 * loginEvent.apply(_ as RequestContext, mode, 'users.signup', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE } 0 * _ @@ -156,7 +156,7 @@ class AppSecEventTrackerSpecification extends DDSpecification { then: if (mode != DISABLED) { - 1 * loginEvent.apply(_ as RequestContext, mode, 'login.success', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + 1 * loginEvent.apply(_ as RequestContext, mode, 'users.login.success', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE } 0 * _ @@ -170,7 +170,7 @@ class AppSecEventTrackerSpecification extends DDSpecification { then: if (mode != DISABLED) { - 1 * loginEvent.apply(_ as RequestContext, mode, 'login.failure', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + 1 * loginEvent.apply(_ as RequestContext, mode, 'users.login.failure', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE } 0 * _ @@ -259,7 +259,7 @@ class AppSecEventTrackerSpecification extends DDSpecification { void 'test blocking on a userId'() { setup: final action = new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO) - loginEvent.apply(_ as RequestContext, SDK, 'login.success', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> new ActionFlow(action: action) + loginEvent.apply(_ as RequestContext, SDK, 'users.login.success', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> new ActionFlow(action: action) when: tracker.onLoginSuccessEvent(SDK, USER_ID, ['key1': 'value1', 'key2': 'value2']) diff --git a/dd-java-agent/instrumentation/spring-security-5/src/main/java/datadog/trace/instrumentation/springsecurity5/SpringSecurityUserEventDecorator.java b/dd-java-agent/instrumentation/spring-security-5/src/main/java/datadog/trace/instrumentation/springsecurity5/SpringSecurityUserEventDecorator.java index 4ece8a890ba..63cd1624b96 100644 --- a/dd-java-agent/instrumentation/spring-security-5/src/main/java/datadog/trace/instrumentation/springsecurity5/SpringSecurityUserEventDecorator.java +++ b/dd-java-agent/instrumentation/spring-security-5/src/main/java/datadog/trace/instrumentation/springsecurity5/SpringSecurityUserEventDecorator.java @@ -138,6 +138,9 @@ public void onUser(final Authentication authentication) { UserIdCollectionMode mode = UserIdCollectionMode.get(); String username = authentication.getName(); + if (missingUserId(username)) { + return; + } tracker.onUserEvent(mode, username); } @@ -163,6 +166,14 @@ private static String findRootAuthentication(Class authentication) { return Authentication.class.getName(); // set this a default for really custom impls } + private static boolean missingUserId(final String username) { + if (username == null || username.isEmpty()) { + WafMetricCollector.get().missingUserId(LoginFramework.SPRING_SECURITY); + return true; + } + return false; + } + private static boolean missingUsername(final String username, final LoginEvent event) { if (username == null || username.isEmpty()) { WafMetricCollector.get().missingUserLogin(LoginFramework.SPRING_SECURITY, event); diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java index dbe63756317..352f2304be3 100644 --- a/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java +++ b/internal-api/src/main/java/datadog/trace/api/appsec/AppSecEventTracker.java @@ -3,9 +3,6 @@ import static datadog.trace.api.UserIdCollectionMode.DISABLED; import static datadog.trace.api.UserIdCollectionMode.SDK; import static datadog.trace.api.gateway.Events.EVENTS; -import static datadog.trace.api.telemetry.LoginEvent.LOGIN_FAILURE; -import static datadog.trace.api.telemetry.LoginEvent.LOGIN_SUCCESS; -import static datadog.trace.api.telemetry.LoginEvent.SIGN_UP; import datadog.appsec.api.blocking.BlockingException; import datadog.trace.api.EventTracker; @@ -91,7 +88,7 @@ public void onSignupEvent( } dispatch( EVENTS.loginEvent(), - (ctx, callback) -> callback.apply(ctx, mode, SIGN_UP.getSpanTag(), null, userId, metadata)); + (ctx, callback) -> callback.apply(ctx, mode, "users.signup", null, userId, metadata)); } public void onLoginSuccessEvent( @@ -101,7 +98,7 @@ public void onLoginSuccessEvent( } dispatch( EVENTS.loginEvent(), - (ctx, cb) -> cb.apply(ctx, mode, LOGIN_SUCCESS.getSpanTag(), null, userId, metadata)); + (ctx, cb) -> cb.apply(ctx, mode, "users.login.success", null, userId, metadata)); } public void onLoginFailureEvent( @@ -114,7 +111,7 @@ public void onLoginFailureEvent( } dispatch( EVENTS.loginEvent(), - (ctx, cb) -> cb.apply(ctx, mode, LOGIN_FAILURE.getSpanTag(), exists, userId, metadata)); + (ctx, cb) -> cb.apply(ctx, mode, "users.login.failure", exists, userId, metadata)); } public void onCustomEvent( diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/LoginEvent.java b/internal-api/src/main/java/datadog/trace/api/telemetry/LoginEvent.java index fa2b9ab3f1d..550d21ad4b5 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/LoginEvent.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/LoginEvent.java @@ -1,30 +1,20 @@ package datadog.trace.api.telemetry; public enum LoginEvent { - LOGIN_SUCCESS("login.success", "login_success"), - LOGIN_FAILURE("login.failure", "login_failure"), + LOGIN_SUCCESS("login_success"), + LOGIN_FAILURE("login_failure"), SIGN_UP("signup"); private static final int numValues = LoginEvent.values().length; - private final String spanTag; - private final String telemetryTag; + private final String tag; LoginEvent(final String tag) { - this(tag, tag); + this.tag = tag; } - LoginEvent(final String spanTag, final String telemetryTag) { - this.spanTag = spanTag; - this.telemetryTag = telemetryTag; - } - - public String getTelemetryTag() { - return telemetryTag; - } - - public String getSpanTag() { - return spanTag; + public String getTag() { + return tag; } public static int getNumValues() { diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java index 115abaebcdb..76914d4b725 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/WafMetricCollector.java @@ -42,9 +42,12 @@ private WafMetricCollector() { new AtomicLongArray(RuleType.getNumValues()); private static final AtomicLongArray missingUserLoginQueue = new AtomicLongArray(LoginFramework.getNumValues() * LoginEvent.getNumValues()); + private static final AtomicLongArray missingUserIdQueue = + new AtomicLongArray(LoginFramework.getNumValues()); /** WAF version that will be initialized with wafInit and reused for all metrics. */ private static String wafVersion = ""; + /** * Rules version that will be updated on each wafInit and wafUpdates. This is not entirely * accurate, since wafRequest metrics might be collected for a period where a rules update happens @@ -105,6 +108,10 @@ public void missingUserLogin(final LoginFramework framework, final LoginEvent ev framework.ordinal() * LoginEvent.getNumValues() + eventType.ordinal()); } + public void missingUserId(final LoginFramework framework) { + missingUserLoginQueue.incrementAndGet(framework.ordinal()); + } + @Override public Collection drain() { if (!rawMetricsQueue.isEmpty()) { @@ -215,7 +222,7 @@ public void prepareMetrics() { long counter = missingUserLoginQueue.getAndSet(ordinal, 0); if (counter > 0) { if (!rawMetricsQueue.offer( - new MissingUserLoginMetric(counter, framework.getTag(), event.getTelemetryTag()))) { + new MissingUserLoginMetric(counter, framework.getTag(), event.getTag()))) { return; } } @@ -260,6 +267,17 @@ public MissingUserLoginMetric(long counter, String framework, String type) { } } + public static class MissingUserIdMetric extends WafMetric { + + public MissingUserIdMetric(long counter, String framework) { + super( + "instrum.user_auth.missing_user_id", + counter, + "framework:" + framework, + "event_type:authenticated_request"); + } + } + public static class WafRequestsRawMetric extends WafMetric { public WafRequestsRawMetric( final long counter, diff --git a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy index 145a6ae17d0..13f76958f93 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/telemetry/WafMetricCollectorTest.groovy @@ -253,6 +253,49 @@ class WafMetricCollectorTest extends DDSpecification { } } + void 'test missing user id event metric'() { + given: + def collector = WafMetricCollector.get() + final count = 6 + final latch = new CountDownLatch(1) + final executors = Executors.newFixedThreadPool(4) + final action = { LoginFramework framework -> + latch.await() + collector.missingUserId(framework) + } + + when: + (1..count).each { + executors.submit { + action.call(LoginFramework.SPRING_SECURITY) + } + } + + latch.countDown() + executors.shutdown() + final finished = executors.awaitTermination(5, TimeUnit.SECONDS) + + then: + finished + collector.prepareMetrics() + final drained = collector.drain() + final metrics = drained.findAll { + it.metricName == 'instrum.user_auth.missing_user_id' + } + metrics.size() == 1 + metrics.forEach { metric -> + assert metric.namespace == 'appsec' + assert metric.type == 'count' + assert metric.value == count + final tags = metric.tags.collectEntries { + final parts = it.split(":") + return [(parts[0]): parts[1]] + } + assert tags["framework"] == LoginFramework.SPRING_SECURITY.getTag() + assert tags["event_type"] == "authenticated_request" + } + } + def "test Rasp #ruleType metrics"() { when: WafMetricCollector.get().wafInit('waf_ver1', 'rules.1')