Merge pull request #2305 from mposolda/master

KEYCLOAK-2542 User can't set password for account created over social…
This commit is contained in:
Marek Posolda 2016-03-01 11:54:52 +01:00
commit 4add134dff
8 changed files with 128 additions and 16 deletions

View file

@ -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

View file

@ -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) {

View file

@ -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) {

View file

@ -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()) {

View file

@ -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;
@ -37,7 +39,11 @@ import java.util.Set;
*/
public class DummyUserFederationProvider implements UserFederationProvider {
private static Map<String, UserModel> users = new HashMap<String, UserModel>();
private final Map<String, UserModel> users;
public DummyUserFederationProvider(Map<String, UserModel> users) {
this.users = users;
}
@Override
public UserModel validateAndProxy(RealmModel realm, UserModel local) {
@ -97,27 +103,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

View file

@ -25,6 +25,7 @@ import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderFactory;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserFederationSyncResult;
import org.keycloak.models.UserModel;
import org.keycloak.provider.ConfiguredProvider;
import org.keycloak.provider.ProviderConfigProperty;
@ -43,9 +44,11 @@ public class DummyUserFederationProviderFactory implements UserFederationProvide
private AtomicInteger fullSyncCounter = new AtomicInteger();
private AtomicInteger changedSyncCounter = new AtomicInteger();
private Map<String, UserModel> users = new HashMap<String, UserModel>();
@Override
public UserFederationProvider getInstance(KeycloakSession session, UserFederationProviderModel model) {
return new DummyUserFederationProvider();
return new DummyUserFederationProvider(users);
}
@Override
@ -57,7 +60,7 @@ public class DummyUserFederationProviderFactory implements UserFederationProvide
@Override
public UserFederationProvider create(KeycloakSession session) {
return new DummyUserFederationProvider();
return new DummyUserFederationProvider(users);
}
@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.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;
@ -103,6 +104,9 @@ public abstract class AbstractIdentityProviderTest {
@WebResource
protected OAuthGrantPage grantPage;
@WebResource
AccountUpdateProfilePage accountUpdateProfilePage;
@WebResource
protected AccountPasswordPage changePasswordPage;

View file

@ -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();
}
}
}