From 5a0ee2e7ede8c07e7836afa1cfbc3f7c243bec2d Mon Sep 17 00:00:00 2001 From: simon-id Date: Thu, 26 Dec 2024 17:11:46 +0100 Subject: [PATCH] push stuff --- packages/dd-trace/src/appsec/channels.js | 2 +- packages/dd-trace/src/appsec/index.js | 31 +++++++------ packages/dd-trace/src/appsec/user_tracking.js | 43 +++++++++++-------- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/packages/dd-trace/src/appsec/channels.js b/packages/dd-trace/src/appsec/channels.js index aa701f975dc..1fe8d632041 100644 --- a/packages/dd-trace/src/appsec/channels.js +++ b/packages/dd-trace/src/appsec/channels.js @@ -13,8 +13,8 @@ module.exports = { apolloServerCoreChannel: dc.tracingChannel('datadog:apollo-server-core:request'), incomingHttpRequestStart: dc.channel('dd-trace:incomingHttpRequestStart'), incomingHttpRequestEnd: dc.channel('dd-trace:incomingHttpRequestEnd'), - passportUser: dc.channel('datadog:passport:deserializeUser:finish'), passportVerify: dc.channel('datadog:passport:verify:finish'), + passportUser: dc.channel('datadog:passport:deserializeUser:finish'), queryParser: dc.channel('datadog:query:read:finish'), setCookieChannel: dc.channel('datadog:iast:set-cookie'), nextBodyParsed: dc.channel('apm:next:body-parsed'), diff --git a/packages/dd-trace/src/appsec/index.js b/packages/dd-trace/src/appsec/index.js index 736be7bb20b..bcbf0de138e 100644 --- a/packages/dd-trace/src/appsec/index.js +++ b/packages/dd-trace/src/appsec/index.js @@ -9,8 +9,8 @@ const { multerParser, incomingHttpRequestStart, incomingHttpRequestEnd, - passportUser, passportVerify, + passportUser, queryParser, nextBodyParsed, nextQueryParsed, @@ -67,8 +67,8 @@ function enable (_config) { cookieParser.subscribe(onRequestCookieParser) incomingHttpRequestStart.subscribe(incomingHttpStartTranslator) incomingHttpRequestEnd.subscribe(incomingHttpEndTranslator) - passportUser.subscribe(onPassportDeserializeUser) passportVerify.subscribe(onPassportVerify) // possible optimization: only subscribe if collection mode is enabled + passportUser.subscribe(onPassportDeserializeUser) queryParser.subscribe(onRequestQueryParsed) nextBodyParsed.subscribe(onRequestBodyParsed) nextQueryParsed.subscribe(onRequestQueryParsed) @@ -185,28 +185,32 @@ function incomingHttpEndTranslator ({ req, res }) { Reporter.finishRequest(req, res) } -function onPassportDeserializeUser ({ req, user, sessionId, abordController }) { - UserTracking.trackUser(user) +function onPassportVerify ({ framework, login, user, success, abortController }) { + const store = storage.getStore() + const rootSpan = store?.req && web.root(store.req) - if (sessionId && typeof sessionId === 'string') { - const results = waf.run({ - persistent: { - 'usr.session_id': sessionId - } - }) + if (!rootSpan) { + log.warn('[ASM] No rootSpan found in onPassportVerify') + return } + + const results = UserTracking.trackLogin(framework, login, user, success, rootSpan) + + handleResults(results, store.req, store.req.res, rootSpan, abortController) } -function onPassportVerify ({ framework, login, user, success, abortController }) { +function onPassportDeserializeUser ({ user, abortController }) { const store = storage.getStore() const rootSpan = store?.req && web.root(store.req) if (!rootSpan) { - log.warn('[ASM] No rootSpan found in onPassportVerify') + log.warn('[ASM] No rootSpan found in onPassportDeserializeUser') return } - const results = UserTracking.trackLogin(framework, login, user, success, rootSpan) + const results = UserTracking.trackUser(user, rootSpan) + + console.log(results) handleResults(results, store.req, store.req.res, rootSpan, abortController) } @@ -324,6 +328,7 @@ function disable () { if (incomingHttpRequestStart.hasSubscribers) incomingHttpRequestStart.unsubscribe(incomingHttpStartTranslator) if (incomingHttpRequestEnd.hasSubscribers) incomingHttpRequestEnd.unsubscribe(incomingHttpEndTranslator) if (passportVerify.hasSubscribers) passportVerify.unsubscribe(onPassportVerify) + if (passportUser.hasSubscribers) passportUser.unsubscribe(onPassportDeserializeUser) if (queryParser.hasSubscribers) queryParser.unsubscribe(onRequestQueryParsed) if (nextBodyParsed.hasSubscribers) nextBodyParsed.unsubscribe(onRequestBodyParsed) if (nextQueryParsed.hasSubscribers) nextQueryParsed.unsubscribe(onRequestQueryParsed) diff --git a/packages/dd-trace/src/appsec/user_tracking.js b/packages/dd-trace/src/appsec/user_tracking.js index 04522d97cba..7865ee78d26 100644 --- a/packages/dd-trace/src/appsec/user_tracking.js +++ b/packages/dd-trace/src/appsec/user_tracking.js @@ -49,10 +49,16 @@ function obfuscateIfNeeded (str) { } } +// what if we have a stack of auth strategies and only last one is sucessful ? we shouldn't send a million failures +// what if sdk is called after automated user, will the waf detect it ? will the tags be overriden ? + // TODO: should we find other ways to get the user ID ? function getUserId (user) { if (!user) return + // should we iterate on user keyss instead to be case independent + // but if we iterate over user then we're missing the inherited props ? + for (const field of USER_ID_FIELDS) { let id = user[field] @@ -73,11 +79,6 @@ function getUserId (user) { function trackLogin (framework, login, user, success, rootSpan) { if (!collectionMode || collectionMode === 'disabled') return - if (!rootSpan) { - log.error('[ASM] No rootSpan found in AppSec trackLogin') - return - } - if (typeof login !== 'string') { log.error('[ASM] Invalid login provided to AppSec trackLogin') @@ -162,32 +163,38 @@ function trackLogin (framework, login, user, success, rootSpan) { return waf.run({ persistent }) } -function trackUser (user, abortController) { +function trackUser (user, rootSpan) { if (!collectionMode || collectionMode === 'disabled') return const userId = getUserId(user) - // must get user ID with USER_ID_FIELDS - if (!userId) { - + log.error('[ASM] Invalid login provided to AppSec trackLogin') + return } - // ANONYMISE user id if needed + const currentTags = rootSpan.context()._tags + + // used to not overwrite tags set by SDK + function shouldSetTag (tag) { + return !(isSdkCalled && currentTags[tag]) + } - // don't override tags if already set by "sdk" but still add the _dd.appsec.usr.id - rootSpan.addTags({ - 'usr.id': userId, + const newTags = { '_dd.appsec.usr.id': userId, // always AND only send when automated - '_dd.appsec.user.collection_mode': collectionMode // short or long form ? - }) + '_dd.appsec.user.collection_mode': collectionMode + } - // _dd.appsec.user.collection_mode: collectionMode // sdk/ident/anon + if (!currentTags['usr.id']) { + newTags['usr.id'] = userId + } + + rootSpan.addTags(newTags) // If the user monitoring SDK has already resulted in a call to libddwaf before any automated instrumentation or collection method has been executed, no extra call should be made. - const results = waf.run({ + return waf.run({ persistent: { - [USER_ID]: userID + [addresses.USER_ID]: userId } }) }