From 3bb0842eae7eea86e1c96a07a5ad637aa8f17329 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Fri, 25 Jul 2014 21:05:45 -0400 Subject: [PATCH] federation iteration 2 --- .../ldap/LDAPFederationProvider.java | 149 ++++------ .../models/UserFederationManager.java | 264 +++++++++--------- .../models/UserFederationProvider.java | 54 +++- .../java/org/keycloak/models/UserModel.java | 2 +- .../org/keycloak/models/UserProvider.java | 2 +- .../cache/DefaultCacheUserProvider.java | 8 +- .../models/cache/NoCacheUserProvider.java | 4 +- .../keycloak/models/jpa/JpaUserProvider.java | 14 +- .../keycloak/adapters/MongoUserProvider.java | 6 +- .../ups/security/UpsSecurityApplication.java | 2 +- .../services/managers/UserManager.java | 2 +- .../resources/admin/UsersResource.java | 2 +- .../exportimport/ExportImportTest.java | 6 +- .../keycloak/testsuite/model/AdapterTest.java | 3 +- .../testsuite/model/MultipleRealmsTest.java | 5 +- .../model/UserSessionProviderTest.java | 6 +- .../test/tools/jobs/DeleteUsersJob.java | 3 +- 17 files changed, 274 insertions(+), 258 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java index 49b84f3785..c42f74c43a 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java @@ -18,8 +18,11 @@ import org.picketlink.idm.credential.Password; import org.picketlink.idm.credential.UsernamePasswordCredentials; import org.picketlink.idm.model.basic.BasicModel; import org.picketlink.idm.model.basic.User; +import org.picketlink.idm.query.IdentityQuery; +import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -30,6 +33,7 @@ import java.util.Set; */ public class LDAPFederationProvider implements UserFederationProvider { private static final Logger logger = Logger.getLogger(LDAPFederationProvider.class); + public static final String LDAP_ID = "LDAP_ID"; protected KeycloakSession session; protected UserFederationProviderModel model; @@ -82,7 +86,12 @@ public class LDAPFederationProvider implements UserFederationProvider { } @Override - public UserModel addUser(RealmModel realm, UserModel user) { + public boolean isRegistrationSupported() { + return true; + } + + @Override + public UserModel register(RealmModel realm, UserModel user) { IdentityManager identityManager = getIdentityManager(); try { @@ -95,6 +104,7 @@ public class LDAPFederationProvider implements UserFederationProvider { } catch (IdentityManagementException ie) { throw convertIDMException(ie); } + } @Override @@ -114,48 +124,59 @@ public class LDAPFederationProvider implements UserFederationProvider { } @Override - public String getAdminPage() { - return null; + public List searchByAttributes(Map attributes, RealmModel realm) { + IdentityManager identityManager = getIdentityManager(); + List searchResults =new LinkedList(); + try { + Map results = new HashMap(); + if (attributes.containsKey(USERNAME)) { + User user = BasicModel.getUser(identityManager, attributes.get(USERNAME)); + if (user != null) results.put(user.getLoginName(), user); + } else if (attributes.containsKey(EMAIL)) { + User user = queryByEmail(identityManager, attributes.get(EMAIL)); + if (user != null) results.put(user.getLoginName(), user); + } else if (attributes.containsKey(FIRST_NAME) || attributes.containsKey(LAST_NAME)) { + IdentityQuery query = identityManager.createIdentityQuery(User.class); + if (attributes.containsKey(FIRST_NAME)) { + query.setParameter(User.FIRST_NAME, attributes.get(FIRST_NAME)); + } + if (attributes.containsKey(LAST_NAME)) { + query.setParameter(User.LAST_NAME, attributes.get(LAST_NAME)); + } + List agents = query.getResultList(); + for (User user : agents) { + results.put(user.getLoginName(), user); + } + } + for (User user : results.values()) { + if (session.userStorage().getUserByUsername(user.getLoginName(), realm) == null) { + UserModel imported = importUserFromPicketlink(realm, user); + searchResults.add(imported); + } + } + } catch (IdentityManagementException ie) { + throw convertIDMException(ie); + } + return searchResults; } @Override - public Class getAdminClass() { - return null; //To change body of implemented methods use File | Settings | File Templates. + public boolean isValid(UserModel local) { + IdentityManager identityManager = getIdentityManager(); + + try { + User picketlinkUser = BasicModel.getUser(identityManager, local.getUsername()); + if (picketlinkUser == null) { + return false; + } + return picketlinkUser.getId().equals(local.getAttribute(LDAP_ID)); + } catch (IdentityManagementException ie) { + throw convertIDMException(ie); + } } @Override - public UserModel addUser(RealmModel realm, String id, String username, boolean addDefaultRoles) { - throw new IllegalAccessError("Not allowed to call this"); - } - - @Override - public UserModel addUser(RealmModel realm, String username) { - throw new IllegalAccessError("Not allowed to call this"); - } - - @Override - public boolean removeUser(RealmModel realm, String name) { - throw new IllegalAccessError("Not allowed to call this"); - } - - @Override - public void addSocialLink(RealmModel realm, UserModel user, SocialLinkModel socialLink) { - session.userStorage().addSocialLink(realm, user, socialLink); - } - - @Override - public boolean removeSocialLink(RealmModel realm, UserModel user, String socialProvider) { - return session.userStorage().removeSocialLink(realm, user, socialProvider); - } - - @Override - public UserModel getUserById(String id, RealmModel realm) { - return null; - } - - - @Override - public UserModel getUserByUsername(String username, RealmModel realm) { + public UserModel getUserByUsername(RealmModel realm, String username) { IdentityManager identityManager = getIdentityManager(); try { @@ -182,6 +203,7 @@ public class LDAPFederationProvider implements UserFederationProvider { imported.setFirstName(picketlinkUser.getFirstName()); imported.setLastName(picketlinkUser.getLastName()); imported.setFederationLink(model.getId()); + imported.setAttribute(LDAP_ID, picketlinkUser.getId()); return proxy(imported); } @@ -200,7 +222,7 @@ public class LDAPFederationProvider implements UserFederationProvider { @Override - public UserModel getUserByEmail(String email, RealmModel realm) { + public UserModel getUserByEmail(RealmModel realm, String email) { IdentityManager identityManager = getIdentityManager(); try { @@ -208,63 +230,12 @@ public class LDAPFederationProvider implements UserFederationProvider { if (picketlinkUser == null) { return null; } - return importUserFromPicketlink(realm, picketlinkUser); } catch (IdentityManagementException ie) { throw convertIDMException(ie); } } - @Override - public UserModel getUserBySocialLink(SocialLinkModel socialLink, RealmModel realm) { - return null; - } - - @Override - public List getUsers(RealmModel realm) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public int getUsersCount(RealmModel realm) { - return -1; - } - - @Override - public List getUsers(RealmModel realm, int firstResult, int maxResults) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public List searchForUser(String search, RealmModel realm) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public List searchForUser(String search, RealmModel realm, int firstResult, int maxResults) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public List searchForUserByAttributes(Map attributes, RealmModel realm) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public List searchForUserByAttributes(Map attributes, RealmModel realm, int firstResult, int maxResults) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public Set getSocialLinks(UserModel user, RealmModel realm) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public SocialLinkModel getSocialLink(UserModel user, String socialProvider, RealmModel realm) { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - @Override public void preRemove(RealmModel realm) { // complete Don't think we have to do anything diff --git a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java index f67a821474..14be8cc194 100755 --- a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java +++ b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java @@ -2,6 +2,7 @@ package org.keycloak.models; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -22,7 +23,9 @@ public class UserFederationManager implements UserProvider { UserModel user = session.userStorage().addUser(realm, id, username, addDefaultRoles); for (UserFederationProviderModel federation : realm.getUserFederationProviders()) { UserFederationProvider fed = session.getProvider(UserFederationProvider.class, federation.getProviderName()); - return fed.addUser(realm, user); + if (fed.isRegistrationSupported()) { + return fed.register(realm, user); + } } return user; } @@ -38,7 +41,9 @@ public class UserFederationManager implements UserProvider { UserModel user = session.userStorage().addUser(realm, username); for (UserFederationProviderModel federation : realm.getUserFederationProviders()) { UserFederationProvider fed = getFederationProvider(federation); - return fed.addUser(realm, user); + if (fed.isRegistrationSupported()) { + return fed.register(realm, user); + } } return user; } @@ -54,34 +59,67 @@ public class UserFederationManager implements UserProvider { } @Override - public boolean removeUser(RealmModel realm, String name) { - UserModel user = session.userStorage().getUserByUsername(name, realm); - if (user == null) return false; + public boolean removeUser(RealmModel realm, UserModel user) { UserFederationProvider link = getFederationLink(realm, user); if (link != null) { return link.removeUser(realm, user); } - return session.userStorage().removeUser(realm, name); + return session.userStorage().removeUser(realm, user); } + protected void validateUser(RealmModel realm, UserModel user) { + UserFederationProvider link = getFederationLink(realm, user); + if (link != null && !link.isValid(user)) { + deleteInvalidUser(realm, user); + throw new IllegalStateException("Federated user no longer valid"); + } + + } + + protected void deleteInvalidUser(RealmModel realm, UserModel user) { + KeycloakSession tx = session.getKeycloakSessionFactory().create(); + try { + tx.getTransaction().begin(); + RealmModel realmModel = tx.realms().getRealm(realm.getId()); + UserModel deletedUser = tx.userStorage().getUserById(user.getId(), realmModel); + tx.userStorage().removeUser(realmModel, deletedUser); + tx.getTransaction().commit(); + } finally { + tx.close(); + } + } + + protected boolean isValid(RealmModel realm, UserModel user) { + UserFederationProvider link = getFederationLink(realm, user); + if (link != null) return link.isValid(user); + return true; + } + + + protected UserModel validateAndProxyUser(RealmModel realm, UserModel user) { + UserFederationProvider link = getFederationLink(realm, user); + if (link != null) { + if (isValid(realm, user)) { + return link.proxy(user); + } else { + deleteInvalidUser(realm, user); + return null; + } + } + return user; + } + @Override public void addSocialLink(RealmModel realm, UserModel user, SocialLinkModel socialLink) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - link.addSocialLink(realm, user, socialLink); - return; - } + validateUser(realm, user); session.userStorage().addSocialLink(realm, user, socialLink); - } @Override public boolean removeSocialLink(RealmModel realm, UserModel user, String socialProvider) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - return link.removeSocialLink(realm, user, socialProvider); - } + validateUser(realm, user); + if (user == null) throw new IllegalStateException("Federated user no longer valid"); return session.userStorage().removeSocialLink(realm, user, socialProvider); } @@ -89,16 +127,7 @@ public class UserFederationManager implements UserProvider { public UserModel getUserById(String id, RealmModel realm) { UserModel user = session.userStorage().getUserById(id, realm); if (user != null) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - return link.proxy(user); - } - return user; - } - for (UserFederationProviderModel federation : realm.getUserFederationProviders()) { - UserFederationProvider fed = getFederationProvider(federation); - user = fed.getUserById(id, realm); - if (user != null) return user; + user = validateAndProxyUser(realm, user); } return user; } @@ -107,15 +136,12 @@ public class UserFederationManager implements UserProvider { public UserModel getUserByUsername(String username, RealmModel realm) { UserModel user = session.userStorage().getUserByUsername(username, realm); if (user != null) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - return link.proxy(user); - } - return user; + user = validateAndProxyUser(realm, user); + if (user != null) return user; } for (UserFederationProviderModel federation : realm.getUserFederationProviders()) { UserFederationProvider fed = getFederationProvider(federation); - user = fed.getUserByUsername(username, realm); + user = fed.getUserByUsername(realm, username); if (user != null) return user; } return user; @@ -125,15 +151,12 @@ public class UserFederationManager implements UserProvider { public UserModel getUserByEmail(String email, RealmModel realm) { UserModel user = session.userStorage().getUserByEmail(email, realm); if (user != null) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - return link.proxy(user); - } - return user; + user = validateAndProxyUser(realm, user); + if (user != null) return user; } for (UserFederationProviderModel federation : realm.getUserFederationProviders()) { UserFederationProvider fed = getFederationProvider(federation); - user = fed.getUserByEmail(email, realm); + user = fed.getUserByEmail(realm, email); if (user != null) return user; } return user; @@ -143,16 +166,7 @@ public class UserFederationManager implements UserProvider { public UserModel getUserBySocialLink(SocialLinkModel socialLink, RealmModel realm) { UserModel user = session.userStorage().getUserBySocialLink(socialLink, realm); if (user != null) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - return link.proxy(user); - } - return user; - } - for (UserFederationProviderModel federation : realm.getUserFederationProviders()) { - UserFederationProvider fed = getFederationProvider(federation); - user = fed.getUserBySocialLink(socialLink, realm); - if (user != null) return user; + user = validateAndProxyUser(realm, user); } return user; } @@ -168,33 +182,40 @@ public class UserFederationManager implements UserProvider { return session.userStorage().getUsersCount(realm); } + interface PaginatedQuery { + List query(RealmModel realm, int first, int max); + } + + protected List query(PaginatedQuery pagedQuery, RealmModel realm, int firstResult, int maxResults) { + List results = new LinkedList(); + if (maxResults <= 0) return results; + int first = firstResult; + int max = maxResults; + do { + List query = pagedQuery.query(realm, first, max); + if (query == null || query.size() == 0) return results; + int added = 0; + for (UserModel user : query) { + user = validateAndProxyUser(realm, user); + if (user == null) continue; + results.add(user); + added++; + } + if (results.size() == maxResults) return results; + if (query.size() < max) return results; + first = query.size(); + max -= added; + } while (true); + } + @Override public List getUsers(RealmModel realm, int firstResult, int maxResults) { - Map users = new HashMap(); - List query = session.userStorage().getUsers(realm, firstResult, maxResults); - for (UserModel user : query) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - users.put(user.getUsername(), link.proxy(user)); - } else { - users.put(user.getUsername(), user); + return query(new PaginatedQuery() { + @Override + public List query(RealmModel realm, int first, int max) { + return session.userStorage().getUsers(realm, first, max); } - } - if (users.size() >= maxResults) { - List results = new ArrayList(users.size()); - results.addAll(users.values()); - return results; - } - List federationProviders = realm.getUserFederationProviders(); - for (int i = federationProviders.size() - 1; i >= 0; i--) { - UserFederationProviderModel federation = federationProviders.get(i); - UserFederationProvider fed = getFederationProvider(federation); - query = fed.getUsers(realm, firstResult, maxResults); - for (UserModel user : query) users.put(user.getUsername(), user); - } - List results = new ArrayList(users.size()); - results.addAll(users.values()); - return results; + }, realm, firstResult, maxResults); } @Override @@ -202,33 +223,36 @@ public class UserFederationManager implements UserProvider { return searchForUser(search, realm, 0, Integer.MAX_VALUE); } - @Override - public List searchForUser(String search, RealmModel realm, int firstResult, int maxResults) { - Map users = new HashMap(); - List query = session.userStorage().searchForUser(search, realm, firstResult, maxResults); - for (UserModel user : query) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - users.put(user.getUsername(), link.proxy(user)); - } else { - users.put(user.getUsername(), user); - } - } - if (users.size() >= maxResults) { - List results = new ArrayList(users.size()); - results.addAll(users.values()); - return results; - } - List federationProviders = realm.getUserFederationProviders(); - for (int i = federationProviders.size() - 1; i >= 0; i--) { - UserFederationProviderModel federation = federationProviders.get(i); + void federationLoad(RealmModel realm, Map attributes) { + for (UserFederationProviderModel federation : realm.getUserFederationProviders()) { UserFederationProvider fed = getFederationProvider(federation); - query = fed.searchForUser(search, realm, firstResult, maxResults); - for (UserModel user : query) users.put(user.getUsername(), user); + fed.searchByAttributes(attributes, realm); } - List results = new ArrayList(users.size()); - results.addAll(users.values()); - return results; + } + + @Override + public List searchForUser(final String search, RealmModel realm, int firstResult, int maxResults) { + Map attributes = new HashMap(); + int spaceIndex = search.lastIndexOf(' '); + if (spaceIndex > -1) { + String firstName = search.substring(0, spaceIndex).trim(); + String lastName = search.substring(spaceIndex).trim(); + attributes.put(UserModel.FIRST_NAME, firstName); + attributes.put(UserModel.LAST_NAME, lastName); + } else if (search.indexOf('@') > -1) { + attributes.put(UserModel.USERNAME, search.trim()); + attributes.put(UserModel.EMAIL, search.trim()); + } else { + attributes.put(UserModel.LAST_NAME, search.trim()); + attributes.put(UserModel.USERNAME, search.trim()); + } + federationLoad(realm, attributes); + return query(new PaginatedQuery() { + @Override + public List query(RealmModel realm, int first, int max) { + return session.userStorage().searchForUser(search, realm, first, max); + } + }, realm, firstResult, maxResults); } @Override @@ -237,49 +261,27 @@ public class UserFederationManager implements UserProvider { } @Override - public List searchForUserByAttributes(Map attributes, RealmModel realm, int firstResult, int maxResults) { - Map users = new HashMap(); - List query = session.userStorage().searchForUserByAttributes(attributes, realm, firstResult, maxResults); - for (UserModel user : query) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - users.put(user.getUsername(), link.proxy(user)); - } else { - users.put(user.getUsername(), user); + public List searchForUserByAttributes(final Map attributes, RealmModel realm, int firstResult, int maxResults) { + federationLoad(realm, attributes); + return query(new PaginatedQuery() { + @Override + public List query(RealmModel realm, int first, int max) { + return session.userStorage().searchForUserByAttributes(attributes, realm, first, max); } - } - if (users.size() >= maxResults) { - List results = new ArrayList(users.size()); - results.addAll(users.values()); - return results; - } - List federationProviders = realm.getUserFederationProviders(); - for (int i = federationProviders.size() - 1; i >= 0; i--) { - UserFederationProviderModel federation = federationProviders.get(i); - UserFederationProvider fed = getFederationProvider(federation); - query = fed.searchForUserByAttributes(attributes, realm, firstResult, maxResults); - for (UserModel user : query) users.put(user.getUsername(), user); - } - List results = new ArrayList(users.size()); - results.addAll(users.values()); - return results; + }, realm, firstResult, maxResults); } @Override public Set getSocialLinks(UserModel user, RealmModel realm) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - return link.getSocialLinks(user, realm); - } + validateUser(realm, user); + if (user == null) throw new IllegalStateException("Federated user no longer valid"); return session.userStorage().getSocialLinks(user, realm); } @Override public SocialLinkModel getSocialLink(UserModel user, String socialProvider, RealmModel realm) { - UserFederationProvider link = getFederationLink(realm, user); - if (link != null) { - return link.getSocialLink(user, socialProvider, realm); - } + validateUser(realm, user); + if (user == null) throw new IllegalStateException("Federated user no longer valid"); return session.userStorage().getSocialLink(user, socialProvider, realm); } @@ -305,6 +307,7 @@ public class UserFederationManager implements UserProvider { public boolean validCredentials(RealmModel realm, UserModel user, List input) { UserFederationProvider link = getFederationLink(realm, user); if (link != null) { + validateUser(realm, user); if (link.getSupportedCredentialTypes().size() > 0) { List fedCreds = new ArrayList(); List localCreds = new ArrayList(); @@ -328,6 +331,7 @@ public class UserFederationManager implements UserProvider { public boolean validCredentials(RealmModel realm, UserModel user, UserCredentialModel... input) { UserFederationProvider link = getFederationLink(realm, user); if (link != null) { + validateUser(realm, user); Set supportedCredentialTypes = link.getSupportedCredentialTypes(); if (supportedCredentialTypes.size() > 0) { List fedCreds = new ArrayList(); diff --git a/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java b/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java index 9c25bbace9..2ea322f286 100755 --- a/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java +++ b/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java @@ -1,5 +1,9 @@ package org.keycloak.models; +import org.keycloak.provider.Provider; + +import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -7,11 +11,51 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public interface UserFederationProvider extends UserProvider { +public interface UserFederationProvider extends Provider { + + public static final String USERNAME = UserModel.USERNAME; + public static final String EMAIL = UserModel.EMAIL; + public static final String FIRST_NAME = UserModel.EMAIL; + public static final String LAST_NAME = UserModel.LAST_NAME; + UserModel proxy(UserModel local); - UserModel addUser(RealmModel realm, UserModel user); + boolean isRegistrationSupported(); + UserModel register(RealmModel realm, UserModel user); boolean removeUser(RealmModel realm, UserModel user); + + /** + * Required to import into local storage any user found. + * + * @param realm + * @param username + * @return + */ + UserModel getUserByUsername(RealmModel realm, String username); + + /** + * Required to import into local storage any user found. + * + * @param realm + * @param email + * @return + */ + UserModel getUserByEmail(RealmModel realm, String email); + + /** + * Required to import into local storage any user found. Must not import if user already exists in KeycloakSession.userStorage()! + * Currently only attributes USERNAME, EMAIL, FIRST_NAME and LAST_NAME will be used. + * + * @param attributes + * @param realm + * @return + */ + List searchByAttributes(Map attributes, RealmModel realm); + + void preRemove(RealmModel realm); + void preRemove(RealmModel realm, RoleModel role); + + boolean isValid(UserModel local); Set getSupportedCredentialTypes(); - String getAdminPage(); - Class getAdminClass(); -} + boolean validCredentials(RealmModel realm, UserModel user, List input); + boolean validCredentials(RealmModel realm, UserModel user, UserCredentialModel... input); + void close();} diff --git a/model/api/src/main/java/org/keycloak/models/UserModel.java b/model/api/src/main/java/org/keycloak/models/UserModel.java index d440c62a19..52f398eff7 100755 --- a/model/api/src/main/java/org/keycloak/models/UserModel.java +++ b/model/api/src/main/java/org/keycloak/models/UserModel.java @@ -9,7 +9,7 @@ import java.util.Set; * @version $Revision: 1 $ */ public interface UserModel { - public static final String LOGIN_NAME = "username"; + public static final String USERNAME = "username"; public static final String LAST_NAME = "lastName"; public static final String FIRST_NAME = "firstName"; public static final String EMAIL = "email"; diff --git a/model/api/src/main/java/org/keycloak/models/UserProvider.java b/model/api/src/main/java/org/keycloak/models/UserProvider.java index b64f73128a..0e063d1168 100755 --- a/model/api/src/main/java/org/keycloak/models/UserProvider.java +++ b/model/api/src/main/java/org/keycloak/models/UserProvider.java @@ -15,7 +15,7 @@ public interface UserProvider extends Provider { UserModel addUser(RealmModel realm, String id, String username, boolean addDefaultRoles); UserModel addUser(RealmModel realm, String username); - boolean removeUser(RealmModel realm, String name); + boolean removeUser(RealmModel realm, UserModel user); public void addSocialLink(RealmModel realm, UserModel user, SocialLinkModel socialLink); public boolean removeSocialLink(RealmModel realm, UserModel user, String socialProvider); diff --git a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/DefaultCacheUserProvider.java b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/DefaultCacheUserProvider.java index 68c61494ed..6fd6eea663 100755 --- a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/DefaultCacheUserProvider.java +++ b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/DefaultCacheUserProvider.java @@ -248,12 +248,10 @@ public class DefaultCacheUserProvider implements CacheUserProvider { } @Override - public boolean removeUser(RealmModel realm, String name) { - if (!cache.isEnabled()) return getDelegate().removeUser(realm, name); - UserModel user = getUserByUsername(name, realm); - if (user == null) return false; + public boolean removeUser(RealmModel realm, UserModel user) { + if (!cache.isEnabled()) return getDelegate().removeUser(realm, user); registerUserInvalidation(realm, user.getId()); - return getDelegate().removeUser(realm, name); + return getDelegate().removeUser(realm, user); } @Override diff --git a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/NoCacheUserProvider.java b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/NoCacheUserProvider.java index 95f7614932..6831a4347c 100755 --- a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/NoCacheUserProvider.java +++ b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/NoCacheUserProvider.java @@ -125,8 +125,8 @@ public class NoCacheUserProvider implements CacheUserProvider { } @Override - public boolean removeUser(RealmModel realm, String name) { - return getDelegate().removeUser(realm, name); + public boolean removeUser(RealmModel realm, UserModel user) { + return getDelegate().removeUser(realm, user); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java index b93fdff0f3..b3e5bf767c 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java @@ -15,7 +15,6 @@ import org.keycloak.models.utils.CredentialValidation; import org.keycloak.models.utils.KeycloakModelUtils; import javax.persistence.EntityManager; -import javax.persistence.Query; import javax.persistence.TypedQuery; import java.util.ArrayList; import java.util.HashSet; @@ -71,13 +70,10 @@ public class JpaUserProvider implements UserProvider { } @Override - public boolean removeUser(RealmModel realm, String name) { - TypedQuery query = em.createNamedQuery("getRealmUserByUsername", UserEntity.class); - query.setParameter("username", name); - query.setParameter("realmId", realm.getId()); - List results = query.getResultList(); - if (results.size() == 0) return false; - removeUser(results.get(0)); + public boolean removeUser(RealmModel realm, UserModel user) { + UserEntity userEntity = em.find(UserEntity.class, user.getId()); + if (userEntity == null) return false; + removeUser(userEntity); return true; } @@ -256,7 +252,7 @@ public class JpaUserProvider implements UserProvider { boolean first = true; for (Map.Entry entry : attributes.entrySet()) { String attribute = null; - if (entry.getKey().equals(UserModel.LOGIN_NAME)) { + if (entry.getKey().equals(UserModel.USERNAME)) { attribute = "lower(username)"; } else if (entry.getKey().equalsIgnoreCase(UserModel.FIRST_NAME)) { attribute = "lower(firstName)"; diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserProvider.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserProvider.java index df60d25d33..bba9de4eb3 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserProvider.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/MongoUserProvider.java @@ -192,7 +192,7 @@ public class MongoUserProvider implements UserProvider { .and("realmId").is(realm.getId()); for (Map.Entry entry : attributes.entrySet()) { - if (entry.getKey().equals(UserModel.LOGIN_NAME)) { + if (entry.getKey().equals(UserModel.USERNAME)) { queryBuilder.and("username").regex(Pattern.compile("(?i:" + entry.getValue() + "$)")); } else if (entry.getKey().equalsIgnoreCase(UserModel.FIRST_NAME)) { queryBuilder.and(UserModel.FIRST_NAME).regex(Pattern.compile("(?i:" + entry.getValue() + "$)")); @@ -285,9 +285,9 @@ public class MongoUserProvider implements UserProvider { } @Override - public boolean removeUser(RealmModel realm, String name) { + public boolean removeUser(RealmModel realm, UserModel user) { DBObject query = new QueryBuilder() - .and("username").is(name) + .and("id").is(user.getId()) .and("realmId").is(realm.getId()) .get(); return getMongoStore().removeEntities(MongoUserEntity.class, query, invocationContext); diff --git a/project-integrations/aerogear-ups/auth-server/src/main/java/org/aerogear/ups/security/UpsSecurityApplication.java b/project-integrations/aerogear-ups/auth-server/src/main/java/org/aerogear/ups/security/UpsSecurityApplication.java index af9fbed834..c4f8c9be24 100755 --- a/project-integrations/aerogear-ups/auth-server/src/main/java/org/aerogear/ups/security/UpsSecurityApplication.java +++ b/project-integrations/aerogear-ups/auth-server/src/main/java/org/aerogear/ups/security/UpsSecurityApplication.java @@ -31,7 +31,7 @@ public class UpsSecurityApplication extends KeycloakApplication { RealmManager manager = new RealmManager(session); RealmModel master = manager.getKeycloakAdminstrationRealm(); UserModel admin = session.users().getUserByUsername("admin", master); - if (admin != null) session.users().removeUser(master, admin.getUsername()); + if (admin != null) session.users().removeUser(master, admin); session.getTransaction().commit(); } finally { session.close(); diff --git a/services/src/main/java/org/keycloak/services/managers/UserManager.java b/services/src/main/java/org/keycloak/services/managers/UserManager.java index eb7c98f83d..c6732b7425 100755 --- a/services/src/main/java/org/keycloak/services/managers/UserManager.java +++ b/services/src/main/java/org/keycloak/services/managers/UserManager.java @@ -17,7 +17,7 @@ public class UserManager { } public boolean removeUser(RealmModel realm, UserModel user) { - if (session.users().removeUser(realm, user.getUsername())) { + if (session.users().removeUser(realm, user)) { UserSessionProvider sessions = session.sessions(); if (sessions != null) { sessions.onUserRemoved(realm, user); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index 184299f279..8c1cffa285 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -341,7 +341,7 @@ public class UsersResource { attributes.put(UserModel.EMAIL, email); } if (username != null) { - attributes.put(UserModel.LOGIN_NAME, username); + attributes.put(UserModel.USERNAME, username); } userModels = session.users().searchForUserByAttributes(attributes, realm, firstResult, maxResults); for (UserModel user : userModels) { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java index c20eb76ef5..2eaaaa5a37 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java @@ -213,7 +213,8 @@ public class ExportImportTest { Assert.assertEquals(1, realmProvider.getRealms().size()); RealmModel master = realmProvider.getRealmByName(Config.getAdminRealm()); - session.users().removeUser(master, "admin2"); + UserModel admin2 = session.users().getUserByUsername("admin2", master); + session.users().removeUser(master, admin2); assertNotAuthenticated(userProvider, realmProvider, Config.getAdminRealm(), "admin2", "admin2"); assertNotAuthenticated(userProvider, realmProvider, "test", "test-user@localhost", "password"); assertNotAuthenticated(userProvider, realmProvider, "test", "user1", "password"); @@ -263,7 +264,8 @@ public class ExportImportTest { Assert.assertEquals(1, realmProvider.getRealms().size()); RealmModel master = realmProvider.getRealmByName(Config.getAdminRealm()); - session.users().removeUser(master, "admin2"); + UserModel admin2 = session.users().getUserByUsername("admin2", master); + session.users().removeUser(master, admin2); assertNotAuthenticated(userProvider, realmProvider, Config.getAdminRealm(), "admin2", "admin2"); assertNotAuthenticated(userProvider, realmProvider, "test", "test-user@localhost", "password"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java index b72d5ec3cb..f357c8c9ee 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java @@ -181,8 +181,7 @@ public class AdapterTest extends AbstractModelTest { commit(); realmModel = model.getRealm("JUGGLER"); - Assert.assertTrue(realmManager.getSession().users().removeUser(realmModel, "bburke")); - Assert.assertFalse(realmManager.getSession().users().removeUser(realmModel, "bburke")); + Assert.assertTrue(realmManager.getSession().users().removeUser(realmModel, user)); assertNull(realmManager.getSession().users().getUserByUsername("bburke", realmModel)); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/MultipleRealmsTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/MultipleRealmsTest.java index 4c60766b7d..023a6fb8f5 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/MultipleRealmsTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/MultipleRealmsTest.java @@ -52,8 +52,9 @@ public class MultipleRealmsTest extends AbstractModelTest { realm1 = model.getRealm("id1"); realm2 = model.getRealm("id2"); - session.users().removeUser(realm1, "user1"); - session.users().removeUser(realm1, "user2"); + session.users().removeUser(realm1, r1user1); + UserModel user2 = session.users().getUserByUsername("user2", realm1); + session.users().removeUser(realm1, user2); Assert.assertEquals(0, session.users().searchForUser("user", realm1).size()); Assert.assertEquals(2, session.users().searchForUser("user", realm2).size()); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java index 6bec2d8bf7..c089781306 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserSessionProviderTest.java @@ -45,8 +45,10 @@ public class UserSessionProviderTest { public void after() { resetSession(); session.sessions().removeUserSessions(realm); - session.users().removeUser(realm, "user1"); - session.users().removeUser(realm, "user2"); + UserModel user1 = session.users().getUserByUsername("user1", realm); + UserModel user2 = session.users().getUserByUsername("user2", realm); + session.users().removeUser(realm, user1); + session.users().removeUser(realm, user2); kc.stopSession(session, true); } diff --git a/testsuite/tools/src/main/java/org/keycloak/test/tools/jobs/DeleteUsersJob.java b/testsuite/tools/src/main/java/org/keycloak/test/tools/jobs/DeleteUsersJob.java index b215e89958..03ee710f43 100755 --- a/testsuite/tools/src/main/java/org/keycloak/test/tools/jobs/DeleteUsersJob.java +++ b/testsuite/tools/src/main/java/org/keycloak/test/tools/jobs/DeleteUsersJob.java @@ -32,7 +32,6 @@ public class DeleteUsersJob extends UsersJob { @Override protected void runIteration(KeycloakSession session, RealmModel realm, Map apps, Set realmRoles, Map> appRoles, int counter) { - String username = users.next().getUsername(); - session.users().removeUser(realm, username); + session.users().removeUser(realm, users.next()); } }