push stuff

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'),

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)

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
     }
   })
 }