NaturalHistoryMuseum / ckanext-ldap

A CKAN extension that provides LDAP authentication.
GNU General Public License v3.0
34 stars 39 forks source link

Cannot use dn search strings for searching at the domain level? #73

Open reedv opened 3 years ago

reedv commented 3 years ago

Description A clear and concise description of what the bug is.

Extension throwing errors when using search string that searches multiple AD (sub)paths for users, eg.

base_dn = DC=myorg,DC=local
search_filter = (&({login}=sAMAccountName)(|(memberOf=CN=zone1,OU=zones,OU=datagroups,DC=myorg,DC=local)(memberOf=CN=zone2,OU=zones,OU=datagroups,DC=myorg,DC=local)))

Getting errors like...

Bad username or password

Basically, my issue is that I have a base DN like...

OU=zones,OU=datagroups,DC=myorg,DC=local

that has a bunch of Group CNs under it like...

myorg.local
    datagroups
        zones
            CN=group1
            CN=group2
            ...

...(where the CN groups contain users/Person objects that are defined in various other places) and I am trying to get all of the users (objectCategory=Person) to be able to log in using the ldap extension.

Is the extension not able to accommodate such a structure? Can I only point to a single place where user objects themselves are explicitly defined?

Expected Behaviour What you expected to happen instead. Be able to sign in using AD users defined in either of the DN paths described in the example search query above.

To Reproduce Steps to reproduce the bug. Set

base_dn = DC=myorg,DC=local
search_filter = (&({login}=sAMAccountName)(|(memberOf=CN=zone1,OU=zones,OU=datagroups,DC=myorg,DC=local)(memberOf=CN=zone2,OU=zones,OU=datagroups,DC=myorg,DC=local)))

or some other set of multiple DN paths to search for AD users in.

Error Log Paste any relevant error logs below:

Screenshots Add screenshots to illustrate the bug if you want.

Your Setup

Anything Else? Note that when I use ADExplorer to look at the AD properties, I see that the objectCategory value for our users is not actually just "Person", but CN=Person,CN=Schema,CN=Configuration,DC=myorg,DC=local. Don't work with AD internals much (nor LDAP querying at all) so not sure if this is an issue (and I may be misunderstanding something else here as well).

reedv commented 3 years ago

Adding some debugging statements to the code (https://github.com/NaturalHistoryMuseum/ckanext-ldap/blob/afded53f0ae8ba9744a9e9f4aa9bf8944eb88895/ckanext/ldap/lib/search.py#L93) like...

def ldap_search(cnx, filter_str, attributes, non_unique='raise'):
   .
   .
   .
    try:
        res = cnx.search_s(toolkit.config['ckanext.ldap.base_dn'], ldap.SCOPE_SUBTREE,
                           filterstr=filter_str, attrlist=attributes)
        log.debug("\n==========\n")
        log.debug(res)
        log.debug("\n==========\n")

I can see that the filter does appear to be working (kinda), but is returning multiple entries in the res list...

2021-04-06 22:07:10,224 DEBUG [ckanext.ldap]
==========

2021-04-06 22:07:10,224 DEBUG [ckanext.ldap] [(u'CN=zone1,OU=zones,OU=datagroups,DC=myorg,DC=local', {u'sAMAccountName': ['myuser']}), (None, [u'ldap://DomainDnsZones.myorg.local/DC=DomainDnsZones,DC=myorg,DC=local']), (None, [u'ldap://ForestDnsZones.myorg.local/DC=ForestDnsZones,DC=myorg,DC=local']), (None, [u'ldap://myorg.local/CN=Configuration,DC=myorg,DC=local'])]
2021-04-06 22:07:10,224 DEBUG [ckanext.ldap]
==========

2021-04-06 22:07:10,224 ERROR [ckanext.ldap] LDAP search.filter search returned more than one entry, ignoring. Fix the search to return only 1 or 0 results.

...(note that tuple with the "myuser" sAMAccountName is the user I want to sign in as) but can't really tell what I am looking at since the docs for python-ldap.search_s() (https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html?highlight=search_s#ldap.LDAPObject.search_s) are not very descriptive of what exactly is being returned here (and IDK enough about LDAP itself to infer what these tuple are).

Not really sure what to do about / make of this.

alycejenni commented 3 years ago

I expect this is a problem with your LDAP search filter rather than this extension. The search filter needs to be constructed so that it can only return a maximum of one result.

Try sAMAccountName={login} instead of {login}=sAMAccountName in your filter.

reedv commented 3 years ago

IC, thank you for the info. TBH, that suggestion didn't work though. What is the difference in the order (I've had other people tell me the opposite)?

reedv commented 3 years ago

The issue appear to be with the python-ldap package the extension uses. Looking at the python-ldap docs (https://www.python-ldap.org/en/python-ldap-3.3.0/faq.html) these extra results appear to be "search continuations" that are included when the search base is at the domain level and it says that they can be turned off by including the code like...

l = ldap.initialize('ldap://foobar')
l.set_option(ldap.OPT_REFERRALS,0)

...yet adding this code (around here: https://github.com/NaturalHistoryMuseum/ckanext-ldap/blob/afded53f0ae8ba9744a9e9f4aa9bf8944eb88895/ckanext/ldap/lib/search.py#L25) does not change the behavior at all and I get the same results.

Also tried to change the order like

ldap.set_option(ldap.OPT_REFERRALS,0)
res = cnx.search_s(toolkit.config['ckanext.ldap.base_dn'], ldap.SCOPE_SUBTREE, filterstr=filter_str, attrlist=attributes)

and still getting the referrals in the result list from the search_s() function.

Would you know what could be done about this? Have no other users run into the issue before (I feel like my use case is pretty reasonable)?

reedv commented 3 years ago

Just talked to someone else more familiar with python-ldap and was told that OPT_REFERRALS is controlling if you automatically follow the referral, but it doesn't stop AD from sending them.

For now, the only approach they recommended was to filter these values with something like:

results = ldap.search_s(...)
results = [ x for x in results if x.0 is not None ]

Noting that the structure of the results returned from search_s() is

[
    ( dn, {
         attrname: [ value, value, ... ],
         attrname: [ value, value, ... ],
    }),
]

When it's a referralm it's a dn of None and the entry dict is replaced with an array of uri's.

(Note that in the search_s call you can request specific attributes to be returned in your search too)

Ultimately I modified the search.py so that... https://github.com/NaturalHistoryMuseum/ckanext-ldap/blob/afded53f0ae8ba9744a9e9f4aa9bf8944eb88895/ckanext/ldap/lib/search.py#L93 became

res = cnx.search_s(toolkit.config['ckanext.ldap.base_dn'], ldap.SCOPE_SUBTREE,
                                filterstr=filter_str, attrlist=attributes)
res = [ x for x in results if x[0] is not None ]

and also https://github.com/NaturalHistoryMuseum/ckanext-ldap/blob/afded53f0ae8ba9744a9e9f4aa9bf8944eb88895/ckanext/ldap/lib/search.py#L25 became

cnx = ldap.initialize(toolkit.config['ckanext.ldap.uri'], bytes_mode=False,
                          trace_level=toolkit.config['ckanext.ldap.trace_level'])
cnx.set_option(ldap.OPT_NETWORK_TIMEOUT, 10)
cnx.set_option(ldap.OPT_REFERRALS,0)

(just to stop the search_s() from actually going down the referral paths (which was adding a few seconds to the search time (again, since I was using a domain level base DN))).

Again, I believe that this problem is due to the base DN being a domain level path (which I still contend from my original use case that it is not that unreasonable a use (unless there is some other base_dn or search.filter I could use for that fact that the group users are scattered across various AD paths in the domain that I'm missing)).

* For others, if you are going to change the extension code, you need to reload the cached package in ckan libs, eg...

sudo rm -rf /usr/lib/ckan/default/lib/python2.7/site-packages/ckanext_ldap-*
cd /usr/lib/ckan/default/src/ckanext-ldap; sudo /usr/lib/ckan/default/bin/python setup install

(if you did not install ckan as a package, then no need to do the sudo and abs. python path reference).