Skip to content

Make user_id optional for track_login_failure#3841

Merged
y9v merged 1 commit into
masterfrom
fix-track-login-failure
Aug 12, 2024
Merged

Make user_id optional for track_login_failure#3841
y9v merged 1 commit into
masterfrom
fix-track-login-failure

Conversation

@y9v

@y9v y9v commented Aug 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #3840.

What does this PR do?
This PR fixes a bug when Datadog::Kit::AppSec::Events.track_login_failure is raising an ArgumentError when user_id is not provided and user_exists flag is set to false.

Motivation:
Issue #3840

@y9v
Make user_id optional for track_login_failure
44834a4 
Fixes #3840.

`Datadog::Kit::AppSec::Events.track_login_failure` should not raise
when `user_id` is not provided and `user_exists` flag set to false.
@y9v y9v self-assigned this Aug 12, 2024
@y9v
y9v requested a review from a team as a code owner August 12, 2024 14:34
@codecov-commenter

codecov-commenter commented Aug 12, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.83%. Comparing base (46c8c8b) to head (44834a4).
Report is 3 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #3841   +/-   ##
=======================================
  Coverage   97.83%   97.83%           
=======================================
  Files        1264     1264           
  Lines       75678    75682    +4     
  Branches     3720     3720           
=======================================
+ Hits        74039    74045    +6     
+ Misses       1639     1637    -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pr-commenter

pr-commenter Bot commented Aug 12, 2024

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2024-08-12 15:09:08

Comparing candidate commit 44834a4 in PR branch fix-track-login-failure with baseline commit 46c8c8b in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 22 metrics, 2 unstable metrics.

scenario:hold / resume

  • 🟩 throughput [+50457.227op/s; +52408.229op/s] or [+2.453%; +2.548%]

@y9v

y9v commented Aug 12, 2024

Copy link
Copy Markdown
Member Author

/merge

@dd-devflow

dd-devflow Bot commented Aug 12, 2024

Copy link
Copy Markdown

🚂 MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

@y9v

y9v commented Aug 12, 2024

Copy link
Copy Markdown
Member Author

/merge -c

@dd-devflow

dd-devflow Bot commented Aug 12, 2024

Copy link
Copy Markdown

⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

If you need support, contact us on Slack #devflow!

@y9v
y9v merged commit 6b50227 into master Aug 12, 2024
@y9v
y9v deleted the fix-track-login-failure branch August 12, 2024 18:37
@github-actions github-actions Bot added this to the 2.3.0 milestone Aug 12, 2024
@TonyCTHsu TonyCTHsu added the appsec Application Security monitoring product label Aug 14, 2024
@TonyCTHsu TonyCTHsu mentioned this pull request Aug 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@p-datadog p-datadog p-datadog approved these changes

Assignees

@y9v y9v

Labels

appsec Application Security monitoring product mergequeue-status: removed

Projects

None yet

Milestone

2.3.0

Development

Successfully merging this pull request may close these issues.

user_id cannot be nil when manually triggering login failure appsec events

4 participants

@y9v @codecov-commenter @p-datadog @TonyCTHsu