-
Notifications
You must be signed in to change notification settings - Fork 261
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
Tests related to brute force attack prevention. #2245
Tests related to brute force attack prevention. #2245
Conversation
Bwc tests are broken on main. Tracking issue here: #2221 |
3585cb3
to
a9cec7b
Compare
Signed-off-by: Lukasz Soszynski <lukasz.soszynski@eliatra.com>
Signed-off-by: Lukasz Soszynski <lukasz.soszynski@eliatra.com>
a9cec7b
to
3f4a665
Compare
Codecov Report
@@ Coverage Diff @@
## main #2245 +/- ##
============================================
- Coverage 61.02% 61.00% -0.03%
+ Complexity 3267 3265 -2
============================================
Files 259 259
Lines 18337 18336 -1
Branches 3248 3248
============================================
- Hits 11191 11185 -6
Misses 5561 5561
- Partials 1585 1590 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
||
HttpResponse response = client.getAuthInfo(); | ||
|
||
response.assertStatusCode(SC_UNAUTHORIZED); |
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.
Out of curiosity, how do you test that an IP is blocked after multiple failed logins? I mean, that end user will still see UNAUTHORIZED error but on server side how do we know that the IP is blocked?
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.
I added an explicit check of logs in the commit c95fbdb
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.
nice
@@ -50,6 +57,9 @@ public CloseableHttpClient getHTTPClient() { | |||
.setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(60, TimeUnit.SECONDS).build()) | |||
.build(); | |||
hcb.setConnectionManager(cm); | |||
if(routePlanner != null) { | |||
hcb.setRoutePlanner(routePlanner); | |||
} |
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.
out of curiosity, why do we need routePlanner? i see it being used in TestRestClient, but couldn't understand the purpose completely
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.
In order to set the request source IP address, this is done by org.opensearch.test.framework.cluster.LocalAddressRoutePlanner
which implements HttpRoutePlanner
.
The Apache HTTP Client in version 4 contained a convenient method to set the request source IP address RequestConfig.custom().setLocalAddress(inetAddress).build();
. But the method was removed in version 5 of Apache HTTP Client
…ventionTests extended to verify unauthorized response reason. Signed-off-by: Lukasz Soszynski <lukasz.soszynski@eliatra.com>
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.
LGTM!
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.
Looks good to me!
} | ||
|
||
@Test | ||
public void shouldBlockIpWhenFailureAuthenticationCountIsGraterThanAllowedTries() { |
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.
nit: Greater
} | ||
|
||
@Test | ||
public void shouldBlockUserWhenNumberOfFailureLoginAttemptIsGraterThanLimit() { |
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.
Same here.
Signed-off-by: Lukasz Soszynski lukasz.soszynski@eliatra.com
Description
[Describe what this change achieves]
Test related to brute-force attack prevention and a minor correction for TlsTests.
Issues Resolved
[List any issues this PR will resolve]
Is this a backport? If so, please add backport PR # and/or commits #
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
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.