Juan Hernandez has posted comments on this change.
Change subject: [WIP] Add LDAP directory
......................................................................
Patch Set 3:
(5 comments)
The problem with TLS and LDAP is the following: when you want to configure TLS
for JNDI you have to specify the class *name* of the socket factory in the hash
table that you pass to the constructor of the initial context, something like
this:
public class MySocketFactory ... {
...
}
HashTable env = new HashTable();
env.put("java.naming.ldap.factory.socket", "MySocketFactory"):
Note that this is the class *name*, not an instance. The JNDI infrastructure
will then create an instance of this class and use it to create sockets.
This is fine if you only need one type of socket factory. But what if you need
to use different socket factories for different LDAP connections? The typical
approach will be to extend that class, something like this:
public MySocketFactoryA extends MySocketFactory {
...
}
public MySocketFactoryB extends MySocketFactory {
...
}
Then you can create different LDAP contexts with different socket factories.
But how do you make this configurable? What if you need to connect to 100
different LDAP servers with different socket factories? You will have to create
100 different classes. That is exactly what I am using "asm" for: creating
those 100 different classes dynamically instead of statically.
Regarding support for different directories what you need to do is to create an
authentication profile for each, and thus a configuration file in "auth.conf.d"
for each as well.
....................................................
File
backend/manager/modules/authentication/src/main/java/org/ovirt/engine/core/authentication/ldap/LdapDirectory.java
Line 19: import org.ovirt.engine.core.common.utils.ExternalId;
Line 20: import org.slf4j.Logger;
Line 21: import org.slf4j.LoggerFactory;
Line 22:
Line 23: public class LdapDirectory implements Directory {
If I understand correctly this is used to automatically find the base
distinguished name for searches. We don't need that if we allow the user to
explicitly configure it. But if we want to do it automatically we can. Remember
that this class is far from finished.
Line 24: private static final Logger log =
LoggerFactory.getLogger(LdapDirectory.class);
Line 25:
Line 26: /**
Line 27: * The name of the directory.
Line 70: controls.setSearchScope(SearchControls.SUBTREE_SCOPE);
Line 71:
Line 72: NamingEnumeration<SearchResult> results = null;
Line 73: try {
Line 74: results = ctx.search(base, "(uid=" + uid + ")",
controls);
Correct, the queries are specific to the LDAP schema used by the server. My
idea was to make this configurable, using templates. For example, the
configuration file could contain something like this:
user.find.filter=(uid=${id})
That would be the default value, but the system administrator could then change
it to match its schema and its use of the schema. Say, for example, that in a
specific scheme the attribute that identifies the user is "cn" instead of
"uid". The system administrator could then change the configuration to this:
user.find.filter=(cn=${id})
These templates would be evaluated in a context populated with variables, in
this case the only relevant variable will be "id" and it would be populated
with the identifier of the user. But in general more variables will be
required, for example the name of the directory, so that system administrators
can do this if required:
user.find.filter=(&(objectClass=user)(userPrincipalName=${id}@${directory}))
I already proposed this long ago, before proposing the refactory:
http://gerrit.ovirt.org/14650 - Simple template engine
http://gerrit.ovirt.org/14655 - Use map for LDAP query parameters
http://gerrit.ovirt.org/14723 - Move LDAP filter templates out of code
My plan was to use these same concepts here.
Line 75: if (results.hasMore()) {
Line 76: SearchResult result = results.next();
Line 77: return mapUser(result.getAttributes());
Line 78: }
Line 145: @Override
Line 146: public List<DirectoryUser> findUsers(List<ExternalId> ids) {
Line 147: List<DirectoryUser> result = new ArrayList<>(ids.size());
Line 148: for (ExternalId id : ids) {
Line 149: DirectoryUser user = findUser(id);
Sure, that should be implementation here, including grouping the users in
blocks if there are too many. This is what we do in the current infrastructure,
didn't replicate it here yet.
Line 150: if (user != null) {
Line 151: result.add(user);
Line 152: }
Line 153: }
Line 188: // Create the LDAP context:
Line 189: ctx = contextFactory.create();
Line 190:
Line 191: // Do the search:
Line 192: SearchControls controls = new SearchControls();
Yes, of course, this is just an starting point.
Line 193: controls.setSearchScope(SearchControls.SUBTREE_SCOPE);
Line 194:
Line 195: NamingEnumeration<SearchResult> results = null;
Line 196: try {
Line 230: }
Line 231:
Line 232: private DirectoryUser mapUser(Attributes attr) throws
NamingException {
Line 233: // Get the basic attribures:
Line 234: String uid = (String) attr.get("uid").get();
No, this isn't general. My plan here was to use templates again. We can have
the mapping specified in the configuration file:
user.id=${uid}
user.firstName=${givenName}
user.lastNmae=${sn}
user.email=${mail}
In this case we would populate the context for template evaluation with all
LDAP attributes, and then we can use reflection to go over the properties of
DirectoryUser, find the corresponding template in the configuration and
evaluate it to get the value to assign. Thus the system administrator can
decide how to do the mapping. For example, a system administrator may want to
use the "dn" as the identifier of the user:
user.id=${dn}
Or it may want to compose the e-mail from the user name and the directory name:
user.email=${uid}@${directory}
So on.
Line 235: ExternalId id = new ExternalId(uid.getBytes());
Line 236:
Line 237: // Create the user with the basic attributes:
Line 238: DirectoryUser user = new DirectoryUser(this, id, uid);
--
To view, visit http://gerrit.ovirt.org/22386
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8d1883145a86a4f338cd17f2d36c150e67ab653
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
