keycloak-aplcache

KEYCLOAK-2542 User can't set password for account created

3/1/2016 6:11:26 AM

Details

diff --git a/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java b/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
index 45077e2..8f809f3 100755
--- a/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
+++ b/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java
@@ -55,9 +55,7 @@ public class UserFederationManager implements UserProvider {
     }
 
     protected UserFederationProvider getFederationProvider(UserFederationProviderModel model) {
-        UserFederationProviderFactory factory = (UserFederationProviderFactory)session.getKeycloakSessionFactory().getProviderFactory(UserFederationProvider.class, model.getProviderName());
-        return factory.getInstance(session, model);
-
+        return KeycloakModelUtils.getFederationProviderInstance(session, model);
     }
 
     @Override
diff --git a/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
index ff5c38e..c4906d6 100755
--- a/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
+++ b/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
@@ -35,6 +35,8 @@ import org.keycloak.models.RoleModel;
 import org.keycloak.models.ScopeContainerModel;
 import org.keycloak.models.UserCredentialModel;
 import org.keycloak.models.UserFederationMapperModel;
+import org.keycloak.models.UserFederationProvider;
+import org.keycloak.models.UserFederationProviderFactory;
 import org.keycloak.models.UserFederationProviderModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.representations.idm.CertificateRepresentation;
@@ -406,6 +408,12 @@ public final class KeycloakModelUtils {
         return mapperModel;
     }
 
+    public static UserFederationProvider getFederationProviderInstance(KeycloakSession session, UserFederationProviderModel model) {
+        UserFederationProviderFactory factory = (UserFederationProviderFactory)session.getKeycloakSessionFactory().getProviderFactory(UserFederationProvider.class, model.getProviderName());
+        return factory.getInstance(session, model);
+
+    }
+
     // END USER FEDERATION RELATED STUFF
 
     public static String toLowerCaseSafe(String str) {
diff --git a/services/src/main/java/org/keycloak/forms/account/freemarker/model/AccountFederatedIdentityBean.java b/services/src/main/java/org/keycloak/forms/account/freemarker/model/AccountFederatedIdentityBean.java
index ec02eba..a20810e 100755
--- a/services/src/main/java/org/keycloak/forms/account/freemarker/model/AccountFederatedIdentityBean.java
+++ b/services/src/main/java/org/keycloak/forms/account/freemarker/model/AccountFederatedIdentityBean.java
@@ -79,7 +79,7 @@ public class AccountFederatedIdentityBean {
         this.identities = new LinkedList<FederatedIdentityEntry>(orderedSet); 
 
         // Removing last social provider is not possible if you don't have other possibility to authenticate
-        this.removeLinkPossible = availableIdentities > 1 || user.getFederationLink() != null || AccountService.isPasswordSet(user);
+        this.removeLinkPossible = availableIdentities > 1 || user.getFederationLink() != null || AccountService.isPasswordSet(session, realm, user);
     }
 
     private FederatedIdentityModel getIdentity(Set<FederatedIdentityModel> identities, String providerId) {
diff --git a/services/src/main/java/org/keycloak/services/resources/AccountService.java b/services/src/main/java/org/keycloak/services/resources/AccountService.java
index 3241363..4907707 100755
--- a/services/src/main/java/org/keycloak/services/resources/AccountService.java
+++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java
@@ -30,16 +30,20 @@ import org.keycloak.models.ClientModel;
 import org.keycloak.models.ClientSessionModel;
 import org.keycloak.models.FederatedIdentityModel;
 import org.keycloak.models.IdentityProviderModel;
+import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelDuplicateException;
 import org.keycloak.models.ModelException;
 import org.keycloak.models.ModelReadOnlyException;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserCredentialModel;
 import org.keycloak.models.UserCredentialValueModel;
+import org.keycloak.models.UserFederationProvider;
+import org.keycloak.models.UserFederationProviderModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.models.utils.CredentialValidation;
 import org.keycloak.models.utils.FormMessage;
+import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.utils.RedirectUtils;
@@ -298,7 +302,7 @@ public class AccountService extends AbstractSecuredLocalService {
     @GET
     public Response passwordPage() {
         if (auth != null) {
-            account.setPasswordSet(isPasswordSet(auth.getUser()));
+            account.setPasswordSet(isPasswordSet(session, realm, auth.getUser()));
         }
 
         return forwardToPage("password", AccountPages.PASSWORD);
@@ -601,7 +605,7 @@ public class AccountService extends AbstractSecuredLocalService {
         csrfCheck(formData);
         UserModel user = auth.getUser();
 
-        boolean requireCurrent = isPasswordSet(user);
+        boolean requireCurrent = isPasswordSet(session, realm, user);
         account.setPasswordSet(requireCurrent);
 
         String password = formData.getFirst("password");
@@ -723,7 +727,7 @@ public class AccountService extends AbstractSecuredLocalService {
                 if (link != null) {
 
                     // Removing last social provider is not possible if you don't have other possibility to authenticate
-                    if (session.users().getFederatedIdentities(user, realm).size() > 1 || user.getFederationLink() != null || isPasswordSet(user)) {
+                    if (session.users().getFederatedIdentities(user, realm).size() > 1 || user.getFederationLink() != null || isPasswordSet(session, realm, user)) {
                         session.users().removeFederatedIdentity(realm, user, providerId);
 
                         logger.debugv("Social provider {0} removed successfully from user {1}", providerId, user.getUsername());
@@ -758,11 +762,25 @@ public class AccountService extends AbstractSecuredLocalService {
         return Urls.accountBase(uriInfo.getBaseUri()).path("/").build(realm.getName());
     }
 
-    public static boolean isPasswordSet(UserModel user) {
+    public static boolean isPasswordSet(KeycloakSession session, RealmModel realm, UserModel user) {
         boolean passwordSet = false;
 
+        // See if password is set for user on linked UserFederationProvider
         if (user.getFederationLink() != null) {
-            passwordSet = true;
+
+            UserFederationProvider federationProvider = null;
+            for (UserFederationProviderModel fedProviderModel : realm.getUserFederationProviders()) {
+                if (fedProviderModel.getId().equals(user.getFederationLink())) {
+                    federationProvider = KeycloakModelUtils.getFederationProviderInstance(session, fedProviderModel);
+                }
+            }
+
+            if (federationProvider != null) {
+                Set<String> supportedCreds = federationProvider.getSupportedCredentialTypes(user);
+                if (supportedCreds.contains(UserCredentialModel.PASSWORD)) {
+                    passwordSet = true;
+                }
+            }
         }
 
         for (UserCredentialValueModel c : user.getCredentialsDirectly()) {
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java
index 241116e..51a9044 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java
@@ -38,6 +38,7 @@ import org.keycloak.testsuite.broker.util.UserSessionStatusServlet;
 import org.keycloak.testsuite.broker.util.UserSessionStatusServlet.UserSessionStatus;
 import org.keycloak.testsuite.pages.AccountFederatedIdentityPage;
 import org.keycloak.testsuite.pages.AccountPasswordPage;
+import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
 import org.keycloak.testsuite.pages.LoginPage;
 import org.keycloak.testsuite.pages.LoginUpdateProfilePage;
 import org.keycloak.testsuite.pages.OAuthGrantPage;
@@ -104,6 +105,9 @@ public abstract class AbstractIdentityProviderTest {
     protected OAuthGrantPage grantPage;
 
     @WebResource
+    AccountUpdateProfilePage accountUpdateProfilePage;
+
+    @WebResource
     protected AccountPasswordPage changePasswordPage;
 
     @WebResource
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
index 402f91e..b5f21a0 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
@@ -19,6 +19,7 @@ package org.keycloak.testsuite.broker;
 
 import java.io.IOException;
 import java.net.URI;
+import java.util.HashMap;
 import java.util.Set;
 
 import javax.mail.MessagingException;
@@ -39,9 +40,11 @@ import org.keycloak.models.FederatedIdentityModel;
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleModel;
+import org.keycloak.models.UserFederationProviderModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.services.Urls;
+import org.keycloak.testsuite.DummyUserFederationProviderFactory;
 import org.keycloak.testsuite.broker.util.UserSessionStatusServlet;
 import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
@@ -531,4 +534,64 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent
 
     protected abstract void doAssertTokenRetrieval(String pageSource);
 
+    @Test
+    public void testWithLinkedFederationProvider() throws Exception {
+        // Add federationProvider to realm. It's configured with sync registrations
+        RealmModel realm = getRealm();
+        UserFederationProviderModel dummyModel = realm.addUserFederationProvider(DummyUserFederationProviderFactory.PROVIDER_NAME, new HashMap<String, String>(), 1, "test-dummy", -1, -1, 0);
+        setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF);
+
+        brokerServerRule.stopSession(session, true);
+        session = brokerServerRule.startSession();
+
+        try {
+            // Login as user "test-user" to account management.
+            authenticateWithIdentityProvider(getIdentityProviderModel(), "test-user", false);
+            changePasswordPage.realm("realm-with-broker");
+            changePasswordPage.open();
+            assertTrue(changePasswordPage.isCurrent());
+
+            // Assert changing password with old password "secret" as this is the password from federationProvider (See DummyUserFederationProvider)
+            changePasswordPage.changePassword("new-password", "new-password");
+            Assert.assertEquals("Please specify password.", accountUpdateProfilePage.getError());
+
+            changePasswordPage.changePassword("bad", "new-password", "new-password");
+            Assert.assertEquals("Invalid existing password.", accountUpdateProfilePage.getError());
+
+            changePasswordPage.changePassword("secret", "new-password", "new-password");
+            Assert.assertEquals("Your password has been updated.", accountUpdateProfilePage.getSuccess());
+
+            // Logout
+            driver.navigate().to("http://localhost:8081/test-app/logout");
+
+
+            // Login as user "test-user-noemail" .
+            authenticateWithIdentityProvider(getIdentityProviderModel(), "test-user-noemail", false);
+            changePasswordPage.open();
+            assertTrue(changePasswordPage.isCurrent());
+
+            //  Assert old password is not required as federationProvider doesn't have it for this user
+            changePasswordPage.changePassword("new-password", "new-password");
+            Assert.assertEquals("Your password has been updated.", accountUpdateProfilePage.getSuccess());
+
+            // Now it is required as it's set on model
+            changePasswordPage.changePassword("new-password2", "new-password2");
+            Assert.assertEquals("Please specify password.", accountUpdateProfilePage.getError());
+
+            changePasswordPage.changePassword("new-password", "new-password2", "new-password2");
+            Assert.assertEquals("Your password has been updated.", accountUpdateProfilePage.getSuccess());
+
+            // Logout
+            driver.navigate().to("http://localhost:8081/test-app/logout");
+        } finally {
+
+            // remove dummy federation provider for this realm
+            realm = getRealm();
+            realm.removeUserFederationProvider(dummyModel);
+
+            brokerServerRule.stopSession(session, true);
+            session = brokerServerRule.startSession();
+        }
+    }
+
 }
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/DummyUserFederationProvider.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/DummyUserFederationProvider.java
index 974e5c2..50ff01b 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/DummyUserFederationProvider.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/DummyUserFederationProvider.java
@@ -25,8 +25,10 @@ import org.keycloak.models.UserCredentialModel;
 import org.keycloak.models.UserFederationProvider;
 import org.keycloak.models.UserModel;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -97,27 +99,39 @@ public class DummyUserFederationProvider implements UserFederationProvider {
 
     @Override
     public boolean isValid(RealmModel realm, UserModel local) {
-        return false;
+        String username = local.getUsername();
+        return users.containsKey(username);
     }
 
     @Override
     public Set<String> getSupportedCredentialTypes(UserModel user) {
-        return Collections.emptySet();
+        // Just user "test-user" is able to validate password with this federationProvider
+        if (user.getUsername().equals("test-user")) {
+            return Collections.singleton(UserCredentialModel.PASSWORD);
+        } else {
+            return Collections.emptySet();
+        }
     }
 
     @Override
     public Set<String> getSupportedCredentialTypes() {
-        return Collections.emptySet();
+        return Collections.singleton(UserCredentialModel.PASSWORD);
     }
 
     @Override
     public boolean validCredentials(RealmModel realm, UserModel user, List<UserCredentialModel> input) {
+        if (user.getUsername().equals("test-user") && input.size() == 1) {
+            UserCredentialModel password = input.get(0);
+            if (password.getType().equals(UserCredentialModel.PASSWORD)) {
+                return "secret".equals(password.getValue());
+            }
+        }
         return false;
     }
 
     @Override
     public boolean validCredentials(RealmModel realm, UserModel user, UserCredentialModel... input) {
-        return false;
+        return validCredentials(realm, user, Arrays.asList(input));
     }
 
     @Override