libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

KEYCLOAK-1823

Browse source 
Annoying behaviour of validations in user registration form
This commit is contained in:
Stian Thorgersen 2015-10-13 08:14:39 +02:00
parent 07c3772b08
commit ef56dca050
7 changed files with 55 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java
View file

@ -18,10 +18,7 @@ import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo; import javax.ws.rs.core.UriInfo;
import java.net.URI; import java.net.URI;
import java.util.HashMap; import java.util.*;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
/** /**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a> * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@ -116,6 +113,7 @@ public class FormAuthenticationFlow implements AuthenticationFlow {
private class ValidationContextImpl extends FormContextImpl implements ValidationContext { private class ValidationContextImpl extends FormContextImpl implements ValidationContext {
FormAction action; FormAction action;
String error;
private ValidationContextImpl(AuthenticationExecutionModel executionModel, FormAction action) { private ValidationContextImpl(AuthenticationExecutionModel executionModel, FormAction action) {
super(executionModel); super(executionModel);
@ -131,6 +129,10 @@ public class FormAuthenticationFlow implements AuthenticationFlow {
this.formData = formData; this.formData = formData;
} }
public void error(String error) {
this.error = error;
}
@Override @Override
public void success() { public void success() {
success = true; success = true;
@ -145,6 +147,7 @@ public class FormAuthenticationFlow implements AuthenticationFlow {
Map<String, ClientSessionModel.ExecutionStatus> executionStatus = new HashMap<>(); Map<String, ClientSessionModel.ExecutionStatus> executionStatus = new HashMap<>();
List<FormAction> requiredActions = new LinkedList<>(); List<FormAction> requiredActions = new LinkedList<>();
List<ValidationContextImpl> successes = new LinkedList<>(); List<ValidationContextImpl> successes = new LinkedList<>();
List<ValidationContextImpl> errors = new LinkedList<>();
for (AuthenticationExecutionModel formActionExecution : formActionExecutions) { for (AuthenticationExecutionModel formActionExecution : formActionExecutions) {
if (!formActionExecution.isEnabled()) { if (!formActionExecution.isEnabled()) {
executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.SKIPPED); executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.SKIPPED);
@ -183,12 +186,28 @@ public class FormAuthenticationFlow implements AuthenticationFlow {
executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.SUCCESS); executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.SUCCESS);
successes.add(result); successes.add(result);
} else { } else {
processor.logFailure();
executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED); executionStatus.put(formActionExecution.getId(), ClientSessionModel.ExecutionStatus.CHALLENGED);
return renderForm(result.formData, result.errors); errors.add(result);
} }
} }
if (!errors.isEmpty()) {
processor.logFailure();
List<FormMessage> messages = new LinkedList<>();
Set<String> 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) { for (ValidationContextImpl context : successes) {
context.action.success(context); context.action.success(context);
} }

2
services/src/main/java/org/keycloak/authentication/ValidationContext.java
View file

@ -21,6 +21,8 @@ public interface ValidationContext extends FormContext {
*/ */
void validationError(MultivaluedMap<String, String> formData, List<FormMessage> errors); void validationError(MultivaluedMap<String, String> formData, List<FormMessage> errors);
void error(String error);
/** /**
* Mark this validation as sucessful * Mark this validation as sucessful
* *

2
services/src/main/java/org/keycloak/authentication/forms/RegistrationPassword.java
View file

@ -59,7 +59,7 @@ public class RegistrationPassword implements FormAction, FormActionFactory {
} }
if (errors.size() > 0) { 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);
formData.remove(RegistrationPage.FIELD_PASSWORD_CONFIRM); formData.remove(RegistrationPage.FIELD_PASSWORD_CONFIRM);
context.validationError(formData, errors); context.validationError(formData, errors);

7
services/src/main/java/org/keycloak/authentication/forms/RegistrationProfile.java
View file

@ -56,14 +56,17 @@ public class RegistrationProfile implements FormAction, FormActionFactory {
} }
String email = formData.getFirst(Validation.FIELD_EMAIL); String email = formData.getFirst(Validation.FIELD_EMAIL);
boolean emailValid = true;
if (Validation.isBlank(email)) { if (Validation.isBlank(email)) {
errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.MISSING_EMAIL)); errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.MISSING_EMAIL));
emailValid = false;
} else if (!Validation.isEmailValid(email)) { } else if (!Validation.isEmailValid(email)) {
context.getEvent().detail(Details.EMAIL, email); context.getEvent().detail(Details.EMAIL, email);
errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.INVALID_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; eventError = Errors.EMAIL_IN_USE;
formData.remove(Validation.FIELD_EMAIL); formData.remove(Validation.FIELD_EMAIL);
context.getEvent().detail(Details.EMAIL, email); context.getEvent().detail(Details.EMAIL, email);
@ -71,7 +74,7 @@ public class RegistrationProfile implements FormAction, FormActionFactory {
} }
if (errors.size() > 0) { if (errors.size() > 0) {
context.getEvent().error(eventError); context.error(eventError);
context.validationError(formData, errors); context.validationError(formData, errors);
return; return;

2
services/src/main/java/org/keycloak/authentication/forms/RegistrationRecaptcha.java
View file

@ -108,7 +108,7 @@ public class RegistrationRecaptcha implements FormAction, FormActionFactory, Con
} else { } else {
errors.add(new FormMessage(null, Messages.RECAPTCHA_FAILED)); errors.add(new FormMessage(null, Messages.RECAPTCHA_FAILED));
formData.remove(G_RECAPTCHA_RESPONSE); formData.remove(G_RECAPTCHA_RESPONSE);
context.getEvent().error(Errors.INVALID_REGISTRATION); context.error(Errors.INVALID_REGISTRATION);
context.validationError(formData, errors); context.validationError(formData, errors);
return; return;

18
services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java
View file

@ -56,9 +56,8 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory {
String usernameField = RegistrationPage.FIELD_USERNAME; String usernameField = RegistrationPage.FIELD_USERNAME;
if (context.getRealm().isRegistrationEmailAsUsername()) { if (context.getRealm().isRegistrationEmailAsUsername()) {
username = email; context.getEvent().detail(Details.USERNAME, email);
context.getEvent().detail(Details.USERNAME, username);
usernameField = RegistrationPage.FIELD_EMAIL;
if (Validation.isBlank(email)) { if (Validation.isBlank(email)) {
errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.MISSING_EMAIL)); errors.add(new FormMessage(RegistrationPage.FIELD_EMAIL, Messages.MISSING_EMAIL));
} else if (!Validation.isEmailValid(email)) { } else if (!Validation.isEmailValid(email)) {
@ -66,33 +65,32 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory {
formData.remove(Validation.FIELD_EMAIL); formData.remove(Validation.FIELD_EMAIL);
} }
if (errors.size() > 0) { if (errors.size() > 0) {
context.getEvent().error(Errors.INVALID_REGISTRATION); context.error(Errors.INVALID_REGISTRATION);
context.validationError(formData, errors); context.validationError(formData, errors);
return; return;
} }
if (email != null && context.getSession().users().getUserByEmail(email, context.getRealm()) != null) { 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); 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); context.validationError(formData, errors);
return; return;
} }
} else { } else {
if (Validation.isBlank(username)) { 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)); errors.add(new FormMessage(RegistrationPage.FIELD_USERNAME, Messages.MISSING_USERNAME));
context.validationError(formData, errors); context.validationError(formData, errors);
return; return;
} }
}
if (context.getSession().users().getUserByUsername(username, context.getRealm()) != null) { if (context.getSession().users().getUserByUsername(username, context.getRealm()) != null) {
context.getEvent().error(Errors.USERNAME_IN_USE); context.error(Errors.USERNAME_IN_USE);
errors.add(new FormMessage(usernameField, Messages.USERNAME_EXISTS)); errors.add(new FormMessage(usernameField, Messages.USERNAME_EXISTS));
formData.remove(Validation.FIELD_USERNAME); formData.remove(Validation.FIELD_USERNAME);
formData.remove(Validation.FIELD_EMAIL);
context.validationError(formData, errors); context.validationError(formData, errors);
return; return;
}
} }
context.success(); context.success();

16
testsuite/integration/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java
View file

@ -84,7 +84,7 @@ public class RegisterTest {
// assert form keeps form fields on error // assert form keeps form fields on error
assertEquals("firstName", registerPage.getFirstName()); assertEquals("firstName", registerPage.getFirstName());
assertEquals("lastName", registerPage.getLastName()); assertEquals("lastName", registerPage.getLastName());
assertEquals("", registerPage.getEmail()); assertEquals("registerExistingUser@email", registerPage.getEmail());
assertEquals("", registerPage.getUsername()); assertEquals("", registerPage.getUsername());
assertEquals("", registerPage.getPassword()); assertEquals("", registerPage.getPassword());
assertEquals("", registerPage.getPasswordConfirm()); assertEquals("", registerPage.getPasswordConfirm());
@ -199,15 +199,15 @@ public class RegisterTest {
loginPage.clickRegister(); loginPage.clickRegister();
registerPage.assertCurrent(); registerPage.assertCurrent();
registerPage.register(null, null, null, "registerUserManyErrors", null, "password"); registerPage.register(null, null, null, null, null, null);
registerPage.assertCurrent(); registerPage.assertCurrent();
System.out.println(registerPage.getError()); assertEquals("Please specify username.\n" +
"Please specify first name.\n" +
assertEquals("Please specify first name.\n" +
"Please specify last 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") events.expectRegister(null, "registerUserMissingUsername@email")
.removeDetail(Details.USERNAME) .removeDetail(Details.USERNAME)
@ -290,9 +290,9 @@ public class RegisterTest {
registerPage.registerWithEmailAsUsername("firstName", "lastName", "test-user@localhost", "password", "password"); registerPage.registerWithEmailAsUsername("firstName", "lastName", "test-user@localhost", "password", "password");
registerPage.assertCurrent(); 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 { } finally {
configureRelamRegistrationEmailAsUsername(false); configureRelamRegistrationEmailAsUsername(false);
} }