keycloak-uncached

KEYCLOAK-5443 Fix update user account when both email as username

12/20/2017 7:52:57 AM

Details

diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
index c430fef..fa6261f 100755
--- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
@@ -99,7 +99,7 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator {
 
         RealmModel realm = context.getRealm();
 
-        List<FormMessage> errors = Validation.validateUpdateProfileForm(!realm.isRegistrationEmailAsUsername(), formData);
+        List<FormMessage> errors = Validation.validateUpdateProfileForm(realm, formData);
         if (errors != null && !errors.isEmpty()) {
             Response challenge = context.form()
                     .setErrors(errors)
diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java
index ca0185e..3ed1f12 100644
--- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java
+++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java
@@ -63,7 +63,7 @@ public class UpdateProfile implements RequiredActionProvider, RequiredActionFact
         RealmModel realm = context.getRealm();
 
 
-        List<FormMessage> errors = Validation.validateUpdateProfileForm(realm.isEditUsernameAllowed(), formData);
+        List<FormMessage> errors = Validation.validateUpdateProfileForm(realm, formData);
         if (errors != null && !errors.isEmpty()) {
             Response challenge = context.form()
                     .setErrors(errors)
diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java
index 68973cf..d3c0260 100755
--- a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java
+++ b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java
@@ -323,7 +323,7 @@ public class AccountFormService extends AbstractSecuredLocalService {
 
         event.event(EventType.UPDATE_PROFILE).client(auth.getClient()).user(auth.getUser());
 
-        List<FormMessage> errors = Validation.validateUpdateProfileForm(realm.isEditUsernameAllowed(), formData);
+        List<FormMessage> errors = Validation.validateUpdateProfileForm(realm, formData);
         if (errors != null && !errors.isEmpty()) {
             setReferrerOnPage();
             return account.setErrors(Response.Status.BAD_REQUEST, errors).setProfileFormData(formData).createResponse(AccountPages.ACCOUNT);
@@ -753,7 +753,7 @@ public class AccountFormService extends AbstractSecuredLocalService {
     private void updateUsername(String username, UserModel user, KeycloakSession session) {
         RealmModel realm = session.getContext().getRealm();
         boolean usernameChanged = username == null || !user.getUsername().equals(username);
-        if (realm.isEditUsernameAllowed()) {
+        if (realm.isEditUsernameAllowed() && !realm.isRegistrationEmailAsUsername()) {
             if (usernameChanged) {
                 UserModel existing = session.users().getUserByUsername(username, realm);
                 if (existing != null && !existing.getId().equals(user.getId())) {
diff --git a/services/src/main/java/org/keycloak/services/validation/Validation.java b/services/src/main/java/org/keycloak/services/validation/Validation.java
index fbd18af..46499b3 100755
--- a/services/src/main/java/org/keycloak/services/validation/Validation.java
+++ b/services/src/main/java/org/keycloak/services/validation/Validation.java
@@ -86,14 +86,10 @@ public class Validation {
         errors.add(new FormMessage(field, message));
     }
 
-    public static List<FormMessage> validateUpdateProfileForm(MultivaluedMap<String, String> formData) {
-        return validateUpdateProfileForm(false, formData);
-    }
-
-    public static List<FormMessage> validateUpdateProfileForm(boolean editUsernameAllowed, MultivaluedMap<String, String> formData) {
+    public static List<FormMessage> validateUpdateProfileForm(RealmModel realm, MultivaluedMap<String, String> formData) {
         List<FormMessage> errors = new ArrayList<>();
         
-        if (editUsernameAllowed && isBlank(formData.getFirst(FIELD_USERNAME))) {
+        if (!realm.isRegistrationEmailAsUsername() && realm.isEditUsernameAllowed() && isBlank(formData.getFirst(FIELD_USERNAME))) {
             addError(errors, FIELD_USERNAME, Messages.MISSING_USERNAME);
         }
 
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
index 462e10b..ca8a270 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
@@ -577,6 +577,42 @@ public class AccountFormServiceTest extends AbstractTestRealmKeycloakTest {
 
     }
 
+    // KEYCLOAK-5443
+    @Test
+    public void changeProfileEmailAsUsernameAndEditUsernameEnabled() throws Exception {
+        setEditUsernameAllowed(true);
+        setRegistrationEmailAsUsername(true);
+
+        profilePage.open();
+        loginPage.login("test-user@localhost", "password");
+        assertFalse(driver.findElements(By.id("username")).size() > 0);
+
+        profilePage.updateProfile("New First", "New Last", "new-email@email");
+
+        Assert.assertEquals("Your account has been updated.", profilePage.getSuccess());
+        Assert.assertEquals("New First", profilePage.getFirstName());
+        Assert.assertEquals("New Last", profilePage.getLastName());
+        Assert.assertEquals("new-email@email", profilePage.getEmail());
+
+        List<UserRepresentation> list = adminClient.realm("test").users().search(null, null, null, "new-email@email", null, null);
+        assertEquals(1, list.size());
+
+        UserRepresentation user = list.get(0);
+
+        assertEquals("new-email@email", user.getUsername());
+
+        // Revert
+
+        user.setUsername("test-user@localhost");
+        user.setFirstName("Tom");
+        user.setLastName("Brady");
+        user.setEmail("test-user@localhost");
+        adminClient.realm("test").users().get(user.getId()).update(user);
+
+        setRegistrationEmailAsUsername(false);
+        setEditUsernameAllowed(false);
+    }
+
     private void setEditUsernameAllowed(boolean allowed) {
         RealmRepresentation testRealm = testRealm().toRepresentation();
         testRealm.setEditUsernameAllowed(allowed);