Merge pull request #3070 from mstruk/KEYCLOAK-2571

KEYCLOAK-2571 RESET_PASSWORD_ERROR and UPDATE_PASSWORD_ERROR events not fired
This commit is contained in:
Bill Burke 2016-07-28 07:23:32 -04:00 committed by GitHub
commit 5d9fe09599
6 changed files with 41 additions and 23 deletions

View file

@ -71,4 +71,8 @@ public interface Errors {
String INVALID_EMAIL = "invalid_email"; String INVALID_EMAIL = "invalid_email";
String IDENTITY_PROVIDER_LOGIN_FAILURE = "identity_provider_login_failure"; String IDENTITY_PROVIDER_LOGIN_FAILURE = "identity_provider_login_failure";
String IDENTITY_PROVIDER_ERROR = "identity_provider_error"; String IDENTITY_PROVIDER_ERROR = "identity_provider_error";
String PASSWORD_CONFIRM_ERROR = "password_confirm_error";
String PASSWORD_MISSING = "password_missing";
String PASSWORD_REJECTED = "password_rejected";
} }

View file

@ -21,6 +21,8 @@ import org.keycloak.Config;
import org.keycloak.authentication.RequiredActionContext; import org.keycloak.authentication.RequiredActionContext;
import org.keycloak.authentication.RequiredActionFactory; import org.keycloak.authentication.RequiredActionFactory;
import org.keycloak.authentication.RequiredActionProvider; import org.keycloak.authentication.RequiredActionProvider;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventBuilder; import org.keycloak.events.EventBuilder;
import org.keycloak.events.EventType; import org.keycloak.events.EventType;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
@ -84,17 +86,23 @@ public class UpdatePassword implements RequiredActionProvider, RequiredActionFac
String passwordNew = formData.getFirst("password-new"); String passwordNew = formData.getFirst("password-new");
String passwordConfirm = formData.getFirst("password-confirm"); String passwordConfirm = formData.getFirst("password-confirm");
EventBuilder errorEvent = event.clone().event(EventType.UPDATE_PASSWORD_ERROR)
.client(context.getClientSession().getClient())
.user(context.getClientSession().getUserSession().getUser());
if (Validation.isBlank(passwordNew)) { if (Validation.isBlank(passwordNew)) {
Response challenge = context.form() Response challenge = context.form()
.setError(Messages.MISSING_PASSWORD) .setError(Messages.MISSING_PASSWORD)
.createResponse(UserModel.RequiredAction.UPDATE_PASSWORD); .createResponse(UserModel.RequiredAction.UPDATE_PASSWORD);
context.challenge(challenge); context.challenge(challenge);
errorEvent.error(Errors.PASSWORD_MISSING);
return; return;
} else if (!passwordNew.equals(passwordConfirm)) { } else if (!passwordNew.equals(passwordConfirm)) {
Response challenge = context.form() Response challenge = context.form()
.setError(Messages.NOTMATCH_PASSWORD) .setError(Messages.NOTMATCH_PASSWORD)
.createResponse(UserModel.RequiredAction.UPDATE_PASSWORD); .createResponse(UserModel.RequiredAction.UPDATE_PASSWORD);
context.challenge(challenge); context.challenge(challenge);
errorEvent.error(Errors.PASSWORD_CONFIRM_ERROR);
return; return;
} }
@ -102,12 +110,14 @@ public class UpdatePassword implements RequiredActionProvider, RequiredActionFac
context.getSession().users().updateCredential(context.getRealm(), context.getUser(), UserCredentialModel.password(passwordNew)); context.getSession().users().updateCredential(context.getRealm(), context.getUser(), UserCredentialModel.password(passwordNew));
context.success(); context.success();
} catch (ModelException me) { } catch (ModelException me) {
errorEvent.detail(Details.REASON, me.getMessage()).error(Errors.PASSWORD_REJECTED);
Response challenge = context.form() Response challenge = context.form()
.setError(me.getMessage(), me.getParameters()) .setError(me.getMessage(), me.getParameters())
.createResponse(UserModel.RequiredAction.UPDATE_PASSWORD); .createResponse(UserModel.RequiredAction.UPDATE_PASSWORD);
context.challenge(challenge); context.challenge(challenge);
return; return;
} catch (Exception ape) { } catch (Exception ape) {
errorEvent.detail(Details.REASON, ape.getMessage()).error(Errors.PASSWORD_REJECTED);
Response challenge = context.form() Response challenge = context.form()
.setError(ape.getMessage()) .setError(ape.getMessage())
.createResponse(UserModel.RequiredAction.UPDATE_PASSWORD); .createResponse(UserModel.RequiredAction.UPDATE_PASSWORD);

View file

@ -16,7 +16,7 @@
*/ */
package org.keycloak.services.resources; package org.keycloak.services.resources;
import org.jboss.logging.Logger; import org.keycloak.events.Errors;
import org.keycloak.forms.account.AccountPages; import org.keycloak.forms.account.AccountPages;
import org.keycloak.forms.account.AccountProvider; import org.keycloak.forms.account.AccountProvider;
import org.keycloak.events.Details; import org.keycloak.events.Details;
@ -612,26 +612,34 @@ public class AccountService extends AbstractSecuredLocalService {
String passwordNew = formData.getFirst("password-new"); String passwordNew = formData.getFirst("password-new");
String passwordConfirm = formData.getFirst("password-confirm"); String passwordConfirm = formData.getFirst("password-confirm");
EventBuilder errorEvent = event.clone().event(EventType.UPDATE_PASSWORD_ERROR)
.client(auth.getClient())
.user(auth.getClientSession().getUserSession().getUser());
if (requireCurrent) { if (requireCurrent) {
if (Validation.isBlank(password)) { if (Validation.isBlank(password)) {
setReferrerOnPage(); setReferrerOnPage();
errorEvent.error(Errors.PASSWORD_MISSING);
return account.setError(Messages.MISSING_PASSWORD).createResponse(AccountPages.PASSWORD); return account.setError(Messages.MISSING_PASSWORD).createResponse(AccountPages.PASSWORD);
} }
UserCredentialModel cred = UserCredentialModel.password(password); UserCredentialModel cred = UserCredentialModel.password(password);
if (!session.users().validCredentials(session, realm, user, cred)) { if (!session.users().validCredentials(session, realm, user, cred)) {
setReferrerOnPage(); setReferrerOnPage();
errorEvent.error(Errors.INVALID_USER_CREDENTIALS);
return account.setError(Messages.INVALID_PASSWORD_EXISTING).createResponse(AccountPages.PASSWORD); return account.setError(Messages.INVALID_PASSWORD_EXISTING).createResponse(AccountPages.PASSWORD);
} }
} }
if (Validation.isBlank(passwordNew)) { if (Validation.isBlank(passwordNew)) {
setReferrerOnPage(); setReferrerOnPage();
errorEvent.error(Errors.PASSWORD_MISSING);
return account.setError(Messages.MISSING_PASSWORD).createResponse(AccountPages.PASSWORD); return account.setError(Messages.MISSING_PASSWORD).createResponse(AccountPages.PASSWORD);
} }
if (!passwordNew.equals(passwordConfirm)) { if (!passwordNew.equals(passwordConfirm)) {
setReferrerOnPage(); setReferrerOnPage();
errorEvent.error(Errors.PASSWORD_CONFIRM_ERROR);
return account.setError(Messages.INVALID_PASSWORD_CONFIRM).createResponse(AccountPages.PASSWORD); return account.setError(Messages.INVALID_PASSWORD_CONFIRM).createResponse(AccountPages.PASSWORD);
} }
@ -639,14 +647,17 @@ public class AccountService extends AbstractSecuredLocalService {
session.users().updateCredential(realm, user, UserCredentialModel.password(passwordNew)); session.users().updateCredential(realm, user, UserCredentialModel.password(passwordNew));
} catch (ModelReadOnlyException mre) { } catch (ModelReadOnlyException mre) {
setReferrerOnPage(); setReferrerOnPage();
errorEvent.error(Errors.NOT_ALLOWED);
return account.setError(Messages.READ_ONLY_PASSWORD).createResponse(AccountPages.PASSWORD); return account.setError(Messages.READ_ONLY_PASSWORD).createResponse(AccountPages.PASSWORD);
}catch (ModelException me) { } catch (ModelException me) {
logger.failedToUpdatePassword(me); logger.failedToUpdatePassword(me);
setReferrerOnPage(); setReferrerOnPage();
errorEvent.detail(Details.REASON, me.getMessage()).error(Errors.PASSWORD_REJECTED);
return account.setError(me.getMessage(), me.getParameters()).createResponse(AccountPages.PASSWORD); return account.setError(me.getMessage(), me.getParameters()).createResponse(AccountPages.PASSWORD);
}catch (Exception ape) { } catch (Exception ape) {
logger.failedToUpdatePassword(ape); logger.failedToUpdatePassword(ape);
setReferrerOnPage(); setReferrerOnPage();
errorEvent.detail(Details.REASON, ape.getMessage()).error(Errors.PASSWORD_REJECTED);
return account.setError(ape.getMessage()).createResponse(AccountPages.PASSWORD); return account.setError(ape.getMessage()).createResponse(AccountPages.PASSWORD);
} }

View file

@ -892,7 +892,7 @@ public class LoginActionsService {
return AuthenticationManager.finishedRequiredActions(session, userSession, clientSession, clientConnection, request, uriInfo, event); return AuthenticationManager.finishedRequiredActions(session, userSession, clientSession, clientConnection, request, uriInfo, event);
} }
} }
if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) { if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) {
return context.getChallenge(); return context.getChallenge();
} }

View file

@ -21,6 +21,7 @@ import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType; import org.keycloak.events.EventType;
import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.models.utils.TimeBasedOTP;
import org.keycloak.services.resources.AccountService; import org.keycloak.services.resources.AccountService;
@ -168,22 +169,20 @@ public class AccountTest extends TestRealmKeycloakTest {
EventRepresentation event = events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent(); EventRepresentation event = events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
String sessionId = event.getSessionId(); String sessionId = event.getSessionId();
String userId = event.getUserId(); String userId = event.getUserId();
changePasswordPage.changePassword("", "new-password", "new-password");
changePasswordPage.changePassword("", "new-password", "new-password");
Assert.assertEquals("Please specify password.", profilePage.getError()); Assert.assertEquals("Please specify password.", profilePage.getError());
events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_MISSING).assertEvent();
changePasswordPage.changePassword("password", "new-password", "new-password2"); changePasswordPage.changePassword("password", "new-password", "new-password2");
Assert.assertEquals("Password confirmation doesn't match.", profilePage.getError()); Assert.assertEquals("Password confirmation doesn't match.", profilePage.getError());
events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_CONFIRM_ERROR).assertEvent();
changePasswordPage.changePassword("password", "new-password", "new-password"); changePasswordPage.changePassword("password", "new-password", "new-password");
Assert.assertEquals("Your password has been updated.", profilePage.getSuccess()); Assert.assertEquals("Your password has been updated.", profilePage.getSuccess());
events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent(); events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
changePasswordPage.logout(); changePasswordPage.logout();
events.expectLogout(sessionId).detail(Details.REDIRECT_URI, changePasswordPage.getPath()).assertEvent(); events.expectLogout(sessionId).detail(Details.REDIRECT_URI, changePasswordPage.getPath()).assertEvent();
loginPage.open(); loginPage.open();
@ -191,7 +190,7 @@ public class AccountTest extends TestRealmKeycloakTest {
Assert.assertEquals("Invalid username or password.", loginPage.getError()); Assert.assertEquals("Invalid username or password.", loginPage.getError());
events.expectLogin().session((String) null).error("invalid_user_credentials") events.expectLogin().session((String) null).error(Errors.INVALID_USER_CREDENTIALS)
.removeDetail(Details.CONSENT) .removeDetail(Details.CONSENT)
.assertEvent(); .assertEvent();
@ -214,18 +213,14 @@ public class AccountTest extends TestRealmKeycloakTest {
changePasswordPage.open(); changePasswordPage.open();
loginPage.login("test-user@localhost", "password"); loginPage.login("test-user@localhost", "password");
events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent(); events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
changePasswordPage.changePassword("", "new", "new"); changePasswordPage.changePassword("", "new", "new");
Assert.assertEquals("Please specify password.", profilePage.getError()); Assert.assertEquals("Please specify password.", profilePage.getError());
events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_MISSING).assertEvent();
changePasswordPage.changePassword("password", "new-password", "new-password"); changePasswordPage.changePassword("password", "new-password", "new-password");
Assert.assertEquals("Your password has been updated.", profilePage.getSuccess()); Assert.assertEquals("Your password has been updated.", profilePage.getSuccess());
events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent(); events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
} }
@ -235,31 +230,26 @@ public class AccountTest extends TestRealmKeycloakTest {
changePasswordPage.open(); changePasswordPage.open();
loginPage.login("test-user@localhost", "password"); loginPage.login("test-user@localhost", "password");
events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent(); events.expectLogin().client("account").detail(Details.REDIRECT_URI, ACCOUNT_REDIRECT + "?path=password").assertEvent();
changePasswordPage.changePassword("password", "password", "password"); changePasswordPage.changePassword("password", "password", "password");
Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError()); Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError());
events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).assertEvent();
changePasswordPage.changePassword("password", "password1", "password1"); changePasswordPage.changePassword("password", "password1", "password1");
Assert.assertEquals("Your password has been updated.", profilePage.getSuccess()); Assert.assertEquals("Your password has been updated.", profilePage.getSuccess());
events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent(); events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
changePasswordPage.changePassword("password1", "password", "password"); changePasswordPage.changePassword("password1", "password", "password");
Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError()); Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError());
events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).assertEvent();
changePasswordPage.changePassword("password1", "password1", "password1"); changePasswordPage.changePassword("password1", "password1", "password1");
Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError()); Assert.assertEquals("Invalid password: must not be equal to any of last 2 passwords.", profilePage.getError());
events.expectAccount(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).assertEvent();
changePasswordPage.changePassword("password1", "password2", "password2"); changePasswordPage.changePassword("password1", "password2", "password2");
Assert.assertEquals("Your password has been updated.", profilePage.getSuccess()); Assert.assertEquals("Your password has been updated.", profilePage.getSuccess());
events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent(); events.expectAccount(EventType.UPDATE_PASSWORD).assertEvent();
} }

View file

@ -314,6 +314,7 @@ public class ResetPasswordTest extends TestRealmKeycloakTest {
assertTrue(updatePasswordPage.isCurrent()); assertTrue(updatePasswordPage.isCurrent());
assertEquals(error, updatePasswordPage.getError()); assertEquals(error, updatePasswordPage.getError());
events.expectRequiredAction(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).user(userId).detail(Details.USERNAME, "login-test").assertEvent().getSessionId();
} }
@Test @Test
@ -544,6 +545,8 @@ public class ResetPasswordTest extends TestRealmKeycloakTest {
assertEquals("Invalid password: minimum length 8.", resetPasswordPage.getErrorMessage()); assertEquals("Invalid password: minimum length 8.", resetPasswordPage.getErrorMessage());
events.expectRequiredAction(EventType.UPDATE_PASSWORD_ERROR).error(Errors.PASSWORD_REJECTED).user(userId).detail(Details.USERNAME, "login-test").assertEvent().getSessionId();
updatePasswordPage.changePassword("resetPasswordWithPasswordPolicy", "resetPasswordWithPasswordPolicy"); updatePasswordPage.changePassword("resetPasswordWithPasswordPolicy", "resetPasswordWithPasswordPolicy");
String sessionId = events.expectRequiredAction(EventType.UPDATE_PASSWORD).user(userId).detail(Details.USERNAME, "login-test").assertEvent().getSessionId(); String sessionId = events.expectRequiredAction(EventType.UPDATE_PASSWORD).user(userId).detail(Details.USERNAME, "login-test").assertEvent().getSessionId();