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

… edit username are enabled
keycloak · Dec 20, 2017 · f0c5752 · f0c5752
1 parent 351dbff
commit f0c5752
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 10 deletions.
diff --git a/...java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java b/...java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
@@ -99,7 +99,7 @@ protected void actionImpl(AuthenticationFlowContext context, SerializedBrokeredI
 
         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
@@ -63,7 +63,7 @@ public void processAction(RequiredActionContext context) {
         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
@@ -323,7 +323,7 @@ public Response processAccountUpdate(final MultivaluedMap<String, String> formDa
 
         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 static AccountSocialAction getAction(String action) {
     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
@@ -86,14 +86,10 @@ private static void addError(List<FormMessage> errors, String field, String mess
         errors.add(new FormMessage(field, message));
     }
 
-    public static List<FormMessage> validateUpdateProfileForm(MultivaluedMap<String, String> formData) {
+    public static List<FormMessage> validateUpdateProfileForm(RealmModel realm, MultivaluedMap<String, String> formData) {
-        return validateUpdateProfileForm(false, formData);
-    }
-
-    public static List<FormMessage> validateUpdateProfileForm(boolean editUsernameAllowed, 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/...llian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java b/...llian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
@@ -577,6 +577,42 @@ public void changeProfileEmailAsUsernameEnabled() throws Exception {
 
     }
 
+    // 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);