Skip to content

Commit 0a01b6a

Browse files
fix(aap): fixing missing tracking user tags in the new sdk [backport 3.9] (#13703)
Backport ff35f5b from #13701 to 3.9. The new SDK for tracking user was reporting name, email, scope and role of tracked user with different tags than the legacy SDK, leading to missing information in the security UI. - this PR fixes that by setting properly legacy tags. - add a regression test Ticket #2164160 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) Co-authored-by: Christophe Papazian <[email protected]>
1 parent 588c74c commit 0a01b6a

File tree

3 files changed

+46
-2
lines changed

3 files changed

+46
-2
lines changed

ddtrace/appsec/track_user_sdk.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,21 @@ def track_user(
7373
span.set_tag_str(_constants.APPSEC.USER_LOGIN_USERID, str(user_id))
7474
if login:
7575
span.set_tag_str(_constants.APPSEC.USER_LOGIN_USERNAME, str(login))
76-
77-
_trace_utils.set_user(None, user_id, session_id=session_id, may_block=False)
76+
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")
81+
_trace_utils.set_user(
82+
None,
83+
user_id,
84+
name=usr_name if isinstance(usr_name, str) else None,
85+
email=usr_email if isinstance(usr_email, str) else None,
86+
scope=usr_scope if isinstance(usr_scope, str) else None,
87+
role=usr_role if isinstance(usr_role, str) else None,
88+
session_id=session_id,
89+
may_block=False,
90+
)
7891
if metadata:
7992
_trace_utils.track_custom_event(None, "auth_sdk", metadata=metadata)
8093
span.set_tag_str(_constants.APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE, _constants.LOGIN_EVENTS_MODE.SDK)
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 the new ATO SDK track_user was reporting differently email, name, scope and role of the tracked user.

tests/appsec/appsec/test_appsec_trace_utils.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from ddtrace.appsec.trace_utils import track_user_login_failure_event
1212
from ddtrace.appsec.trace_utils import track_user_login_success_event
1313
from ddtrace.appsec.trace_utils import track_user_signup_event
14+
from ddtrace.appsec.track_user_sdk import track_user
1415
from ddtrace.contrib.internal.trace_utils import set_user
1516
from ddtrace.ext import user
1617
import tests.appsec.rules as rules
@@ -93,6 +94,7 @@ def test_track_user_login_event_success_in_span_without_metadata(self):
9394
assert (
9495
user_span.get_tag(user.SESSION_ID) == "test_session_id" and parent_span.get_tag(user.SESSION_ID) is None
9596
)
97+
user_span.finish()
9698

9799
def test_track_user_login_event_success_auto_mode_safe(self):
98100
with asm_context(tracer=self.tracer, span_name="test_success1", config=config_asm):
@@ -239,6 +241,31 @@ def test_set_user_blocked(self):
239241
assert span.get_tag("usr.id") == str(self._BLOCKED_USER)
240242
assert is_blocked(span)
241243

244+
def test_track_user_blocked(self):
245+
with asm_context(tracer=self.tracer, span_name="fake_span", config=config_good_rules) as span:
246+
track_user(
247+
self.tracer,
248+
user_id=self._BLOCKED_USER,
249+
session_id="usr.session_id",
250+
metadata={
251+
"email": "usr.email",
252+
"name": "usr.name",
253+
"session_id": "usr.session_id",
254+
"role": "usr.role",
255+
"scope": "usr.scope",
256+
},
257+
)
258+
assert span.get_tag(user.ID)
259+
assert span.get_tag(user.EMAIL)
260+
assert span.get_tag(user.SESSION_ID)
261+
assert span.get_tag(user.NAME)
262+
assert span.get_tag(user.ROLE)
263+
assert span.get_tag(user.SCOPE)
264+
assert span.get_tag(user.SESSION_ID)
265+
assert span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_COLLECTION_MODE) == LOGIN_EVENTS_MODE.SDK
266+
assert span.get_tag("usr.id") == str(self._BLOCKED_USER)
267+
assert is_blocked(span)
268+
242269
def test_no_span_doesnt_raise(self):
243270
from ddtrace.trace import tracer
244271

0 commit comments

Comments
 (0)