Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic userID tracking and blocking #4670

Draft
wants to merge 142 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
142 commits
Select commit Hold shift + click to select a range
81f6ab5
instrument passport
simon-id Sep 11, 2024
023828c
lint
simon-id Sep 11, 2024
98090fb
update config
simon-id Sep 11, 2024
5cc7274
add new RC capability
simon-id Sep 11, 2024
3343878
Merge branch 'master' into new_user_collection
simon-id Sep 11, 2024
4e127de
add RC handler
simon-id Sep 12, 2024
b4daf8f
fix require path
simon-id Sep 12, 2024
5f85b8e
add setCollectionMode()
simon-id Sep 12, 2024
1b074ad
use setCollectionMode() in appsec index
simon-id Sep 12, 2024
cbc8cf3
DRY up passport strategies instrumentation
simon-id Sep 17, 2024
729241d
simplify passport strategies instrumentations
simon-id Sep 17, 2024
6ffc21f
simplify instrumentation
simon-id Sep 17, 2024
4433baa
note for later
simon-id Sep 17, 2024
cf39858
add blocking to login
simon-id Sep 17, 2024
2da5bef
Merge branch 'master' into new_user_collection
simon-id Sep 17, 2024
d8bed47
add abortController in listener
simon-id Sep 17, 2024
5105006
cleanup
simon-id Oct 20, 2024
65c680b
update typings
simon-id Oct 20, 2024
23f965d
cleanup
simon-id Oct 20, 2024
b5b336a
RC config update
simon-id Oct 20, 2024
53a7791
push everything
simon-id Oct 30, 2024
2b7d2e6
simplify passport strategies instrumentation
simon-id Oct 30, 2024
24c2828
fixes
simon-id Oct 30, 2024
cdb89f6
simplify code
simon-id Oct 30, 2024
32edfae
cleanup
simon-id Oct 30, 2024
50b3605
revert some changes
simon-id Oct 30, 2024
b5aa0d4
delete passport.js
simon-id Oct 30, 2024
1f30a3c
Update packages/datadog-instrumentations/src/passport-utils.js
simon-id Oct 30, 2024
9fee2bc
update verify subscriber
simon-id Oct 30, 2024
be3115a
rollback config changes
simon-id Oct 30, 2024
38b4419
fix config
simon-id Oct 30, 2024
40c9df1
add blocking for passport strategies
simon-id Oct 30, 2024
e34bb72
update typings and docs
simon-id Oct 30, 2024
d255702
update appsec index
simon-id Oct 30, 2024
b6d4584
update RC
simon-id Oct 30, 2024
75782f9
Merge branch 'master' into new_user_collection
simon-id Oct 30, 2024
4ff8379
push some stuff
simon-id Oct 30, 2024
be473e6
push some stuff
simon-id Oct 30, 2024
4a566a8
cleanup
simon-id Oct 30, 2024
882bc8b
cleanup
simon-id Oct 30, 2024
d4345ef
add new usr.login waf address
simon-id Oct 31, 2024
be4a06c
commit some stuff
simon-id Oct 31, 2024
f129890
cleanup
simon-id Oct 31, 2024
58897c8
Merge branch 'new_user_collection' into automatic_userid_blocking
simon-id Oct 31, 2024
cceac23
cleanup
simon-id Oct 31, 2024
654d260
add comment for later
simon-id Oct 31, 2024
066da87
add some ideas
simon-id Oct 31, 2024
da9b270
cleanup
simon-id Nov 1, 2024
01c8240
aaaaa
simon-id Nov 1, 2024
cc86e3a
pass abort controller for blocking
simon-id Nov 1, 2024
69d12a9
push some notes
simon-id Nov 14, 2024
ce604c1
change config default
simon-id Nov 15, 2024
56c30e8
handle duplicate RC confs for auto_user_instrum.mode
simon-id Nov 15, 2024
1d3773a
refactor sdk/track_event.js to only be used by the SDK
simon-id Nov 15, 2024
a9f5c01
Merge branch 'new_user_collection' into automatic_userid_blocking
simon-id Nov 15, 2024
a72695f
push some stuff
simon-id Nov 19, 2024
91a59db
remove some comments
simon-id Nov 19, 2024
faea3aa
pass login to WAF in SDK login success event
simon-id Nov 19, 2024
fe70c84
add comments
simon-id Nov 19, 2024
e32f78e
add framework name to passport strategy instrum
simon-id Nov 20, 2024
806ab23
add framework name and waf handleResults() to onPassportVerify()
simon-id Nov 20, 2024
69be031
Merge branch 'master' into new_user_collection
simon-id Nov 20, 2024
1e1bb47
finally commit trackLogin()
simon-id Nov 21, 2024
8eac055
fix tag override condition
simon-id Nov 21, 2024
a444cfe
add telemetry function
simon-id Nov 21, 2024
187b42f
Merge branch 'master' into new_user_collection
simon-id Nov 21, 2024
72a4a8b
Merge branch 'master' into new_user_collection
simon-id Nov 22, 2024
9fa94f6
move user_tracking into a file instead of a subfolder
simon-id Nov 22, 2024
d51e434
Merge branch 'master' into new_user_collection
simon-id Nov 22, 2024
8b22248
remove changes
simon-id Nov 22, 2024
25abd50
temp revert changes
simon-id Nov 22, 2024
52a0601
temp revert changes
simon-id Nov 22, 2024
b72ce34
Merge branch 'new_user_collection' into automatic_userid_blocking
simon-id Nov 22, 2024
5a10fc0
fix conflict
simon-id Nov 22, 2024
9869c4b
wrong file
simon-id Nov 22, 2024
76e2e65
add some stuff
simon-id Nov 22, 2024
80ae693
push some stuff
simon-id Nov 25, 2024
7e3374d
Use addresses enum instead of hardcoded business logic events
simon-id Nov 25, 2024
c099a2a
Update default value of collection mode in typings
simon-id Nov 25, 2024
96cb722
Add string check in getUserId()
simon-id Nov 25, 2024
0a347e8
Merge branch 'master' into new_user_collection
simon-id Nov 26, 2024
85ad6f2
fix existing RC tests
simon-id Nov 26, 2024
70dcbab
add tests for RC collection mode
simon-id Nov 26, 2024
ff72271
fix env var ordering
simon-id Nov 26, 2024
7338bd6
cleanup track_event.spec.js
simon-id Nov 29, 2024
b71ed06
do not export or test trackEvent()
simon-id Nov 29, 2024
166bc23
update test, cleanup, and add missing coverage
simon-id Nov 29, 2024
a93b79a
change ordering of code to match tests
simon-id Nov 29, 2024
a83c4fb
delete passport-utils tests because it's useless
simon-id Nov 29, 2024
6cf7ef2
fix test to correctly use passReqToCallback
simon-id Nov 29, 2024
29c0d62
update passport-local tests
simon-id Nov 29, 2024
a6211b2
Merge branch 'new_user_collection' into automatic_userid_blocking
simon-id Dec 2, 2024
afafb8a
update tests for passpot-http
simon-id Dec 2, 2024
956cb9b
allow empty login strings
simon-id Dec 2, 2024
62ae42b
update TS tests
simon-id Dec 2, 2024
375f688
update telemetry tests
simon-id Dec 2, 2024
79a7ff5
Merge branch 'new_user_collection' into automatic_userid_blocking
simon-id Dec 2, 2024
b47e44a
Merge branch 'master' into automatic_userid_blocking
simon-id Dec 17, 2024
5a0ee2e
push stuff
simon-id Dec 26, 2024
775bf9a
remove session id support
simon-id Dec 27, 2024
cb78746
add telemetry function
simon-id Jan 13, 2025
a259b22
use telemetry
simon-id Jan 13, 2025
96cf8e7
cleanup
simon-id Jan 13, 2025
c519d6e
bad copy paste
simon-id Jan 15, 2025
8767533
remove comments
simon-id Jan 15, 2025
a508649
fix sdk
simon-id Jan 15, 2025
42d9fda
cleanup
simon-id Jan 15, 2025
d643691
cleanup
simon-id Jan 15, 2025
d910370
cleanup
simon-id Jan 15, 2025
28254ce
better original res.end call
simon-id Jan 16, 2025
2ec2e5b
revert old copy paste or something
simon-id Jan 16, 2025
bd3a597
cleanup
simon-id Jan 16, 2025
bc2b2d0
cleanup
simon-id Jan 16, 2025
be29564
Merge branch 'master' into automatic_userid_blocking
simon-id Jan 17, 2025
c0ae70b
cleanup
simon-id Jan 17, 2025
60789f4
don't put user tracking tag when calling login tracking
simon-id Jan 17, 2025
1cb83fb
Merge branch 'master' into automatic_userid_blocking
simon-id Jan 18, 2025
03e18df
Merge branch 'master' into automatic_userid_blocking
simon-id Jan 20, 2025
8c3e97b
add telemetry test
simon-id Jan 20, 2025
7adb49a
fix blocking tests
simon-id Jan 20, 2025
d80b41c
fix appsec index blocking tests
simon-id Jan 20, 2025
8dfbb3d
fix setuser tests
simon-id Jan 20, 2025
1beff4f
remove outdated test
simon-id Jan 21, 2025
2dd9cb5
fix user blocking test
simon-id Jan 21, 2025
8eea578
Merge branch 'master' into automatic_userid_blocking
simon-id Jan 21, 2025
be219e1
trackUser() tests
simon-id Jan 21, 2025
115e808
update set_user tests
simon-id Jan 21, 2025
09aadcd
update user blocking tests
simon-id Jan 21, 2025
589271d
update appsec index tests
simon-id Jan 21, 2025
67482db
lint
simon-id Jan 21, 2025
b51c666
Merge branch 'master' into automatic_userid_blocking
simon-id Jan 22, 2025
1fe6b89
add passport in ci tests
simon-id Jan 22, 2025
e600ec0
try to break a test
simon-id Jan 22, 2025
f00dfbe
try to break another test
simon-id Jan 22, 2025
c203530
Merge branch 'master' into automatic_userid_blocking
simon-id Jan 23, 2025
0b15e9b
Update passport-http.spec.js
simon-id Jan 23, 2025
24328ab
add express-session in the whitelist of WeakHashAnalyzer
simon-id Jan 23, 2025
2ee8209
fix passport instrmentation tests not run in CI
simon-id Jan 24, 2025
89b9cfe
revert debug
simon-id Jan 24, 2025
14d6760
upload non finished passport test to check CI
simon-id Jan 24, 2025
0310e40
revert autoformat
simon-id Jan 24, 2025
0c51a40
cleanup
simon-id Jan 24, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion .github/workflows/appsec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,17 @@ jobs:
version:
- 18
- latest
range: ['>=10.2.0 <11', '>=11.0.0 <13', '11.1.4', '>=13.0.0 <14', '13.2.0', '>=14.0.0 <=14.2.6', '>=14.2.7 <15', '>=15.0.0']
range:
[
'>=10.2.0 <11',
'>=11.0.0 <13',
'11.1.4',
'>=13.0.0 <14',
'13.2.0',
'>=14.0.0 <=14.2.6',
'>=14.2.7 <15',
'>=15.0.0',
]
include:
- range: '>=10.2.0 <11'
range_clean: gte.10.2.0.and.lt.11
Expand Down
24 changes: 24 additions & 0 deletions .github/workflows/plugins.yml
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,30 @@ jobs:
suffix: plugins-${{ github.job }}
- uses: codecov/codecov-action@v3

passport:
runs-on: ubuntu-latest
env:
PLUGINS: passport
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test

passport-http:
runs-on: ubuntu-latest
env:
PLUGINS: passport-http
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test

passport-local:
runs-on: ubuntu-latest
env:
PLUGINS: passport-local
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test

# TODO: re-enable upstream tests if it ever stops being flaky
pino:
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions packages/datadog-instrumentations/src/helpers/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ module.exports = {
oracledb: () => require('../oracledb'),
openai: () => require('../openai'),
paperplane: () => require('../paperplane'),
passport: () => require('../passport'),
'passport-http': () => require('../passport-http'),
'passport-local': () => require('../passport-local'),
pg: () => require('../pg'),
Expand Down
45 changes: 45 additions & 0 deletions packages/datadog-instrumentations/src/passport.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict'

const shimmer = require('../../datadog-shimmer')
const { channel, addHook } = require('./helpers/instrument')

const onPassportDeserializeUserChannel = channel('datadog:passport:deserializeUser:finish')

function wrapDone (done) {
return function wrappedDone (err, user) {
if (!err && user) {
const abortController = new AbortController()

onPassportDeserializeUserChannel.publish({ user, abortController })

if (abortController.signal.aborted) return
}

return done.apply(this, arguments)
}
}

function wrapDeserializeUser (deserializeUser) {
return function wrappedDeserializeUser (fn, req, done) {
if (typeof fn === 'function') return deserializeUser.apply(this, arguments)

if (typeof req === 'function') {
done = req
arguments[1] = wrapDone(done)
} else {
arguments[2] = wrapDone(done)
}

return deserializeUser.apply(this, arguments)
}
}

addHook({
name: 'passport',
file: 'lib/authenticator.js',
versions: ['>=0.2.0']
}, Authenticator => {
shimmer.wrap(Authenticator.prototype, 'deserializeUser', wrapDeserializeUser)

return Authenticator
})
118 changes: 118 additions & 0 deletions packages/datadog-instrumentations/test/passport.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
'use strict'

const agent = require('../../dd-trace/test/plugins/agent')
const axios = require('axios').create({ validateStatus: null })
const dc = require('dc-polyfill')
const { storage } = require('../../datadog-core')

const users = [{
id: 1,
username: 'test',
password: '1234',
email: 'testuser@ddog.com'
}]

withVersions('passport', 'passport', version => {
describe('passport instrumentation', () => {
const passportDeserializeUserChannel = dc.channel('datadog:passport:deserializeUser:finish')
let port, server, subscriberStub

before(() => {
return agent.load(['http', 'express', 'express-session', 'passport', 'passport-local'], { client: false })
})

before((done) => {
const express = require('../../../versions/express').get()
const expressSession = require('../../../versions/express-session').get()
const passport = require(`../../../versions/passport@${version}`).get()
const LocalStrategy = require(`../../../versions/passport-local@${version}`).get().Strategy

const app = express()
app.use(expressSession({
secret: 'secret',
resave: false,
rolling: true,
saveUninitialized: true
}))

app.use(passport.initialize())
app.use(passport.session())

passport.serializeUser((user, done) => {
done(null, user.id)
})

passport.deserializeUser((id, done) => {
const user = users.find((user) => user.id === id)

done(null, user)
})

passport.use('local', new LocalStrategy((username, password, done) => {
const user = users.find((user) => user.username === username && user.password === password)

return done(null, user)
}))

app.get('/', passport.authenticate('local'))

passportDeserializeUserChannel.subscribe((data) => subscriberStub(data))

server = app.listen(0, () => {
port = server.address().port
done()
})
})

beforeEach(async () => {
subscriberStub = sinon.stub()

const res = await axios.post(`http://localhost:${port}/`, { username: 'test', password: '1234' })

console.log(res.headers['set-cookie'])

Check failure on line 72 in packages/datadog-instrumentations/test/passport.spec.js

View workflow job for this annotation

GitHub Actions / lint

Unexpected console statement
})

after(() => {
server.close()
return agent.close({ ritmReset: false })
})

it('should not call subscriber when an error occurs', async () => {
const res = await axios.post(`http://localhost:${port}/`, { username: 'error', password: '1234' })

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.post(`http://localhost:${port}/`, { username: 'test', password: '1234' })

expect(res.status).to.equal(200)
expect(res.data).to.equal('Granted')
expect(subscriberStub).to.be.calledOnceWithExactly({
framework: 'passport-local',
login: 'test',
user: { _id: 1, username: 'test', password: '1234', email: 'testuser@ddog.com' },
success: true,
abortController: new AbortController()
})
})

it('should block when subscriber aborts', async () => {
subscriberStub = sinon.spy(({ abortController }) => {
storage.getStore().req.res.writeHead(403).end('Blocked')
abortController.abort()
})


Check failure on line 107 in packages/datadog-instrumentations/test/passport.spec.js

View workflow job for this annotation

GitHub Actions / lint

More than 1 blank line not allowed
const res = await axios.post(`http://localhost:${port}/`, { username: 'test', password: '1234' })

expect(res.status).to.equal(403)
expect(res.data).to.equal('Blocked')
expect(subscriberStub).to.be.calledOnceWithExactly({
user: { _id: 1, username: 'test', password: '1234', email: 'testuser@ddog.com' },
abortController: new AbortController()
})
})
})
})
5 changes: 4 additions & 1 deletion packages/dd-trace/src/appsec/blocking.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ function block (req, res, rootSpan, abortController, actionParameters = defaultB
res.removeHeader(headerName)
}

res.writeHead(statusCode, headers).end(body)
res.writeHead(statusCode, headers)

// this is needed to call the original end method, since express-session replaces it
res.constructor.prototype.end.call(res, body)

responseBlockedSet.add(res)

Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/appsec/channels.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module.exports = {
incomingHttpRequestStart: dc.channel('dd-trace:incomingHttpRequestStart'),
incomingHttpRequestEnd: dc.channel('dd-trace:incomingHttpRequestEnd'),
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'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const EXCLUDED_LOCATIONS = getNodeModulesPaths(
'sqreen/lib/package-reader/index.js',
'ws/lib/websocket-server.js',
'google-gax/build/src/grpc.js',
'cookie-signature/index.js'
'cookie-signature/index.js',
'express-session/index.js'
)

const EXCLUDED_PATHS_FROM_STACK = [
Expand Down
17 changes: 17 additions & 0 deletions packages/dd-trace/src/appsec/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const {
incomingHttpRequestStart,
incomingHttpRequestEnd,
passportVerify,
passportUser,
queryParser,
nextBodyParsed,
nextQueryParsed,
Expand Down Expand Up @@ -67,6 +68,7 @@ function enable (_config) {
incomingHttpRequestStart.subscribe(incomingHttpStartTranslator)
incomingHttpRequestEnd.subscribe(incomingHttpEndTranslator)
passportVerify.subscribe(onPassportVerify) // possible optimization: only subscribe if collection mode is enabled
passportUser.subscribe(onPassportDeserializeUser)
queryParser.subscribe(onRequestQueryParsed)
nextBodyParsed.subscribe(onRequestBodyParsed)
nextQueryParsed.subscribe(onRequestQueryParsed)
Expand Down Expand Up @@ -197,6 +199,20 @@ function onPassportVerify ({ framework, login, user, success, abortController })
handleResults(results, store.req, store.req.res, rootSpan, 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 onPassportDeserializeUser')
return
}

const results = UserTracking.trackUser(user, rootSpan)

handleResults(results, store.req, store.req.res, rootSpan, abortController)
}

function onRequestQueryParsed ({ req, res, query, abortController }) {
if (!query || typeof query !== 'object') return

Expand Down Expand Up @@ -310,6 +326,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)
Expand Down
15 changes: 15 additions & 0 deletions packages/dd-trace/src/appsec/sdk/set_user.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

const { getRootSpan } = require('./utils')
const log = require('../../log')
const waf = require('../waf')
const addresses = require('../addresses')

function setUserTags (user, rootSpan) {
for (const k of Object.keys(user)) {
Expand All @@ -22,6 +24,19 @@ function setUser (tracer, user) {
}

setUserTags(user, rootSpan)
rootSpan.setTag('_dd.appsec.user.collection_mode', 'sdk')

const persistent = {}

if (user.id) {
persistent[addresses.USER_ID] = '' + user.id
}

if (user.login) {
persistent[addresses.USER_LOGIN] = '' + user.login
}

waf.run({ persistent })
}

module.exports = {
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/appsec/sdk/user_blocking.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function checkUserAndSetUser (tracer, user) {
if (rootSpan) {
if (!rootSpan.context()._tags['usr.id']) {
setUserTags(user, rootSpan)
rootSpan.setTag('_dd.appsec.user.collection_mode', 'sdk')
}
} else {
log.warn('[ASM] Root span not available in isUserBlocked')
Expand Down
10 changes: 10 additions & 0 deletions packages/dd-trace/src/appsec/telemetry.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,15 @@ function incrementMissingUserLoginMetric (framework, eventType) {
}).inc()
}

function incrementMissingUserIdMetric (framework, eventType) {
if (!enabled) return

appsecMetrics.count('instrum.user_auth.missing_user_id', {
framework,
event_type: eventType
}).inc()
}

function getRequestMetrics (req) {
if (req) {
const store = getStore(req)
Expand All @@ -203,6 +212,7 @@ module.exports = {
incrementWafUpdatesMetric,
incrementWafRequestsMetric,
incrementMissingUserLoginMetric,
incrementMissingUserIdMetric,

getRequestMetrics
}
Loading
Loading