jeremycx / node-LDAP

LDAP binding for node.js
MIT License
221 stars 43 forks source link

LDAP search not working - reopen #81

Closed user0403 closed 8 years ago

user0403 commented 9 years ago

I am reopening the ldap serach issue which I have already mentioned with code in #79. Firstly thanks for the update so soon which has resolved the error problem . But still not getting the expected result which I got in older node version. Search output in old version [ { dc: [ 'abc' ], o: [ 'abc' ], objectClass: [ 'dcObject', 'organization' ], dn: 'dc=abc' }, { dc: [ 'allusers' ], objectClass: [ 'dcObject', 'organizationalUnit' ], ou: [ 'allusers' ], dn: 'dc=allusers,dc=abc' } ]

Search output in new version [ { dc: [ 'cts' ], o: [ 'cts' ], objectClass: [ 'dcObject', 'organization' ], dn: 'dc=cts' } ]

Tried passing values like scope: ldap.SUBTREE, filter: '(objectClass=)', attrs: ' +', got error again { [LDAPError: Timeout] name: 'LDAPError', message: 'Timeout' }

jeremycx commented 9 years ago

Can you send me some code that exhibits the issues?

jeremycx commented 9 years ago

Try LDAP.SUBTREE instead of ldap.SUBTREE

user0403 commented 9 years ago

Below is the code with new LDAP update. LDAP.SUBTREE gives an error saying LDAP is undefined var LDAP = require('ldap-client'); var bind_options = { binddn: 'cn=admin,dc=nodomain', password: 'password' }; var ldapObj = new LDAP({ uri: "ldap://IP", version: 3, connecttimeout: 1, reconnect: true },function(err){ ldapObj.simplebind(bind_options, function (err) { if (err) { console.log(err); } else { var search_options = { base: "dc=nodomain", scope: LDAP.SUBTREE, filter: '(objectClass=)', attrs: ' \ ', pagesize: 2 }; ldapObj.search(search_options, function (err, data) { if (err) { console.log(err) } else { console.log(data); } }) }) })

The only difference is search options which was working in old version var search_options = { base: "dc=nodomain", scope: '', filter: '', attrs: '', pagesize: 2 };

jeremycx commented 9 years ago

In the above code, you're not waiting on the "ready" callback to make your bind.

var ldapObj = new ldap({
    uri: "ldap://IP",
    version: 3,
    connecttimeout: 1,
    reconnect: true
}, function(err) {
   // now you can do stuff
});

The reason why the callback is optional: in a long-running service, you should be handling Timeout/Can't Connect errors anyway, to ensure you can survive a restarted LDAP server. In these long-running cases, you can just get started without waiting on the callback, handle a few errors until the connection is established, then just proceed normally. Imagine you're wiring this up with a nice little Express.js REST interface -- while the LDAP connection is connecting (or reconnecting) you should be returning 500 errors anyway.

The ready callback is also a good place to put bind()s in general, as it will be executed on every reconnect (so you can re-bind, for example).

This is all in the README here: https://github.com/jeremycx/node-LDAP#reconnection

user0403 commented 8 years ago

I have tried the callback but still same error :( Can you post the search code which is working for you.

jeremycx commented 8 years ago

Look at test/index.js -- it has lots of examples that work (for me, at least). Let's see if you can run the test suite.

Now you have a custom ldap server running on ports 1234 and 1235 with some basic demo data in it. In a separate terminal cd to node-LDAP and npm test will run all the tests against the demo server.

jeremycx commented 8 years ago

Any more info? I'd really like to put this to bed.

user0403 commented 8 years ago

Search option is working. But I am confused with the output. In older version I am getting two records in array whereas in newer version of ldap only one record in array. I have mentioned the result in the first post of this thread.

jeremycx commented 8 years ago

No quotes on the scope. It should be:

scope: LDAP.SUBTREE

In the search options (assuming you have used var LDAP=require('ldap-client');

user0403 commented 8 years ago

I tried with scope: LDAP.SUBTREE. Still did not get the expected result. I have updated my code which I am testing.

jeremycx commented 8 years ago

It sounds like scope is being set to the value 0 - which corresponds to LDAP.BASE, which is the result you appear to be getting.

Try setting scope to 2 in your search options to see if this is indeed the case:

{
    scope: 2
}
marianomerlo commented 8 years ago

This is also happening to me. I'm trying to update from LDAP.js 1.2.0 to this new one and facing the same issue. I'm just receiving only 1 result in my searches.

Have already tried manually setting scope: 2 in the init params.

jeremycx commented 8 years ago

If scope: 2 doesn't do the trick, this is obviously something more nefarious.

Can I get:

  1. LDAP Server you are searching against (Windows, openLDAP, ?)
  2. Version of openLDAP client libs in place (something like 2.4.xx)
  3. Some sample code that exhibits the issue
marianomerlo commented 8 years ago

@jeremycx i'll come again with you with that info asap. But it seems that it's happening what you said before: The scope is being overwritten. (Despite i was not able to find where in the code).

But i was playing with ldapsearch directly against the server, and when setting the scope to base, it returns 1 result, otherwise it's retuning everything as expected.

jeremycx commented 8 years ago

It's very fishy that we're getting essentially

scope: 0

When we specify LDAP.SUBTREE (=2).

So, yeah, I assume something's getting overwritten with 0 somewhere along the line. I'm not seeing this on FreeBSD 10 with openLDAP client libs 2.4.42.

marianomerlo commented 8 years ago

in the meantime, is there a way i can log the value of:

https://github.com/jeremycx/node-LDAP/blob/master/LDAPCnx.cc#L305 ? (It's obvious a newbie question but i don't have the slightest idea about it). I guess i will need to change the code and recompile this? How can i do that?

PD: still waiting for the requested information.

jeremycx commented 8 years ago

Easiest way: check out the whole project from github, then at line 317, drop a printf:

printf("Scope is %i\n", scope);

Recompile (node-gyp rebuild) - to test it, write a new test in test/issues.js (and run the test with npm test tests/issues.js)

marianomerlo commented 8 years ago

no way, Scope is 2 is printed everywhere. so i guess the problem is even deeper? Anyway, will bother you again when i have the server information.

Thanks for your help.

marianomerlo commented 8 years ago

last thing i can think about: is there a debug mode in which i can print exactly the command that is being executed against the LDAP?

jeremycx commented 8 years ago

Haha -- took me a while to figure this one out. You need to get version of the openLDAP client libs compiled with the DEBUG switch - without this compiled in, nothing will happen.

Then, you set the debug option when you instantiate the LDAP object to whatever makes most sense:

var ldap = new LDAP({
    ...
    debug: 255
    ...
});
marianomerlo commented 8 years ago

So, it is openLdap with 2.4.23 openLDAP client libs

jeremycx commented 8 years ago

Can you try with 2.4.42? There were a lot of changes in libldap (though none that were officially compatibility-breaking) between .23 and .42

marianomerlo commented 8 years ago

Ok, so, i was able to fix the issue. In fact, the problem was that, since we are wrapping the new LDAP({}) with promisifyAll and using .spread. Replacing it with .then made everything work again. I don't understand 100% why it stopped working, but i'm able to move on.

jeremycx commented 8 years ago

Could I get some example code that uses promises and doesn't work properly? I have a promises guy that I'd like to show it to.

marianomerlo commented 8 years ago
  var ldap = Promise.promisifyAll(new LDAP({
    uri : {uri},
    connecttimeout : {connectTimeoutSeconds},
    timeout : {operationTimeoutMs},
    reconnect: function () {
      ldap.bind({ 
        binddn : {binddn},
        password : {password}
      });
    }
  }));

This returns unexpected

ldap.searchAsync({
  base : {b},
  filter : {f},
  attrs : {attrs},
  scope: 2
}).spread(function (data) {
  // data here is corrupted. Only 1 result.
  return _.map(data, fromDb);
});

This returns expected

ldap.searchAsync({
  base : {b},
  filter : {f},
  attrs : {attrs},
  scope: 2
}).then(function (data) {
  return _.map(data, fromDb);
});

So, there is an obvious difference between .spread and .then . I'm just wondering why it was working with previous version and no after the lib upgrade.

jeremycx commented 8 years ago

(I can't take credit for this (thanks @tcort )

.spread() puts the resultant array into the arguments list - so in the "unexpected" example above, the data variable is only the first element of the arguments array. To make the example work properly, the return should be return _.map(arguments, fromDb);