Skip to content
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 account lockout test #3406

Draft
wants to merge 1 commit into
base: 4.x
Choose a base branch
from
Draft

Conversation

kariuwu
Copy link
Contributor

@kariuwu kariuwu commented Apr 23, 2024

No description provided.

@kariuwu kariuwu requested a review from mmuzikar April 23, 2024 20:17
Copy link

github-actions bot commented Apr 23, 2024

Test Results

  4 files  ±0    4 suites  ±0   14m 48s ⏱️ +28s
 66 tests +1   65 ✅ +1   1 💤 ±0  0 ❌ ±0 
268 runs  +4  258 ✅ +4  10 💤 ±0  0 ❌ ±0 

Results for commit eac59bb. ± Comparison against base commit 758fdb9.

♻️ This comment has been updated with latest results.

@hawtio-ci
Copy link

hawtio-ci bot commented Apr 23, 2024

Test results

Run attempt: 1568
Detailed summary

NAME TESTS PASSED ✅ SKIPPED 💤 FAILED ❌ ERRORS 🚫 TIME 🕖
results-quarkus-17-firefox 67 65 2 0 0 221.96
results-quarkus-21-firefox 67 65 2 0 0 218.387
results-springboot-17-firefox 67 64 3 0 0 225.562
results-springboot-21-firefox 67 64 3 0 0 223.023

Comment on lines 32 to 34
String pass = "gojo";
int maxRetries = 7;
int retries = 0;
while (!warning.isDisplayed() && retries < maxRetries) {
Selenide.Wait().until(ExpectedConditions.visibilityOf(loginButton));
loginInput.shouldBe(editable).setValue(username);
passwordInput.shouldBe(editable).setValue(pass);
loginButton.shouldBe(enabled).click();
retries++;
}


Selenide.Wait().until(ExpectedConditions.visibilityOf(loginButton));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create another method to test the throttling behavior and create a Cucumber scenario for it.
We should be testing that the throttling time is increasing (2 or 3 attempts is enough).

Copy link
Contributor

@mmuzikar mmuzikar left a comment

Choose a reason for hiding this comment

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

@kariuwu please change this to a cucumber scenario


public class ThrottlingStepDefs {
private final LoginPage loginPage = new LoginPage();
private final static SelenideElement warning = $("p.pf-c-form__helper-text.pf-m-error");
Copy link
Contributor

Choose a reason for hiding this comment

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

It's preferable for this to be just By with the selector and you call the selenide function $ when you need to access the element. This is because sometimes the elements on the webpage can change and leads to Selenide throwing an exception that the reference to the element is stale, it's rare but annoying 😸

Also it's best practice to name constants in ALL_UPPERCASE, in this case warning is a constant.

Comment on lines 44 to 43
@Then("the user should see a message indicating account lockout second time")
public void theUserShouldSeeAMessageIndicatingAccountLockoutSecondTime() {
warning.shouldHave(text("Login attempt blocked. Retry after 3 seconds"));
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make this a universal step "the user should see a message indicating account lockout for {int} seconds" and format the text condition to contain the int parameter.

@kariuwu kariuwu force-pushed the kariuwu-authen branch 2 times, most recently from c616efd to 8d24375 Compare May 20, 2024 12:49
Comment on lines 23 to 29
@When("the user attempts to log in with incorrect credentials {int} times")
public void theUserAttemptsToLogInWithIncorrectCredentialsTimes(int attempts) {
for (int i = 0; i < attempts; i++) {
loginPage.throttling("username", "invalid");
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

The same step is on line 37, the only difference is just credentials -> credential, one can be removed

@@ -0,0 +1,8 @@
Feature: Account Lockout (Throttling)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would but the throttling.feature file in some other folder. This implies there will be more throttling feature files to come which it hopefully wont. What about security/auth folder?

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.

None yet

3 participants