From 892fa041301235f37d0530de884974dc30a0aa3e Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 26 Aug 2014 18:04:26 +0200 Subject: [PATCH 1/3] KEYCLOAK-645 Disable username field when editing user in admin console --- .../theme/admin/base/resources/partials/user-detail.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html index 5ce68ff08c..41afecc345 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/user-detail.html @@ -41,7 +41,7 @@
+ required ng-pattern="/^[^\<\>\\\/]*$/" data-ng-readonly="!create">
From b3b480c25f461b247f8163d6cc19d6e19b394088 Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 26 Aug 2014 19:10:48 +0200 Subject: [PATCH 2/3] Display proper error message if LDAP-linked user couldn't be deleted due to read-only mode --- .../ldap/LDAPFederationProvider.java | 5 ++++- .../base/resources/js/controllers/users.js | 2 ++ .../models/UserFederationManager.java | 6 +++++ .../resources/admin/UsersResource.java | 9 ++++++-- .../FederationProvidersIntegrationTest.java | 22 +++++++++++++++++++ 5 files changed, 41 insertions(+), 3 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 cb948baba3..45bd0dfa46 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 @@ -126,7 +126,10 @@ public class LDAPFederationProvider implements UserFederationProvider { @Override public boolean removeUser(RealmModel realm, UserModel user) { - if (editMode == EditMode.READ_ONLY || editMode == EditMode.UNSYNCED) return false; + if (editMode == EditMode.READ_ONLY || editMode == EditMode.UNSYNCED) { + logger.warnf("User '%s' can't be deleted in LDAP as editMode is '%s'", user.getUsername(), editMode.toString()); + return false; + } try { return LDAPUtils.removeUser(partitionManager, user.getUsername()); diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js index 8543323b21..304d66c0a4 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/controllers/users.js @@ -253,6 +253,8 @@ module.controller('UserDetailCtrl', function($scope, realm, user, User, UserFede }, function() { $location.url("/realms/" + realm.realm + "/users"); Notifications.success("The user has been deleted."); + }, function() { + Notifications.error("User couldn't be deleted"); }); }); }; 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 083886d00a..13faf73cf2 100755 --- a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java +++ b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java @@ -7,11 +7,16 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.jboss.logging.Logger; + /** * @author Bill Burke * @version $Revision: 1 $ */ public class UserFederationManager implements UserProvider { + + private static final Logger logger = Logger.getLogger(UserFederationManager.class); + protected KeycloakSession session; public UserFederationManager(KeycloakSession session) { @@ -83,6 +88,7 @@ public class UserFederationManager implements UserProvider { RealmModel realmModel = tx.realms().getRealm(realm.getId()); UserModel deletedUser = tx.userStorage().getUserById(user.getId(), realmModel); tx.userStorage().removeUser(realmModel, deletedUser); + logger.debugf("Removed invalid user '%s'", user.getUsername()); tx.getTransaction().commit(); } finally { tx.close(); 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 52e1e65be7..13de4df0c4 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 @@ -305,7 +305,7 @@ public class UsersResource { @Path("{username}") @DELETE @NoCache - public void deleteUser(final @PathParam("username") String username) { + public Response deleteUser(final @PathParam("username") String username) { auth.requireManage(); UserModel user = session.users().getUserByUsername(username, realm); @@ -313,7 +313,12 @@ public class UsersResource { throw new NotFoundException("User not found"); } - new UserManager(session).removeUser(realm, user); + boolean removed = new UserManager(session).removeUser(realm, user); + if (removed) { + return Response.noContent().build(); + } else { + return Flows.errors().error("User couldn't be deleted", Response.Status.BAD_REQUEST); + } } /** diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java index 09a07e6f09..d0b2080df4 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java @@ -236,6 +236,23 @@ public class FederationProvidersIntegrationTest { } } + @Test + public void testRemoveFederatedUser() { + KeycloakSession session = keycloakRule.startSession(); + try { + RealmModel appRealm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("registerUserSuccess2", appRealm); + Assert.assertNotNull(user); + Assert.assertNotNull(user.getFederationLink()); + Assert.assertEquals(user.getFederationLink(), ldapModel.getId()); + + Assert.assertTrue(session.users().removeUser(appRealm, user)); + Assert.assertNull(session.users().getUserByUsername("registerUserSuccess2", appRealm)); + } finally { + keycloakRule.stopSession(session, true); + } + } + @Test public void testReadonly() { KeycloakSession session = keycloakRule.startSession(); @@ -275,6 +292,8 @@ public class FederationProvidersIntegrationTest { } catch (ModelReadOnlyException e) { } + + Assert.assertFalse(session.users().removeUser(appRealm, user)); } finally { keycloakRule.stopSession(session, false); } @@ -311,6 +330,9 @@ public class FederationProvidersIntegrationTest { // LDAP password is still unchanged Assert.assertTrue(LDAPUtils.validatePassword(getPartitionManager(session, model), "johnkeycloak", "new-password")); + + // ATM it's not permitted to delete user in unsynced mode. Should be user deleted just locally instead? + Assert.assertFalse(session.users().removeUser(appRealm, user)); } finally { keycloakRule.stopSession(session, false); } From fc0889cd2e6d0fc65d7e052cf162dc60198b53d6 Mon Sep 17 00:00:00 2001 From: mposolda Date: Tue, 26 Aug 2014 20:24:25 +0200 Subject: [PATCH 3/3] KEYCLOAK-644 Searching by email from LDAP doesn't work --- .../ldap/LDAPFederationProvider.java | 62 ++++++++++------ .../FederationProvidersIntegrationTest.java | 71 ++++++++++++++----- .../testsuite/forms/SyncProvidersTest.java | 2 +- 3 files changed, 94 insertions(+), 41 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 45bd0dfa46..1813e8ab1a 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 @@ -140,31 +140,10 @@ public class LDAPFederationProvider implements UserFederationProvider { @Override public List searchByAttributes(Map attributes, RealmModel realm, int maxResults) { - 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)); - } - query.setLimit(maxResults); - List agents = query.getResultList(); - for (User user : agents) { - results.put(user.getLoginName(), user); - } - } - for (User user : results.values()) { + Map plUsers = searchPicketlink(attributes, maxResults); + for (User user : plUsers.values()) { if (session.userStorage().getUserByUsername(user.getLoginName(), realm) == null) { UserModel imported = importUserFromPicketlink(realm, user); searchResults.add(imported); @@ -176,6 +155,43 @@ public class LDAPFederationProvider implements UserFederationProvider { return searchResults; } + protected Map searchPicketlink(Map attributes, int maxResults) { + IdentityManager identityManager = getIdentityManager(); + Map results = new HashMap(); + if (attributes.containsKey(USERNAME)) { + User user = BasicModel.getUser(identityManager, attributes.get(USERNAME)); + if (user != null) { + results.put(user.getLoginName(), user); + return results; + } + } + + if (attributes.containsKey(EMAIL)) { + User user = queryByEmail(identityManager, attributes.get(EMAIL)); + if (user != null) { + results.put(user.getLoginName(), user); + return results; + } + } + + 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)); + } + query.setLimit(maxResults); + List agents = query.getResultList(); + for (User user : agents) { + results.put(user.getLoginName(), user); + } + } + + return results; + } + @Override public boolean isValid(UserModel local) { try { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java index d0b2080df4..3caae8bc06 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java @@ -236,23 +236,6 @@ public class FederationProvidersIntegrationTest { } } - @Test - public void testRemoveFederatedUser() { - KeycloakSession session = keycloakRule.startSession(); - try { - RealmModel appRealm = session.realms().getRealmByName("test"); - UserModel user = session.users().getUserByUsername("registerUserSuccess2", appRealm); - Assert.assertNotNull(user); - Assert.assertNotNull(user.getFederationLink()); - Assert.assertEquals(user.getFederationLink(), ldapModel.getId()); - - Assert.assertTrue(session.users().removeUser(appRealm, user)); - Assert.assertNull(session.users().getUserByUsername("registerUserSuccess2", appRealm)); - } finally { - keycloakRule.stopSession(session, true); - } - } - @Test public void testReadonly() { KeycloakSession session = keycloakRule.startSession(); @@ -307,6 +290,60 @@ public class FederationProvidersIntegrationTest { } } + @Test + public void testRemoveFederatedUser() { + KeycloakSession session = keycloakRule.startSession(); + try { + RealmModel appRealm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("registerUserSuccess2", appRealm); + Assert.assertNotNull(user); + Assert.assertNotNull(user.getFederationLink()); + Assert.assertEquals(user.getFederationLink(), ldapModel.getId()); + + Assert.assertTrue(session.users().removeUser(appRealm, user)); + Assert.assertNull(session.users().getUserByUsername("registerUserSuccess2", appRealm)); + } finally { + keycloakRule.stopSession(session, true); + } + } + + @Test + public void testSearch() { + KeycloakSession session = keycloakRule.startSession(); + PartitionManager partitionManager = getPartitionManager(session, ldapModel); + try { + RealmModel appRealm = session.realms().getRealmByName("test"); + LDAPUtils.addUser(partitionManager, "username1", "John1", "Doel1", "user1@email.org"); + LDAPUtils.addUser(partitionManager, "username2", "John2", "Doel2", "user2@email.org"); + LDAPUtils.addUser(partitionManager, "username3", "John3", "Doel3", "user3@email.org"); + LDAPUtils.addUser(partitionManager, "username4", "John4", "Doel4", "user4@email.org"); + + // Users are not at local store at this moment + Assert.assertNull(session.userStorage().getUserByUsername("username1", appRealm)); + Assert.assertNull(session.userStorage().getUserByUsername("username2", appRealm)); + Assert.assertNull(session.userStorage().getUserByUsername("username3", appRealm)); + Assert.assertNull(session.userStorage().getUserByUsername("username4", appRealm)); + + // search by username + session.users().searchForUser("username1", appRealm); + SyncProvidersTest.assertUserImported(session.userStorage(), appRealm, "username1", "John1", "Doel1", "user1@email.org"); + + // search by email + session.users().searchForUser("user2@email.org", appRealm); + SyncProvidersTest.assertUserImported(session.userStorage(), appRealm, "username2", "John2", "Doel2", "user2@email.org"); + + // search by lastName + session.users().searchForUser("Doel3", appRealm); + SyncProvidersTest.assertUserImported(session.userStorage(), appRealm, "username3", "John3", "Doel3", "user3@email.org"); + + // search by firstName + lastName + session.users().searchForUser("John4 Doel4", appRealm); + SyncProvidersTest.assertUserImported(session.userStorage(), appRealm, "username4", "John4", "Doel4", "user4@email.org"); + } finally { + keycloakRule.stopSession(session, true); + } + } + @Test public void testUnsynced() { KeycloakSession session = keycloakRule.startSession(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/SyncProvidersTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/SyncProvidersTest.java index 35f863b473..4d2da8e063 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/SyncProvidersTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/SyncProvidersTest.java @@ -185,7 +185,7 @@ public class SyncProvidersTest { } } - private void assertUserImported(UserProvider userProvider, RealmModel realm, String username, String expectedFirstName, String expectedLastName, String expectedEmail) { + public static void assertUserImported(UserProvider userProvider, RealmModel realm, String username, String expectedFirstName, String expectedLastName, String expectedEmail) { UserModel user = userProvider.getUserByUsername(username, realm); Assert.assertNotNull(user); Assert.assertEquals(expectedFirstName, user.getFirstName());