Skip to content

Conversation

@rjd15372
Copy link
Member

@rjd15372 rjd15372 commented Jun 20, 2025

In this commit we introduce a new module API event called ValkeyModuleEvent_AuthenticationAttempt to track successful/failed authentication attempts.

This event will fill a struct, called ValkeyModuleAuthenticationInfo, with the client ID of the connection, the username, the module name that handle the authentication event, and the result of the authentication attempt.

Fixes: #2211

In this commit we introduce a new module API event called
`ValkeyModuleEvent_AuthenticationAttempt` to track successful/failed
authentication attempts.

This event will fill a struct, called `ValkeyModuleAuthenticationInfo`,
with the client ID of the connection, the IP address and port, the
username, the module name  that handle the authentication event, and
the result of the authentication attempt.

Fixes: valkey-io#2211

Signed-off-by: Ricardo Dias <[email protected]>
@rjd15372 rjd15372 requested a review from madolson June 20, 2025 09:15
Signed-off-by: Ricardo Dias <[email protected]>
@codecov
Copy link

codecov bot commented Jun 20, 2025

Codecov Report

❌ Patch coverage is 23.52941% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.22%. Comparing base (a739531) to head (36c9f5e).
⚠️ Report is 108 commits behind head on unstable.

Files with missing lines Patch % Lines
src/module.c 7.14% 26 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2237      +/-   ##
============================================
+ Coverage     71.41%   72.22%   +0.81%     
============================================
  Files           123      126       +3     
  Lines         67139    70659    +3520     
============================================
+ Hits          47947    51036    +3089     
- Misses        19192    19623     +431     
Files with missing lines Coverage Δ
src/acl.c 90.61% <100.00%> (+0.02%) ⬆️
src/module.h 0.00% <ø> (ø)
src/server.h 100.00% <ø> (ø)
src/module.c 9.70% <7.14%> (+0.14%) ⬆️

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Ricardo Dias <[email protected]>
- Used an enumeration for the authentication result
- removed IP and port from auth info structure
- added tests to test the module blocking authentication

Signed-off-by: Ricardo Dias <[email protected]>
@rjd15372
Copy link
Member Author

rjd15372 commented Jul 9, 2025

@madolson can we get this in for 9.0?

@madolson madolson moved this to Todo in Valkey 9.0 Jul 21, 2025
@madolson madolson moved this from Todo to Optional for next release candidate in Valkey 9.0 Jul 21, 2025
@rjd15372 rjd15372 requested a review from enjoy-binbin July 25, 2025 08:29
@enjoy-binbin enjoy-binbin added release-notes This issue should get a line item in the release notes run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) major-decision-pending Major decision pending by TSC team labels Jul 30, 2025
Copy link
Member

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall.

@madolson madolson moved this from Optional for next release candidate to Todo in Valkey 9.0 Aug 6, 2025
@madolson madolson moved this from Todo to In Progress in Valkey 9.0 Aug 6, 2025
@zuiderkwast zuiderkwast added major-decision-approved Major decision approved by TSC team and removed major-decision-pending Major decision pending by TSC team labels Aug 25, 2025
Signed-off-by: Ricardo Dias <[email protected]>
@rjd15372 rjd15372 merged commit b44de37 into valkey-io:unstable Sep 1, 2025
70 of 71 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Valkey 9.0 Sep 1, 2025
rjd15372 added a commit to rjd15372/valkey that referenced this pull request Sep 19, 2025
…2237)

In this commit we introduce a new module API event called
`ValkeyModuleEvent_AuthenticationAttempt` to track successful/failed
authentication attempts.

This event will fill a struct, called `ValkeyModuleAuthenticationInfo`,
with the client ID of the connection, the username, the module name that
handle the authentication event, and the result of the authentication
attempt.

Fixes: valkey-io#2211

---------

Signed-off-by: Ricardo Dias <[email protected]>
rjd15372 added a commit that referenced this pull request Sep 23, 2025
In this commit we introduce a new module API event called
`ValkeyModuleEvent_AuthenticationAttempt` to track successful/failed
authentication attempts.

This event will fill a struct, called `ValkeyModuleAuthenticationInfo`,
with the client ID of the connection, the username, the module name that
handle the authentication event, and the result of the authentication
attempt.

Fixes: #2211

---------

Signed-off-by: Ricardo Dias <[email protected]>
hpatro pushed a commit to hpatro/valkey that referenced this pull request Oct 3, 2025
…2237)

In this commit we introduce a new module API event called
`ValkeyModuleEvent_AuthenticationAttempt` to track successful/failed
authentication attempts.

This event will fill a struct, called `ValkeyModuleAuthenticationInfo`,
with the client ID of the connection, the username, the module name that
handle the authentication event, and the result of the authentication
attempt.

Fixes: valkey-io#2211

---------

Signed-off-by: Ricardo Dias <[email protected]>
Signed-off-by: Harkrishn Patro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

major-decision-approved Major decision approved by TSC team release-notes This issue should get a line item in the release notes run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[NEW] Extend module-API with Authentication result

6 participants