Skip to content

Tests related to brute force attack prevention.#2245

Merged
DarshitChanpura merged 3 commits intoopensearch-project:mainfrom
eliatra:brute-force-attacks-prevention-tests
Nov 21, 2022
Merged

Tests related to brute force attack prevention.#2245
DarshitChanpura merged 3 commits intoopensearch-project:mainfrom
eliatra:brute-force-attacks-prevention-tests

Conversation

@lukasz-soszynski-eliatra
Copy link
Copy Markdown
Contributor

Signed-off-by: Lukasz Soszynski lukasz.soszynski@eliatra.com

Description

[Describe what this change achieves]

  • Category (Integration tests)
  • Why these changes are required?
  • What is the old behaviour before changes and new behaviour after changes?

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

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.

@lukasz-soszynski-eliatra lukasz-soszynski-eliatra requested a review from a team November 8, 2022 12:55
@DarshitChanpura
Copy link
Copy Markdown
Member

Bwc tests are broken on main. Tracking issue here: #2221

@lukasz-soszynski-eliatra lukasz-soszynski-eliatra force-pushed the brute-force-attacks-prevention-tests branch from 3585cb3 to a9cec7b Compare November 10, 2022 13:09
Signed-off-by: Lukasz Soszynski <lukasz.soszynski@eliatra.com>
Signed-off-by: Lukasz Soszynski <lukasz.soszynski@eliatra.com>
@lukasz-soszynski-eliatra lukasz-soszynski-eliatra force-pushed the brute-force-attacks-prevention-tests branch from a9cec7b to 3f4a665 Compare November 11, 2022 11:13
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 11, 2022

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.16%. Comparing base (93fe633) to head (c95fbdb).
⚠️ Report is 1373 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2245      +/-   ##
============================================
+ Coverage     61.02%   61.16%   +0.13%     
- Complexity     3267     3274       +7     
============================================
  Files           259      259              
  Lines         18337    18337              
  Branches       3248     3248              
============================================
+ Hits          11191    11216      +25     
+ Misses         5561     5536      -25     
  Partials       1585     1585              

see 6 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.


HttpResponse response = client.getAuthInfo();

response.assertStatusCode(SC_UNAUTHORIZED);
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice

hcb.setConnectionManager(cm);
if(routePlanner != null) {
hcb.setRoutePlanner(routePlanner);
}
Copy link
Copy Markdown
Member

@DarshitChanpura DarshitChanpura Nov 18, 2022

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

Copy link
Copy Markdown
Contributor Author

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>
Copy link
Copy Markdown
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Member

@cwperks cwperks left a 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() {
Copy link
Copy Markdown
Member

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() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

@DarshitChanpura DarshitChanpura merged commit 25ea092 into opensearch-project:main Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants