From 2910db559584a45f1bcfef70e86daa16aaeb525e Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Fri, 16 Oct 2015 11:23:54 +0200 Subject: [PATCH] KEYCLOAK-1973 Clear user from authentication context is password is not valid --- .../AuthenticationFlowContext.java | 5 ++ .../AuthenticationProcessor.java | 10 ++++ .../AbstractUsernameFormAuthenticator.java | 2 + .../keycloak/testsuite/forms/LoginTest.java | 51 +++++++++++++++++-- 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java b/services/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java index 681e76c484..9f69e636e0 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationFlowContext.java @@ -32,6 +32,11 @@ public interface AuthenticationFlowContext extends AbstractAuthenticationFlowCon */ void setUser(UserModel user); + /** + * Clear the user from the flow. + */ + void clearUser(); + void attachUserSession(UserSessionModel userSession); diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 35b2ab94c8..2d7f0ebc9c 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -215,6 +215,9 @@ public class AuthenticationProcessor { getClientSession().setAuthenticatedUser(user); } + public void clearAuthenticatedUser() { + getClientSession().setAuthenticatedUser(null); + } public class Result implements AuthenticationFlowContext, ClientAuthenticationFlowContext { AuthenticatorConfigModel authenticatorConfig; @@ -332,6 +335,8 @@ public class AuthenticationProcessor { } + + @Override public UserModel getUser() { return getClientSession().getAuthenticatedUser(); @@ -342,6 +347,11 @@ public class AuthenticationProcessor { setAutheticatedUser(user); } + @Override + public void clearUser() { + clearAuthenticatedUser(); + } + @Override public RealmModel getRealm() { return AuthenticationProcessor.this.getRealm(); 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 062d0ba0c8..294d220696 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 @@ -140,6 +140,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth context.getEvent().error(Errors.INVALID_USER_CREDENTIALS); Response challengeResponse = invalidCredentials(context); context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse); + context.clearUser(); return false; } credentials.add(UserCredentialModel.password(password)); @@ -149,6 +150,7 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth context.getEvent().error(Errors.INVALID_USER_CREDENTIALS); Response challengeResponse = invalidCredentials(context); context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse); + context.clearUser(); return false; } return true; 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 bb6292267e..de7fff48b4 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 @@ -66,19 +66,28 @@ public class LoginTest { @ClassRule public static KeycloakRule keycloakRule = new KeycloakRule(new KeycloakRule.KeycloakSetup() { + @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + UserCredentialModel creds = new UserCredentialModel(); + creds.setType(CredentialRepresentation.PASSWORD); + creds.setValue("password"); + UserModel user = manager.getSession().users().addUser(appRealm, "login-test"); user.setEmail("login@test.com"); user.setEnabled(true); userId = user.getId(); - UserCredentialModel creds = new UserCredentialModel(); - creds.setType(CredentialRepresentation.PASSWORD); - creds.setValue("password"); - user.updateCredential(creds); + + UserModel user2 = manager.getSession().users().addUser(appRealm, "login-test2"); + user2.setEmail("login2@test.com"); + user2.setEnabled(true); + + user2Id = user2.getId(); + + user2.updateCredential(creds); } }); @@ -108,6 +117,8 @@ public class LoginTest { private static String userId; + private static String user2Id; + @Test public void testBrowserSecurityHeaders() { Client client = ClientBuilder.newClient(); @@ -122,6 +133,31 @@ public class LoginTest { response.close(); } + @Test + public void loginChangeUserAfterInvalidPassword() { + loginPage.open(); + loginPage.login("login-test2", "invalid"); + + loginPage.assertCurrent(); + + Assert.assertEquals("login-test2", loginPage.getUsername()); + Assert.assertEquals("", loginPage.getPassword()); + + Assert.assertEquals("Invalid username or password.", loginPage.getError()); + + events.expectLogin().user(user2Id).session((String) null).error("invalid_user_credentials") + .detail(Details.USERNAME, "login-test2") + .removeDetail(Details.CONSENT) + .assertEvent(); + + loginPage.login("login-test", "password"); + + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + + events.expectLogin().user(userId).detail(Details.USERNAME, "login-test").assertEvent(); + } + @Test public void loginInvalidPassword() { loginPage.open(); @@ -247,6 +283,13 @@ public class LoginTest { .detail(Details.USERNAME, "invalid") .removeDetail(Details.CONSENT) .assertEvent(); + + loginPage.login("login-test", "password"); + + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + + events.expectLogin().user(userId).detail(Details.USERNAME, "login-test").assertEvent(); } @Test