Merge pull request #3841 from DataDog/fix-track-login-failure

Make user_id optional for track_login_failure

lib/datadog/kit/appsec/events.rb

@@ -53,13 +53,11 @@ def track_login_success(trace = nil, span = nil, user:, **others)
           # @param user_exists [bool] Whether the user id that did a login attempt exists.
           # @param others [Hash<String || Symbol, String>] Additional free-form
           #   event information to attach to the trace.
-          def track_login_failure(trace = nil, span = nil, user_id:, user_exists:, **others)
+          def track_login_failure(trace = nil, span = nil, user_exists:, user_id: nil, **others)
             set_trace_and_span_context('track_login_failure', trace, span) do |active_trace, active_span|
-              raise ArgumentError, 'user_id cannot be nil' if user_id.nil?
-
               track(LOGIN_FAILURE_EVENT, active_trace, active_span, **others)
 
-              active_span.set_tag('appsec.events.users.login.failure.usr.id', user_id)
+              active_span.set_tag('appsec.events.users.login.failure.usr.id', user_id) if user_id
               active_span.set_tag('appsec.events.users.login.failure.usr.exists', user_exists)
             end
           end

spec/datadog/kit/appsec/events_spec.rb

@@ -120,20 +120,29 @@
       end
     end
 
-    it 'sets user non-existence  on trace' do
-      trace_op.measure('root') do |span, _trace|
-        described_class.track_login_failure(trace_op, user_id: '42', user_exists: false)
-        expect(span.tags).to include('appsec.events.users.login.failure.usr.exists' => 'false')
-      end
-    end
-
     it 'sets other keys on trace' do
       trace_op.measure('root') do |span, _trace|
         described_class.track_login_failure(trace_op, user_id: '42', user_exists: true, foo: 'bar')
         expect(span.tags).to include('appsec.events.users.login.failure.foo' => 'bar')
       end
     end
 
+    context 'when user does not exist' do
+      it 'sets user non-existence on trace' do
+        trace_op.measure('root') do |span, _trace|
+          described_class.track_login_failure(trace_op, user_exists: false)
+          expect(span.tags).to include('appsec.events.users.login.failure.usr.exists' => 'false')
+        end
+      end
+
+      it 'does not set user id on trace' do
+        trace_op.measure('root') do |span, _trace|
+          described_class.track_login_failure(trace_op, user_exists: false)
+          expect(span.tags).not_to have_key('appsec.events.users.login.failure.usr.id')
+        end
+      end
+    end
+
     it_behaves_like 'uses AppSec scope' do
       let(:event_tag) { 'appsec.events.users.login.failure.track' }
       subject(:event) { described_class.track_login_failure(trace_op, user_id: '42', user_exists: true) }