Unhandled 'error' event

[csulok]

As per the discussion in vesse/passport-ldapauth#29, I'm investigating an Unhandled error event crash in my application related to DNS errors with the LDAP server's address.

I found 2 error handlers are missing, one on _userClient and one on _adminClient. I have a "working" version that I tried to format into a pull request. The handlers are both required to resolve the issue, but I couldn't find a proper place to handle the _userClient error.

The obvious choice would have been - code snippet below - in LdapAuth.prototype.authenticate, attaching a one time handler before the userclient binds, but that didn't work. The nature of these errors and the connection order is something I still don't understand.

self._userClient.once('error', function(err) {
  callback(err);
});
self._userClient.bind(user[self.opts.bindProperty], password, function (err) {

Does this help with coming up with a nice solution for users of passport-ldapauth?

[vesse]

I'll try to look at this not this weekend but the next when I've done renovating and have moved in, hopefully then I have some spare time. Thanks for your patience.

[mir4ef]

Yeah, that would be great if we can get an error in the response, rather than bombing out the application. Also, I think a handler for invalid/expired/missing certificates is needed. I get errors like these:

events.js:85
      throw er; // Unhandled 'error' event
            ^
Error: Hostname/IP doesn't match certificate's altnames: "Host: company.com. is not in the cert's altnames: othername:<unsupported>, DNS:SERVER1.company.com"
    at Object.checkServerIdentity (tls.js:210:15)
    at TLSSocket.<anonymous> (_tls_wrap.js:934:31)
    at TLSSocket.emit (events.js:104:17)
    at TLSSocket._finishInit (_tls_wrap.js:460:8)

or

events.js:85
      throw er; // Unhandled 'error' event
            ^
Error: unable to verify the first certificate
    at Error (native)
    at TLSSocket.<anonymous> (_tls_wrap.js:929:36)
    at TLSSocket.emit (events.js:104:17)
    at TLSSocket._finishInit (_tls_wrap.js:460:8)

(it always says 'first', no matter what the order of the certificates is and I have few certificates)
Not sure if you want me to create a new issue with these errors or attach it to this one. Please let me know.

BTW, I am going through renovation, as well :) fun fun fun :) good luck with yours!

[csulok]

Have you had any chance to look at it so far?

[vesse]

Not really, sorry. I was thinking about it though and currently I feel like LdapAuth should inherit EventEmitter and re-emit those events. Since passport-ldapauth recreates the instance every time (due to this one not reconnecting) it could easily attach listeners in the handleAuthentication.

[adalinesimonian]

Is there any update on consensus for this and/or related solutions?

[vesse]

This should be resolved with the latest version (2.4.0) using ldapjs 1.0.0.

[vesse]

My bad, poor test case did not test the right thing. Still throws with ldapjs 1.0.0

[chanep]

When will be merged this??

[hcderaad]

Many thanks for this PR, I can confirm the solution from this pull request working on our setup. Which includes an AD LDAP lookup. Hopefully this will soon be merged. However, there are other events that are possibly unhandled as well. Is there any strategy/plan to tackle those, perhaps also upstream in ldapjs?