-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ignore_hosts
config option for auth failure listener
#4538
Add ignore_hosts
config option for auth failure listener
#4538
Conversation
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding this enhancement @cwperks. What would happen in a scenario where a user is blocked but the IP address is allowed and that user requests through this IP address?
Thanks for the changes! However, this would not fully resolve #4262 as this would completely disable rate-limiting for a particular IP. For example, as a user of dashboards, I would like that 'X-Forwarded-For' is used to rate-limit the clients and not disable my dashboards IP. |
@shikharj05 Understood, this PR only partially resolves the associated issue by implementing one of the new configuration options described in the issue. |
It would be blocked by the username and I can add a test to prove it. IP Address blocking is performed before authentication here: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/auth/BackendRegistry.java#L202 Username block is performed after authentication here:https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/auth/BackendRegistry.java#L279 |
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
This PR makes this config value only applicable to IP-based auth failure listener. It could be extended to all types of auth failure listener, but I chose to apply it only to IP-based ones |
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
src/main/java/org/opensearch/security/auth/limiting/AbstractRateLimiter.java
Show resolved
Hide resolved
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4538 +/- ##
==========================================
- Coverage 65.28% 65.26% -0.03%
==========================================
Files 317 317
Lines 22272 22296 +24
Branches 3582 3588 +6
==========================================
+ Hits 14541 14551 +10
- Misses 5939 5951 +12
- Partials 1792 1794 +2
|
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
bc92a89
into
opensearch-project:main
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-4538-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 bc92a8900ea147f92eed0c69eb9e787f4a5d3a65
# Push it to GitHub
git push --set-upstream origin backport/backport-4538-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x Then, create a pull request where the |
@cwperks Would you create a manual backport? |
…h-project#4538) Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit bc92a89)
Description
This PR adds a new configuration for IP-based auth failure listener to allow a cluster admin to specify a list of hosts where rate limiting should not apply. If requests originate from a host matching one of the
ignore_hosts
then the auth failure listener would not apply to the request.Companion Documentation PR: opensearch-project/documentation-website#7859
Enhancement
Issues Resolved
Partially resolves #4262
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.