ldapjs / node-ldapjs

LDAP Client and Server API for node.js
http://ldapjs.org
MIT License
1.61k stars 448 forks source link

req.dn is a string in Server.bind #944

Open gramakri opened 1 year ago

gramakri commented 1 year ago

In 3.x, req.dn has become a string in Server.bind handlers instead of the DN Object. Server.search route handler still has req.dn has an object.

Example:

var ldap = require('ldapjs');

var server = ldap.createServer();

server.bind('dc=example', function(req, res, next) {
 console.log('bind: type is', typeof req.dn);
 next();
});

server.search('dc=example', function(req, res, next) {
  var obj = {
    dn: req.dn.toString(),
    attributes: {
      objectclass: ['organization', 'top'],
      o: 'example'
    }
  };

  if (req.filter.matches(obj.attributes))
  res.send(obj);

  res.end();
});

server.listen(1389, function() {
  console.log('ldapjs listening at ' + server.url);
});

When testing with ldapsearch -x -H "ldap://127.0.0.1:1389" -D "ou=sftp,dc=example" -w "password" -b "dc=example" :

$ node test.js 
ldapjs listening at ldap://127.0.0.1:1389
bind: type is string
jsumners commented 1 year ago

Are you suggesting a note be added to the v3 release notes? Or are you suggesting something else?

gramakri commented 1 year ago

@jsumners Oh, I thought this was a bug. Is this difference intentional? Per https://github.com/ldapjs/node-ldapjs/blob/master/docs/server.md#bind it still uses req.dn.toString() like in v2.

jsumners commented 1 year ago

Here's what I can tell you:

  1. The direct change to the server.bind method for v3 was to utilize @ldapjs/protocol -- https://github.com/ldapjs/node-ldapjs/blame/bec2ff8e7399155ebdcbf86eec2077a792b8510b/lib/server.js#L577
  2. The same link in 1 indicates that it has been expected to be a string for 12 years.
  3. The _mount method (which builds the arguments for the handler) has a change that returns an @ldapjs/dn instance under the right conditions -- https://github.com/ldapjs/node-ldapjs/blame/bec2ff8e7399155ebdcbf86eec2077a792b8510b/lib/server.js#L908