0

In my NodeJS server, run by PM2, I authenticate my users with a LAPD service using npm module "ldap-authentication".

const { authenticate } = require('ldap-authentication');
...
try {
   const auth = await authenticate(options);
   return auth;
} catch(err){
   return err;
}

It works fine when credentials are Ok.

When credentials are wrong function throws a proper error that can be caught and easily handled.

The problem comes when there is no connection and the "getaddrinfo ENOTFOUND" error arises,

Error: getaddrinfo ENOTFOUND xxx.xxx.corp
at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:60:26)

as the code crashes without catching any error causing an inadmissible problem (despite PM2 restarts the code). I need to handle a possible broken connection and avoid the server crash.

It looks like is not the problem about catching errors from an async function as other types of errors for this function are caught. Attending to this link seems it is a problem with this particular error in this particular module.

Any ideas?

jaume
  • 493
  • 1
  • 7
  • 17

2 Answers2

1

Unfortunately this seems to be bug in the underlying ldapjs library. The ENOTFOUND needs to be handled in the Socket.on('error') however this is not set up until after the call to Socket.connect() so it is not available when the DNS error occurs.

You can see the code here https://github.com/ldapjs/node-ldapjs/blob/master/lib/client/client.js#L827

if (server && server.secure) {
  socket = tls.connect(port, host, self.tlsOptions)
  socket.once('secureConnect', onConnect)
} else {
  socket = net.connect(port, host)
  socket.once('connect', onConnect)
}
socket.once('error', onResult)

To handle the DNS error it would need to look something like the following

// reference to the socket
let socket;
if (server && server.secure) {
  // create TLS socket and connection handler
  socket = new tls.TLSSocket(self.tlsOptions);
  socket.once('secureConnect', onConnect)
} else {
  // create net socket and connection handler
  socket = new net.Socket();
  socket.once('connect', onConnect)
}
// set up error handler - including DNS 
socket.once('error', onResult)
// connect the socket after we have an error handler
socket.connect(port, host)

I have not tested but it's possible that adding an error handler to the module you are using here would also work - https://github.com/shaozi/ldap-authentication/blob/master/index.js#L8

var client = ldap.createClient(ldapOpts)
client.on('error', (err) = { /* set up before connect */ });

Outside of submitting a PR to fix the underlying library my best idea is to use a wrapper to do a DNS lookup before trying to connect - https://nodejs.org/api/dns.html#dns_dns_lookup_hostname_options_callback

const dns = require('dns');

const auth = await new Promise((resolve, reject) => { 
  dns.lookup('xxx.xxx.corp', (err, address, family) => {
    // console.log('address: %j family: IPv%s', address, family));
    if (err) {
      return reject(err);
    }
    return resolve(authenticate(options));
  }
});
doublesharp
  • 26,888
  • 6
  • 52
  • 73
  • Thanks for your confirmation of the bug and the alternatives, this explanation is pretty clear. The second seems logical, but still wondering if it will work as the very same unhandled error may rise from the same script (not sure though). The first is even more elegant and eficient and at a glance it should work but need to test too. When I have the time soon I will test both and post back. – jaume Oct 10 '20 at 09:41
  • 1
    @jaume no, the `dns.lookup()` call should handle it, and in my example the promise rejection should be catchable, whereas the ENOTFOUND from the ldap library is not. I tested it and it works as expected with a bad hostname. – doublesharp Oct 10 '20 at 16:39
  • finally I tried the second as I was not confident about fiddling the module itself. Yo were right and it works like a charm. Thanks. – jaume Oct 13 '20 at 15:47
  • Yet I made a bit of modification to distinguish between the error for bad credentials and the broken communication case, I will post in answers. – jaume Oct 13 '20 at 15:48
0

A little modifications to distinguish between the error for bad credentials and the broken communication case in the calling function.

If error a rejection is thrown, if bad credentials it is resolved without a param.

return new Promise((resolve, reject) => {

  dns.lookup('xxx.xxx.corp', async (err, address, family) => {

    if (err) {

      return reject(err);

    };

    try {

      const x = await(authenticate(options));

      return resolve(x);

    } catch(err){

      return resolve();

    }

  });

});
jaume
  • 493
  • 1
  • 7
  • 17