Skip to content

Commit 0940d73

Browse files
fix(aap): track user default arguments must not generate security event (#13742)
Ensure track_user to not generate additional security events when used with expected metadata (name, email, scope or role). Also: - update (and fix) related unit tests APPSEC-58040 ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent 6fae86b commit 0940d73

File tree

3 files changed

+32
-27
lines changed

3 files changed

+32
-27
lines changed

ddtrace/appsec/track_user_sdk.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ def track_user(
7474
if login:
7575
span.set_tag_str(_constants.APPSEC.USER_LOGIN_USERNAME, str(login))
7676
meta = metadata or {}
77-
usr_name = meta.get("name") or meta.get("usr.name")
78-
usr_email = meta.get("email") or meta.get("usr.email")
79-
usr_scope = meta.get("scope") or meta.get("usr.scope")
80-
usr_role = meta.get("role") or meta.get("usr.role")
77+
usr_name = meta.pop("name", None) or meta.pop("usr.name", None)
78+
usr_email = meta.pop("email", None) or meta.pop("usr.email", None)
79+
usr_scope = meta.pop("scope", None) or meta.pop("usr.scope", None)
80+
usr_role = meta.pop("role", None) or meta.pop("usr.role", None)
8181
_trace_utils.set_user(
8282
None,
8383
user_id,
@@ -88,8 +88,8 @@ def track_user(
8888
session_id=session_id,
8989
may_block=False,
9090
)
91-
if metadata:
92-
_trace_utils.track_custom_event(None, "auth_sdk", metadata=metadata)
91+
if meta:
92+
_trace_utils.track_custom_event(None, "auth_sdk", metadata=meta)
9393
span.set_tag_str(_constants.APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE, _constants.LOGIN_EVENTS_MODE.SDK)
9494
if _asm_request_context.in_asm_context():
9595
custom_data = {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
AAP: This fix resolves an issue where track_user was generating additional unexpected security activity for customers.

tests/appsec/appsec/test_appsec_trace_utils.py

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -261,16 +261,16 @@ def test_set_user_blocked(self):
261261
role="usr.role",
262262
scope="usr.scope",
263263
)
264-
assert span.get_tag(user.ID)
265-
assert span.get_tag(user.EMAIL)
266-
assert span.get_tag(user.SESSION_ID)
267-
assert span.get_tag(user.NAME)
268-
assert span.get_tag(user.ROLE)
269-
assert span.get_tag(user.SCOPE)
270-
assert span.get_tag(user.SESSION_ID)
271-
assert span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE) == LOGIN_EVENTS_MODE.SDK
272-
assert span.get_tag("usr.id") == str(self._BLOCKED_USER)
273-
assert is_blocked(span)
264+
assert span.get_tag(user.ID)
265+
assert span.get_tag(user.EMAIL)
266+
assert span.get_tag(user.SESSION_ID)
267+
assert span.get_tag(user.NAME)
268+
assert span.get_tag(user.ROLE)
269+
assert span.get_tag(user.SCOPE)
270+
assert span.get_tag(user.SESSION_ID)
271+
assert span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE) == LOGIN_EVENTS_MODE.SDK
272+
assert span.get_tag("usr.id") == str(self._BLOCKED_USER)
273+
assert is_blocked(span)
274274

275275
def test_track_user_blocked(self):
276276
with asm_context(tracer=self.tracer, span_name="fake_span", config=config_good_rules) as span:
@@ -281,21 +281,22 @@ def test_track_user_blocked(self):
281281
metadata={
282282
"email": "usr.email",
283283
"name": "usr.name",
284-
"session_id": "usr.session_id",
285284
"role": "usr.role",
286285
"scope": "usr.scope",
287286
},
288287
)
289-
assert span.get_tag(user.ID)
290-
assert span.get_tag(user.EMAIL)
291-
assert span.get_tag(user.SESSION_ID)
292-
assert span.get_tag(user.NAME)
293-
assert span.get_tag(user.ROLE)
294-
assert span.get_tag(user.SCOPE)
295-
assert span.get_tag(user.SESSION_ID)
296-
assert span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE) == LOGIN_EVENTS_MODE.SDK
297-
assert span.get_tag("usr.id") == str(self._BLOCKED_USER)
298-
assert is_blocked(span)
288+
assert span.get_tag(user.ID)
289+
assert span.get_tag(user.EMAIL)
290+
assert span.get_tag(user.SESSION_ID)
291+
assert span.get_tag(user.NAME)
292+
assert span.get_tag(user.ROLE)
293+
assert span.get_tag(user.SCOPE)
294+
assert span.get_tag(user.SESSION_ID)
295+
assert span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE) == LOGIN_EVENTS_MODE.SDK
296+
# assert metadata tags are not set for usual data
297+
assert span.get_tag("appsec.events.auth_sdk.track") is None
298+
assert span.get_tag("usr.id") == str(self._BLOCKED_USER)
299+
assert is_blocked(span)
299300

300301
def test_no_span_doesnt_raise(self):
301302
from ddtrace.trace import tracer

0 commit comments

Comments
 (0)