Merge pull request #2373 from mposolda/master

LDAP fixes
This commit is contained in:
Marek Posolda 2016-03-14 10:32:20 +01:00
commit 975dcb68a9
7 changed files with 60 additions and 34 deletions

View file

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

View file

@ -19,6 +19,7 @@ package org.keycloak.federation.ldap;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -229,6 +230,45 @@ public class LDAPUtils {
}
/**
* Load all LDAP objects corresponding to given query. We will load them paginated, so we allow to bypass the limitation of 1000
* maximum loaded objects in single query in MSAD
*
* @param ldapQuery
* @param ldapProvider
* @return
*/
public static List<LDAPObject> loadAllLDAPObjects(LDAPQuery ldapQuery, LDAPFederationProvider ldapProvider) {
LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig();
boolean pagination = ldapConfig.isPagination();
if (pagination) {
// For now reuse globally configured batch size in LDAP provider page
int pageSize = ldapConfig.getBatchSizeForSync();
List<LDAPObject> result = new LinkedList<>();
boolean nextPage = true;
while (nextPage) {
ldapQuery.setLimit(pageSize);
final List<LDAPObject> currentPageGroups = ldapQuery.getResultList();
result.addAll(currentPageGroups);
nextPage = ldapQuery.getPaginationContext() != null;
}
return result;
} else {
// LDAP pagination not available. Do everything in single transaction
return ldapQuery.getResultList();
}
}
/**
* Validate configured customFilter matches the requested format
*
* @param customFilter
* @throws FederationConfigValidationException
*/
public static void validateCustomLdapFilter(String customFilter) throws FederationConfigValidationException {
if (customFilter != null) {

View file

@ -343,28 +343,7 @@ public class GroupLDAPFederationMapper extends AbstractLDAPFederationMapper impl
// Send LDAP query to retrieve all groups
protected List<LDAPObject> getAllLDAPGroups() {
LDAPQuery ldapGroupQuery = createGroupQuery();
LDAPConfig ldapConfig = ldapProvider.getLdapIdentityStore().getConfig();
boolean pagination = ldapConfig.isPagination();
if (pagination) {
// For now reuse globally configured batch size in LDAP provider page
int pageSize = ldapConfig.getBatchSizeForSync();
List<LDAPObject> result = new LinkedList<>();
boolean nextPage = true;
while (nextPage) {
ldapGroupQuery.setLimit(pageSize);
final List<LDAPObject> currentPageGroups = ldapGroupQuery.getResultList();
result.addAll(currentPageGroups);
nextPage = ldapGroupQuery.getPaginationContext() != null;
}
return result;
} else {
// LDAP pagination not available. Do everything in single transaction
return ldapGroupQuery.getResultList();
}
return LDAPUtils.loadAllLDAPObjects(ldapGroupQuery, ldapProvider);
}

View file

@ -88,9 +88,9 @@ public class GroupLDAPFederationMapperFactory extends AbstractLDAPFederationMapp
for (MembershipType membershipType : MembershipType.values()) {
membershipTypes.add(membershipType.toString());
}
ProviderConfigProperty membershipType = createConfigProperty(RoleMapperConfig.MEMBERSHIP_ATTRIBUTE_TYPE, "Membership Attribute Type",
"DN means that LDAP role has it's members declared in form of their full DN. For example 'member: uid=john,ou=users,dc=example,dc=com' . " +
"UID means that LDAP role has it's members declared in form of pure user uids. For example 'memberUid: john' .",
ProviderConfigProperty membershipType = createConfigProperty(GroupMapperConfig.MEMBERSHIP_ATTRIBUTE_TYPE, "Membership Attribute Type",
"DN means that LDAP group has it's members declared in form of their full DN. For example 'member: uid=john,ou=users,dc=example,dc=com' . " +
"UID means that LDAP group has it's members declared in form of pure user uids. For example 'memberUid: john' .",
ProviderConfigProperty.LIST_TYPE, membershipTypes);
configProperties.add(membershipType);
@ -165,6 +165,7 @@ public class GroupLDAPFederationMapperFactory extends AbstractLDAPFederationMapp
defaultValues.put(GroupMapperConfig.PRESERVE_GROUP_INHERITANCE, "true");
defaultValues.put(GroupMapperConfig.MEMBERSHIP_LDAP_ATTRIBUTE, LDAPConstants.MEMBER);
defaultValues.put(GroupMapperConfig.MEMBERSHIP_ATTRIBUTE_TYPE, MembershipType.DN.toString());
String mode = config.getEditMode() == UserFederationProvider.EditMode.WRITABLE ? LDAPGroupMapperMode.LDAP_ONLY.toString() : LDAPGroupMapperMode.READ_ONLY.toString();
defaultValues.put(GroupMapperConfig.MODE, mode);

View file

@ -122,9 +122,9 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper imple
logger.debugf("Syncing roles from LDAP into Keycloak DB. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getDisplayName());
// Send LDAP query
LDAPQuery ldapQuery = createRoleQuery();
List<LDAPObject> ldapRoles = ldapQuery.getResultList();
// Send LDAP query to load all roles
LDAPQuery ldapRoleQuery = createRoleQuery();
List<LDAPObject> ldapRoles = LDAPUtils.loadAllLDAPObjects(ldapRoleQuery, ldapProvider);
RoleContainerModel roleContainer = getTargetRoleContainer();
String rolesRdnAttr = config.getRoleNameLdapAttribute();

View file

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

View file

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