From 8c89366ca882295976a403883752be645d3f5d14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Wed, 18 Dec 2024 11:50:58 +0100 Subject: [PATCH] Update user lifecycle tracking to V3 --- .../java/com/datadog/appsec/AppSecSystem.java | 3 +- .../appsec/event/data/KnownAddresses.java | 4 + .../appsec/gateway/AppSecRequestContext.java | 29 ++ .../datadog/appsec/gateway/GatewayBridge.java | 242 +++++++++++++---- .../appsec/user/AppSecEventTrackerImpl.java | 257 ------------------ .../data/KnownAddressesSpecification.groovy | 3 +- .../gateway/GatewayBridgeSpecification.groovy | 234 +++++++++++++--- .../AppSecEventTrackerSpecification.groovy | 207 +++----------- .../SpringSecurityUserEventDecorator.java | 35 ++- .../SpringBootBasedTest.groovy | 61 +++-- .../SpringBootBasedTest.groovy | 61 +++-- .../trace/api/appsec/AppSecEventTracker.java | 143 +++++++++- .../trace/api/appsec/LoginEventCallback.java | 17 ++ .../datadog/trace/api/gateway/Events.java | 30 +- .../api/gateway/InstrumentationGateway.java | 32 ++- .../trace/api/telemetry/LoginEvent.java | 33 +++ .../trace/api/telemetry/LoginFramework.java | 21 ++ .../api/telemetry/WafMetricCollector.java | 32 ++- .../trace/api/UserIdCollectionModeTest.groovy | 13 +- .../telemetry/WafMetricCollectorTest.groovy | 72 ++++- .../gateway/InstrumentationGatewayTest.java | 35 ++- 21 files changed, 917 insertions(+), 647 deletions(-) delete mode 100644 dd-java-agent/appsec/src/main/java/com/datadog/appsec/user/AppSecEventTrackerImpl.java create mode 100644 internal-api/src/main/java/datadog/trace/api/appsec/LoginEventCallback.java create mode 100644 internal-api/src/main/java/datadog/trace/api/telemetry/LoginEvent.java create mode 100644 internal-api/src/main/java/datadog/trace/api/telemetry/LoginFramework.java diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java index ea37e9c5c74..b87573c339e 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/AppSecSystem.java @@ -8,7 +8,6 @@ import com.datadog.appsec.event.ReplaceableEventProducerService; import com.datadog.appsec.gateway.GatewayBridge; import com.datadog.appsec.powerwaf.PowerWAFModule; -import com.datadog.appsec.user.AppSecEventTrackerImpl; import com.datadog.appsec.util.AbortStartupException; import com.datadog.appsec.util.StandardizedLogging; import datadog.appsec.api.blocking.Blocking; @@ -99,7 +98,7 @@ private static void doStart(SubscriptionService gw, SharedCommunicationObjects s Blocking.setBlockingService(new BlockingServiceImpl(REPLACEABLE_EVENT_PRODUCER)); - AppSecEventTracker.setEventTracker(new AppSecEventTrackerImpl()); + AppSecEventTracker.setEventTracker(new AppSecEventTracker()); STARTED.set(true); diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java index 65649e149ec..1e440556676 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/event/data/KnownAddresses.java @@ -111,6 +111,8 @@ public interface KnownAddresses { Address USER_ID = new Address<>("usr.id"); + Address USER_LOGIN = new Address<>("usr.login"); + Address SESSION_ID = new Address<>("usr.session_id"); /** The URL of a network resource being requested (outgoing request) */ @@ -189,6 +191,8 @@ static Address forName(String name) { return SERVER_GRAPHQL_ALL_RESOLVERS; case "usr.id": return USER_ID; + case "usr.login": + return USER_LOGIN; case "usr.session_id": return SESSION_ID; case "server.io.net.url": diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java index 503e71bacd1..d917ac12f40 100644 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java +++ b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java @@ -8,6 +8,7 @@ import com.datadog.appsec.report.AppSecEvent; import com.datadog.appsec.util.StandardizedLogging; import datadog.trace.api.Config; +import datadog.trace.api.UserIdCollectionMode; import datadog.trace.api.http.StoredBodySupplier; import datadog.trace.api.internal.TraceSegment; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -125,6 +126,10 @@ public class AppSecRequestContext implements DataBundle, Closeable { // keep a reference to the last published usr.id private volatile String userId; + private volatile UserIdCollectionMode userIdSource; + // keep a reference to the last published usr.login + private volatile String userLogin; + private volatile UserIdCollectionMode userLoginSource; // keep a reference to the last published usr.session_id private volatile String sessionId; @@ -435,6 +440,30 @@ public void setUserId(String userId) { this.userId = userId; } + public UserIdCollectionMode getUserIdSource() { + return userIdSource; + } + + public void setUserIdSource(UserIdCollectionMode userIdSource) { + this.userIdSource = userIdSource; + } + + public String getUserLogin() { + return userLogin; + } + + public void setUserLogin(String userLogin) { + this.userLogin = userLogin; + } + + public UserIdCollectionMode getUserLoginSource() { + return userLoginSource; + } + + public void setUserLoginSource(UserIdCollectionMode userLoginSource) { + this.userLoginSource = userLoginSource; + } + public void setSessionId(String sessionId) { this.sessionId = sessionId; } 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 6b98dad668c..53441cc3968 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 @@ -1,10 +1,16 @@ package com.datadog.appsec.gateway; import static com.datadog.appsec.event.data.MapDataBundle.Builder.CAPACITY_0_2; +import static com.datadog.appsec.event.data.MapDataBundle.Builder.CAPACITY_3_4; import static com.datadog.appsec.event.data.MapDataBundle.Builder.CAPACITY_6_10; import static com.datadog.appsec.gateway.AppSecRequestContext.DEFAULT_REQUEST_HEADERS_ALLOW_LIST; import static com.datadog.appsec.gateway.AppSecRequestContext.REQUEST_HEADERS_ALLOW_LIST; import static com.datadog.appsec.gateway.AppSecRequestContext.RESPONSE_HEADERS_ALLOW_LIST; +import static datadog.trace.api.UserIdCollectionMode.ANONYMIZATION; +import static datadog.trace.api.UserIdCollectionMode.DISABLED; +import static datadog.trace.api.UserIdCollectionMode.SDK; +import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; +import static datadog.trace.util.Strings.toHexString; import com.datadog.appsec.AppSecSystem; import com.datadog.appsec.api.security.ApiSecurityRequestSampler; @@ -22,7 +28,6 @@ import com.datadog.appsec.report.AppSecEventWrapper; import datadog.trace.api.Config; import datadog.trace.api.UserIdCollectionMode; -import datadog.trace.api.function.TriFunction; import datadog.trace.api.gateway.Events; import datadog.trace.api.gateway.Flow; import datadog.trace.api.gateway.IGSpanInfo; @@ -41,6 +46,8 @@ import java.net.URISyntaxException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -51,7 +58,9 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,6 +74,10 @@ public class GatewayBridge { private static final Pattern QUERY_PARAM_SPLITTER = Pattern.compile("&"); private static final Map> EMPTY_QUERY_PARAMS = Collections.emptyMap(); + private static final int HASH_SIZE_BYTES = 16; // 128 bits + private static final String ANON_PREFIX = "anon_"; + private static final AtomicBoolean SHA_MISSING_REPORTED = new AtomicBoolean(false); + /** User tracking tags that will force the collection of request headers */ private static final String[] USER_TRACKING_TAGS = { "appsec.events.users.login.success.track", "appsec.events.users.login.failure.track" @@ -91,7 +104,8 @@ public class GatewayBridge { private volatile DataSubscriberInfo ioNetUrlSubInfo; private volatile DataSubscriberInfo ioFileSubInfo; private volatile DataSubscriberInfo sessionIdSubInfo; - private final ConcurrentHashMap, DataSubscriberInfo> userIdSubInfo = + private volatile DataSubscriberInfo userIdSubInfo; + private final ConcurrentHashMap loginEventSubInfo = new ConcurrentHashMap<>(); public GatewayBridge( @@ -134,11 +148,8 @@ public void init() { subscriptionService.registerCallback(EVENTS.networkConnection(), this::onNetworkConnection); subscriptionService.registerCallback(EVENTS.fileLoaded(), this::onFileLoaded); subscriptionService.registerCallback(EVENTS.requestSession(), this::onRequestSession); - subscriptionService.registerCallback(EVENTS.userId(), this.onUserEvent(KnownAddresses.USER_ID)); - subscriptionService.registerCallback( - EVENTS.loginSuccess(), this.onUserEvent(KnownAddresses.LOGIN_SUCCESS)); - subscriptionService.registerCallback( - EVENTS.loginFailure(), this.onUserEvent(KnownAddresses.LOGIN_FAILURE)); + subscriptionService.registerCallback(EVENTS.user(), this::onUser); + subscriptionService.registerCallback(EVENTS.loginEvent(), this::onLoginEvent); if (additionalIGEvents.contains(EVENTS.requestPathParams())) { subscriptionService.registerCallback(EVENTS.requestPathParams(), this::onRequestPathParams); @@ -149,55 +160,157 @@ public void init() { } } - private TriFunction> onUserEvent( - final Address address) { - return (ctx_, mode, userId) -> { - final AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC); - if (userId == null || ctx == null) { + private Flow onUser( + final RequestContext ctx_, final UserIdCollectionMode mode, final String originalUser) { + 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; + } + final TraceSegment segment = ctx_.getTraceSegment(); + + // span with ASM data + segment.setTagTop(Tags.ASM_KEEP, true); + segment.setTagTop(Tags.PROPAGATED_APPSEC, true); + + // skip event if we have an SDK one + if (mode != SDK) { + segment.setTagTop("_dd.appsec.usr.id", user); + if (ctx.getUserIdSource() == SDK) { return NoopFlow.INSTANCE; } - final TraceSegment segment = ctx_.getTraceSegment(); - // user id can be set by the SDK overriding the auto event, always update the segment - segment.setTagTop("usr.id", userId); - segment.setTagTop("_dd.appsec.user.collection_mode", mode.shortName()); - final List> addresses = new ArrayList<>(2); - final boolean newUserId = !userId.equals(ctx.getUserId()); - if (newUserId) { - // unlikely that multiple threads will update the value at the same time - ctx.setUserId(userId); - addresses.add(KnownAddresses.USER_ID); - } - if (address != KnownAddresses.USER_ID) { - addresses.add(address); - } - if (addresses.isEmpty()) { - // nothing to publish so short-circuit here + } + + // update span tags + segment.setTagTop("usr.id", user); + segment.setTagTop("_dd.appsec.user.collection_mode", mode.fullName()); + + // update current context with new user id + ctx.setUserIdSource(mode); + final boolean newUserId = !user.equals(ctx.getUserId()); + if (!newUserId) { + return NoopFlow.INSTANCE; + } + ctx.setUserId(user); + + // call waf if we have a new user id + while (true) { + DataSubscriberInfo subInfo = userIdSubInfo; + if (subInfo == null) { + subInfo = producerService.getDataSubscribers(KnownAddresses.USER_ID); + userIdSubInfo = subInfo; + } + if (subInfo == null || subInfo.isEmpty()) { return NoopFlow.INSTANCE; } - final Address[] addressArray = addresses.toArray(new Address[0]); - while (true) { - DataSubscriberInfo subInfo = - userIdSubInfo.computeIfAbsent( - address, k -> producerService.getDataSubscribers(addressArray)); - if (subInfo == null || subInfo.isEmpty()) { - return NoopFlow.INSTANCE; - } - MapDataBundle.Builder bundle = new MapDataBundle.Builder(CAPACITY_0_2); - if (newUserId) { - bundle.add(KnownAddresses.USER_ID, userId); - } - if (address != KnownAddresses.USER_ID) { - // we don't support null values for the address so we use an invalid placeholder here - bundle.add(address, "invalid"); - } - try { - GatewayContext gwCtx = new GatewayContext(false); - return producerService.publishDataEvent(subInfo, ctx, bundle.build(), gwCtx); - } catch (ExpiredSubscriberInfoException e) { - userIdSubInfo.remove(address); - } + DataBundle bundle = + new MapDataBundle.Builder(CAPACITY_0_2).add(KnownAddresses.USER_ID, user).build(); + try { + GatewayContext gwCtx = new GatewayContext(false); + return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); + } catch (ExpiredSubscriberInfoException e) { + userIdSubInfo = null; + } + } + } + + private Flow onLoginEvent( + final RequestContext ctx_, + final UserIdCollectionMode mode, + final String eventName, + final Boolean exists, + final String originalUser, + final Map metadata) { + 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; + } + final TraceSegment segment = ctx_.getTraceSegment(); + + // span with ASM data + segment.setTagTop(Tags.ASM_KEEP, true); + segment.setTagTop(Tags.PROPAGATED_APPSEC, true); + + // 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 current context with new user login + ctx.setUserLoginSource(mode); + final boolean newUserLogin = !user.equals(ctx.getUserLogin()); + if (!newUserLogin) { + return NoopFlow.INSTANCE; + } + ctx.setUserLogin(user); + + // call waf if we have a new user login + final List> addresses = new ArrayList<>(3); + addresses.add(KnownAddresses.USER_LOGIN); + addresses.add(KnownAddresses.USER_ID); + if (KnownAddresses.LOGIN_SUCCESS.getKey().endsWith(eventName)) { + addresses.add(KnownAddresses.LOGIN_SUCCESS); + } else if (KnownAddresses.LOGIN_FAILURE.getKey().endsWith(eventName)) { + addresses.add(KnownAddresses.LOGIN_FAILURE); + } + final MapDataBundle.Builder bundleBuilder = + new MapDataBundle.Builder(addresses.size() == 2 ? CAPACITY_0_2 : CAPACITY_3_4); + bundleBuilder.add(KnownAddresses.USER_ID, user); + bundleBuilder.add(KnownAddresses.USER_LOGIN, user); + if (addresses.size() == 3) { + // we don't support null values for the address so we use an invalid placeholder here + bundleBuilder.add(addresses.get(2), "invalid"); + } + final DataBundle bundle = bundleBuilder.build(); + final String subInfoKey = + addresses.stream().map(Address::getKey).collect(Collectors.joining("|")); + while (true) { + DataSubscriberInfo subInfo = + loginEventSubInfo.computeIfAbsent( + subInfoKey, + t -> producerService.getDataSubscribers(addresses.toArray(new Address[0]))); + if (subInfo == null || subInfo.isEmpty()) { + return NoopFlow.INSTANCE; + } + try { + GatewayContext gwCtx = new GatewayContext(false); + return producerService.publishDataEvent(subInfo, ctx, bundle, gwCtx); + } catch (ExpiredSubscriberInfoException e) { + loginEventSubInfo.remove(subInfoKey); } - }; + } } private Flow onRequestSession(final RequestContext ctx_, final String sessionId) { @@ -940,6 +1053,33 @@ private static int byteToDigit(byte b) { return -1; } + protected static String anonymizeUser(final UserIdCollectionMode mode, final String userId) { + if (mode != ANONYMIZATION || userId == null) { + return userId; + } + MessageDigest digest; + try { + // TODO avoid lookup a new instance every time + digest = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + if (!SHA_MISSING_REPORTED.getAndSet(true)) { + log.error( + SEND_TELEMETRY, + "Missing SHA-256 digest, user collection in 'anon' mode cannot continue", + e); + } + return null; + } + digest.update(userId.getBytes()); + byte[] hash = digest.digest(); + if (hash.length > HASH_SIZE_BYTES) { + byte[] temp = new byte[HASH_SIZE_BYTES]; + System.arraycopy(hash, 0, temp, 0, temp.length); + hash = temp; + } + return ANON_PREFIX + toHexString(hash); + } + private static class IGAppSecEventDependencies { private static final Map, Collection>> diff --git a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/user/AppSecEventTrackerImpl.java b/dd-java-agent/appsec/src/main/java/com/datadog/appsec/user/AppSecEventTrackerImpl.java deleted file mode 100644 index e7cf336fa11..00000000000 --- a/dd-java-agent/appsec/src/main/java/com/datadog/appsec/user/AppSecEventTrackerImpl.java +++ /dev/null @@ -1,257 +0,0 @@ -package com.datadog.appsec.user; - -import static datadog.trace.api.UserIdCollectionMode.ANONYMIZATION; -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.LogCollector.SEND_TELEMETRY; -import static datadog.trace.util.Strings.toHexString; - -import datadog.appsec.api.blocking.BlockingException; -import datadog.trace.api.UserIdCollectionMode; -import datadog.trace.api.appsec.AppSecEventTracker; -import datadog.trace.api.function.TriFunction; -import datadog.trace.api.gateway.BlockResponseFunction; -import datadog.trace.api.gateway.CallbackProvider; -import datadog.trace.api.gateway.EventType; -import datadog.trace.api.gateway.Flow; -import datadog.trace.api.gateway.RequestContext; -import datadog.trace.api.gateway.RequestContextSlot; -import datadog.trace.api.internal.TraceSegment; -import datadog.trace.api.telemetry.WafMetricCollector; -import datadog.trace.bootstrap.ActiveSubsystems; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import datadog.trace.bootstrap.instrumentation.api.AgentTracer; -import datadog.trace.bootstrap.instrumentation.api.Tags; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; -import javax.annotation.Nonnull; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class AppSecEventTrackerImpl extends AppSecEventTracker { - - private static final Logger LOGGER = LoggerFactory.getLogger(AppSecEventTrackerImpl.class); - private static final int HASH_SIZE_BYTES = 16; // 128 bits - private static final String ANON_PREFIX = "anon_"; - private static final AtomicBoolean SHA_MISSING_REPORTED = new AtomicBoolean(false); - - private static final String LOGIN_FAILURE_NO_USER_TAG = - "appsec.events.users.login.failure.usr.exists"; - private static final String LOGIN_FAILURE_USER_ID_EXTRA_TAG = - "appsec.events.users.login.failure.usr.id"; - private static final String USER_COLLECTION_MODE_TAG = "_dd.appsec.user.collection_mode"; - - protected boolean isEnabled(final UserIdCollectionMode mode) { - return ActiveSubsystems.APPSEC_ACTIVE && mode != DISABLED; - } - - @Override - public void onUserNotFound(final UserIdCollectionMode mode) { - TraceSegment segment = beforeEvent(mode); - if (segment == null) { - return; - } - segment.setTagTop(LOGIN_FAILURE_NO_USER_TAG, false); - } - - @Override - public void onSignupEvent( - final UserIdCollectionMode mode, final String userId, final Map metadata) { - TraceSegment segment = beforeEvent(mode, userId); - if (segment == null) { - return; - } - onUserId(mode, segment, userId, EVENTS.userId()); - onEvent(mode, segment, "users.signup", false, metadata); - } - - @Override - public void onLoginSuccessEvent( - final UserIdCollectionMode mode, final String userId, final Map metadata) { - TraceSegment segment = beforeEvent(mode, userId); - if (segment == null) { - return; - } - onUserId(mode, segment, userId, EVENTS.loginSuccess()); - onEvent(mode, segment, "users.login.success", false, metadata); - } - - @Override - public void onLoginFailureEvent( - final UserIdCollectionMode mode, - final String userId, - final Boolean exists, - final Map metadata) { - TraceSegment segment = beforeEvent(mode, userId); - if (segment == null) { - return; - } - onUserId(mode, segment, userId, EVENTS.loginFailure(), LOGIN_FAILURE_USER_ID_EXTRA_TAG); - onEvent(mode, segment, "users.login.failure", false, metadata); - if (exists != null) { - segment.setTagTop(LOGIN_FAILURE_NO_USER_TAG, exists, false); - } - } - - @Override - public void onUserEvent(UserIdCollectionMode mode, String userId) { - TraceSegment segment = beforeEvent(mode, userId); - if (segment == null) { - return; - } - onUserId(mode, segment, userId, EVENTS.userId()); - } - - @Override - public void onCustomEvent( - final UserIdCollectionMode mode, final String eventName, final Map metadata) { - if (eventName == null || eventName.isEmpty()) { - throw new IllegalArgumentException("EventName is null or empty"); - } - TraceSegment segment = beforeEvent(mode); - if (segment == null) { - return; - } - onEvent(mode, segment, eventName, true, metadata); - } - - /** Takes care of user anonymization if required. */ - protected void onUserId( - @Nonnull final UserIdCollectionMode mode, - final TraceSegment segment, - final String userId, - final EventType>> event, - final String... extraTags) { - if (mode != SDK && isSdkCollectedUser(segment)) { - return; // do not overwrite users set by the SDK - } - final String finalUserId = mode == ANONYMIZATION ? anonymize(userId) : userId; - for (String tag : extraTags) { - segment.setTagTop(tag, finalUserId, false); - } - final AgentSpan span = tracer().activeSpan(); - if (span == null) { - return; - } - final RequestContext ctx = span.getRequestContext(); - if (ctx == null) { - return; - } - final Flow flow = callIGCallbackUserId(tracer().activeSpan(), mode, finalUserId, event); - final Flow.Action action = flow.getAction(); - if (action instanceof Flow.Action.RequestBlockingAction) { - final BlockResponseFunction brf = ctx.getBlockResponseFunction(); - if (brf != null) { - Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; - brf.tryCommitBlockingResponse( - ctx.getTraceSegment(), - rba.getStatusCode(), - rba.getBlockingContentType(), - rba.getExtraHeaders()); - } - throw new BlockingException("Blocked request (for user id)"); - } - } - - private void onEvent( - @Nonnull final UserIdCollectionMode mode, - @Nonnull final TraceSegment segment, - String eventName, - boolean sanitize, - Map metadata) { - segment.setTagTop("appsec.events." + eventName + ".track", true, sanitize); - segment.setTagTop(Tags.ASM_KEEP, true); - segment.setTagTop(Tags.PROPAGATED_APPSEC, true); - if (mode == SDK) { - segment.setTagTop("_dd.appsec.events." + eventName + ".sdk", true, sanitize); - } else { - segment.setTagTop("_dd.appsec.events." + eventName + ".auto.mode", mode.fullName(), sanitize); - } - if (metadata != null && !metadata.isEmpty()) { - segment.setTagTop("appsec.events." + eventName, metadata, sanitize); - } - } - - protected TraceSegment beforeEvent(final UserIdCollectionMode mode, final String userId) { - if (userId == null || userId.isEmpty()) { - if (mode == SDK) { - throw new IllegalArgumentException("UserId is null or empty"); - } else { - // increment metric and ignore the event - WafMetricCollector.get().missingUserId(); - return null; - } - } - return beforeEvent(mode); - } - - protected TraceSegment beforeEvent(final UserIdCollectionMode mode) { - if (!isEnabled(mode)) { - return null; - } - return tracer().getTraceSegment(); - } - - protected boolean isSdkCollectedUser(final TraceSegment segment) { - if (segment == null) { - return false; - } - final Object currentMode = segment.getTagTop(USER_COLLECTION_MODE_TAG); - return SDK.shortName().equals(currentMode); - } - - protected static String anonymize(String userId) { - if (userId == null) { - return null; - } - MessageDigest digest; - try { - // TODO avoid lookup a new instance every time - digest = MessageDigest.getInstance("SHA-256"); - } catch (NoSuchAlgorithmException e) { - if (!SHA_MISSING_REPORTED.getAndSet(true)) { - LOGGER.error( - SEND_TELEMETRY, - "Missing SHA-256 digest, user collection in 'anon' mode cannot continue", - e); - } - return null; - } - digest.update(userId.getBytes()); - byte[] hash = digest.digest(); - if (hash.length > HASH_SIZE_BYTES) { - byte[] temp = new byte[HASH_SIZE_BYTES]; - System.arraycopy(hash, 0, temp, 0, temp.length); - hash = temp; - } - return ANON_PREFIX + toHexString(hash); - } - - @SuppressWarnings("UnusedReturnValue") - private Flow callIGCallbackUserId( - final AgentSpan span, - final UserIdCollectionMode mode, - final String userId, - final EventType>> - event) { - final CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC); - final RequestContext requestContext = span.getRequestContext(); - if (cbp == null || requestContext == null) { - return Flow.ResultFlow.empty(); - } - final TriFunction> addrCallback = - cbp.getCallback(event); - if (addrCallback == null) { - return Flow.ResultFlow.empty(); - } - return addrCallback.apply(requestContext, mode, userId); - } - - // Extract this to allow for easier testing - protected AgentTracer.TracerAPI tracer() { - return AgentTracer.get(); - } -} diff --git a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy index 1d285fc7913..2938671c89c 100644 --- a/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy +++ b/dd-java-agent/appsec/src/test/groovy/com/datadog/appsec/event/data/KnownAddressesSpecification.groovy @@ -36,6 +36,7 @@ class KnownAddressesSpecification extends Specification { 'server.db.system', 'server.db.statement', 'usr.id', + 'usr.login', 'usr.session_id', 'server.business_logic.users.login.failure', 'server.business_logic.users.login.success', @@ -45,7 +46,7 @@ class KnownAddressesSpecification extends Specification { void 'number of known addresses is expected number'() { expect: - Address.instanceCount() == 35 + Address.instanceCount() == 36 KnownAddresses.WAF_CONTEXT_PROCESSOR.serial == Address.instanceCount() - 1 } } 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 2cc6cbc5170..3f992da906e 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 @@ -9,6 +9,7 @@ import com.datadog.appsec.event.data.KnownAddresses import com.datadog.appsec.report.AppSecEvent import com.datadog.appsec.report.AppSecEventWrapper import datadog.trace.api.UserIdCollectionMode +import datadog.trace.api.appsec.LoginEventCallback import datadog.trace.api.function.TriConsumer import datadog.trace.api.function.TriFunction import datadog.trace.api.gateway.BlockResponseFunction @@ -29,9 +30,17 @@ import java.util.function.BiFunction import java.util.function.Function import java.util.function.Supplier +import static datadog.trace.api.UserIdCollectionMode.ANONYMIZATION +import static datadog.trace.api.UserIdCollectionMode.DISABLED +import static datadog.trace.api.UserIdCollectionMode.IDENTIFICATION +import static datadog.trace.api.UserIdCollectionMode.SDK import static datadog.trace.api.gateway.Events.EVENTS class GatewayBridgeSpecification extends DDSpecification { + + private static final String USER_ID = 'user' + private static final String ANONYMIZED_USER_ID = 'anon_04f8996da763b7a969b1028ee3007569' + SubscriptionService ig = Mock() EventDispatcher eventDispatcher = Mock() AppSecRequestContext arCtx = new AppSecRequestContext() @@ -89,10 +98,8 @@ class GatewayBridgeSpecification extends DDSpecification { BiFunction> networkConnectionCB BiFunction> fileLoadedCB BiFunction> requestSessionCB - TriFunction> userIdCB - TriFunction> loginSuccessCB - TriFunction> loginFailureCB - + TriFunction> userCB + LoginEventCallback loginEventCB void setup() { callInitAndCaptureCBs() @@ -429,9 +436,9 @@ class GatewayBridgeSpecification extends DDSpecification { 1 * ig.registerCallback(EVENTS.networkConnection(), _) >> { networkConnectionCB = it[1]; null } 1 * ig.registerCallback(EVENTS.fileLoaded(), _) >> { fileLoadedCB = it[1]; null } 1 * ig.registerCallback(EVENTS.requestSession(), _) >> { requestSessionCB = it[1]; null } - 1 * ig.registerCallback(EVENTS.userId(), _) >> { userIdCB = it[1]; null } - 1 * ig.registerCallback(EVENTS.loginSuccess(), _) >> { loginSuccessCB = it[1]; null } - 1 * ig.registerCallback(EVENTS.loginFailure(), _) >> { loginFailureCB = it[1]; null } + 1 * ig.registerCallback(EVENTS.user(), _) >> { userCB = it[1]; null } + 1 * ig.registerCallback(EVENTS.loginEvent(), _) >> { loginEventCB = it[1]; null } + 0 * ig.registerCallback(_, _) bridge.init() @@ -1001,49 +1008,208 @@ class GatewayBridgeSpecification extends DDSpecification { gatewayContext.isTransient == false } - void 'process user ids'() { + void "test onUserEvent (#mode)"() { setup: - DataBundle bundle - GatewayContext gatewayContext + final expectedUser = mode == ANONYMIZATION ? ANONYMIZED_USER_ID : USER_ID eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo - final userId = 'admin' - final mode = UserIdCollectionMode.SDK - final TriFunction> callback = this[callbackField] when: - callback.apply(ctx, mode, userId) + userCB.apply(ctx, mode, USER_ID) then: - 1 * traceSegment.setTagTop('usr.id', userId) - 1 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', mode.shortName()) - 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> - { a, b, db, gw -> bundle = db; gatewayContext = gw; NoopFlow.INSTANCE } - bundle.get(KnownAddresses.USER_ID) == userId - if (businessAddress != null) { - assert bundle.hasAddress(businessAddress) + if (mode == DISABLED) { + 0 * _ + } else { + 1 * traceSegment.setTagTop('usr.id', expectedUser) + if (mode != SDK) { + 1 * traceSegment.setTagTop('_dd.appsec.usr.id', expectedUser) + } + 1 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', mode.fullName()) + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> + assert db.get(KnownAddresses.USER_ID) == expectedUser + assert !gw.isTransient + return NoopFlow.INSTANCE + } + } + + when: + userCB.apply(ctx, mode, USER_ID) + + then: 'no call to the WAF for duplicated calls' + 0 * eventDispatcher.publishDataEvent + + where: + mode << UserIdCollectionMode.values() + } + + void "test onSignup (#mode)"() { + setup: + final expectedUser = mode == ANONYMIZATION ? ANONYMIZED_USER_ID : USER_ID + final metadata = ['key1': 'value1', 'key2': 'value2'] + eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo + + when: + loginEventCB.apply(ctx, mode, 'signup', null, USER_ID, metadata) + + then: + if (mode == DISABLED) { + 0 * _ + } else { + 1 * traceSegment.setTagTop('appsec.events.users.signup.usr.login', expectedUser, true) + 1 * traceSegment.setTagTop('appsec.events.users.signup.usr.id', expectedUser, true) + if (mode != SDK) { + 1 * traceSegment.setTagTop('_dd.appsec.usr.login', expectedUser) + 1 * traceSegment.setTagTop('_dd.appsec.usr.id', expectedUser) + 1 * traceSegment.setTagTop('_dd.appsec.events.users.signup.auto.mode', mode.fullName(), true) + } else { + 1 * traceSegment.setTagTop('_dd.appsec.events.users.signup.sdk', true, true) + } + 1 * traceSegment.setTagTop('appsec.events.users.signup.track', true, true) + 1 * traceSegment.setTagTop('appsec.events.users.signup', ['key1': 'value1', 'key2': 'value2'], true) + 1 * traceSegment.setTagTop('asm.keep', true) + 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> + assert db.get(KnownAddresses.USER_ID) == expectedUser + assert db.get(KnownAddresses.USER_LOGIN) == expectedUser + assert !gw.isTransient + return NoopFlow.INSTANCE + } } - gatewayContext.isTransient == false where: - callbackField | businessAddress - 'userIdCB' | null - 'loginSuccessCB' | KnownAddresses.LOGIN_SUCCESS - 'loginFailureCB' | KnownAddresses.LOGIN_FAILURE + mode << UserIdCollectionMode.values() } - void 'ensure that the same user id is not published twice'() { + void "test onLoginSuccess (#mode)"() { setup: + final expectedUser = mode == ANONYMIZATION ? ANONYMIZED_USER_ID : USER_ID + final metadata = ['key1': 'value1', 'key2': 'value2'] eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo - final userId = 'admin' when: - userIdCB.apply(ctx, UserIdCollectionMode.IDENTIFICATION, userId) - userIdCB.apply(ctx, UserIdCollectionMode.SDK, userId) + loginEventCB.apply(ctx, mode, 'login.success', null, USER_ID, metadata) + + then: + if (mode == DISABLED) { + 0 * _ + } else { + 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', expectedUser, true) + 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.id', expectedUser, true) + if (mode != SDK) { + 1 * traceSegment.setTagTop('_dd.appsec.usr.login', expectedUser) + 1 * traceSegment.setTagTop('_dd.appsec.usr.id', expectedUser) + 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', mode.fullName(), true) + } else { + 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.sdk', true, true) + } + 1 * traceSegment.setTagTop('appsec.events.users.login.success.track', true, true) + 1 * traceSegment.setTagTop('appsec.events.users.login.success', ['key1': 'value1', 'key2': 'value2'], true) + 1 * traceSegment.setTagTop('asm.keep', true) + 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> + assert db.get(KnownAddresses.USER_ID) == expectedUser + assert db.get(KnownAddresses.USER_LOGIN) == expectedUser + assert db.get(KnownAddresses.LOGIN_SUCCESS) != null + assert !gw.isTransient + return NoopFlow.INSTANCE + } + } + + where: + mode << UserIdCollectionMode.values() + } + + void "test onLoginFailure (#mode)"() { + setup: + final expectedUser = mode == ANONYMIZATION ? ANONYMIZED_USER_ID : USER_ID + final metadata = ['key1': 'value1', 'key2': 'value2'] + eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo + + when: + loginEventCB.apply(ctx, mode, 'login.failure', false, USER_ID, metadata) + + then: + if (mode == DISABLED) { + 0 * _ + } else { + 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.login', expectedUser, true) + 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.id', expectedUser, true) + if (mode != SDK) { + 1 * traceSegment.setTagTop('_dd.appsec.usr.login', expectedUser) + 1 * traceSegment.setTagTop('_dd.appsec.usr.id', expectedUser) + 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.auto.mode', mode.fullName(), true) + } else { + 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.sdk', true, true) + } + 1 * traceSegment.setTagTop('appsec.events.users.login.failure.track', true, true) + 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.exists', false, true) + 1 * traceSegment.setTagTop('appsec.events.users.login.failure', ['key1': 'value1', 'key2': 'value2'], true) + 1 * traceSegment.setTagTop('asm.keep', true) + 1 * traceSegment.setTagTop('_dd.p.appsec', true) + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> { a, b, DataBundle db, GatewayContext gw -> + assert db.get(KnownAddresses.USER_ID) == expectedUser + assert db.get(KnownAddresses.USER_LOGIN) == expectedUser + assert db.get(KnownAddresses.LOGIN_FAILURE) != null + assert !gw.isTransient + return NoopFlow.INSTANCE + } + } + + where: + mode << UserIdCollectionMode.values() + } + + void "test onUserEvent (automated login events should not overwrite SDK)"() { + setup: + final firstUser = 'first-user' + final secondUser = 'second-user' + eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo + + when: + userCB.apply(ctx, SDK, firstUser) + + then: + 1 * traceSegment.setTagTop('usr.id', firstUser) + 1 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', SDK.fullName()) + 0 * traceSegment.setTagTop('_dd.appsec.usr.id', _) + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> NoopFlow.INSTANCE + + when: + userCB.apply(ctx, IDENTIFICATION, secondUser) + + then: 'SDK data remains untouched' + 0 * traceSegment.setTagTop('usr.id', _) + 0 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', _) + 1 * traceSegment.setTagTop('_dd.appsec.usr.id', secondUser) + 0 * eventDispatcher.publishDataEvent + } + + void "test onLoginSuccess (automated login events should not overwrite SDK)"() { + setup: + final firstUser = 'user1' + final secondUser = 'user2' + eventDispatcher.getDataSubscribers(_) >> nonEmptyDsInfo + + when: + loginEventCB.apply(ctx, SDK, 'login.success', null, firstUser, null) + + then: + 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', firstUser, true) + 1 * traceSegment.setTagTop('appsec.events.users.login.success.usr.id', firstUser, true) + 0 * traceSegment.setTagTop('_dd.appsec.usr.login', _) + 0 * traceSegment.setTagTop('_dd.appsec.usr.id', _) + 0 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', _, _) + 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) >> NoopFlow.INSTANCE + + when: + loginEventCB.apply(ctx, IDENTIFICATION, 'login.success', null, secondUser, null) then: - 2 * traceSegment.setTagTop('usr.id', userId) - 1 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', UserIdCollectionMode.IDENTIFICATION.shortName()) - 1 * traceSegment.setTagTop('_dd.appsec.user.collection_mode', UserIdCollectionMode.SDK.shortName()) - 1 * eventDispatcher.publishDataEvent(nonEmptyDsInfo, ctx.data, _ as DataBundle, _ as GatewayContext) + 0 * traceSegment.setTagTop('appsec.events.users.login.success.usr.login', _, _) + 0 * traceSegment.setTagTop('appsec.events.users.login.success.usr.id', _, _) + 1 * traceSegment.setTagTop('_dd.appsec.usr.login', secondUser) + 1 * traceSegment.setTagTop('_dd.appsec.usr.id', secondUser) + 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', IDENTIFICATION.fullName(), true) + 0 * eventDispatcher.publishDataEvent } } 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 f38200fff1b..d41f4bd0751 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 @@ -6,58 +6,57 @@ import datadog.appsec.api.blocking.BlockingException import datadog.trace.api.EventTracker import datadog.trace.api.GlobalTracer import datadog.trace.api.UserIdCollectionMode +import datadog.trace.api.appsec.AppSecEventTracker +import datadog.trace.api.appsec.LoginEventCallback import datadog.trace.api.function.TriFunction import datadog.trace.api.gateway.CallbackProvider import datadog.trace.api.gateway.Flow import datadog.trace.api.gateway.RequestContext import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.internal.TraceSegment -import datadog.trace.api.telemetry.WafMetricCollector import datadog.trace.bootstrap.ActiveSubsystems import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI import datadog.trace.test.util.DDSpecification import spock.lang.Shared +import static datadog.trace.api.UserIdCollectionMode.DISABLED import static datadog.trace.api.UserIdCollectionMode.SDK import static datadog.trace.api.gateway.Events.EVENTS class AppSecEventTrackerSpecification extends DDSpecification { private static final String USER_ID = 'user' - private static final String ANONYMIZED_USER_ID = 'anon_04f8996da763b7a969b1028ee3007569' @Shared private static boolean appSecActiveBefore = ActiveSubsystems.APPSEC_ACTIVE @Shared private static EventTracker eventTrackerBefore = GlobalTracer.getEventTracker() - private AppSecEventTrackerImpl tracker + private AppSecEventTracker tracker private TraceSegment traceSegment private TracerAPI tracer private AgentSpan span private CallbackProvider provider - private TriFunction> userCallback - private TriFunction> loginSuccessCallback - private TriFunction> loginFailureCallback + private TriFunction> user + private LoginEventCallback loginEvent void setup() { traceSegment = Mock(TraceSegment) span = Stub(AgentSpan) - userCallback = Mock(TriFunction) - loginSuccessCallback = Mock(TriFunction) - loginFailureCallback = Mock(TriFunction) + user = Mock(TriFunction) + loginEvent = Mock(LoginEventCallback) + provider = Stub(CallbackProvider) { - getCallback(EVENTS.userId()) >> userCallback - getCallback(EVENTS.loginSuccess()) >> loginSuccessCallback - getCallback(EVENTS.loginFailure()) >> loginFailureCallback + getCallback(EVENTS.user()) >> user + getCallback(EVENTS.loginEvent()) >> loginEvent } tracer = Stub(TracerAPI) { getTraceSegment() >> traceSegment activeSpan() >> span getCallbackProvider(RequestContextSlot.APPSEC) >> provider } - tracker = new AppSecEventTrackerImpl() { + tracker = new AppSecEventTracker() { @Override protected TracerAPI tracer() { return tracer @@ -73,54 +72,29 @@ class AppSecEventTrackerSpecification extends DDSpecification { } def 'test track login success event (SDK)'() { - given: - final sanitize = false // non custom event - when: GlobalTracer.getEventTracker().trackLoginSuccessEvent('user1', ['key1': 'value1', 'key2': 'value2']) then: - 1 * traceSegment.setTagTop('appsec.events.users.login.success.track', true, sanitize) - 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.sdk', true, sanitize) - 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) - 1 * traceSegment.setTagTop('appsec.events.users.login.success', ['key1': 'value1', 'key2': 'value2'], sanitize) - 1 * loginSuccessCallback.apply(_ as RequestContext, SDK, 'user1') >> NoopFlow.INSTANCE + 1 * loginEvent.apply(_ as RequestContext, SDK, 'login.success', null, 'user1', ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE 0 * _ } def 'test track login failure event (SDK)'() { - given: - final sanitize = false // non custom event - when: GlobalTracer.getEventTracker().trackLoginFailureEvent('user1', true, ['key1': 'value1', 'key2': 'value2']) then: - 1 * traceSegment.setTagTop('appsec.events.users.login.failure.track', true, sanitize) - 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.sdk', true, sanitize) - 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.id', 'user1', sanitize) - 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.exists', true, sanitize) - 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) - 1 * traceSegment.setTagTop('appsec.events.users.login.failure', ['key1': 'value1', 'key2': 'value2'], sanitize) - 1 * loginFailureCallback.apply(_ as RequestContext, SDK, 'user1') >> NoopFlow.INSTANCE + 1 * loginEvent.apply(_ as RequestContext, SDK, 'login.failure', true, 'user1', ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE 0 * _ } def 'test track custom event (SDK)'() { - given: - final sanitize = true // custom event - when: GlobalTracer.getEventTracker().trackCustomEvent('myevent', ['key1': 'value1', 'key2': 'value2']) then: - 1 * traceSegment.setTagTop('appsec.events.myevent.track', true, sanitize) - 1 * traceSegment.setTagTop('_dd.appsec.events.myevent.sdk', true, sanitize) - 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) - 1 * traceSegment.setTagTop('appsec.events.myevent', ['key1': 'value1', 'key2': 'value2'], sanitize) + 1 * loginEvent.apply(_ as RequestContext, SDK, 'myevent', null, null, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE 0 * _ } @@ -163,114 +137,73 @@ class AppSecEventTrackerSpecification extends DDSpecification { } def "test onSignup (#mode)"() { - setup: - final sanitize = false // non custom event - final collectionMode = UserIdCollectionMode.fromString(mode, null) - when: - tracker.onSignupEvent(collectionMode, USER_ID, ['key1': 'value1', 'key2': 'value2']) + tracker.onSignupEvent(mode, USER_ID, ['key1': 'value1', 'key2': 'value2']) then: - 1 * traceSegment.setTagTop('_dd.appsec.events.users.signup.auto.mode', modeTag, sanitize) - 1 * traceSegment.getTagTop('_dd.appsec.user.collection_mode') >> null - 1 * traceSegment.setTagTop('appsec.events.users.signup.track', true, sanitize) - 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) - 1 * traceSegment.setTagTop('appsec.events.users.signup', ['key1': 'value1', 'key2': 'value2'], sanitize) - 1 * userCallback.apply(_ as RequestContext, collectionMode, expectedUserId) >> NoopFlow.INSTANCE + if (mode != DISABLED) { + 1 * loginEvent.apply(_ as RequestContext, mode, 'signup', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + } 0 * _ where: - mode | modeTag | expectedUserId - 'anon' | 'anonymization' | ANONYMIZED_USER_ID - 'ident' | 'identification' | USER_ID + mode << UserIdCollectionMode.values() } def "test onLoginSuccess (#mode)"() { - setup: - final sanitize = false // non custom event - final collectionMode = UserIdCollectionMode.fromString(mode, null) - when: - tracker.onLoginSuccessEvent(collectionMode, USER_ID, ['key1': 'value1', 'key2': 'value2']) + tracker.onLoginSuccessEvent(mode, USER_ID, ['key1': 'value1', 'key2': 'value2']) then: - 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.success.auto.mode', modeTag, sanitize) - 1 * traceSegment.getTagTop('_dd.appsec.user.collection_mode') >> null - 1 * traceSegment.setTagTop('appsec.events.users.login.success.track', true, sanitize) - 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) - 1 * traceSegment.setTagTop('appsec.events.users.login.success', ['key1': 'value1', 'key2': 'value2'], sanitize) - 1 * loginSuccessCallback.apply(_ as RequestContext, collectionMode, expectedUserId) >> NoopFlow.INSTANCE + if (mode != DISABLED) { + 1 * loginEvent.apply(_ as RequestContext, mode, 'login.success', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + } 0 * _ where: - mode | modeTag | expectedUserId - 'anon' | 'anonymization' | ANONYMIZED_USER_ID - 'ident' | 'identification' | USER_ID + mode << UserIdCollectionMode.values() } - def "test onLoginFailed #description (#mode)"() { - setup: - final sanitize = false // non custom event - final collectionMode = UserIdCollectionMode.fromString(mode, null) - + def "test onLoginFailed (#mode)"() { when: - tracker.onLoginFailureEvent(collectionMode, USER_ID, null, ['key1': 'value1', 'key2': 'value2']) + tracker.onLoginFailureEvent(mode, USER_ID, null, ['key1': 'value1', 'key2': 'value2']) then: - 1 * traceSegment.setTagTop('_dd.appsec.events.users.login.failure.auto.mode', modeTag, sanitize) - 1 * traceSegment.getTagTop('_dd.appsec.user.collection_mode') >> null - 1 * traceSegment.setTagTop('appsec.events.users.login.failure.track', true, sanitize) - 1 * traceSegment.setTagTop('asm.keep', true) - 1 * traceSegment.setTagTop('_dd.p.appsec', true) - 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.id', expectedUserId, sanitize) - 1 * traceSegment.setTagTop('appsec.events.users.login.failure', ['key1': 'value1', 'key2': 'value2'], sanitize) - 1 * loginFailureCallback.apply(_ as RequestContext, collectionMode, expectedUserId) >> NoopFlow.INSTANCE + if (mode != DISABLED) { + 1 * loginEvent.apply(_ as RequestContext, mode, 'login.failure', null, USER_ID, ['key1': 'value1', 'key2': 'value2']) >> NoopFlow.INSTANCE + } 0 * _ where: - mode | modeTag | description | expectedUserId - 'anon' | 'anonymization' | 'with existing user' | ANONYMIZED_USER_ID - 'anon' | 'anonymization' | 'user doesn\'t exist' | ANONYMIZED_USER_ID - 'ident' | 'identification' | 'with existing user' | USER_ID - 'ident' | 'identification' | 'user doesn\'t exist' | USER_ID + mode << UserIdCollectionMode.values() } def "test onUserEvent (#mode)"() { - setup: - final collectionMode = UserIdCollectionMode.fromString(mode, null) - when: - tracker.onUserEvent(collectionMode, USER_ID) + tracker.onUserEvent(mode, USER_ID) then: - 1 * traceSegment.getTagTop('_dd.appsec.user.collection_mode') >> null - 1 * userCallback.apply(_ as RequestContext, collectionMode, expectedUserId) >> NoopFlow.INSTANCE + if (mode != DISABLED) { + 1 * user.apply(_ as RequestContext, mode, USER_ID) >> NoopFlow.INSTANCE + } 0 * _ where: - mode | modeTag | expectedUserId - 'anon' | 'anonymization' | ANONYMIZED_USER_ID - 'ident' | 'identification' | USER_ID + mode << UserIdCollectionMode.values() } - def "test onUserNotFound (#mode)"() { - setup: - final collectionMode = UserIdCollectionMode.fromString(mode, null) - when: - tracker.onUserNotFound(collectionMode) + tracker.onUserNotFound(mode) then: - 1 * traceSegment.setTagTop("appsec.events.users.login.failure.usr.exists", false) + if (mode != DISABLED) { + 1 * traceSegment.setTagTop('appsec.events.users.login.failure.usr.exists', false) + } 0 * _ where: - mode | modeTag - 'anon' | 'ANONYMIZATION' - 'ident' | 'IDENTIFIED' + mode << UserIdCollectionMode.values() } def "test isEnabled (appsec = #appsec, tracking = #trackingMode, collection = #collectionMode)"() { @@ -323,66 +256,10 @@ class AppSecEventTrackerSpecification extends DDSpecification { true | 'anon' | 'disabled' | true } - void 'test missing user id callback'() { - setup: - final collector = WafMetricCollector.get() - collector.prepareMetrics() - collector.drain() - final collectionMode = UserIdCollectionMode.fromString('ident', null) - - when: - tracker.onLoginSuccessEvent(collectionMode, null, [:]) - - then: - collector.prepareMetrics() - final metrics = collector.drain() - metrics.size() == 1 - final metric = metrics.first() - metric.namespace == 'appsec' - metric.type == 'count' - metric.metricName == 'instrum.user_auth.missing_user_id' - metric.value == 1 - 0 * _ - } - - void 'test user id anonymization of #userId'() { - when: - final anonymized = AppSecEventTrackerImpl.anonymize(userId) - - then: - anonymized == expected - - where: - userId | expected - null | null - 'zouzou@sansgluten.com' | 'anon_0c76692372ebf01a7da6e9570fb7d0a1' - } - - void 'test that SDK produced events are not overridden by auto login events'() { - setup: - traceSegment.getTagTop('_dd.appsec.user.collection_mode') >> 'sdk' - - when: - tracker.onLoginSuccessEvent(mode, USER_ID, ['key1': 'value1', 'key2': 'value2']) - - then: - if (mode == SDK) { - // the SDK can override itself - 1 * loginSuccessCallback.apply(_, SDK, USER_ID) >> NoopFlow.INSTANCE - } else { - // auto login events should not modify SDK produced ones - 0 * loginSuccessCallback.apply(_, _, _, _) - } - - where: - mode << [SDK, UserIdCollectionMode.IDENTIFICATION] - } - void 'test blocking on a userId'() { setup: final action = new Flow.Action.RequestBlockingAction(403, BlockingContentType.AUTO) - loginSuccessCallback.apply(_ as RequestContext, SDK, USER_ID) >> new ActionFlow(action: action) - traceSegment.getTagTop('_dd.appsec.user.collection_mode') >> 'sdk' + loginEvent.apply(_ as RequestContext, SDK, '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 25c20548ec0..4ece8a890ba 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 @@ -5,6 +5,9 @@ import datadog.trace.api.UserIdCollectionMode; import datadog.trace.api.appsec.AppSecEventTracker; +import datadog.trace.api.telemetry.LoginEvent; +import datadog.trace.api.telemetry.LoginFramework; +import datadog.trace.api.telemetry.WafMetricCollector; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -35,7 +38,6 @@ public void onUserNotFound() { if (tracker == null) { return; } - tracker.onUserNotFound(UserIdCollectionMode.get()); } @@ -52,7 +54,10 @@ public void onSignup(UserDetails user, Throwable throwable) { } UserIdCollectionMode mode = UserIdCollectionMode.get(); - String userId = user.getUsername(); + String username = user.getUsername(); + if (missingUsername(username, LoginEvent.SIGN_UP)) { + return; + } Map metadata = null; if (mode == IDENTIFICATION) { metadata = new HashMap<>(); @@ -62,7 +67,7 @@ public void onSignup(UserDetails user, Throwable throwable) { user.getAuthorities().stream().map(Object::toString).collect(Collectors.joining(","))); } - tracker.onSignupEvent(mode, userId, metadata); + tracker.onSignupEvent(mode, username, metadata); } public void onLogin(Authentication authentication, Throwable throwable, Authentication result) { @@ -80,8 +85,11 @@ public void onLogin(Authentication authentication, Throwable throwable, Authenti } UserIdCollectionMode mode = UserIdCollectionMode.get(); - String userId = result != null ? result.getName() : authentication.getName(); + String username = result != null ? result.getName() : authentication.getName(); if (throwable == null && result != null && result.isAuthenticated()) { + if (missingUsername(username, LoginEvent.LOGIN_SUCCESS)) { + return; + } Map metadata = null; Object principal = result.getPrincipal(); if (principal instanceof User) { @@ -95,8 +103,11 @@ public void onLogin(Authentication authentication, Throwable throwable, Authenti metadata.put("accountNonLocked", String.valueOf(user.isAccountNonLocked())); metadata.put("credentialsNonExpired", String.valueOf(user.isCredentialsNonExpired())); } - tracker.onLoginSuccessEvent(mode, userId, metadata); + tracker.onLoginSuccessEvent(mode, username, metadata); } else if (throwable != null) { + if (missingUsername(username, LoginEvent.LOGIN_FAILURE)) { + return; + } // On bad password, throwable would be // org.springframework.security.authentication.BadCredentialsException, // on user not found, throwable can be BadCredentials or @@ -107,7 +118,7 @@ public void onLogin(Authentication authentication, Throwable throwable, Authenti // 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. - tracker.onLoginFailureEvent(mode, userId, null, null); + tracker.onLoginFailureEvent(mode, username, null, null); } } @@ -126,8 +137,8 @@ public void onUser(final Authentication authentication) { } UserIdCollectionMode mode = UserIdCollectionMode.get(); - String userId = authentication.getName(); - tracker.onUserEvent(mode, userId); + String username = authentication.getName(); + tracker.onUserEvent(mode, username); } private static boolean shouldSkipAuthentication(final Authentication authentication) { @@ -151,4 +162,12 @@ private static String findRootAuthentication(Class authentication) { } return Authentication.class.getName(); // set this a default for really custom impls } + + private static boolean missingUsername(final String username, final LoginEvent event) { + if (username == null || username.isEmpty()) { + WafMetricCollector.get().missingUserLogin(LoginFramework.SPRING_SECURITY, event); + return true; + } + return false; + } } diff --git a/dd-java-agent/instrumentation/spring-security-5/src/test/groovy/datadog/trace/instrumentation/springsecurity5/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-security-5/src/test/groovy/datadog/trace/instrumentation/springsecurity5/SpringBootBasedTest.groovy index 4fcaa867bed..d33953c8cdf 100644 --- a/dd-java-agent/instrumentation/spring-security-5/src/test/groovy/datadog/trace/instrumentation/springsecurity5/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-security-5/src/test/groovy/datadog/trace/instrumentation/springsecurity5/SpringBootBasedTest.groovy @@ -151,11 +151,14 @@ class SpringBootBasedTest extends AppSecHttpServerTest metadata) { + if (userId == null || userId.isEmpty()) { + throw new IllegalArgumentException("UserId is null or empty"); + } onLoginSuccessEvent(SDK, userId, metadata); } @Override public final void trackLoginFailureEvent( String userId, boolean exists, Map metadata) { + if (userId == null || userId.isEmpty()) { + throw new IllegalArgumentException("UserId is null or empty"); + } onLoginFailureEvent(SDK, userId, exists, metadata); } @Override public final void trackCustomEvent(String eventName, Map metadata) { + if (eventName == null || eventName.isEmpty()) { + throw new IllegalArgumentException("EventName is null or empty"); + } onCustomEvent(SDK, eventName, metadata); } - public abstract void onUserNotFound(UserIdCollectionMode mode); + public void onUserNotFound(final UserIdCollectionMode mode) { + if (!isEnabled(mode)) { + return; + } + final AgentTracer.TracerAPI tracer = tracer(); + if (tracer == null) { + return; + } + final TraceSegment segment = tracer.getTraceSegment(); + if (segment == null) { + return; + } + segment.setTagTop("appsec.events.users.login.failure.usr.exists", false); + } + + public void onUserEvent(final UserIdCollectionMode mode, final String userId) { + if (!isEnabled(mode)) { + return; + } + dispatch(EVENTS.user(), (ctx, cb) -> cb.apply(ctx, mode, userId)); + } + + public void onSignupEvent( + final UserIdCollectionMode mode, final String userId, final Map metadata) { + if (!isEnabled(mode)) { + return; + } + dispatch( + EVENTS.loginEvent(), + (ctx, callback) -> callback.apply(ctx, mode, SIGN_UP.getSpanTag(), null, userId, metadata)); + } + + public void onLoginSuccessEvent( + final UserIdCollectionMode mode, final String userId, final Map metadata) { + if (!isEnabled(mode)) { + return; + } + dispatch( + EVENTS.loginEvent(), + (ctx, cb) -> cb.apply(ctx, mode, LOGIN_SUCCESS.getSpanTag(), null, userId, metadata)); + } - public abstract void onSignupEvent( - UserIdCollectionMode mode, String userId, Map metadata); + public void onLoginFailureEvent( + final UserIdCollectionMode mode, + final String userId, + final Boolean exists, + final Map metadata) { + if (!isEnabled(mode)) { + return; + } + dispatch( + EVENTS.loginEvent(), + (ctx, cb) -> cb.apply(ctx, mode, LOGIN_FAILURE.getSpanTag(), exists, userId, metadata)); + } - public abstract void onLoginSuccessEvent( - UserIdCollectionMode mode, String userId, Map metadata); + public void onCustomEvent( + final UserIdCollectionMode mode, final String eventName, final Map metadata) { + if (!isEnabled(mode)) { + return; + } + dispatch( + EVENTS.loginEvent(), (ctx, cb) -> cb.apply(ctx, mode, eventName, null, null, metadata)); + } - public abstract void onLoginFailureEvent( - UserIdCollectionMode mode, String userId, Boolean exists, Map metadata); + private void dispatch( + final EventType event, final BiFunction> consumer) { + final AgentTracer.TracerAPI tracer = tracer(); + if (tracer == null) { + return; + } + final CallbackProvider cbp = tracer.getCallbackProvider(RequestContextSlot.APPSEC); + if (cbp == null) { + return; + } + final AgentSpan span = tracer.activeSpan(); + if (span == null) { + return; + } + final RequestContext ctx = span.getRequestContext(); + if (ctx == null) { + return; + } + final T callback = cbp.getCallback(event); + final Flow flow = consumer.apply(ctx, callback); + if (flow == null) { + return; + } + final Flow.Action action = flow.getAction(); + if (action instanceof Flow.Action.RequestBlockingAction) { + final BlockResponseFunction brf = ctx.getBlockResponseFunction(); + if (brf != null) { + Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action; + brf.tryCommitBlockingResponse( + ctx.getTraceSegment(), + rba.getStatusCode(), + rba.getBlockingContentType(), + rba.getExtraHeaders()); + } + throw new BlockingException("Blocked request (for user)"); + } + } - public abstract void onUserEvent(UserIdCollectionMode mode, String userId); + protected boolean isEnabled(final UserIdCollectionMode mode) { + return ActiveSubsystems.APPSEC_ACTIVE && mode != DISABLED; + } - public abstract void onCustomEvent( - UserIdCollectionMode mode, String eventName, Map metadata); + // Extract this to allow for easier testing + protected AgentTracer.TracerAPI tracer() { + return AgentTracer.get(); + } } diff --git a/internal-api/src/main/java/datadog/trace/api/appsec/LoginEventCallback.java b/internal-api/src/main/java/datadog/trace/api/appsec/LoginEventCallback.java new file mode 100644 index 00000000000..11e12103904 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/appsec/LoginEventCallback.java @@ -0,0 +1,17 @@ +package datadog.trace.api.appsec; + +import datadog.trace.api.UserIdCollectionMode; +import datadog.trace.api.gateway.Flow; +import datadog.trace.api.gateway.RequestContext; +import java.util.Map; + +public interface LoginEventCallback { + + Flow apply( + RequestContext context, + UserIdCollectionMode mode, + String eventName, + Boolean exists, + String user, + Map metadata); +} diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java index df111af28ee..305bd97b772 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/Events.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/Events.java @@ -1,6 +1,7 @@ package datadog.trace.api.gateway; import datadog.trace.api.UserIdCollectionMode; +import datadog.trace.api.appsec.LoginEventCallback; import datadog.trace.api.function.TriConsumer; import datadog.trace.api.function.TriFunction; import datadog.trace.api.http.StoredBodySupplier; @@ -276,36 +277,21 @@ public EventType>> requestSession( @SuppressWarnings("rawtypes") private static final EventType USER = new ET<>("user", USER_ID); - /** A user with the mode used for the collection */ + /** Triggered in every authenticated request to the application */ @SuppressWarnings("unchecked") - public EventType>> userId() { + public EventType>> user() { return (EventType>>) USER; } - static final int LOGIN_SUCCESS_ID = 23; + static final int LOGIN_EVENT_ID = 23; @SuppressWarnings("rawtypes") - private static final EventType LOGIN_SUCCESS = new ET<>("login.success", LOGIN_SUCCESS_ID); + private static final EventType LOGIN_EVENT = new ET<>("login.event", LOGIN_EVENT_ID); - /** The logged user with the mode used for the collection */ + /** Triggered when the SDK sends a custom event */ @SuppressWarnings("unchecked") - public EventType>> - loginSuccess() { - return (EventType>>) - LOGIN_SUCCESS; - } - - static final int LOGIN_FAILURE_ID = 24; - - @SuppressWarnings("rawtypes") - private static final EventType LOGIN_FAILURE = new ET<>("login.failure", LOGIN_FAILURE_ID); - - /** The user tha failed to log in with the mode used for the collection */ - @SuppressWarnings("unchecked") - public EventType>> - loginFailure() { - return (EventType>>) - LOGIN_FAILURE; + public EventType loginEvent() { + return (EventType) LOGIN_EVENT; } static final int MAX_EVENTS = nextId.get(); diff --git a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java index 45d734f0d11..0833b02634c 100644 --- a/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java +++ b/internal-api/src/main/java/datadog/trace/api/gateway/InstrumentationGateway.java @@ -6,8 +6,7 @@ import static datadog.trace.api.gateway.Events.GRAPHQL_SERVER_REQUEST_MESSAGE_ID; import static datadog.trace.api.gateway.Events.GRPC_SERVER_METHOD_ID; import static datadog.trace.api.gateway.Events.GRPC_SERVER_REQUEST_MESSAGE_ID; -import static datadog.trace.api.gateway.Events.LOGIN_FAILURE_ID; -import static datadog.trace.api.gateway.Events.LOGIN_SUCCESS_ID; +import static datadog.trace.api.gateway.Events.LOGIN_EVENT_ID; import static datadog.trace.api.gateway.Events.MAX_EVENTS; import static datadog.trace.api.gateway.Events.NETWORK_CONNECTION_ID; import static datadog.trace.api.gateway.Events.REQUEST_BODY_CONVERTED_ID; @@ -28,6 +27,7 @@ import static datadog.trace.api.gateway.Events.USER_ID; import datadog.trace.api.UserIdCollectionMode; +import datadog.trace.api.appsec.LoginEventCallback; import datadog.trace.api.function.TriConsumer; import datadog.trace.api.function.TriFunction; import datadog.trace.api.http.StoredBodySupplier; @@ -385,16 +385,36 @@ public void accept(RequestContext ctx, String arg) { } }; case USER_ID: - case LOGIN_SUCCESS_ID: - case LOGIN_FAILURE_ID: return (C) new TriFunction>() { @Override - public Flow apply(RequestContext ctx, UserIdCollectionMode mode, String arg) { + public Flow apply( + RequestContext ctx, UserIdCollectionMode mode, String userId) { try { return ((TriFunction>) callback) - .apply(ctx, mode, arg); + .apply(ctx, mode, userId); + } catch (Throwable t) { + log.warn("Callback for {} threw.", eventType, t); + return Flow.ResultFlow.empty(); + } + } + }; + case LOGIN_EVENT_ID: + return (C) + new LoginEventCallback() { + + @Override + public Flow apply( + RequestContext ctx, + UserIdCollectionMode mode, + String eventName, + Boolean exists, + String user, + Map metadata) { + try { + return ((LoginEventCallback) callback) + .apply(ctx, mode, eventName, exists, user, metadata); } catch (Throwable t) { log.warn("Callback for {} threw.", eventType, t); return Flow.ResultFlow.empty(); 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 new file mode 100644 index 00000000000..fa2b9ab3f1d --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/LoginEvent.java @@ -0,0 +1,33 @@ +package datadog.trace.api.telemetry; + +public enum LoginEvent { + LOGIN_SUCCESS("login.success", "login_success"), + LOGIN_FAILURE("login.failure", "login_failure"), + SIGN_UP("signup"); + + private static final int numValues = LoginEvent.values().length; + + private final String spanTag; + private final String telemetryTag; + + LoginEvent(final String 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 static int getNumValues() { + return numValues; + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/LoginFramework.java b/internal-api/src/main/java/datadog/trace/api/telemetry/LoginFramework.java new file mode 100644 index 00000000000..c0b00b47d3f --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/LoginFramework.java @@ -0,0 +1,21 @@ +package datadog.trace.api.telemetry; + +public enum LoginFramework { + SPRING_SECURITY("spring_security"); + + private static final int numValues = LoginFramework.values().length; + + private final String tag; + + LoginFramework(final String tag) { + this.tag = tag; + } + + public String getTag() { + return tag; + } + + public static int getNumValues() { + return numValues; + } +} 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 3f093106d28..a604f2dbc43 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 @@ -40,7 +40,8 @@ private WafMetricCollector() { new AtomicLongArray(RuleType.getNumValues()); private static final AtomicLongArray raspTimeoutCounter = new AtomicLongArray(RuleType.getNumValues()); - private static final AtomicRequestCounter missingUserIdCounter = new AtomicRequestCounter(); + private static final AtomicLongArray missingUserLoginQueue = + new AtomicLongArray(LoginFramework.getNumValues() * LoginEvent.getNumValues()); /** WAF version that will be initialized with wafInit and reused for all metrics. */ private static String wafVersion = ""; @@ -99,8 +100,9 @@ public void raspTimeout(final RuleType ruleType) { raspTimeoutCounter.incrementAndGet(ruleType.ordinal()); } - public void missingUserId() { - missingUserIdCounter.increment(); + public void missingUserLogin(final LoginFramework framework, final LoginEvent eventType) { + missingUserLoginQueue.incrementAndGet( + framework.ordinal() * LoginEvent.getNumValues() + eventType.ordinal()); } @Override @@ -207,10 +209,16 @@ public void prepareMetrics() { } // Missing user id - long missingUserId = missingUserIdCounter.getAndReset(); - if (missingUserId > 0) { - if (!rawMetricsQueue.offer(new MissingUserIdMetric(missingUserId))) { - return; + for (LoginFramework framework : LoginFramework.values()) { + for (LoginEvent event : LoginEvent.values()) { + final int ordinal = framework.ordinal() * LoginEvent.getNumValues() + event.ordinal(); + long counter = missingUserLoginQueue.getAndSet(ordinal, 0); + if (counter > 0) { + if (!rawMetricsQueue.offer( + new MissingUserLoginMetric(counter, framework.getTag(), event.getTelemetryTag()))) { + return; + } + } } } } @@ -241,10 +249,14 @@ public WafUpdatesRawMetric( } } - public static class MissingUserIdMetric extends WafMetric { + public static class MissingUserLoginMetric extends WafMetric { - public MissingUserIdMetric(long counter) { - super("instrum.user_auth.missing_user_id", counter); + public MissingUserLoginMetric(long counter, String framework, String type) { + super( + "instrum.user_auth.missing_user_login", + counter, + "framework:" + framework, + "event_type:" + type); } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/UserIdCollectionModeTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/UserIdCollectionModeTest.groovy index 01c917fcbbd..910936be9c1 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/UserIdCollectionModeTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/UserIdCollectionModeTest.groovy @@ -58,30 +58,27 @@ class UserIdCollectionModeTest extends DDSpecification { } - void 'test user id collection mode "#mode" with remote config "#rc"'() { + void 'test user id collection mode "#local" with remote config "#remote"'() { setup: - if (mode == null) { + if (local == null) { removeSysConfig(AppSecConfig.APPSEC_AUTO_USER_INSTRUMENTATION_MODE) } else { - injectSysConfig(AppSecConfig.APPSEC_AUTO_USER_INSTRUMENTATION_MODE, mode) + injectSysConfig(AppSecConfig.APPSEC_AUTO_USER_INSTRUMENTATION_MODE, local) } when: - UserIdCollectionMode.fromRemoteConfig(rc) + UserIdCollectionMode.fromRemoteConfig(remote) then: UserIdCollectionMode.get() == expected where: - mode | rc | expected - // by default + local | remote | expected null | null | IDENTIFICATION - // rc wins 'ident' | 'yolo' | DISABLED 'ident' | 'disabled' | DISABLED 'anon' | 'ident' | IDENTIFICATION 'ident' | 'anon' | ANONYMIZATION - // rc not present 'yolo' | null | DISABLED 'disabled' | null | DISABLED 'ident' | null | IDENTIFICATION 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 7baafe02e8c..7972f7b448e 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 @@ -2,6 +2,10 @@ package datadog.trace.api.telemetry import datadog.trace.test.util.DDSpecification +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + class WafMetricCollectorTest extends DDSpecification { def "no metrics - drain empty list"() { @@ -185,21 +189,69 @@ class WafMetricCollectorTest extends DDSpecification { collector.drain().size() == limit } - void 'test missing user id event metric'() { + void 'test missing user login event metric'() { given: def collector = WafMetricCollector.get() + final loginSuccessCount = 6 + final loginFailureCount = 3 + final signupCount = 2 + final latch = new CountDownLatch(1) + final executors = Executors.newFixedThreadPool(4) + final action = { LoginFramework framework, LoginEvent event -> + latch.await() + collector.missingUserLogin(framework, event) + } when: - collector.missingUserId() - collector.prepareMetrics() + (1..loginSuccessCount).each { + executors.submit { + action.call(LoginFramework.SPRING_SECURITY, LoginEvent.LOGIN_SUCCESS) + } + } + (1..loginFailureCount).each { + executors.submit { + action.call(LoginFramework.SPRING_SECURITY, LoginEvent.LOGIN_FAILURE) + } + } + (1..signupCount).each { + executors.submit { + action.call(LoginFramework.SPRING_SECURITY, LoginEvent.SIGN_UP) + } + } + + latch.countDown() + executors.shutdown() + final finished = executors.awaitTermination(5, TimeUnit.SECONDS) then: - noExceptionThrown() - def metrics = collector.drain() - def metric = metrics.find { it.metricName == 'instrum.user_auth.missing_user_id'} - metric.namespace == 'appsec' - metric.type == 'count' - metric.value == 1 - metric.tags == [] + finished + collector.prepareMetrics() + final drained = collector.drain() + final metrics = drained.findAll { + it.metricName == 'instrum.user_auth.missing_user_login' + } + metrics.size() == 3 + metrics.forEach { metric -> + assert metric.namespace == 'appsec' + assert metric.type == 'count' + final tags = metric.tags.collectEntries { + final parts = it.split(":") + return [(parts[0]): parts[1]] + } + assert tags["framework"] == LoginFramework.SPRING_SECURITY.getTag() + switch (tags["event_type"]) { + case LoginEvent.LOGIN_SUCCESS.getTelemetryTag(): + assert metric.value == loginSuccessCount + break + case LoginEvent.LOGIN_FAILURE.getTelemetryTag(): + assert metric.value == loginFailureCount + break + case LoginEvent.SIGN_UP.getTelemetryTag(): + assert metric.value == signupCount + break + default: + throw new IllegalArgumentException("Invalid event_type " + tags["event_type"]) + } + } } } diff --git a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java index ea441fd372a..0ad59ecd56d 100644 --- a/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java +++ b/internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import datadog.appsec.api.blocking.BlockingContentType; +import datadog.trace.api.appsec.LoginEventCallback; import datadog.trace.api.function.TriConsumer; import datadog.trace.api.function.TriFunction; import datadog.trace.api.http.StoredBodySupplier; @@ -213,14 +214,12 @@ public void testNormalCalls() { cbp.getCallback(events.networkConnection()).apply(null, null); ss.registerCallback(events.fileLoaded(), callback); cbp.getCallback(events.fileLoaded()).apply(null, null); - ss.registerCallback(events.userId(), callback); - cbp.getCallback(events.userId()).apply(null, null, null); + ss.registerCallback(events.user(), callback); + cbp.getCallback(events.user()).apply(null, null, null); + ss.registerCallback(events.loginEvent(), callback.asLoginEventCallback()); + cbp.getCallback(events.loginEvent()).apply(null, null, null, null, null, null); ss.registerCallback(events.requestSession(), callback); cbp.getCallback(events.requestSession()).apply(null, null); - ss.registerCallback(events.loginSuccess(), callback); - cbp.getCallback(events.loginSuccess()).apply(null, null, null); - ss.registerCallback(events.loginFailure(), callback); - cbp.getCallback(events.loginFailure()).apply(null, null, null); assertThat(callback.count).isEqualTo(Events.MAX_EVENTS); } @@ -281,14 +280,12 @@ public void testThrowableBlocking() { cbp.getCallback(events.networkConnection()).apply(null, null); ss.registerCallback(events.fileLoaded(), throwback); cbp.getCallback(events.fileLoaded()).apply(null, null); - ss.registerCallback(events.userId(), throwback); - cbp.getCallback(events.userId()).apply(null, null, null); + ss.registerCallback(events.user(), throwback); + cbp.getCallback(events.user()).apply(null, null, null); + ss.registerCallback(events.loginEvent(), throwback.asLoginEventCallback()); + cbp.getCallback(events.loginEvent()).apply(null, null, null, null, null, null); ss.registerCallback(events.requestSession(), throwback); cbp.getCallback(events.requestSession()).apply(null, null); - ss.registerCallback(events.loginSuccess(), throwback); - cbp.getCallback(events.loginSuccess()).apply(null, null, null); - ss.registerCallback(events.loginFailure(), throwback); - cbp.getCallback(events.loginFailure()).apply(null, null, null); assertThat(throwback.count).isEqualTo(Events.MAX_EVENTS); } @@ -502,6 +499,13 @@ public Flow apply(RequestContext requestContext, T t, T t2) { count++; return flow; } + + public LoginEventCallback asLoginEventCallback() { + return (context, mode, eventName, exists, user, metadata) -> { + count++; + return flow; + }; + } } private static class Throwback @@ -571,6 +575,13 @@ public BiFunction> asRequestBodyD }; } + public LoginEventCallback asLoginEventCallback() { + return (context, mode, eventName, exists, user, metadata) -> { + count++; + throw new IllegalArgumentException(); + }; + } + public int getCount() { return count; }