vesse / node-ldapauth-fork

Simple node.js module to authenticate against an LDAP server
Other
127 stars 79 forks source link

Updated ldapjs Dependency to Fix TypeError When Using LDAPS in Certain Environments #18

Closed mjhasbach closed 9 years ago

mjhasbach commented 9 years ago

This PR fixes a TypeError when using the LDAPS protocol in certain environments (tested on Windows 7 x64, OpenSSL 1.0.2a and 1.0.1e):

TypeError: Cannot read property 'on' of undefined
    at setupSocket (c:\app\node_modules\passport-ldapauth\node_modules\ldapauth-fork\node_modules\ldapjs\lib\client\client.js:111:14)
    at Client._connect (c:\app\node_modules\passport-ldapauth\node_modules\ldapauth-fork\node_modules\ldapjs\lib\client\client.js:742:3)
    at new Client (c:\app\node_modules\passport-ldapauth\node_modules\ldapauth-fork\node_modules\ldapjs\lib\client\client.js:247:22)
    at Object.createClient (c:\app\node_modules\passport-ldapauth\node_modules\ldapauth-fork\node_modules\ldapjs\lib\client\index.js:60:12)
    at new LdapAuth (c:\app\node_modules\passport-ldapauth\node_modules\ldapauth-fork\lib\ldapauth.js:129:28)
    at handleAuthentication (c:\app\node_modules\passport-ldapauth\lib\passport-ldapauth\strategy.js:140:10)
    at callback (c:\app\node_modules\passport-ldapauth\lib\passport-ldapauth\strategy.js:181:26)
    at null.getOptions (c:\app\auth.js:31:17)
    at Strategy.authenticate (c:\app\node_modules\passport-ldapauth\lib\passport-ldapauth\strategy.js:187:10)
    at attempt (c:\app\node_modules\passport\lib\middleware\authenticate.js:341:16)

See this StackOverflow question where another user encountered the same error.

If you decide to pull this, I kindly request that you release a new version of node-ldapauth-fork and passport-ldapauth.

vesse commented 9 years ago

Thanks, but I am not yet going to depend on an unreleased version as mentioned in vesse/passport-ldapauth#26 and vesse/node-ldapauth-fork#16, especially because the problems happen only with Node 0.12 and specific platforms. You can do that in your project by defining ldapjs dependency in your package.json:

{
  "dependencies": {
    "ldapjs": "mcavage/node-ldapjs",
    "ldapauth-fork": "2.3.1"
  }
}

Since the version number in ldapjs master is still 0.7.1 this will satisfy the dependency for ldapauth-fork and thus it will use the unreleased version.

mjhasbach commented 9 years ago

@vesse Thanks for your suggestion! The following worked perfectly for passport-ldapauth:

"dependencies": {
    "passport-ldapauth": "~0.3.0",
    "ldapjs": "mcavage/node-ldapjs"
  }
snowshine09 commented 9 years ago

Hello! I did based on your instruction, and follow the steps described in the first answer to this question (http://stackoverflow.com/questions/28773546/nodejs-passport-ldapauth-cannot-read-on-property-of-undefined), but I kept getting socket hang up error. The details in the error report are as follows:

       throw e;
              ^
Error: socket hang up
    at onHangUp (_tls_wrap.js:961:21)
    at g (events.js:199:16)
    at TLSSocket.emit (events.js:129:20)
    at _stream_readable.js:908:16
    at process._tickCallback (node.js:355:11)
---------------------------------------------
    at Client.connect (/Users/nasun/Workspace/test/testldap/node_modules/passport-ldapauth/node_modules/ldapauth-fork/node_modules/ldapjs/lib/client/client.js:1218:9)
    at new Client (/Users/nasun/Workspace/test/testldap/node_modules/passport-ldapauth/node_modules/ldapauth-fork/node_modules/ldapjs/lib/client/client.js:360:8)
    at Object.createClient (/Users/nasun/Workspace/test/testldap/node_modules/passport-ldapauth/node_modules/ldapauth-fork/node_modules/ldapjs/lib/client/index.js:53:12)
    at new LdapAuth (/Users/nasun/Workspace/test/testldap/node_modules/passport-ldapauth/node_modules/ldapauth-fork/lib/ldapauth.js:129:28)
    at handleAuthentication (/Users/nasun/Workspace/test/testldap/node_modules/passport-ldapauth/lib/passport-ldapauth/strategy.js:140:10)
    at Strategy.authenticate (/Users/nasun/Workspace/test/testldap/node_modules/passport-ldapauth/lib/passport-ldapauth/strategy.js:175:33)
    at attempt (/Users/nasun/Workspace/test/testldap/node_modules/passport/lib/middleware/authenticate.js:341:16)
    at authenticate (/Users/nasun/Workspace/test/testldap/node_modules/passport/lib/middleware/authenticate.js:342:7)
    at handle (/Users/nasun/Workspace/test/testldap/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/nasun/Workspace/test/testldap/node_modules/express/lib/router/route.js:131:13)
    at Route.dispatch (/Users/nasun/Workspace/test/testldap/node_modules/express/lib/router/route.js:112:3)

My request are as follows:

app.put('/userlogin', passport.authenticate('ldapauth', {session: false}), function(req, res) {
        if(err) {
            console.log("error in userlogin");
            console.dir(err);
        }
        res.send({status: 'ok'});
    });

or

app.put('/userlogin', function(req, res, next){
        passport.authenticate('ldapauth',{session: false}, function(err, user, info){
            if (err) res.send(err);//next(err);
            console.log('enter login post ldapauth');
            if(!user) {
                return res.send({ success : false, message : 'authentication failed' });
            }
            req.logIn(user, function(err) {
                if (err) { return next(err); }
                return res.redirect('/home');
            });
            return res.send({ success : true, message : 'authentication succeeded' });
        })(req, res, next);
    });

However, either case above did not work.

vesse commented 9 years ago

Your error is obviously not the same as the original poster's. Sorry, but I don't know why your connection hangs up. Maybe your remote server never responds, maybe there is a bug in the current master version of ldapjs, maybe something else. Unfortunately I cannot really help you with this.