Merge lp:~matt.hall/endroid/ldap into lp:endroid

Proposed by Matthew Hall
Status: Merged
Approved by: Matthew Hall
Approved revision: 101
Merged at revision: 101
Proposed branch: lp:~matt.hall/endroid/ldap
Merge into: lp:endroid
Diff against target: 77 lines (+58/-5)
1 file modified
src/endroid/plugins/ldapauth.py (+58/-5)
To merge this branch: bzr merge lp:~matt.hall/endroid/ldap
Reviewer Review Type Date Requested Status
Phil Connell Approve
Review via email: mp+281035@code.launchpad.net

Commit message

Enhance the ldapauth plugin to support LDAP directories that do not allow anonymous access.

Description of the change

Enhance the ldapauth plugin to support LDAP directories that do not allow anonymous access.

The Twisted ldaptor module, used by Endroid's ldapauth plugin, unfortunately doesn't already provide a suitable credential checker for this purpose, so we instead define one in the plugin. This is done by sub-classing ldaptor's existing checker, overriding the callback called once connected to the directory so that it skips the attempt to lookup the user's entry and instead proceeds straight to the authentication attempt (i.e. the LDAP bind.)

Skipping the entry look up means the name of the attribute forming the entry's relative distinguished name (e.g. "cn" or "name") must be provided to the checker some other way, so the plugin now supports an "identityrdn" config option.

To post a comment you must log in.
Revision history for this message
Phil Connell (pconnell) wrote :

This is fine to go in to get things working, but strictly speaking it's a massive hack since it relies on internal details (e.g. _connect method and semantics) of ldaptor.

An ideal outcome would be getting a patch into ldaptor upstream to implement this within ldaptor: https://github.com/twisted/ldaptor

review: Approve
Revision history for this message
Matthew Hall (matt.hall) wrote :

> This is fine to go in to get things working, but strictly speaking it's a
> massive hack since it relies on internal details (e.g. _connect method and
> semantics) of ldaptor.
>
> An ideal outcome would be getting a patch into ldaptor upstream to implement
> this within ldaptor: https://github.com/twisted/ldaptor

Thanks, Phil. Yes, agreed it's a hack. Good point about submitting a patch to ldaptor; I'll try to do that over Xmas :-)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/endroid/plugins/ldapauth.py'
--- src/endroid/plugins/ldapauth.py 2014-04-05 11:42:51 +0000
+++ src/endroid/plugins/ldapauth.py 2015-12-20 12:23:42 +0000
@@ -3,7 +3,6 @@
3#3#
44
5from ldaptor import checkers, config5from ldaptor import checkers, config
6from ldaptor.protocols.ldap import distinguishedname
76
8from endroid.pluginmanager import Plugin7from endroid.pluginmanager import Plugin
98
@@ -11,7 +10,61 @@
11 hidden = True10 hidden = True
12 name = "ldapauth"11 name = "ldapauth"
13 def http_cred_checker(self):12 def http_cred_checker(self):
14 return checkers.LDAPBindingChecker(13 # Check whether we've been configured with the attribute forming the
15 config.LDAPConfig(','.join(self.vars.get("basedn")),14 # relative DN for identity entries within the directory and use the
16 {','.join(self.vars.get("basedn")): (self.vars.get("ldaphost"),15 # appropriate LDAP credential checker object.
17 int(self.vars.get("ldapport", 389)))}))16 rdn = self.vars.get("identityrdn")
17 if rdn:
18 return LDAPAuthedBindingChecker(
19 LDAPAuthedConfig(','.join(self.vars.get("basedn")),
20 {','.join(self.vars.get("basedn")):
21 (self.vars.get("ldaphost"),
22 int(self.vars.get("ldapport", 389)))},
23 identityRelativeDN=rdn))
24 else:
25 return checkers.LDAPBindingChecker(
26 config.LDAPConfig(','.join(self.vars.get("basedn")),
27 {','.join(self.vars.get("basedn")):
28 (self.vars.get("ldaphost"),
29 int(self.vars.get("ldapport", 389)))}))
30
31# Subclass the ldaptor package's LDAPBindingChecker with a variant that
32# supplies the user's credentials to the bind call; do this by overriding the
33# _connected callback method.
34#
35# On the assumption that this will be used when anonymous access to the LDAP
36# server is not allowed, the user's entry is not looked up before binding.
37# Instead the full distinguished name of the user is constructed from this
38# plugin's config.
39#
40# Also subclass LDAPConfig, so that we can attach an extra bit of config to
41# it.
42class LDAPAuthedBindingChecker(checkers.LDAPBindingChecker):
43 def _connected(self, client, filt, credentials):
44 d = client.bind(self.config.getIdentityDN(credentials.username),
45 credentials.password)
46 d.addCallback(self._bound)
47 return d
48
49 def _bound(self, bindresult):
50 # Doesn't matter what we return here: it's never used.
51 return bindresult
52
53class LDAPAuthedConfig(config.LDAPConfig):
54 def __init__(self,
55 baseDN=None,
56 serviceLocationOverrides=None,
57 identityBaseDN=None,
58 identitySearch=None,
59 identityRelativeDN="cn"):
60 super(LDAPAuthedConfig, self).__init__(baseDN,
61 serviceLocationOverrides,
62 identityBaseDN,
63 identitySearch)
64 self.identityRelativeDN = identityRelativeDN
65
66 def getIdentityDN(self, name):
67 return "{}={},{}".format(self.identityRelativeDN,
68 name,
69 str(self.getIdentityBaseDN()))
70

Subscribers

People subscribed via source and target branches