libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

KEYCLOAK-13054 Unblock temporarily disabled user on password reset, and remove invalid error message

Browse source
This commit is contained in:
stianst 2020-02-21 08:33:59 +01:00 committed by Stian Thorgersen
parent de8ba75399
commit 950eae090f
3 changed files with 94 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java
View file

@ -652,22 +652,6 @@ public class AuthenticationProcessor {
}
}
protected void logSuccess() {
if (realm.isBruteForceProtected()) {
String username = authenticationSession.getAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME);
// TODO: as above, need to handle non form success
if(username == null) {
return;
}
UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username);
if (user != null) {
getBruteForceProtector().successfulLogin(realm, user, connection);
}
}
}
public boolean isSuccessful(AuthenticationExecutionModel model) {
AuthenticationSessionModel.ExecutionStatus status = authenticationSession.getExecutionStatus().get(model.getId());
if (status == null) return false;
@ -1077,12 +1061,6 @@ public class AuthenticationProcessor {
public void validateUser(UserModel authenticatedUser) {
if (authenticatedUser == null) return;
if (!authenticatedUser.isEnabled()) throw new AuthenticationFlowException(AuthenticationFlowError.USER_DISABLED);
if (realm.isBruteForceProtected() && !realm.isPermanentLockout()) {
if (getBruteForceProtector().isTemporarilyDisabled(session, realm, authenticatedUser)) {
getEvent().error(Errors.RESET_CREDENTIAL_DISABLED);
ServicesLogger.LOGGER.passwordResetFailed(new AuthenticationFlowException(AuthenticationFlowError.USER_TEMPORARILY_DISABLED));
}
}
}
protected Response authenticationComplete() {
@ -1094,8 +1072,6 @@ public class AuthenticationProcessor {
return AuthenticationManager.redirectToRequiredActions(session, realm, authenticationSession, uriInfo, nextRequiredAction);
} else {
event.detail(Details.CODE_ID, authenticationSession.getParentSession().getId()); // todo This should be set elsewhere. find out why tests fail. Don't know where this is supposed to be set
// the user has successfully logged in and we can clear his/her previous login failure attempts.
logSuccess();
return AuthenticationManager.finishedRequiredActions(session, authenticationSession, userSession, connection, request, uriInfo, event);
}
}

23
services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
View file

@ -33,6 +33,7 @@ import org.keycloak.authentication.RequiredActionContextResult;
import org.keycloak.authentication.RequiredActionFactory;
import org.keycloak.authentication.RequiredActionProvider;
import org.keycloak.authentication.actiontoken.DefaultActionTokenKey;
import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator;
import org.keycloak.broker.provider.IdentityProvider;
import org.keycloak.common.ClientConnection;
import org.keycloak.common.VerificationException;
@ -797,6 +798,9 @@ public class AuthenticationManager {
clientSession.removeNote(SSO_AUTH);
}
// The user has successfully logged in and we can clear his/her previous login failure attempts.
logSuccess(session, authSession);
return protocol.authenticated(authSession, userSession, clientSessionCtx);
}
@ -1306,4 +1310,23 @@ public class AuthenticationManager {
}
}
protected static void logSuccess(KeycloakSession session, AuthenticationSessionModel authSession) {
RealmModel realm = session.getContext().getRealm();
if (realm.isBruteForceProtected()) {
String username = authSession.getAuthNote(AbstractUsernameFormAuthenticator.ATTEMPTED_USERNAME);
// TODO: as above, need to handle non form success
if(username == null) {
return;
}
UserModel user = KeycloakModelUtils.findUserByNameOrEmail(session, realm, username);
if (user != null) {
BruteForceProtector bruteForceProtector = session.getProvider(BruteForceProtector.class);
bruteForceProtector.successfulLogin(realm, user, session.getContext().getConnection());
}
}
}
}

112
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java
View file

@ -27,29 +27,33 @@ import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.models.Constants;
import org.keycloak.models.utils.TimeBasedOTP;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.AssertEvents.ExpectedEvent;
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.AppPage.RequestType;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.LoginPasswordResetPage;
import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
import org.keycloak.testsuite.pages.LoginTotpPage;
import org.keycloak.testsuite.pages.RegisterPage;
import org.keycloak.testsuite.util.GreenMailRule;
import org.keycloak.testsuite.util.MailUtils;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.RealmRepUtil;
import org.keycloak.testsuite.util.UserBuilder;
import javax.mail.internet.MimeMessage;
import java.net.MalformedURLException;
import java.util.Collections;
import static org.junit.Assert.assertEquals;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.REMOTE;
/**
@ -61,6 +65,34 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
private static String userId;
@Rule
public AssertEvents events = new AssertEvents(this);
@Rule
public GreenMailRule greenMail = new GreenMailRule();
@Page
protected AppPage appPage;
@Page
protected LoginPage loginPage;
@Page
protected LoginPasswordResetPage passwordResetPage;
@Page
protected LoginPasswordUpdatePage passwordUpdatePage;
@Page
private RegisterPage registerPage;
@Page
protected LoginTotpPage loginTotpPage;
private TimeBasedOTP totp = new TimeBasedOTP();
private int lifespan;
@Override
public void configureTestRealm(RealmRepresentation testRealm) {
UserRepresentation user = RealmRepUtil.findUser(testRealm, "test-user@localhost");
@ -118,32 +150,6 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
Thread.sleep(100);
}
@Rule
public AssertEvents events = new AssertEvents(this);
@Rule
public GreenMailRule greenMail = new GreenMailRule();
@Page
protected AppPage appPage;
@Page
protected LoginPage loginPage;
@Page
protected LoginPasswordResetPage resetPasswordPage;
@Page
private RegisterPage registerPage;
@Page
protected LoginTotpPage loginTotpPage;
private TimeBasedOTP totp = new TimeBasedOTP();
private int lifespan;
@Before
public void before() throws MalformedURLException {
totp = new TimeBasedOTP();
@ -411,7 +417,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
// assert
expectPermanentlyDisabled();
Assert.assertFalse(adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0).isEnabled());
assertFalse(adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0).isEnabled());
} finally {
realm.setPermanentLockout(false);
testRealm().update(realm);
@ -437,7 +443,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
loginSuccess();
// assert
Assert.assertTrue(adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0).isEnabled());
assertTrue(adminClient.realm("test").users().search("test-user@localhost", 0, 1).get(0).isEnabled());
} finally {
realm.setPermanentLockout(false);
testRealm().update(realm);
@ -458,24 +464,48 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest {
@Test
@AuthServerContainerExclude(REMOTE) // GreenMailRule is not working atm
public void testResetPassword() throws Exception {
String userId = adminClient.realm("test").users().search("test-user@localhost", null, null, null, 0, 1).get(0).getId();
String userId = adminClient.realm("test").users().search("user2", null, null, null, 0, 1).get(0).getId();
loginSuccess();
loginInvalidPassword();
loginInvalidPassword();
expectTemporarilyDisabled();
expectTemporarilyDisabled("test-user@localhost", null, "invalid");
loginSuccess("user2");
loginInvalidPassword("user2");
loginInvalidPassword("user2");
expectTemporarilyDisabled("user2", userId, "invalid");
loginPage.resetPassword();
resetPasswordPage.assertCurrent();
resetPasswordPage.changePassword("test-user@localhost");
passwordResetPage.assertCurrent();
passwordResetPage.changePassword("user2");
loginPage.assertCurrent();
assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage());
events.expectRequiredAction(EventType.RESET_PASSWORD_ERROR).user(userId);
events.expectRequiredAction(EventType.SEND_RESET_PASSWORD).user(userId).assertEvent();
MimeMessage message = greenMail.getReceivedMessages()[0];
String passwordResetEmailLink = MailUtils.getPasswordResetEmailLink(message);
driver.navigate().to(passwordResetEmailLink.trim());
assertTrue(passwordUpdatePage.isCurrent());
UserRepresentation userRepresentation = testRealm().users().get(userId).toRepresentation();
assertFalse(userRepresentation.isEnabled());
updatePasswordPage.updatePasswords("password", "password");
events.expectRequiredAction(EventType.UPDATE_PASSWORD).user(userId).assertEvent();
userRepresentation = testRealm().users().get(userId).toRepresentation();
assertTrue(userRepresentation.isEnabled());
Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());
appPage.logout();
events.clear();
clearUserFailures();
loginSuccess("user2");
}
public void expectTemporarilyDisabled() throws Exception {