diff --git a/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java index f29ad398d1..c94e8cbc08 100755 --- a/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java +++ b/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java @@ -18,10 +18,7 @@ import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.net.URI; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import java.util.*; /** * @author Bill Burke @@ -116,6 +113,7 @@ public class FormAuthenticationFlow implements AuthenticationFlow { private class ValidationContextImpl extends FormContextImpl implements ValidationContext { FormAction action; + String error; private ValidationContextImpl(AuthenticationExecutionModel executionModel, FormAction action) { super(executionModel); @@ -131,6 +129,10 @@ public class FormAuthenticationFlow implements AuthenticationFlow { this.formData = formData; } + public void error(String error) { + this.error = error; + } + @Override public void success() { success = true; @@ -145,6 +147,7 @@ public class FormAuthenticationFlow implements AuthenticationFlow { Map executionStatus = new HashMap<>(); List requiredActions = new LinkedList<>(); List successes = new LinkedList<>(); + List errors = new LinkedList<>(); for (AuthenticationExecutionModel formActionExecution : formActionExecutions) { if (!formActionExecution.isEnabled()) { executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.SKIPPED); @@ -183,12 +186,28 @@ public class FormAuthenticationFlow implements AuthenticationFlow { executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.SUCCESS); successes.add(result); } else { - processor.logFailure(); executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED); - return renderForm(result.formData, result.errors); + errors.add(result); } } + if (!errors.isEmpty()) { + processor.logFailure(); + List messages = new LinkedList<>(); + Set fields = new HashSet<>(); + for (ValidationContextImpl v : errors) { + for (FormMessage m : v.errors) { + if (!fields.contains(m.getField())) { + fields.add(m.getField()); + messages.add(m); + } + } + } + ValidationContextImpl first = errors.get(0); + first.getEvent().error(first.error); + return renderForm(first.formData, messages); + } + for (ValidationContextImpl context : successes) { context.action.success(context); } diff --git a/services/src/main/java/org/keycloak/authentication/ValidationContext.java b/services/src/main/java/org/keycloak/authentication/ValidationContext.java index b0c456eb22..ce96b682c1 100755 --- a/services/src/main/java/org/keycloak/authentication/ValidationContext.java +++ b/services/src/main/java/org/keycloak/authentication/ValidationContext.java @@ -21,6 +21,8 @@ public interface ValidationContext extends FormContext { */ void validationError(MultivaluedMap formData, List errors); + void error(String error); + /** * Mark this validation as sucessful * diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationPassword.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationPassword.java index ade4e00a48..ff2442f3d5 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationPassword.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationPassword.java @@ -59,7 +59,7 @@ public class RegistrationPassword implements FormAction, FormActionFactory { } if (errors.size() > 0) { - context.getEvent().error(Errors.INVALID_REGISTRATION); + context.error(Errors.INVALID_REGISTRATION); formData.remove(RegistrationPage.FIELD_PASSWORD); formData.remove(RegistrationPage.FIELD_PASSWORD_CONFIRM); context.validationError(formData, errors); diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java index efec19a2cf..3baae6fe3a 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java @@ -56,14 +56,17 @@ public class RegistrationProfile implements FormAction, FormActionFactory { } String email = formData.getFirst(Validation.FIELD_EMAIL); + boolean emailValid = true; if (Validation.isBlank(email)) { errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.MISSING_EMAIL)); + emailValid = false; } else if (!Validation.isEmailValid(email)) { context.getEvent().detail(Details.EMAIL, email); errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.INVALID_EMAIL)); + emailValid = false; } - if (context.getSession().users().getUserByEmail(email, context.getRealm()) != null) { + if (emailValid && context.getSession().users().getUserByEmail(email, context.getRealm()) != null) { eventError = Errors.EMAIL_IN_USE; formData.remove(Validation.FIELD_EMAIL); context.getEvent().detail(Details.EMAIL, email); @@ -71,7 +74,7 @@ public class RegistrationProfile implements FormAction, FormActionFactory { } if (errors.size() > 0) { - context.getEvent().error(eventError); + context.error(eventError); context.validationError(formData, errors); return; diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationRecaptcha.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationRecaptcha.java index 3c1817ce5e..eb10250946 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationRecaptcha.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationRecaptcha.java @@ -108,7 +108,7 @@ public class RegistrationRecaptcha implements FormAction, FormActionFactory, Con } else { errors.add(new FormMessage(null, Messages.RECAPTCHA_FAILED)); formData.remove(G_RECAPTCHA_RESPONSE); - context.getEvent().error(Errors.INVALID_REGISTRATION); + context.error(Errors.INVALID_REGISTRATION); context.validationError(formData, errors); return; diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java index 40d2fb075f..dfc2a89731 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java @@ -56,9 +56,8 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory { String usernameField = RegistrationPage.FIELD_USERNAME; if (context.getRealm().isRegistrationEmailAsUsername()) { - username = email; - context.getEvent().detail(Details.USERNAME, username); - usernameField = RegistrationPage.FIELD_EMAIL; + context.getEvent().detail(Details.USERNAME, email); + if (Validation.isBlank(email)) { errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.MISSING_EMAIL)); } else if (!Validation.isEmailValid(email)) { @@ -66,33 +65,32 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory { formData.remove(Validation.FIELD_EMAIL); } if (errors.size() > 0) { - context.getEvent().error(Errors.INVALID_REGISTRATION); + context.error(Errors.INVALID_REGISTRATION); context.validationError(formData, errors); return; } if (email != null && context.getSession().users().getUserByEmail(email, context.getRealm()) != null) { - context.getEvent().error(Errors.USERNAME_IN_USE); + context.error(Errors.EMAIL_IN_USE); formData.remove(Validation.FIELD_EMAIL); - errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.USERNAME_EXISTS)); + errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.EMAIL_EXISTS)); context.validationError(formData, errors); return; } } else { if (Validation.isBlank(username)) { - context.getEvent().error(Errors.INVALID_REGISTRATION); + context.error(Errors.INVALID_REGISTRATION); errors.add(new FormMessage(RegistrationPage.FIELD_USERNAME, Messages.MISSING_USERNAME)); context.validationError(formData, errors); return; } - } - if (context.getSession().users().getUserByUsername(username, context.getRealm()) != null) { - context.getEvent().error(Errors.USERNAME_IN_USE); - errors.add(new FormMessage(usernameField, Messages.USERNAME_EXISTS)); - formData.remove(Validation.FIELD_USERNAME); - formData.remove(Validation.FIELD_EMAIL); - context.validationError(formData, errors); - return; + if (context.getSession().users().getUserByUsername(username, context.getRealm()) != null) { + context.error(Errors.USERNAME_IN_USE); + errors.add(new FormMessage(usernameField, Messages.USERNAME_EXISTS)); + formData.remove(Validation.FIELD_USERNAME); + context.validationError(formData, errors); + return; + } } context.success(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java index 932630ba09..55c7e618d1 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java @@ -84,7 +84,7 @@ public class RegisterTest { // assert form keeps form fields on error assertEquals("firstName", registerPage.getFirstName()); assertEquals("lastName", registerPage.getLastName()); - assertEquals("", registerPage.getEmail()); + assertEquals("registerExistingUser@email", registerPage.getEmail()); assertEquals("", registerPage.getUsername()); assertEquals("", registerPage.getPassword()); assertEquals("", registerPage.getPasswordConfirm()); @@ -199,15 +199,15 @@ public class RegisterTest { loginPage.clickRegister(); registerPage.assertCurrent(); - registerPage.register(null, null, null, "registerUserManyErrors", null, "password"); + registerPage.register(null, null, null, null, null, null); registerPage.assertCurrent(); - System.out.println(registerPage.getError()); - - assertEquals("Please specify first name.\n" + + assertEquals("Please specify username.\n" + + "Please specify first name.\n" + "Please specify last name.\n" + - "Please specify email.", registerPage.getError()); + "Please specify email.\n" + + "Please specify password.", registerPage.getError()); events.expectRegister(null, "registerUserMissingUsername@email") .removeDetail(Details.USERNAME) @@ -290,9 +290,9 @@ public class RegisterTest { registerPage.registerWithEmailAsUsername("firstName", "lastName", "test-user@localhost", "password", "password"); registerPage.assertCurrent(); - assertEquals("Username already exists.", registerPage.getError()); + assertEquals("Email already exists.", registerPage.getError()); - events.expectRegister("test-user@localhost", "test-user@localhost").user((String) null).error("username_in_use").assertEvent(); + events.expectRegister("test-user@localhost", "test-user@localhost").user((String) null).error("email_in_use").assertEvent(); } finally { configureRelamRegistrationEmailAsUsername(false); }