New automatic user event collection (#4674)

docs/test.ts

@@ -111,7 +111,7 @@ tracer.init({
     blockedTemplateJson: './blocked.json',
     blockedTemplateGraphql: './blockedgraphql.json',
     eventTracking: {
-      mode: 'safe'
+      mode: 'anon'
     },
     apiSecurity: {
       enabled: true,

index.d.ts

@@ -655,12 +655,24 @@ declare namespace tracer {
        */
       eventTracking?: {
         /**
-         * Controls the automated user event tracking mode. Possible values are disabled, safe and extended.
-         * On safe mode, any detected Personally Identifiable Information (PII) about the user will be redacted from the event.
-         * On extended mode, no redaction will take place.
-         * @default 'safe'
+         * Controls the automated user tracking mode for user IDs and logins collections. Possible values:
+         * *  'anonymous': will hash user IDs and user logins before collecting them
+         * *  'anon': alias for 'anonymous'
+         * *  'safe': deprecated alias for 'anonymous'
+         * 
+         * *  'identification': will collect user IDs and logins without redaction
+         * *  'ident': alias for 'identification'
+         * *  'extended': deprecated alias for 'identification'
+         * 
+         * *  'disabled': will not collect user IDs and logins
+         * 
+         * Unknown values will be considered as 'disabled'
+         * @default 'identification'
          */
-        mode?: 'safe' | 'extended' | 'disabled'
+        mode?:
+          'anonymous' | 'anon' | 'safe' |
+          'identification' | 'ident' | 'extended' |
+          'disabled'
       },
       /**
        * Configuration for Api Security

packages/datadog-instrumentations/src/passport-http.js

@@ -1,22 +1,10 @@
 'use strict'
 
-const shimmer = require('../../datadog-shimmer')
 const { addHook } = require('./helpers/instrument')
-const { wrapVerify } = require('./passport-utils')
+const { strategyHook } = require('./passport-utils')
 
 addHook({
   name: 'passport-http',
   file: 'lib/passport-http/strategies/basic.js',
   versions: ['>=0.3.0']
-}, BasicStrategy => {
-  return shimmer.wrapFunction(BasicStrategy, BasicStrategy => function () {
-    const type = 'http'
-
-    if (typeof arguments[0] === 'function') {
-      arguments[0] = wrapVerify(arguments[0], false, type)
-    } else {
-      arguments[1] = wrapVerify(arguments[1], (arguments[0] && arguments[0].passReqToCallback), type)
-    }
-    return BasicStrategy.apply(this, arguments)
-  })
-})
+}, strategyHook)

packages/datadog-instrumentations/src/passport-local.js

@@ -1,22 +1,10 @@
 'use strict'
 
-const shimmer = require('../../datadog-shimmer')
 const { addHook } = require('./helpers/instrument')
-const { wrapVerify } = require('./passport-utils')
+const { strategyHook } = require('./passport-utils')
 
 addHook({
   name: 'passport-local',
   file: 'lib/strategy.js',
   versions: ['>=1.0.0']
-}, Strategy => {
-  return shimmer.wrapFunction(Strategy, Strategy => function () {
-    const type = 'local'
-
-    if (typeof arguments[0] === 'function') {
-      arguments[0] = wrapVerify(arguments[0], false, type)
-    } else {
-      arguments[1] = wrapVerify(arguments[1], (arguments[0] && arguments[0].passReqToCallback), type)
-    }
-    return Strategy.apply(this, arguments)
-  })
-})
+}, strategyHook)

packages/datadog-instrumentations/src/passport-utils.js

@@ -5,33 +5,57 @@ const { channel } = require('./helpers/instrument')
 
 const passportVerifyChannel = channel('datadog:passport:verify:finish')
 
-function wrapVerifiedAndPublish (username, password, verified, type) {
-  if (!passportVerifyChannel.hasSubscribers) {
-    return verified
-  }
+function wrapVerifiedAndPublish (framework, username, verified) {
+  return shimmer.wrapFunction(verified, function wrapVerified (verified) {
+    return function wrappedVerified (err, user) {
+      // if there is an error, it's neither an auth success nor a failure
+      if (!err) {
+        const abortController = new AbortController()
+
+        passportVerifyChannel.publish({ framework, login: username, user, success: !!user, abortController })
+
+        if (abortController.signal.aborted) return
+      }
 
-  // eslint-disable-next-line n/handle-callback-err
-  return shimmer.wrapFunction(verified, verified => function (err, user, info) {
-    const credentials = { type, username }
-    passportVerifyChannel.publish({ credentials, user })
-    return verified.apply(this, arguments)
+      return verified.apply(this, arguments)
+    }
   })
 }
 
-function wrapVerify (verify, passReq, type) {
-  if (passReq) {
-    return function (req, username, password, verified) {
-      arguments[3] = wrapVerifiedAndPublish(username, password, verified, type)
-      return verify.apply(this, arguments)
+function wrapVerify (verify) {
+  return function wrappedVerify (req, username, password, verified) {
+    if (passportVerifyChannel.hasSubscribers) {
+      const framework = `passport-${this.name}`
+
+      // replace the callback with our own wrapper to get the result
+      if (this._passReqToCallback) {
+        arguments[3] = wrapVerifiedAndPublish(framework, arguments[1], arguments[3])
+      } else {
+        arguments[2] = wrapVerifiedAndPublish(framework, arguments[0], arguments[2])
+      }
     }
-  } else {
-    return function (username, password, verified) {
-      arguments[2] = wrapVerifiedAndPublish(username, password, verified, type)
-      return verify.apply(this, arguments)
+
+    return verify.apply(this, arguments)
+  }
+}
+
+function wrapStrategy (Strategy) {
+  return function wrappedStrategy () {
+    // verify function can be either the first or second argument
+    if (typeof arguments[0] === 'function') {
+      arguments[0] = wrapVerify(arguments[0])
+    } else {
+      arguments[1] = wrapVerify(arguments[1])
     }
+
+    return Strategy.apply(this, arguments)
   }
 }
 
+function strategyHook (Strategy) {
+  return shimmer.wrapFunction(Strategy, wrapStrategy)
+}
+
 module.exports = {
-  wrapVerify
+  strategyHook
 }

packages/datadog-instrumentations/test/passport-http.spec.js

@@ -1,16 +1,17 @@
 'use strict'
 
 const agent = require('../../dd-trace/test/plugins/agent')
-const axios = require('axios')
+const axios = require('axios').create({ validateStatus: null })
 const dc = require('dc-polyfill')
+const { storage } = require('../../datadog-core')
 
 withVersions('passport-http', 'passport-http', version => {
   describe('passport-http instrumentation', () => {
     const passportVerifyChannel = dc.channel('datadog:passport:verify:finish')
     let port, server, subscriberStub
 
     before(() => {
-      return agent.load(['express', 'passport', 'passport-http'], { client: false })
+      return agent.load(['http', 'express', 'passport', 'passport-http'], { client: false })
     })
 
     before((done) => {
@@ -19,7 +20,17 @@ withVersions('passport-http', 'passport-http', version => {
       const BasicStrategy = require(`../../../versions/passport-http@${version}`).get().BasicStrategy
       const app = express()
 
-      passport.use(new BasicStrategy((username, password, done) => {
+      function validateUser (req, username, password, done) {
+        // support with or without passReqToCallback
+        if (typeof done !== 'function') {
+          done = password
+          password = username
+          username = req
+        }
+
+        // simulate db error
+        if (username === 'error') return done('error')
+
         const users = [{
           _id: 1,
           username: 'test',
@@ -35,7 +46,18 @@ withVersions('passport-http', 'passport-http', version => {
           return done(null, user)
         }
       }
-      ))
+
+      passport.use('basic', new BasicStrategy({
+        usernameField: 'username',
+        passwordField: 'password',
+        passReqToCallback: false
+      }, validateUser))
+
+      passport.use('basic-withreq', new BasicStrategy({
+        usernameField: 'username',
+        passwordField: 'password',
+        passReqToCallback: true
+      }, validateUser))
 
       app.use(passport.initialize())
       app.use(express.json())
@@ -44,16 +66,14 @@ withVersions('passport-http', 'passport-http', version => {
         passport.authenticate('basic', {
           successRedirect: '/grant',
           failureRedirect: '/deny',
-          passReqToCallback: false,
           session: false
         })
       )
 
-      app.post('/req',
-        passport.authenticate('basic', {
+      app.get('/req',
+        passport.authenticate('basic-withreq', {
           successRedirect: '/grant',
           failureRedirect: '/deny',
-          passReqToCallback: true,
           session: false
         })
       )
@@ -66,9 +86,7 @@ withVersions('passport-http', 'passport-http', version => {
         res.send('Denied')
       })
 
-      passportVerifyChannel.subscribe(function ({ credentials, user, err, info }) {
-        subscriberStub(arguments[0])
-      })
+      passportVerifyChannel.subscribe((data) => subscriberStub(data))
 
       server = app.listen(0, () => {
         port = server.address().port
@@ -85,6 +103,18 @@ withVersions('passport-http', 'passport-http', version => {
       return agent.close({ ritmReset: false })
     })
 
+    it('should not call subscriber when an error occurs', async () => {
+      const res = await axios.get(`http://localhost:${port}/`, {
+        headers: {
+          // error:1234
+          Authorization: 'Basic ZXJyb3I6MTIzNA=='
+        }
+      })
+
+      expect(res.status).to.equal(500)
+      expect(subscriberStub).to.not.be.called
+    })
+
     it('should call subscriber with proper arguments on success', async () => {
       const res = await axios.get(`http://localhost:${port}/`, {
         headers: {
@@ -95,16 +125,17 @@ withVersions('passport-http', 'passport-http', version => {
 
       expect(res.status).to.equal(200)
       expect(res.data).to.equal('Granted')
-      expect(subscriberStub).to.be.calledOnceWithExactly(
-        {
-          credentials: { type: 'http', username: 'test' },
-          user: { _id: 1, username: 'test', password: '1234', email: 'testuser@ddog.com' }
-        }
-      )
+      expect(subscriberStub).to.be.calledOnceWithExactly({
+        framework: 'passport-basic',
+        login: 'test',
+        user: { _id: 1, username: 'test', password: '1234', email: 'testuser@ddog.com' },
+        success: true,
+        abortController: new AbortController()
+      })
     })
 
     it('should call subscriber with proper arguments on success with passReqToCallback set to true', async () => {
-      const res = await axios.get(`http://localhost:${port}/`, {
+      const res = await axios.get(`http://localhost:${port}/req`, {
         headers: {
           // test:1234
           Authorization: 'Basic dGVzdDoxMjM0'
@@ -113,12 +144,13 @@ withVersions('passport-http', 'passport-http', version => {
 
       expect(res.status).to.equal(200)
       expect(res.data).to.equal('Granted')
-      expect(subscriberStub).to.be.calledOnceWithExactly(
-        {
-          credentials: { type: 'http', username: 'test' },
-          user: { _id: 1, username: 'test', password: '1234', email: 'testuser@ddog.com' }
-        }
-      )
+      expect(subscriberStub).to.be.calledOnceWithExactly({
+        framework: 'passport-basic',
+        login: 'test',
+        user: { _id: 1, username: 'test', password: '1234', email: 'testuser@ddog.com' },
+        success: true,
+        abortController: new AbortController()
+      })
     })
 
     it('should call subscriber with proper arguments on failure', async () => {
@@ -131,12 +163,37 @@ withVersions('passport-http', 'passport-http', version => {
 
       expect(res.status).to.equal(200)
       expect(res.data).to.equal('Denied')
-      expect(subscriberStub).to.be.calledOnceWithExactly(
-        {
-          credentials: { type: 'http', username: 'test' },
-          user: false
+      expect(subscriberStub).to.be.calledOnceWithExactly({
+        framework: 'passport-basic',
+        login: 'test',
+        user: false,
+        success: false,
+        abortController: new AbortController()
+      })
+    })
+
+    it('should block when subscriber aborts', async () => {
+      subscriberStub = sinon.spy(({ abortController }) => {
+        storage.getStore().req.res.writeHead(403).end('Blocked')
+        abortController.abort()
+      })
+
+      const res = await axios.get(`http://localhost:${port}/`, {
+        headers: {
+          // test:1234
+          Authorization: 'Basic dGVzdDoxMjM0'
         }
-      )
+      })
+
+      expect(res.status).to.equal(403)
+      expect(res.data).to.equal('Blocked')
+      expect(subscriberStub).to.be.calledOnceWithExactly({
+        framework: 'passport-basic',
+        login: 'test',
+        user: { _id: 1, username: 'test', password: '1234', email: 'testuser@ddog.com' },
+        success: true,
+        abortController: new AbortController()
+      })
     })
   })
 })