Updates ldapjs to 1.0.0

[Mctalian]

Closes #29 by upgrading ldapjs to >0.7.2

[Mctalian]

There seems to be an issue with ECONNRESET after about 15 minutes when using ldapjs v1.0.0. I wouldn't consider this PR until that is better understood.

[sentientcucumber]

This is kind of a naive guess, but could it have to do with something outside of this module? I'm pretty new to LDAP, but given it's a persistent connection, could Node or possibly the LDAP server, close the connection after 15 minutes? I notice some timeouts happening after 5 minutes.

I don't see an unbind happening. I wonder if it would be worth exposing it so developers can determine when/if they want to unbind when they see fit? Not sure if this goes against the protocol standards though.

[Mctalian]

Yes, I agree. It is likely the LDAP server itself forcing the connection closed after 15 minutes. However previously it did not cause the application to crash. So yes, I think it's more of an issue with ldapjs not this passport strategy, but I haven't done much investigation on it yet. On Dec 8, 2015 10:11 AM, "Michael Hunsinger" notifications@github.com wrote:

This is kind of a naive guess, but could it have to do with something outside of this module? I'm pretty new to LDAP, but given it's a persistent connection, could Node or possibly the LDAP server, close the connection after 15 minutes? I notice some timeouts happening after 5 minutes.

I don't see an unbind happening. I wonder if it would be worth exposing it so developers can determine when/if they want to unbind when they see fit? Not sure if this goes against the protocol standards though.

— Reply to this email directly or view it on GitHub https://github.com/auth0/passport-windowsauth/pull/30#issuecomment-162910364 .

[dustinsmith1024]

This fixes my node 5 issues with this module. I have not run it in production yet though.

[dustinsmith1024]

@Mctalian assuming this the timeout issue your talking about? https://github.com/mcavage/node-ldapjs/issues/318

Looks like my client disconnects after about 15 minutes as well. The reconnect option is not working for me either. Has anyone had luck with this yet?

[Mctalian]

I have unfortunately not been able to look into this further yet. I've got one other task that I'm hoping to finish up this weekend then I can start messing with this again. There is a recent comment in that issue you linked though, it might provide some insight.

[dustinsmith1024]

Yea I noticed but hadn't time to play with it either. I should get work time on Monday or Tuesday to see. Might need to make a change here to implement the suggestion.

[dustinsmith1024]

I was able to get reconnect working here: https://github.com/Mctalian/passport-windowsauth/compare/master...dustinsmith1024:master. I will clean the code up and PR tomorrow hopefully.

[Mctalian]

Nice work! That looks pretty solid, I'll have to try it out tomorrow to see if it also works for my scenario.

[Mctalian]

Yup! Confirmed, that works for me, good work!

[dustinsmith1024]

I have had less luck today. It seems to be not handling the error consistently. The second time the idle timeout happens the error is not handled properly. I tried reattaching an error handler after the client is reconnected, but it doesn't seem to help.

[Mctalian]

Ah... you're right. I didn't sit and wait for another timeout. I have the same issue over here.

[dustinsmith1024]

Think I have it worked out. I was making harder than it was. @Mctalian can you try this out? https://github.com/Mctalian/passport-windowsauth/compare/master...dustinsmith1024:error-handling?expand=1

If it works for you I can PR it. Basically credentials options needs to be bindCredentials and the reconnect option was not be passed down to LDAP. Then the last part was we needed to handle uncaught errors in the LdapValidator client instance.

[Mctalian]

Seems to be running fine after near 2 hours, I think this is a solid solution! On Jan 14, 2016 8:59 PM, "Dustin Smith" notifications@github.com wrote:

Think I have it worked out. I was making harder than it was. @Mctalian https://github.com/Mctalian can you try this out? https://github.com/Mctalian/passport-windowsauth/compare/master...dustinsmith1024:error-handling?expand=1

If it works for you I can PR it. Basically credentials options needs to be bindCredentials and the reconnect option was not be passed down to LDAP. Then the last part was we needed to handle uncaught errors in the LdapValidator client instance.

— Reply to this email directly or view it on GitHub https://github.com/auth0/passport-windowsauth/pull/30#issuecomment-171845267 .

[dustinsmith1024]

:+1: Awesome Ill take out the stack logging and submit a PR.

[Mctalian]

Ok, thanks to @dustinsmith1024 , this PR should now work with version ~1.0.0 of ldapjs

@jfromaniello can you check this over and merge it? Or tag someone who can/will?

Thanks!

[jfromaniello]

Thank you all and sorry for the delay. I just merged these changes and released to npm as v1.0.0.

[Mctalian]

Thanks @jfromaniello !

[dcolens]

What about the econnreset errors, does someone know where these come from, I have them too, while I never saw any with ldapjs 0.7.x

[dustinsmith1024]

I am not sure. All I did was implement what the ldpapjs maintainer said to do in the other linked issue. Capture the error and pass reconnect=true.

My guess is the library used to do this behind the scenes. When I checked with our IT team they had Active Directory setup to timeout after 15 minutes. So it seems to me like the new version is acting correctly.

[dcolens]

I agree, I did the same

On Fri, Jan 29, 2016 at 6:37 PM, Dustin Smith notifications@github.com wrote:

I am not sure. All I did was implement what the ldpapjs maintainer said to do in the other linked issue. Capture the error and pass reconnect=true.

My guess is the library used to do this behind the scenes. When I checked with our IT team they had Active Directory setup to timeout after 15 minutes. So it seems to me like the new version is acting correctly.

— Reply to this email directly or view it on GitHub https://github.com/auth0/passport-windowsauth/pull/30#issuecomment-176876866 .

[Mansuro]

Hi, I am using latest versions of ldapjs and passport-windowsauth and I have the problem