KEYCLOAK-18338 Fix update user account with configured SSSD

server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java

@@ -30,6 +30,7 @@
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelException;
 import org.keycloak.models.UserModel;
+import org.keycloak.storage.ReadOnlyException;
 
 /**
  * <p>The default implementation for {@link UserProfile}. Should be reused as much as possible by the different implementations
@@ -146,9 +147,9 @@ private UserModel updateInternal(UserModel user, boolean removeAttributes, Attri
                     }
                 }
             }
-        } catch (ModelException me) {
-            // some client code relies on this exception to react to exceptions from the storage
-            throw me;
+        } catch (ModelException | ReadOnlyException e) {
+            // some client code relies on these exceptions to react to exceptions from the storage
+            throw e;
         } catch (Exception cause) {
             throw new RuntimeException("Unexpected error when persisting user profile", cause);
         }

services/src/main/java/org/keycloak/services/resources/admin/UserResource.java

@@ -197,7 +197,7 @@ public Response updateUser(final UserRepresentation rep) {
         } catch (ModelDuplicateException e) {
             return ErrorResponse.exists("User exists with same username or email");
         } catch (ReadOnlyException re) {
-            return ErrorResponse.exists("User is read only!");
+            return ErrorResponse.error("User is read only!", Status.BAD_REQUEST);
         } catch (ModelException me) {
             logger.warn("Could not update user!", me);
             return ErrorResponse.error("Could not update user!", Status.BAD_REQUEST);

testsuite/integration-arquillian/tests/other/sssd/README.md

@@ -1,27 +1,33 @@
 What is this module about?
 -------------------------
 
-This module containes integration tests for testing the SSSD features of Keycloak.
+This module contains integration tests for testing the SSSD features of Keycloak.
 
 Prerequisites
 -------------
 
-To run tests inside this module, one needs to have a linux machine configured as an `IPA` client having sssd
-  service started with infopipe support.
+To run tests inside this module, one needs to have a linux machine configured as an `IPA` client having SSSD
+  service started with Infopipe support.
 
 How does one run the tests?
 --------------------------
 
 *All the commands are intended to be run from the root `keycloak` project directory.*
 
 First build the distribution of keycloak:
-`mvn clean install -B -DskipTests -Pdistribution`
+```
+mvn clean install -B -DskipTests -Pdistribution
+```
 
 It may fail in the end, but it's not a problem as far as it creates a zip distribution of Keycloak inside
 distribution/server-dist/target.
 
 Then build the integration-arquillian-servers-auth-server-wildfly artifact:
-`mvn clean install -B -Pauth-server-wildfly -f testsuite/integration-arquillian/servers/pom.xml`
+```
+mvn clean install -B -Pauth-server-wildfly -f testsuite/integration-arquillian/servers/pom.xml
+```
 
 And then, finally, it's possible to run the tests:
-`mvn test -f testsuite/integration-arquillian/tests/other/sssd/ -Pauth-server-wildfly -Psssd-testing`
+```
+mvn test -f testsuite/integration-arquillian/tests/other/sssd/ -Pauth-server-wildfly -Psssd-testing
+```

testsuite/integration-arquillian/tests/other/sssd/src/test/java/org/keycloak/testsuite/sssd/SSSDTest.java

@@ -16,22 +16,24 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.common.Profile;
-import org.keycloak.common.constants.GenericConstants;
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.representations.idm.ComponentRepresentation;
 import org.keycloak.representations.idm.GroupRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.storage.UserStorageProvider;
 import org.keycloak.testsuite.AbstractKeycloakTest;
-import org.keycloak.testsuite.Assert;
 import org.keycloak.testsuite.AssertEvents;
 import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
 import org.keycloak.testsuite.pages.AccountPasswordPage;
 import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
 import org.keycloak.testsuite.pages.LoginPage;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThan;
+
 @DisableFeature(value = Profile.Feature.ACCOUNT2, skipRestart = true) // TODO remove this (KEYCLOAK-16228)
 public class SSSDTest extends AbstractKeycloakTest {
 
@@ -104,9 +106,9 @@ public void testInvalidPassword() {
         log.debug("Testing invalid password for user " + username);
 
         profilePage.open();
-        Assert.assertEquals("Browser should be on login page now", "Sign in to " + REALM_NAME, driver.getTitle());
+        assertThat("Browser should be on login page now", driver.getTitle(), is("Sign in to " + REALM_NAME));
         accountLoginPage.login(username, "invalid-password");
-        Assert.assertEquals("Invalid username or password.", accountLoginPage.getInputError());
+        assertThat(accountLoginPage.getInputError(), is("Invalid username or password."));
     }
 
     @Test
@@ -116,10 +118,10 @@ public void testDisabledUser() {
         log.debug("Testing disabled user " + username);
 
         profilePage.open();
-        Assert.assertEquals("Browser should be on login page now", "Sign in to " + REALM_NAME, driver.getTitle());
+        assertThat("Browser should be on login page now", driver.getTitle(), is("Sign in to " + REALM_NAME));
         accountLoginPage.login(username, getPassword(username));
 
-        Assert.assertEquals("Invalid username or password.", accountLoginPage.getInputError());
+        assertThat(accountLoginPage.getInputError(), is("Invalid username or password."));
     }
 
     @Test
@@ -129,9 +131,9 @@ public void testAdmin() {
         log.debug("Testing password for user " + username);
 
         profilePage.open();
-        Assert.assertEquals("Browser should be on login page now", "Sign in to " + REALM_NAME, driver.getTitle());
+        assertThat("Browser should be on login page now", driver.getTitle(), is("Sign in to " + REALM_NAME));
         accountLoginPage.login(username, getPassword(username));
-        Assert.assertTrue(profilePage.isCurrent());
+        assertThat(profilePage.isCurrent(), is(true));
     }
 
     @Test
@@ -140,9 +142,9 @@ public void testExistingUserLogIn() {
 
         for (String username : getUsernames()) {
             profilePage.open();
-            Assert.assertEquals("Browser should be on login page now", "Sign in to " + REALM_NAME, driver.getTitle());
+            assertThat("Browser should be on login page now", driver.getTitle(), is("Sign in to " + REALM_NAME));
             accountLoginPage.login(username, getPassword(username));
-            Assert.assertTrue(profilePage.isCurrent());
+            assertThat(profilePage.isCurrent(), is(true));
             verifyUserGroups(username, getGroups(username));
             profilePage.logout();
         }
@@ -153,9 +155,9 @@ public void testExistingUserWithNoEmailLogIn() {
         log.debug("Testing correct password, but no e-mail provided");
         String username = getUser(NO_EMAIL_USER);
         profilePage.open();
-        Assert.assertEquals("Browser should be on login page now", "Sign in to " + REALM_NAME, driver.getTitle());
+        assertThat("Browser should be on login page now", driver.getTitle(), is("Sign in to " + REALM_NAME));
         accountLoginPage.login(username, getPassword(username));
-        Assert.assertTrue(profilePage.isCurrent());
+        assertThat(profilePage.isCurrent(), is(true));
     }
 
     @Test
@@ -164,32 +166,32 @@ public void testDeleteSSSDFederationProvider() {
 
         profilePage.open();
         String username = getUsername();
-        Assert.assertEquals("Browser should be on login page now", "Sign in to " + REALM_NAME, driver.getTitle());
+        assertThat("Browser should be on login page now", driver.getTitle(), is("Sign in to " + REALM_NAME));
         accountLoginPage.login(username, getPassword(username));
-        Assert.assertTrue(profilePage.isCurrent());
+        assertThat(profilePage.isCurrent(), is(true));
         verifyUserGroups(username, getGroups(username));
 
         int componentsListSize = adminClient.realm(REALM_NAME).components().query().size();
         adminClient.realm(REALM_NAME).components().component(SSSDFederationID).remove();
-        Assert.assertEquals(componentsListSize - 1, adminClient.realm(REALM_NAME).components().query().size());
+        assertThat(adminClient.realm(REALM_NAME).components().query().size(), is(componentsListSize - 1));
     }
 
 
     @Test
-    public void changeReadOnlyProfile() throws Exception {
+    public void changeReadOnlyProfile() {
 
         String username = getUsername();
         profilePage.open();
         accountLoginPage.login(username, getPassword(username));
 
-        Assert.assertEquals(username, profilePage.getUsername());
-        Assert.assertEquals(sssdConfig.getProperty("user." + username + ".firstname"), profilePage.getFirstName());
-        Assert.assertEquals(sssdConfig.getProperty("user." + username + ".lastname"), profilePage.getLastName());
-        Assert.assertEquals(sssdConfig.getProperty("user." + username + ".mail"), profilePage.getEmail());
+        assertThat(profilePage.getUsername(), is(username));
+        assertThat(sssdConfig.getProperty("user." + username + ".firstname"), is(profilePage.getFirstName()));
+        assertThat(sssdConfig.getProperty("user." + username + ".lastname"), is(profilePage.getLastName()));
+        assertThat(sssdConfig.getProperty("user." + username + ".mail"), is(profilePage.getEmail()));
 
         profilePage.updateProfile("New first", "New last", "new@email.com");
 
-        Assert.assertEquals("You can't update your account as it is read-only.", profilePage.getError());
+        assertThat(profilePage.getError(), is("You can't update your account as it is read-only."));
     }
 
     @Test
@@ -199,18 +201,18 @@ public void changeReadOnlyPassword() {
         accountLoginPage.login(username, getPassword(username));
 
         changePasswordPage.changePassword(getPassword(username), "new-password", "new-password");
-        Assert.assertEquals("You can't update your password as your account is read only.", profilePage.getError());
+        assertThat(profilePage.getError(), is("You can't update your password as your account is read only."));
     }
 
     private void verifyUserGroups(String username, List<String> groups) {
         List<UserRepresentation> users = adminClient.realm(REALM_NAME).users().search(username, 0, 1);
-        Assert.assertTrue("There must be at least one user", users.size() > 0);
-        Assert.assertEquals("Exactly our test user", username, users.get(0).getUsername());
+        assertThat("There must be at least one user", users.size(), greaterThan(0));
+        assertThat("Exactly our test user", users.get(0).getUsername(), is(username));
         List<GroupRepresentation> assignedGroups = adminClient.realm(REALM_NAME).users().get(users.get(0).getId()).groups();
-        Assert.assertEquals("User must have exactly " + groups.size() + " groups", groups.size(), assignedGroups.size());
+        assertThat("User must have exactly " + groups.size() + " groups", assignedGroups.size(), is(groups.size()));
 
         for (GroupRepresentation group : assignedGroups) {
-            Assert.assertTrue(groups.contains(group.getName()));
+            assertThat(groups.contains(group.getName()), is(true));
         }
     }