[KEYCLOAK-12044] Fix messages in the UsernameForm (#6548)

This commit is contained in:
Martin Bartoš 2019-12-11 10:59:46 +01:00 committed by Marek Posolda
parent 05493371ca
commit 2cf6483cdf
8 changed files with 109 additions and 5 deletions

View file

@ -104,7 +104,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
if (user == null) {
dummyHash(context);
context.getEvent().error(Errors.USER_NOT_FOUND);
Response challengeResponse = challenge(context, Messages.INVALID_USER);
Response challengeResponse = challenge(context, getDefaultChallengeMessage(context));
context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse);
}
}
@ -138,7 +138,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
String username = inputData.getFirst(AuthenticationManager.FORM_USERNAME);
if (username == null) {
context.getEvent().error(Errors.USER_NOT_FOUND);
Response challengeResponse = challenge(context, Messages.INVALID_USER);
Response challengeResponse = challenge(context, getDefaultChallengeMessage(context));
context.failureChallenge(AuthenticationFlowError.INVALID_USER, challengeResponse);
return null;
}
@ -193,7 +193,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
if (password == null || password.isEmpty()) {
context.getEvent().user(user);
context.getEvent().error(Errors.INVALID_USER_CREDENTIALS);
Response challengeResponse = challenge(context, Messages.INVALID_USER);
Response challengeResponse = challenge(context, getDefaultChallengeMessage(context));
context.forceChallenge(challengeResponse);
context.clearUser();
return false;
@ -206,7 +206,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
} else {
context.getEvent().user(user);
context.getEvent().error(Errors.INVALID_USER_CREDENTIALS);
Response challengeResponse = challenge(context, Messages.INVALID_USER);
Response challengeResponse = challenge(context, getDefaultChallengeMessage(context));
context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse);
if (clearUser) {
context.clearUser();
@ -228,4 +228,8 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth
}
return false;
}
protected String getDefaultChallengeMessage(AuthenticationFlowContext context) {
return Messages.INVALID_USER;
}
}

View file

@ -22,6 +22,7 @@ import org.keycloak.forms.login.LoginFormsProvider;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.services.messages.Messages;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
@ -53,4 +54,9 @@ public class PasswordForm extends UsernamePasswordForm {
protected Response createLoginForm(LoginFormsProvider form) {
return form.createLoginPassword();
}
@Override
protected String getDefaultChallengeMessage(AuthenticationFlowContext context){
return Messages.INVALID_PASSWORD;
}
}

View file

@ -19,6 +19,7 @@ package org.keycloak.authentication.authenticators.browser;
import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.forms.login.LoginFormsProvider;
import org.keycloak.services.messages.Messages;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
@ -43,4 +44,11 @@ public final class UsernameForm extends UsernamePasswordForm {
protected Response createLoginForm(LoginFormsProvider form) {
return form.createLoginUsername();
}
@Override
protected String getDefaultChallengeMessage(AuthenticationFlowContext context) {
if (context.getRealm().isLoginWithEmailAllowed())
return Messages.INVALID_USERNAME_OR_EMAIL;
return Messages.INVALID_USERNAME;
}
}

View file

@ -30,6 +30,7 @@ import org.keycloak.models.UserModel;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.messages.Messages;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
@ -111,4 +112,9 @@ public class UsernamePasswordForm extends AbstractUsernameFormAuthenticator impl
public PasswordCredentialProvider getCredentialProvider(KeycloakSession session) {
return (PasswordCredentialProvider)session.getProvider(CredentialProvider.class, "keycloak-password");
}
@Override
protected String getDefaultChallengeMessage(AuthenticationFlowContext context){
return Messages.INVALID_USER;
}
}

View file

@ -26,6 +26,12 @@ public class Messages {
public static final String INVALID_USER = "invalidUserMessage";
public static final String INVALID_USERNAME = "invalidUsernameMessage";
public static final String INVALID_USERNAME_OR_EMAIL = "invalidUsernameOrEmailMessage";
public static final String INVALID_PASSWORD = "invalidPasswordMessage";
public static final String INVALID_EMAIL = "invalidEmailMessage";
public static final String ACCOUNT_DISABLED = "accountDisabledMessage";

View file

@ -55,7 +55,7 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr
// Try bad password first
passwordPage.login("bad-password");
Assert.assertEquals("Invalid username or password.", passwordPage.getError());
Assert.assertEquals("Invalid password.", passwordPage.getError());
// Try good password
passwordPage.login("password");

View file

@ -1010,6 +1010,77 @@ public class BrowserFlowTest extends AbstractTestRealmKeycloakTest {
}
}
/**
* This test checks the error messages, when the credentials are invalid and UsernameForm and PasswordForm are separated.
*/
@Test
public void testLoginWithWrongCredentialsMessage() {
UserRepresentation user = testRealm().users().search("test-user@localhost").get(0);
Assert.assertNotNull(user);
loginPage.open();
loginPage.assertCurrent();
loginPage.login(user.getUsername(), "wrong_password");
Assert.assertEquals("Invalid username or password.", loginPage.getError());
events.clear();
loginPage.assertCurrent();
loginPage.login(user.getUsername(), "password");
Assert.assertFalse(loginPage.isCurrent());
events.expectLogin()
.user(user)
.detail(Details.USERNAME, "test-user@localhost")
.assertEvent();
}
/**
* This test checks the error messages, when the credentials are invalid and UsernameForm and PasswordForm are separated.
*/
@Test
public void testLoginMultiFactorWithWrongCredentialsMessage() {
UserRepresentation user = testRealm().users().search("test-user@localhost").get(0);
Assert.assertNotNull(user);
configureBrowserFlowWithAlternativeCredentials();
try {
RealmRepresentation realm = testRealm().toRepresentation();
realm.setLoginWithEmailAllowed(false);
testRealm().update(realm);
loginUsernameOnlyPage.open();
loginUsernameOnlyPage.assertCurrent();
loginUsernameOnlyPage.login("non_existing_user");
Assert.assertEquals("Invalid username.", loginUsernameOnlyPage.getError());
realm.setLoginWithEmailAllowed(true);
testRealm().update(realm);
loginUsernameOnlyPage.login("non_existing_user");
Assert.assertEquals("Invalid username or email.", loginUsernameOnlyPage.getError());
loginUsernameOnlyPage.login(user.getUsername());
passwordPage.assertCurrent();
passwordPage.login("wrong_password");
Assert.assertEquals("Invalid password.", passwordPage.getError());
passwordPage.assertCurrent();
events.clear();
passwordPage.login("password");
Assert.assertFalse(loginUsernameOnlyPage.isCurrent());
Assert.assertFalse(passwordPage.isCurrent());
events.expectLogin()
.user(user)
.detail(Details.USERNAME, "test-user@localhost")
.assertEvent();
} finally {
revertFlows("browser - alternative");
}
}
/**
* This flow contains:
* UsernameForm REQUIRED

View file

@ -157,6 +157,9 @@ client_realm-management=Realm Management
client_broker=Broker
invalidUserMessage=Invalid username or password.
invalidUsernameMessage=Invalid username.
invalidUsernameOrEmailMessage=Invalid username or email.
invalidPasswordMessage=Invalid password.
invalidEmailMessage=Invalid email address.
accountDisabledMessage=Account is disabled, contact your administrator.
accountTemporarilyDisabledMessage=Account is temporarily disabled; contact your administrator or retry later.