Merge pull request #2342 from mposolda/master

LDAP fixes
This commit is contained in:
Marek Posolda 2016-03-08 08:46:28 +01:00
commit 229eca302f
7 changed files with 257 additions and 6 deletions

View file

@ -27,6 +27,7 @@ import org.keycloak.federation.ldap.idm.query.internal.LDAPQueryConditionsBuilde
import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore;
import org.keycloak.federation.ldap.kerberos.LDAPProviderKerberosConfig;
import org.keycloak.federation.ldap.mappers.LDAPFederationMapper;
import org.keycloak.federation.ldap.mappers.LDAPMappersComparator;
import org.keycloak.models.CredentialValidationOutput;
import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession;
@ -47,6 +48,7 @@ import org.keycloak.services.managers.UserManager;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@ -128,7 +130,8 @@ public class LDAPFederationProvider implements UserFederationProvider {
}
Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(model.getId());
for (UserFederationMapperModel mapperModel : federationMappers) {
List<UserFederationMapperModel> sortedMappers = sortMappersAsc(federationMappers);
for (UserFederationMapperModel mapperModel : sortedMappers) {
LDAPFederationMapper ldapMapper = getMapper(mapperModel);
proxied = ldapMapper.proxy(mapperModel, this, ldapObject, proxied, realm);
}
@ -313,7 +316,8 @@ public class LDAPFederationProvider implements UserFederationProvider {
imported.setEnabled(true);
Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(getModel().getId());
for (UserFederationMapperModel mapperModel : federationMappers) {
List<UserFederationMapperModel> sortedMappers = sortMappersDesc(federationMappers);
for (UserFederationMapperModel mapperModel : sortedMappers) {
if (logger.isTraceEnabled()) {
logger.tracef("Using mapper %s during import user from LDAP", mapperModel);
}
@ -517,4 +521,13 @@ public class LDAPFederationProvider implements UserFederationProvider {
return ldapMapper;
}
public List<UserFederationMapperModel> sortMappersAsc(Collection<UserFederationMapperModel> mappers) {
return LDAPMappersComparator.sortAsc(getLdapIdentityStore().getConfig(), mappers);
}
protected List<UserFederationMapperModel> sortMappersDesc(Collection<UserFederationMapperModel> mappers) {
return LDAPMappersComparator.sortDesc(getLdapIdentityStore().getConfig(), mappers);
}
}

View file

@ -354,7 +354,8 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi
// Update keycloak user
Set<UserFederationMapperModel> federationMappers = currentRealm.getUserFederationMappersByFederationProvider(fedModel.getId());
for (UserFederationMapperModel mapperModel : federationMappers) {
List<UserFederationMapperModel> sortedMappers = ldapFedProvider.sortMappersDesc(federationMappers);
for (UserFederationMapperModel mapperModel : sortedMappers) {
LDAPFederationMapper ldapMapper = ldapFedProvider.getMapper(mapperModel);
ldapMapper.onImportUserFromLDAP(mapperModel, ldapFedProvider, ldapUser, currentUser, currentRealm, false);
}

View file

@ -19,6 +19,7 @@ package org.keycloak.federation.ldap;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -58,7 +59,8 @@ public class LDAPUtils {
ldapUser.setObjectClasses(ldapConfig.getUserObjectClasses());
Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(ldapProvider.getModel().getId());
for (UserFederationMapperModel mapperModel : federationMappers) {
List<UserFederationMapperModel> sortedMappers = ldapProvider.sortMappersAsc(federationMappers);
for (UserFederationMapperModel mapperModel : sortedMappers) {
LDAPFederationMapper ldapMapper = ldapProvider.getMapper(mapperModel);
ldapMapper.onRegisterUserToLDAP(mapperModel, ldapProvider, ldapUser, user, realm);
}

View file

@ -153,7 +153,8 @@ public class LDAPQuery {
public List<LDAPObject> getResultList() {
// Apply mappers now
for (UserFederationMapperModel mapperModel : mappers) {
List<UserFederationMapperModel> sortedMappers = ldapFedProvider.sortMappersAsc(mappers);
for (UserFederationMapperModel mapperModel : sortedMappers) {
LDAPFederationMapper fedMapper = ldapFedProvider.getMapper(mapperModel);
fedMapper.beforeLDAPQuery(mapperModel, this);
}

View file

@ -0,0 +1,114 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.federation.ldap.mappers;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import org.keycloak.federation.ldap.LDAPConfig;
import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserModel;
/**
* TODO: Possibly add "priority" to UserFederationMapper instead of hardcoding behaviour
*
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class LDAPMappersComparator {
public static List<UserFederationMapperModel> sortAsc(LDAPConfig ldapConfig, Collection<UserFederationMapperModel> mappers) {
Comparator<UserFederationMapperModel> comparator = new ImportantFirstComparator(ldapConfig);
List<UserFederationMapperModel> result = new ArrayList<>(mappers);
Collections.sort(result, comparator);
return result;
}
public static List<UserFederationMapperModel> sortDesc(LDAPConfig ldapConfig, Collection<UserFederationMapperModel> mappers) {
Comparator<UserFederationMapperModel> comparator = new ImportantFirstComparator(ldapConfig).reversed();
List<UserFederationMapperModel> result = new ArrayList<>(mappers);
Collections.sort(result, comparator);
return result;
}
private static class ImportantFirstComparator implements Comparator<UserFederationMapperModel> {
private final LDAPConfig ldapConfig;
public ImportantFirstComparator(LDAPConfig ldapConfig) {
this.ldapConfig = ldapConfig;
}
@Override
public int compare(UserFederationMapperModel o1, UserFederationMapperModel o2) {
// UserAttributeLDAPFederationMapper first
boolean isO1AttrMapper = o1.getFederationMapperType().equals(UserAttributeLDAPFederationMapperFactory.PROVIDER_ID);
boolean isO2AttrMapper = o2.getFederationMapperType().equals(UserAttributeLDAPFederationMapperFactory.PROVIDER_ID);
if (!isO1AttrMapper) {
if (isO2AttrMapper) {
return 1;
} else {
return 0;
}
} else if (!isO2AttrMapper) {
return -1;
}
// Mapper for "username" attribute first
String model1 = o1.getConfig().get(UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE);
String model2 = o2.getConfig().get(UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE);
boolean isO1UsernameMapper = model1 != null && model1.equalsIgnoreCase(UserModel.USERNAME);
boolean isO2UsernameMapper = model2 != null && model2.equalsIgnoreCase(UserModel.USERNAME);
if (!isO1UsernameMapper) {
if (isO2UsernameMapper) {
return 1;
} else {
return 0;
}
} else if (!isO2UsernameMapper) {
return -1;
}
// The username mapper corresponding to the same like configured username for federationProvider is first
String o1LdapAttr = o1.getConfig().get(UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE);
String o2LdapAttr = o2.getConfig().get(UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE);
boolean isO1LdapAttr = o1LdapAttr != null && ldapConfig.getUsernameLdapAttribute().equalsIgnoreCase(o1LdapAttr);
boolean isO2LdapAttr = o2LdapAttr != null && ldapConfig.getUsernameLdapAttribute().equalsIgnoreCase(o2LdapAttr);
if (!isO1LdapAttr) {
if (isO2LdapAttr) {
return 1;
} else {
return 0;
}
} else if (!isO2LdapAttr) {
return -1;
}
return 0;
}
}
}

View file

@ -0,0 +1,117 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.federation.ldap.idm.model;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.junit.Assert;
import org.junit.Test;
import org.keycloak.federation.ldap.LDAPConfig;
import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapper;
import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapperFactory;
import org.keycloak.federation.ldap.mappers.LDAPMappersComparator;
import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapper;
import org.keycloak.federation.ldap.mappers.UserAttributeLDAPFederationMapperFactory;
import org.keycloak.models.LDAPConstants;
import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class LDAPMappersComparatorTest {
@Test
public void testCompareWithCNUsername() {
Map<String, String> cfg = new HashMap<>();
cfg.put(LDAPConstants.USERNAME_LDAP_ATTRIBUTE, LDAPConstants.CN);
LDAPConfig config = new LDAPConfig(cfg);
List<UserFederationMapperModel> sorted = LDAPMappersComparator.sortAsc(config, getMappers());
assertOrder(sorted, "username-cn", "sAMAccountName", "first name", "full name");
sorted = LDAPMappersComparator.sortDesc(config, getMappers());
assertOrder(sorted, "full name", "first name", "sAMAccountName", "username-cn");
}
@Test
public void testCompareWithSAMAccountNameUsername() {
Map<String, String> cfg = new HashMap<>();
cfg.put(LDAPConstants.USERNAME_LDAP_ATTRIBUTE, LDAPConstants.SAM_ACCOUNT_NAME);
LDAPConfig config = new LDAPConfig(cfg);
List<UserFederationMapperModel> sorted = LDAPMappersComparator.sortAsc(config, getMappers());
assertOrder(sorted, "sAMAccountName", "username-cn", "first name", "full name");
sorted = LDAPMappersComparator.sortDesc(config, getMappers());
assertOrder(sorted, "full name", "first name", "username-cn", "sAMAccountName");
}
private void assertOrder(List<UserFederationMapperModel> result, String... names) {
Assert.assertEquals(result.size(), names.length);
for (int i=0 ; i<names.length ; i++) {
Assert.assertEquals(names[i], result.get(i).getName());
}
}
private Set<UserFederationMapperModel> getMappers() {
Set<UserFederationMapperModel> result = new HashSet<>();
UserFederationMapperModel mapperModel = KeycloakModelUtils.createUserFederationMapperModel("first name", "fed-provider", UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.FIRST_NAME,
UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.GIVENNAME,
UserAttributeLDAPFederationMapper.READ_ONLY, "true",
UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "true",
UserAttributeLDAPFederationMapper.IS_MANDATORY_IN_LDAP, "true");
mapperModel.setId("idd1");
result.add(mapperModel);
mapperModel = KeycloakModelUtils.createUserFederationMapperModel("username-cn", "fed-provider", UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.USERNAME,
UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.CN,
UserAttributeLDAPFederationMapper.READ_ONLY, "true",
UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false",
UserAttributeLDAPFederationMapper.IS_MANDATORY_IN_LDAP, "true");
mapperModel.setId("idd2");
result.add(mapperModel);
mapperModel = KeycloakModelUtils.createUserFederationMapperModel("full name", "fed-provider", FullNameLDAPFederationMapperFactory.PROVIDER_ID,
FullNameLDAPFederationMapper.LDAP_FULL_NAME_ATTRIBUTE, LDAPConstants.CN,
UserAttributeLDAPFederationMapper.READ_ONLY, "true");
mapperModel.setId("idd3");
result.add(mapperModel);
mapperModel = KeycloakModelUtils.createUserFederationMapperModel("sAMAccountName", "fed-provider", UserAttributeLDAPFederationMapperFactory.PROVIDER_ID,
UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, UserModel.USERNAME,
UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, LDAPConstants.SAM_ACCOUNT_NAME,
UserAttributeLDAPFederationMapper.READ_ONLY, "false",
UserAttributeLDAPFederationMapper.ALWAYS_READ_VALUE_FROM_LDAP, "false",
UserAttributeLDAPFederationMapper.IS_MANDATORY_IN_LDAP, "true");
mapperModel.setId("idd4");
result.add(mapperModel);
return result;
}
}

View file

@ -96,7 +96,10 @@ public class UserFederationSyncResult {
if (ignored) {
return "Synchronization ignored as it's already in progress";
} else {
String status = String.format("%d imported users, %d updated users, %d removed users", added, updated, removed);
String status = String.format("%d imported users, %d updated users", added, updated);
if (removed > 0) {
status += String.format(", %d removed users", removed);
}
if (failed != 0) {
status += String.format(", %d users failed sync! See server log for more details", failed);
}