search

JackAdams / meteor-accounts-ldap

Authentication against an LDAP server in Meteor
MIT License
21 stars 12 forks source link

Connection to ldaps broken in Meteor 1.4+ #13

Closed migerh closed 8 years ago

migerh commented 8 years ago

Meteor: 1.4 babrahams:accounts-ldap: 0.6.1

I am using this package in connection with an ldaps server which worked fine until around we upgraded to Meteor 1.4. The error originates in ldapjs@0.7.1/lib/client/client.js:111:

I20160926-17:14:06.336(2)? LDAP authentication for: user I20160926-17:14:06.504(2)? Exception while invoking method 'login' TypeError: Cannot read property 'on' of undefined I20160926-17:14:06.504(2)? at setupSocket (~/.meteor/packages/babrahams_accounts-ldap/.0.6.1.1celj0g++os+web.browser+web.cordova/npm/node_modules/ldapjs/lib/client/client.js:111:14) I20160926-17:14:06.505(2)? at Client._connect (~/.meteor/packages/babrahams_accounts-ldap/.0.6.1.1celj0g++os+web.browser+web.cordova/npm/node_modules/ldapjs/lib/client/client.js:742:3) I20160926-17:14:06.505(2)? at new Client (~/.meteor/packages/babrahams_accounts-ldap/.0.6.1.1celj0g++os+web.browser+web.cordova/npm/node_modules/ldapjs/lib/client/client.js:247:22) I20160926-17:14:06.505(2)? at Object.createClient (~/.meteor/packages/babrahams_accounts-ldap/.0.6.1.1celj0g++os+web.browser+web.cordova/npm/node_modules/ldapjs/lib/client/index.js:60:12) I20160926-17:14:06.506(2)? at Object.LDAP._createClient (packages/babrahams_accounts-ldap/ldap_server.js:152:1) I20160926-17:14:06.506(2)? at [object Object]. (packages/babrahams_accounts-ldap/ldap_server.js:362:1) I20160926-17:14:06.506(2)? at packages/accounts-base/accounts_server.js:471:32 I20160926-17:14:06.506(2)? at tryLoginMethod (packages/accounts-base/accounts_server.js:248:14) I20160926-17:14:06.506(2)? at AccountsServer.Ap._runLoginHandlers (packages/accounts-base/accounts_server.js:468:18) I20160926-17:14:06.507(2)? at [object Object].methods.login (packages/accounts-base/accounts_server.js:531:27)

This is the line in ldapjs mentioned above:

https://github.com/mcavage/node-ldapjs/blob/v0.7.1/lib/client/client.js#L108-L111

In line 108 of client.js you can see the switch between tls and plain net socket: if opts.secure use socket.socket, else use just socket. Meteor 1.4 included an upgrade of the nodejs bundled with Meteor from 0.10.x to 4.x. Apparently the tls module API changed between 0.10.x and 4.x. The tls socket no longer has a property called socket. This is already fixed since ldapjs 1.0.0, maybe even in 0.8.0.

To test my assumption I created a local copy of this package and bumped the ldapjs version to 1.0.0 and it fixed this issue.

JackAdams commented 8 years ago

Thanks for finding the fix. I really need to update the ldapjs version to 1.0.0.

The only problem is -- it's working for me with 1.4.1.1 and, with all the changes to Meteor's node version and the new way of including binary builds for packages using binaries like ldapjs, I know I'll have hours of troubleshooting to get it working again.

I will fix it one day, but don't have the time right now to break-and-remake.

migerh commented 8 years ago

Yes, unencrypted ldap connections are not affected by this (our end2end tests work with an unencrypted local ldap server, that's why we didn't notice the issue right away). It's just encrypted connections that don't work anymore.

If someone else has this issue, feel free to copy our local version of this package with the upgraded ldapjs: https://github.com/4minitz/4minitz/tree/develop/packages/meteor-accounts-ldap

JackAdams commented 8 years ago

Ah yes ... you did say ldaps in the issue title. So a drop-in replacement of 1.0.0 solved the issue with no further code changes? I'll have a play around with it later today and, if it all works okay, will publish to atmosphere.

JackAdams commented 8 years ago

Well ... thank you for blazing the trail. I've updated the package (v0.7.0) to use ldapjs v1.0.0 and it's all working remarkably well (already deployed in a production app too).