Merge pull request #1310 from mposolda/ldap2

KEYCLOAK-886 LDAP: Fixes and improvements
This commit is contained in:
Marek Posolda 2015-06-01 18:12:04 +02:00
commit f5c7c14ce0
15 changed files with 75 additions and 194 deletions

View file

@ -49,21 +49,8 @@ public class LDAPConfig {
return config.get(LDAPConstants.SECURITY_PROTOCOL);
}
public Collection<String> getUserDns() {
String value = config.get(LDAPConstants.USER_DNS);
if (value == null) {
return Collections.emptyList();
} else {
return Arrays.asList(value.split(LDAPConstants.CONFIG_DIVIDER));
}
}
public String getSingleUserDn() {
Collection<String> dns = getUserDns();
if (dns.size() == 0) {
throw new IllegalStateException("No user DN configured. User DNS value is " + config.get(LDAPConstants.USER_DNS));
}
return dns.iterator().next();
public String getUsersDn() {
return config.get(LDAPConstants.USERS_DN);
}
public Collection<String> getUserObjectClasses() {

View file

@ -34,7 +34,7 @@ public class LDAPUtils {
ldapUser.setRdnAttributeName(ldapConfig.getRdnLdapAttribute());
ldapUser.setObjectClasses(ldapConfig.getUserObjectClasses());
Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappers();
Set<UserFederationMapperModel> federationMappers = realm.getUserFederationMappersByFederationProvider(ldapProvider.getModel().getId());
for (UserFederationMapperModel mapperModel : federationMappers) {
LDAPFederationMapper ldapMapper = ldapProvider.getMapper(mapperModel);
ldapMapper.onRegisterUserToLDAP(mapperModel, ldapProvider, ldapUser, user, realm);
@ -59,7 +59,7 @@ public class LDAPUtils {
LDAPIdentityQuery ldapQuery = new LDAPIdentityQuery(ldapProvider);
LDAPConfig config = ldapProvider.getLdapIdentityStore().getConfig();
ldapQuery.setSearchScope(config.getSearchScope());
ldapQuery.addSearchDns(config.getUserDns());
ldapQuery.setSearchDn(config.getUsersDn());
ldapQuery.addObjectClasses(config.getUserObjectClasses());
Set<UserFederationMapperModel> mapperModels = realm.getUserFederationMappersByFederationProvider(ldapProvider.getModel().getId());
@ -68,17 +68,17 @@ public class LDAPUtils {
return ldapQuery;
}
// ldapUser has filled attributes, but doesn't have filled dn
public static void computeAndSetDn(LDAPConfig config, LDAPObject ldapObject) {
// ldapUser has filled attributes, but doesn't have filled dn.
private static void computeAndSetDn(LDAPConfig config, LDAPObject ldapUser) {
String rdnLdapAttrName = config.getRdnLdapAttribute();
String rdnLdapAttrValue = ldapObject.getAttributeAsString(rdnLdapAttrName);
String rdnLdapAttrValue = ldapUser.getAttributeAsString(rdnLdapAttrName);
if (rdnLdapAttrValue == null) {
throw new ModelException("RDN Attribute [" + rdnLdapAttrName + "] is not filled. Filled attributes: " + ldapObject.getAttributes());
throw new ModelException("RDN Attribute [" + rdnLdapAttrName + "] is not filled. Filled attributes: " + ldapUser.getAttributes());
}
LDAPDn dn = LDAPDn.fromString(config.getSingleUserDn());
dn.addToHead(rdnLdapAttrName, rdnLdapAttrValue);
ldapObject.setDn(dn);
LDAPDn dn = LDAPDn.fromString(config.getUsersDn());
dn.addFirst(rdnLdapAttrName, rdnLdapAttrValue);
ldapUser.setDn(dn);
}
public static String getUsername(LDAPObject ldapUser, LDAPConfig config) {

View file

@ -2,15 +2,13 @@ package org.keycloak.federation.ldap.idm.model;
import java.util.Deque;
import java.util.LinkedList;
import java.util.List;
import java.util.Queue;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class LDAPDn {
private final Deque<Entry> entries = new LinkedList<Entry>();
private final Deque<Entry> entries = new LinkedList<>();
public static LDAPDn fromString(String dnString) {
LDAPDn dn = new LDAPDn();
@ -18,7 +16,7 @@ public class LDAPDn {
String[] rdns = dnString.split(",");
for (String entryStr : rdns) {
String[] rdn = entryStr.split("=");
dn.addToBottom(rdn[0].trim(), rdn[1].trim());
dn.addLast(rdn[0].trim(), rdn[1].trim());
}
return dn;
@ -62,14 +60,14 @@ public class LDAPDn {
* @return string like "dc=something,dc=org" from the DN like "uid=joe,dc=something,dc=org"
*/
public String getParentDn() {
return new LinkedList<Entry>(entries).remove().toString();
return new LinkedList<>(entries).remove().toString();
}
public void addToHead(String rdnName, String rdnValue) {
public void addFirst(String rdnName, String rdnValue) {
entries.addFirst(new Entry(rdnName, rdnValue));
}
public void addToBottom(String rdnName, String rdnValue) {
public void addLast(String rdnName, String rdnValue) {
entries.addLast(new Entry(rdnName, rdnValue));
}

View file

@ -1,6 +1,5 @@
package org.keycloak.federation.ldap.idm.model;
import java.io.Serializable;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedList;

View file

@ -13,7 +13,6 @@ import org.keycloak.federation.ldap.LDAPFederationProvider;
import org.keycloak.federation.ldap.idm.model.LDAPObject;
import org.keycloak.federation.ldap.idm.query.Condition;
import org.keycloak.federation.ldap.idm.query.Sort;
import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore;
import org.keycloak.federation.ldap.mappers.LDAPFederationMapper;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelException;
@ -34,10 +33,10 @@ public class LDAPIdentityQuery {
private int offset;
private int limit;
private byte[] paginationContext;
private String searchDn;
private final Set<Condition> conditions = new LinkedHashSet<Condition>();
private final Set<Sort> ordering = new LinkedHashSet<Sort>();
private final Set<String> searchDns = new LinkedHashSet<String>();
private final Set<String> returningLdapAttributes = new LinkedHashSet<String>();
// Contains just those returningLdapAttributes, which are read-only. They will be marked as read-only in returned LDAPObject instances as well
@ -62,8 +61,8 @@ public class LDAPIdentityQuery {
return this;
}
public LDAPIdentityQuery addSearchDns(Collection<String> searchDns) {
this.searchDns.addAll(searchDns);
public LDAPIdentityQuery setSearchDn(String searchDn) {
this.searchDn = searchDn;
return this;
}
@ -96,8 +95,8 @@ public class LDAPIdentityQuery {
return unmodifiableSet(this.ordering);
}
public Set<String> getSearchDns() {
return unmodifiableSet(this.searchDns);
public String getSearchDn() {
return this.searchDn;
}
public Set<String> getObjectClasses() {

View file

@ -56,7 +56,7 @@ public class LDAPIdentityStore implements IdentityStore {
this.config = config;
try {
this.operationManager = new LDAPOperationManager(getConfig());
this.operationManager = new LDAPOperationManager(config);
} catch (NamingException e) {
throw new ModelException("Couldn't init operation manager", e);
}
@ -115,8 +115,7 @@ public class LDAPIdentityStore implements IdentityStore {
throw new ModelException("LDAP Identity Store does not yet support sorted queries.");
}
// TODO: proper support for search by more DNs
String baseDN = identityQuery.getSearchDns().iterator().next();
String baseDN = identityQuery.getSearchDn();
for (Condition condition : identityQuery.getConditions()) {
@ -263,6 +262,7 @@ public class LDAPIdentityStore implements IdentityStore {
return filter;
}
protected void applyCondition(StringBuilder filter, Condition condition) {
if (OrCondition.class.isInstance(condition)) {
OrCondition orCondition = (OrCondition) condition;
@ -351,6 +351,7 @@ public class LDAPIdentityStore implements IdentityStore {
}
}
private StringBuilder getObjectClassesFilter(Collection<String> objectClasses) {
StringBuilder builder = new StringBuilder();
@ -365,6 +366,7 @@ public class LDAPIdentityStore implements IdentityStore {
return builder;
}
private LDAPObject populateAttributedType(SearchResult searchResult, Collection<String> readOnlyAttrNames) {
try {
String entryDN = searchResult.getNameInNamespace();
@ -430,67 +432,12 @@ public class LDAPIdentityStore implements IdentityStore {
return ldapObject;
/*LDAPMappingConfiguration entryConfig = getMappingConfig(attributedType.getClass());
if (mappingConfig.getParentMembershipAttributeName() != null) {
StringBuilder filter = new StringBuilder("(&");
String entryBaseDN = entryDN.substring(entryDN.indexOf(LDAPConstants.COMMA) + 1);
filter
.append("(")
.append(getObjectClassesFilter(entryConfig))
.append(")")
.append("(")
.append(mappingConfig.getParentMembershipAttributeName())
.append(LDAPConstants.EQUAL).append("")
.append(getBindingDN(attributedType, false))
.append(LDAPConstants.COMMA)
.append(entryBaseDN)
.append(")");
filter.append(")");
if (logger.isTraceEnabled()) {
logger.tracef("Searching parent entry for DN [%s] using filter [%s].", entryBaseDN, filter.toString());
}
List<SearchResult> search = this.operationManager.search(getConfig().getBaseDN(), filter.toString(), entryConfig);
if (!search.isEmpty()) {
SearchResult next = search.get(0);
Property<IdentityType> parentProperty = PropertyQueries
.<IdentityType>createQuery(attributedType.getClass())
.addCriteria(new TypedPropertyCriteria(attributedType.getClass())).getFirstResult();
if (parentProperty != null) {
String parentDN = next.getNameInNamespace();
String parentBaseDN = parentDN.substring(parentDN.indexOf(",") + 1);
Class<? extends IdentityType> baseDNType = getConfig().getSupportedTypeByBaseDN(parentBaseDN, getEntryObjectClasses(attributes));
if (parentProperty.getJavaClass().isAssignableFrom(baseDNType)) {
if (logger.isTraceEnabled()) {
logger.tracef("Found parent [%s] for entry for DN [%s].", parentDN, entryDN);
}
int hierarchyDepthCount1 = ++hierarchyDepthCount;
parentProperty.setValue(attributedType, populateAttributedType(next, null, hierarchyDepthCount1));
}
}
} else {
if (logger.isTraceEnabled()) {
logger.tracef("No parent entry found for DN [%s] using filter [%s].", entryDN, filter.toString());
}
}
} */
} catch (Exception e) {
throw new ModelException("Could not populate attribute type " + searchResult.getNameInNamespace() + ".", e);
}
}
protected BasicAttributes extractAttributes(LDAPObject ldapObject, boolean isCreate) {
BasicAttributes entryAttributes = new BasicAttributes();
@ -539,80 +486,6 @@ public class LDAPIdentityStore implements IdentityStore {
return entryAttributes;
}
/*public String getBindingDN(IdentityType attributedType, boolean appendBaseDN) {
LDAPMappingConfiguration mappingConfig = getMappingConfig(attributedType.getClass());
String baseDN;
if (mappingConfig.getBaseDN() == null || !appendBaseDN) {
baseDN = "";
} else {
baseDN = LDAPConstants.COMMA + getBaseDN(attributedType);
}
Property<String> bindingDnAttributeProperty = mappingConfig.getBindingDnProperty();
String bindingAttributeName = mappingConfig.getMappedAttributes().get(bindingDnAttributeProperty.getName());
String bindingAttributeValue = mappingConfig.getBindingDnProperty().getValue(attributedType);
return bindingAttributeName + LDAPConstants.EQUAL + bindingAttributeValue + baseDN;
}
private String getBaseDN(IdentityType attributedType) {
LDAPMappingConfiguration mappingConfig = getMappingConfig(attributedType.getClass());
String baseDN = mappingConfig.getBaseDN();
String parentDN = mappingConfig.getParentMapping().get(mappingConfig.getIdProperty().getValue(attributedType));
if (parentDN != null) {
baseDN = parentDN;
} else {
Property<IdentityType> parentProperty = PropertyQueries
.<IdentityType>createQuery(attributedType.getClass())
.addCriteria(new TypedPropertyCriteria(attributedType.getClass())).getFirstResult();
if (parentProperty != null) {
IdentityType parentType = parentProperty.getValue(attributedType);
if (parentType != null) {
Property<String> parentIdProperty = getMappingConfig(parentType.getClass()).getIdProperty();
String parentId = parentIdProperty.getValue(parentType);
String parentBaseDN = mappingConfig.getParentMapping().get(parentId);
if (parentBaseDN != null) {
baseDN = parentBaseDN;
} else {
baseDN = getBaseDN(parentType);
}
}
}
}
return baseDN;
}
protected void addToParentAsMember(final IdentityType attributedType) {
LDAPMappingConfiguration entryConfig = getMappingConfig(attributedType.getClass());
if (entryConfig.getParentMembershipAttributeName() != null) {
Property<IdentityType> parentProperty = PropertyQueries
.<IdentityType>createQuery(attributedType.getClass())
.addCriteria(new TypedPropertyCriteria(attributedType.getClass()))
.getFirstResult();
if (parentProperty != null) {
IdentityType parentType = parentProperty.getValue(attributedType);
if (parentType != null) {
Attributes attributes = this.operationManager.getAttributes(parentType.getId(), getBaseDN(parentType), entryConfig);
Attribute attribute = attributes.get(entryConfig.getParentMembershipAttributeName());
attribute.add(getBindingDN(attributedType, true));
this.operationManager.modifyAttribute(getBindingDN(parentType, true), attribute);
}
}
}
} */
protected String getEntryIdentifier(final LDAPObject ldapObject) {
try {

View file

@ -120,10 +120,12 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
public LDAPIdentityQuery createRoleQuery(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider) {
LDAPIdentityQuery ldapQuery = new LDAPIdentityQuery(ldapProvider);
ldapQuery.setSearchScope(SearchControls.ONELEVEL_SCOPE);
// For now, use same search scope, which is configured "globally" and used for user's search.
ldapQuery.setSearchScope(ldapProvider.getLdapIdentityStore().getConfig().getSearchScope());
String rolesDn = getRolesDn(mapperModel);
ldapQuery.addSearchDns(Arrays.asList(rolesDn));
ldapQuery.setSearchDn(rolesDn);
Collection<String> roleObjectClasses = getRoleObjectClasses(mapperModel);
ldapQuery.addObjectClasses(roleObjectClasses);
@ -205,7 +207,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
ldapObject.setAttribute(roleNameAttribute, roleName);
LDAPDn roleDn = LDAPDn.fromString(getRolesDn(mapperModel));
roleDn.addToHead(roleNameAttribute, roleName);
roleDn.addFirst(roleNameAttribute, roleName);
ldapObject.setDn(roleDn);
// TODO: debug

View file

@ -110,9 +110,9 @@
</div>
</div>
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="ldapUserDns"><span class="required">*</span> User DN</label>
<label class="col-md-2 control-label" for="ldapUsersDn"><span class="required">*</span> Users DN</label>
<div class="col-md-6">
<input class="form-control" id="ldapUserDns" type="text" ng-model="instance.config.userDns" placeholder="LDAP User DN" required>
<input class="form-control" id="ldapUsersDn" type="text" ng-model="instance.config.usersDn" placeholder="LDAP Users DN" required>
</div>
<kc-tooltip>Full DN of LDAP tree where your users are. This DN is parent of LDAP users. It could be for example 'ou=users,dc=example,dc=com' assuming
that your typical user will have DN like 'uid=john,ou=users,dc=example,dc=com'

View file

@ -19,7 +19,7 @@ public class LDAPConstants {
public static final String CONNECTION_URL = "connectionUrl";
public static final String SECURITY_PROTOCOL = "securityProtocol";
public static final String USER_DNS = "userDns";
public static final String USERS_DN = "usersDn";
public static final String BIND_DN = "bindDn";
public static final String BIND_CREDENTIAL = "bindCredential";

View file

@ -13,7 +13,6 @@ import org.keycloak.models.RequiredCredentialModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserCredentialModel;
import org.keycloak.models.UserFederationMapperModel;
import org.keycloak.models.UserFederationProvider;
import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserModel;
import org.keycloak.util.CertificateUtils;
@ -30,7 +29,6 @@ import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.cert.X509Certificate;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -334,7 +332,7 @@ public final class KeycloakModelUtils {
mapperModel.setFederationProviderId(federationProviderId);
mapperModel.setFederationMapperType(mapperType);
Map<String, String> configMap = new HashMap<String, String>();
Map<String, String> configMap = new HashMap<>();
String key = null;
for (String configEntry : config) {
if (key == null) {

View file

@ -1,5 +1,7 @@
package org.keycloak.services.resources.admin;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
@ -202,6 +204,26 @@ public class UserFederationProviderResource {
for (UserFederationMapperModel model : realm.getUserFederationMappersByFederationProvider(this.federationProviderModel.getId())) {
mappers.add(ModelToRepresentation.toRepresentation(realm, model));
}
// Sort mappers by category,type,name
Collections.sort(mappers, new Comparator<UserFederationMapperRepresentation>() {
@Override
public int compare(UserFederationMapperRepresentation o1, UserFederationMapperRepresentation o2) {
UserFederationMapperFactory factory1 = (UserFederationMapperFactory) session.getKeycloakSessionFactory().getProviderFactory(UserFederationMapper.class, o1.getFederationMapperType());
UserFederationMapperFactory factory2 = (UserFederationMapperFactory) session.getKeycloakSessionFactory().getProviderFactory(UserFederationMapper.class, o2.getFederationMapperType());
int compare = factory1.getDisplayCategory().compareTo(factory2.getDisplayCategory());
if (compare != 0) return compare;
compare = factory1.getDisplayType().compareTo(factory2.getDisplayType());
if (compare != 0) return compare;
compare = o1.getName().compareTo(o2.getName());
return compare;
}
});
return mappers;
}

View file

@ -60,6 +60,7 @@ import javax.ws.rs.core.UriInfo;
import javax.ws.rs.WebApplicationException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
@ -121,7 +122,16 @@ public class UsersResource {
if (user == null) {
throw new NotFoundException("User not found");
}
updateUserFromRep(user, rep);
Set<String> attrsToRemove;
if (rep.getAttributes() != null) {
attrsToRemove = new HashSet<>(user.getAttributes().keySet());
attrsToRemove.removeAll(rep.getAttributes().keySet());
} else {
attrsToRemove = Collections.emptySet();
}
updateUserFromRep(user, rep, attrsToRemove);
adminEvent.operation(OperationType.UPDATE).resourcePath(uriInfo).representation(rep).success();
if (session.getTransaction().isActive()) {
@ -157,7 +167,8 @@ public class UsersResource {
try {
UserModel user = session.users().addUser(realm, rep.getUsername());
updateUserFromRep(user, rep);
Set<String> emptySet = Collections.emptySet();
updateUserFromRep(user, rep, emptySet);
adminEvent.operation(OperationType.CREATE).resourcePath(uriInfo, user.getId()).representation(rep).success();
@ -174,7 +185,7 @@ public class UsersResource {
}
}
private void updateUserFromRep(UserModel user, UserRepresentation rep) {
private void updateUserFromRep(UserModel user, UserRepresentation rep, Set<String> attrsToRemove) {
user.setEmail(rep.getEmail());
user.setFirstName(rep.getFirstName());
user.setLastName(rep.getLastName());
@ -200,9 +211,7 @@ public class UsersResource {
user.setAttribute(attr.getKey(), attr.getValue());
}
Set<String> attrToRemove = new HashSet<String>(user.getAttributes().keySet());
attrToRemove.removeAll(rep.getAttributes().keySet());
for (String attr : attrToRemove) {
for (String attr : attrsToRemove) {
user.removeAttribute(attr);
}
}

View file

@ -73,9 +73,6 @@ public class FederationProvidersIntegrationTest {
LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
LDAPUtils.removeAllUsers(ldapFedProvider, appRealm);
// Add sample application TODO: REmove this!!!! It's just temporarily needed in SyncProvidersTest until model for federation mappers is implemented
ClientModel finance = appRealm.addClient("finance");
LDAPObject john = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "johnkeycloak", "John", "Doe", "john@email.org", "1234");
ldapFedProvider.getLdapIdentityStore().updatePassword(john, "Password1");

View file

@ -62,9 +62,6 @@ public class SyncProvidersTest {
LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
LDAPUtils.removeAllUsers(ldapFedProvider, appRealm);
// Add sample application TODO: REmove this!!!! It's just temporarily needed in SyncProvidersTest until model for federation mappers is implemented
ClientModel finance = appRealm.addClient("finance");
for (int i=1 ; i<=5 ; i++) {
LDAPObject ldapUser = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "user" + i, "User" + i + "FN", "User" + i + "LN", "user" + i + "@email.org", "12" + i);
ldapFedProvider.getLdapIdentityStore().updatePassword(ldapUser, "Password1");

View file

@ -30,7 +30,7 @@ public class LDAPTestConfiguration {
PROP_MAPPINGS.put(LDAPConstants.CONNECTION_URL, "idm.test.ldap.connection.url");
PROP_MAPPINGS.put("rolesDnSuffix", "idm.test.ldap.roles.dn.suffix");
PROP_MAPPINGS.put("groupDnSuffix", "idm.test.ldap.group.dn.suffix");
PROP_MAPPINGS.put(LDAPConstants.USER_DNS, "idm.test.ldap.user.dn.suffix");
PROP_MAPPINGS.put(LDAPConstants.USERS_DN, "idm.test.ldap.user.dn.suffix");
PROP_MAPPINGS.put(LDAPConstants.BIND_DN, "idm.test.ldap.bind.dn");
PROP_MAPPINGS.put(LDAPConstants.BIND_CREDENTIAL, "idm.test.ldap.bind.credential");
PROP_MAPPINGS.put(LDAPConstants.VENDOR, "idm.test.ldap.vendor");
@ -54,7 +54,7 @@ public class LDAPTestConfiguration {
DEFAULT_VALUES.put(LDAPConstants.CONNECTION_URL, "ldap://localhost:10389");
DEFAULT_VALUES.put("rolesDnSuffix", "ou=Roles,dc=keycloak,dc=org");
DEFAULT_VALUES.put("groupDnSuffix", "ou=Groups,dc=keycloak,dc=org");
DEFAULT_VALUES.put(LDAPConstants.USER_DNS, "ou=People,dc=keycloak,dc=org");
DEFAULT_VALUES.put(LDAPConstants.USERS_DN, "ou=People,dc=keycloak,dc=org");
DEFAULT_VALUES.put(LDAPConstants.BIND_DN, "uid=admin,ou=system");
DEFAULT_VALUES.put(LDAPConstants.BIND_CREDENTIAL, "secret");
DEFAULT_VALUES.put(LDAPConstants.VENDOR, LDAPConstants.VENDOR_OTHER);