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 010870ee2a..4ddd142f23 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 @@ -179,8 +179,8 @@ public class LDAPFederationProvider implements UserFederationProvider { @Override public boolean removeUser(RealmModel realm, UserModel user) { 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; + logger.warnf("User '%s' can't be deleted in LDAP as editMode is '%s'. Deleting user just from Keycloak DB, but he will be re-imported from LDAP again once searched in Keycloak", user.getUsername(), editMode.toString()); + return true; } LDAPObject ldapObject = loadAndValidateUser(realm, user); 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 8f809f3389..ef8d182534 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java +++ b/server-spi/src/main/java/org/keycloak/models/UserFederationManager.java @@ -96,7 +96,7 @@ public class UserFederationManager implements UserProvider { boolean localRemoved = session.userStorage().removeUser(realm, user); managedUsers.remove(user.getId()); if (!localRemoved) { - logger.warn("User removed from federation provider, but failed to remove him from keycloak model"); + logger.warn("User possibly removed from federation provider, but failed to remove him from keycloak model"); } return localRemoved; } else { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java index 3ee6b70dcf..be8c97bd8d 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java @@ -500,6 +500,8 @@ public class FederationProvidersIntegrationTest { } } + + // TODO: Rather separate test for fullNameMapper to better test all the possibilities @Test public void testFullNameMapper() { KeycloakSession session = keycloakRule.startSession(); @@ -691,7 +693,7 @@ public class FederationProvidersIntegrationTest { } - Assert.assertFalse(session.users().removeUser(appRealm, user)); + Assert.assertTrue(session.users().removeUser(appRealm, user)); } finally { keycloakRule.stopSession(session, false); } @@ -826,8 +828,12 @@ public class FederationProvidersIntegrationTest { LDAPObject ldapUser = ldapProvider.loadLDAPUserByUsername(appRealm, "johnkeycloak"); ldapProvider.getLdapIdentityStore().validatePassword(ldapUser, "Password1"); - // 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)); + // User is deleted just locally + Assert.assertTrue(session.users().removeUser(appRealm, user)); + + // Assert user not available locally, but will be reimported from LDAP once searched + Assert.assertNull(session.userStorage().getUserByUsername("johnkeycloak", appRealm)); + Assert.assertNotNull(session.users().getUserByUsername("johnkeycloak", appRealm)); } finally { keycloakRule.stopSession(session, false); }