keycloak-aplcache
Changes
services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpUsernamePasswordForm.java 2(+1 -1)
services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java 51(+32 -19)
Details
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpUsernamePasswordForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpUsernamePasswordForm.java
index b293d1b..5e732d8 100644
--- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpUsernamePasswordForm.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpUsernamePasswordForm.java
@@ -37,7 +37,7 @@ public class IdpUsernamePasswordForm extends UsernamePasswordForm {
// Restore formData for the case of error
setupForm(context, formData, existingUser);
- return validatePassword(context, formData);
+ return validatePassword(context, existingUser, formData);
}
protected LoginFormsProvider setupForm(AuthenticationFlowContext context, MultivaluedMap<String, String> formData, UserModel existingUser) {
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java
index 294d220..0506f21 100755
--- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java
@@ -71,12 +71,16 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse);
return true;
}
+ return false;
+ }
+
+ public boolean enabledUser(AuthenticationFlowContext context, UserModel user) {
if (!user.isEnabled()) {
context.getEvent().user(user);
context.getEvent().error(Errors.USER_DISABLED);
Response challengeResponse = disabledUser(context);
context.failureChallenge(AuthenticationFlowError.USER_DISABLED, challengeResponse);
- return true;
+ return false;
}
if (context.getRealm().isBruteForceProtected()) {
if (context.getProtector().isTemporarilyDisabled(context.getSession(), context.getRealm(), user.getUsername())) {
@@ -84,13 +88,13 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
context.getEvent().error(Errors.USER_TEMPORARILY_DISABLED);
Response challengeResponse = temporarilyDisabledUser(context);
context.failureChallenge(AuthenticationFlowError.USER_TEMPORARILY_DISABLED, challengeResponse);
- return true;
+ return false;
}
}
- return false;
+ return true;
}
- public boolean validateUser(AuthenticationFlowContext context, MultivaluedMap<String, String> inputData) {
+ public boolean validateUserAndPassword(AuthenticationFlowContext context, MultivaluedMap<String, String> inputData) {
String username = inputData.getFirst(AuthenticationManager.FORM_USERNAME);
if (username == null) {
context.getEvent().error(Errors.USER_NOT_FOUND);
@@ -117,7 +121,18 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
return false;
}
- if (invalidUser(context, user)) return false;
+ if (invalidUser(context, user)){
+ return false;
+ }
+
+ if (!validatePassword(context, user, inputData)){
+ return false;
+ }
+
+ if(!enabledUser(context, user)){
+ return false;
+ }
+
String rememberMe = inputData.getFirst("rememberMe");
boolean remember = rememberMe != null && rememberMe.equalsIgnoreCase("on");
if (remember) {
@@ -130,29 +145,27 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
return true;
}
- public boolean validatePassword(AuthenticationFlowContext context, MultivaluedMap<String, String> inputData) {
+ public boolean validatePassword(AuthenticationFlowContext context, UserModel user, MultivaluedMap<String, String> inputData) {
List<UserCredentialModel> credentials = new LinkedList<>();
String password = inputData.getFirst(CredentialRepresentation.PASSWORD);
if (password == null || password.isEmpty()) {
- if (context.getUser() != null) {
- context.getEvent().user(context.getUser());
- }
- context.getEvent().error(Errors.INVALID_USER_CREDENTIALS);
- Response challengeResponse = invalidCredentials(context);
- context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse);
- context.clearUser();
+ invalidPassword(context, user);
return false;
}
credentials.add(UserCredentialModel.password(password));
- boolean valid = context.getSession().users().validCredentials(context.getRealm(), context.getUser(), credentials);
+ boolean valid = context.getSession().users().validCredentials(context.getRealm(), user, credentials);
if (!valid) {
- context.getEvent().user(context.getUser());
- context.getEvent().error(Errors.INVALID_USER_CREDENTIALS);
- Response challengeResponse = invalidCredentials(context);
- context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse);
- context.clearUser();
+ invalidPassword(context, user);
return false;
}
return true;
}
+
+ private void invalidPassword(AuthenticationFlowContext context, UserModel user) {
+ context.getEvent().user(user);
+ context.getEvent().error(Errors.INVALID_USER_CREDENTIALS);
+ Response challengeResponse = invalidCredentials(context);
+ context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse);
+ context.clearUser();
+ }
}
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java
index 70d9fd9..7463638 100755
--- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java
@@ -38,7 +38,7 @@ public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator impl
}
protected boolean validateForm(AuthenticationFlowContext context, MultivaluedMap<String, String> formData) {
- return validateUser(context, formData) && validatePassword(context, formData);
+ return validateUserAndPassword(context, formData);
}
@Override
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java
index 950c182..30d94fa 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java
@@ -215,9 +215,10 @@ public class LoginTest {
Assert.assertEquals("login-test", loginPage.getUsername());
Assert.assertEquals("", loginPage.getPassword());
- Assert.assertEquals("Account is disabled, contact admin.", loginPage.getError());
+ // KEYCLOAK-2024
+ Assert.assertEquals("Invalid username or password.", loginPage.getError());
- events.expectLogin().user(userId).session((String) null).error("user_disabled")
+ events.expectLogin().user(userId).session((String) null).error("invalid_user_credentials")
.detail(Details.USERNAME, "login-test")
.removeDetail(Details.CONSENT)
.assertEvent();
@@ -250,6 +251,7 @@ public class LoginTest {
Assert.assertEquals("login-test", loginPage.getUsername());
Assert.assertEquals("", loginPage.getPassword());
+ // KEYCLOAK-2024
Assert.assertEquals("Account is disabled, contact admin.", loginPage.getError());
events.expectLogin().user(userId).session((String) null).error("user_disabled")