diff --git a/ddtrace/appsec/_trace_utils.py b/ddtrace/appsec/_trace_utils.py index 40468e5bd5c..3e0b297c135 100644 --- a/ddtrace/appsec/_trace_utils.py +++ b/ddtrace/appsec/_trace_utils.py @@ -127,7 +127,7 @@ def track_user_login_success_event( def track_user_login_failure_event( tracer: Tracer, user_id: Optional[str], - exists: bool, + exists: Optional[bool] = None, metadata: Optional[dict] = None, login_events_mode: str = LOGIN_EVENTS_MODE.SDK, ) -> None: @@ -152,8 +152,10 @@ def track_user_login_failure_event( if user_id: span.set_tag_str("%s.failure.%s" % (APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC, user.ID), str(user_id)) - exists_str = "true" if exists else "false" - span.set_tag_str("%s.failure.%s" % (APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC, user.EXISTS), exists_str) + + if exists is not None: + exists_str = "true" if exists else "false" + span.set_tag_str("%s.failure.%s" % (APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC, user.EXISTS), exists_str) def track_user_signup_event( @@ -312,9 +314,9 @@ def _on_django_login( **user_extra, ) else: - # Login failed and the user is unknown + # Login failed and the user is unknown (may exist or not) user_id = info_retriever.get_userid() - track_user_login_failure_event(pin.tracer, user_id=user_id, exists=False, login_events_mode=mode) + track_user_login_failure_event(pin.tracer, user_id=user_id, login_events_mode=mode) def _on_django_auth(result_user, mode, kwargs, pin, info_retriever): @@ -333,7 +335,7 @@ def _on_django_auth(result_user, mode, kwargs, pin, info_retriever): if not result_user: with pin.tracer.trace("django.contrib.auth.login", span_type=SpanTypes.AUTH): - track_user_login_failure_event(pin.tracer, user_id=user_id, exists=False, login_events_mode=mode) + track_user_login_failure_event(pin.tracer, user_id=user_id, login_events_mode=mode) return False, None diff --git a/releasenotes/notes/asm-fix-login-failure-events-4c79ea4b5c6f5e29.yaml b/releasenotes/notes/asm-fix-login-failure-events-4c79ea4b5c6f5e29.yaml new file mode 100644 index 00000000000..3aed56bd38b --- /dev/null +++ b/releasenotes/notes/asm-fix-login-failure-events-4c79ea4b5c6f5e29.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + ASM: This fix resolves an issue where django login failure events may send wrong information of user existence. + diff --git a/tests/contrib/django/test_django_appsec.py b/tests/contrib/django/test_django_appsec.py index 3831a37d31f..fe49488bb65 100644 --- a/tests/contrib/django/test_django_appsec.py +++ b/tests/contrib/django/test_django_appsec.py @@ -239,7 +239,28 @@ def test_django_login_failure_user_doesnt_exists(client, test_spans, tracer): login_span = test_spans.find_span(name="django.contrib.auth.login") assert login_span.get_tag("appsec.events.users.login.failure.track") == "true" assert login_span.get_tag(APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC + ".failure." + user.ID) == "missing" - assert login_span.get_tag(APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC + ".failure." + user.EXISTS) == "false" + ## TODO: Disabled until we have a proper way to detect whether the user exists + # assert login_span.get_tag(APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC + ".failure." + user.EXISTS) == "false" + assert login_span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_FAILURE_MODE) == "extended" + + +@pytest.mark.django_db +def test_django_login_failure_user_does_exist(client, test_spans, tracer): + from django.contrib.auth import get_user + from django.contrib.auth.models import User + + with override_global_config(dict(_asm_enabled=True, _automatic_login_events_mode="extended")): + test_user = User.objects.create(username="fred") + test_user.set_password("secret") + test_user.save() + assert not get_user(client).is_authenticated + client.login(username="fred", password="wrong") + assert not get_user(client).is_authenticated + login_span = test_spans.find_span(name="django.contrib.auth.login") + assert login_span.get_tag("appsec.events.users.login.failure.track") == "true" + assert login_span.get_tag(APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC + ".failure." + user.ID) == "fred" + ## TODO: Disabled until we have a proper way to detect whether the user exists + # assert login_span.get_tag(APPSEC.USER_LOGIN_EVENT_PREFIX_PUBLIC + ".failure." + user.EXISTS) == "true" assert login_span.get_tag(APPSEC.AUTO_LOGIN_EVENTS_FAILURE_MODE) == "extended"