KEYCLOAK-2542 User can't set password for account created over social login if UserFedarationProvider used

This commit is contained in:
mposolda 2016-03-01 10:11:26 +01:00
parent 47652ae179
commit 0768bcc452
7 changed files with 118 additions and 13 deletions

View file

@ -55,9 +55,7 @@ public class UserFederationManager implements UserProvider {
} }
protected UserFederationProvider getFederationProvider(UserFederationProviderModel model) { protected UserFederationProvider getFederationProvider(UserFederationProviderModel model) {
UserFederationProviderFactory factory = (UserFederationProviderFactory)session.getKeycloakSessionFactory().getProviderFactory(UserFederationProvider.class, model.getProviderName()); return KeycloakModelUtils.getFederationProviderInstance(session, model);
return factory.getInstance(session, model);
} }
@Override @Override

View file

@ -35,6 +35,8 @@ import org.keycloak.models.RoleModel;
import org.keycloak.models.ScopeContainerModel; import org.keycloak.models.ScopeContainerModel;
import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserFederationMapperModel; import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderFactory;
import org.keycloak.models.UserFederationProviderModel; import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.representations.idm.CertificateRepresentation; import org.keycloak.representations.idm.CertificateRepresentation;
@ -406,6 +408,12 @@ public final class KeycloakModelUtils {
return mapperModel; 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 // END USER FEDERATION RELATED STUFF
public static String toLowerCaseSafe(String str) { public static String toLowerCaseSafe(String str) {

View file

@ -79,7 +79,7 @@ public class AccountFederatedIdentityBean {
this.identities = new LinkedList<FederatedIdentityEntry>(orderedSet); this.identities = new LinkedList<FederatedIdentityEntry>(orderedSet);
// Removing last social provider is not possible if you don't have other possibility to authenticate // 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) { private FederatedIdentityModel getIdentity(Set<FederatedIdentityModel> identities, String providerId) {

View file

@ -30,16 +30,20 @@ import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientSessionModel; import org.keycloak.models.ClientSessionModel;
import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelException; import org.keycloak.models.ModelException;
import org.keycloak.models.ModelReadOnlyException; import org.keycloak.models.ModelReadOnlyException;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserCredentialValueModel; import org.keycloak.models.UserCredentialValueModel;
import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.models.UserSessionModel; import org.keycloak.models.UserSessionModel;
import org.keycloak.models.utils.CredentialValidation; import org.keycloak.models.utils.CredentialValidation;
import org.keycloak.models.utils.FormMessage; import org.keycloak.models.utils.FormMessage;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.protocol.oidc.utils.RedirectUtils;
@ -298,7 +302,7 @@ public class AccountService extends AbstractSecuredLocalService {
@GET @GET
public Response passwordPage() { public Response passwordPage() {
if (auth != null) { if (auth != null) {
account.setPasswordSet(isPasswordSet(auth.getUser())); account.setPasswordSet(isPasswordSet(session, realm, auth.getUser()));
} }
return forwardToPage("password", AccountPages.PASSWORD); return forwardToPage("password", AccountPages.PASSWORD);
@ -601,7 +605,7 @@ public class AccountService extends AbstractSecuredLocalService {
csrfCheck(formData); csrfCheck(formData);
UserModel user = auth.getUser(); UserModel user = auth.getUser();
boolean requireCurrent = isPasswordSet(user); boolean requireCurrent = isPasswordSet(session, realm, user);
account.setPasswordSet(requireCurrent); account.setPasswordSet(requireCurrent);
String password = formData.getFirst("password"); String password = formData.getFirst("password");
@ -723,7 +727,7 @@ public class AccountService extends AbstractSecuredLocalService {
if (link != null) { if (link != null) {
// Removing last social provider is not possible if you don't have other possibility to authenticate // 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); session.users().removeFederatedIdentity(realm, user, providerId);
logger.debugv("Social provider {0} removed successfully from user {1}", providerId, user.getUsername()); 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()); 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; boolean passwordSet = false;
// See if password is set for user on linked UserFederationProvider
if (user.getFederationLink() != null) { 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()) { for (UserCredentialValueModel c : user.getCredentialsDirectly()) {

View file

@ -25,8 +25,10 @@ import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserFederationProvider; import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
@ -97,27 +99,39 @@ public class DummyUserFederationProvider implements UserFederationProvider {
@Override @Override
public boolean isValid(RealmModel realm, UserModel local) { public boolean isValid(RealmModel realm, UserModel local) {
return false; String username = local.getUsername();
return users.containsKey(username);
} }
@Override @Override
public Set<String> getSupportedCredentialTypes(UserModel user) { 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 @Override
public Set<String> getSupportedCredentialTypes() { public Set<String> getSupportedCredentialTypes() {
return Collections.emptySet(); return Collections.singleton(UserCredentialModel.PASSWORD);
} }
@Override @Override
public boolean validCredentials(RealmModel realm, UserModel user, List<UserCredentialModel> input) { 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; return false;
} }
@Override @Override
public boolean validCredentials(RealmModel realm, UserModel user, UserCredentialModel... input) { public boolean validCredentials(RealmModel realm, UserModel user, UserCredentialModel... input) {
return false; return validCredentials(realm, user, Arrays.asList(input));
} }
@Override @Override

View file

@ -38,6 +38,7 @@ import org.keycloak.testsuite.broker.util.UserSessionStatusServlet;
import org.keycloak.testsuite.broker.util.UserSessionStatusServlet.UserSessionStatus; import org.keycloak.testsuite.broker.util.UserSessionStatusServlet.UserSessionStatus;
import org.keycloak.testsuite.pages.AccountFederatedIdentityPage; import org.keycloak.testsuite.pages.AccountFederatedIdentityPage;
import org.keycloak.testsuite.pages.AccountPasswordPage; import org.keycloak.testsuite.pages.AccountPasswordPage;
import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.LoginUpdateProfilePage; import org.keycloak.testsuite.pages.LoginUpdateProfilePage;
import org.keycloak.testsuite.pages.OAuthGrantPage; import org.keycloak.testsuite.pages.OAuthGrantPage;
@ -103,6 +104,9 @@ public abstract class AbstractIdentityProviderTest {
@WebResource @WebResource
protected OAuthGrantPage grantPage; protected OAuthGrantPage grantPage;
@WebResource
AccountUpdateProfilePage accountUpdateProfilePage;
@WebResource @WebResource
protected AccountPasswordPage changePasswordPage; protected AccountPasswordPage changePasswordPage;

View file

@ -19,6 +19,7 @@ package org.keycloak.testsuite.broker;
import java.io.IOException; import java.io.IOException;
import java.net.URI; import java.net.URI;
import java.util.HashMap;
import java.util.Set; import java.util.Set;
import javax.mail.MessagingException; import javax.mail.MessagingException;
@ -39,9 +40,11 @@ import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel; import org.keycloak.models.RoleModel;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation;
import org.keycloak.services.Urls; import org.keycloak.services.Urls;
import org.keycloak.testsuite.DummyUserFederationProviderFactory;
import org.keycloak.testsuite.broker.util.UserSessionStatusServlet; import org.keycloak.testsuite.broker.util.UserSessionStatusServlet;
import org.openqa.selenium.By; import org.openqa.selenium.By;
import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.NoSuchElementException;
@ -531,4 +534,64 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent
protected abstract void doAssertTokenRetrieval(String pageSource); 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();
}
}
} }