Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client doesn't unbind #88

Closed
KaiserWolfTaco opened this issue May 20, 2019 · 13 comments
Closed

Client doesn't unbind #88

KaiserWolfTaco opened this issue May 20, 2019 · 13 comments

Comments

@KaiserWolfTaco
Copy link

KaiserWolfTaco commented May 20, 2019

I see a note about the unbind issue affecting node.js v10. Is there a fix for this? The only fix i see in node-ldapjs issue 483 is to use ldapts but I don't think that's possible with this module. Is there another ldap passport strategy i can use?

@scottbell
Copy link

scottbell commented Sep 19, 2019

Looks like this got fixed: ldapjs/node-ldapjs#497

@KaiserWolfTaco
Copy link
Author

Thanks for the heads up Scott. Do you know when this will find its way into passport-ldapauth?

@scottbell
Copy link

It looks like this part of a larger effort to get node-ldapjs to be compatible with Node 10 for node-ldapjs 2.0:
ldapjs/node-ldapjs#524
This passport plugin is pegged:
"ldapjs": "^1.0.1"
so this plugin will need to be updated when node-ldapjs releases 2.0

@scottbell
Copy link

If you wanted to get ahead of the release, you could fork this plugin and have it point to:
https://github.com/ldapjs/node-ldapjs/tree/next
for its node-ldapjs dependency.

@vesse
Copy link
Owner

vesse commented Sep 22, 2019

Hopefully ldapjs 2.0 will be released soon. Looks like it should not even be a breaking release for the client even if major version number is bumped. Quickly tested with the next branch and simple test at least passed without any changes to ldapauth-fork.

josh-engebretson pushed a commit to josh-engebretson/node-ldapauth-fork that referenced this issue Oct 8, 2019
@tnrich
Copy link

tnrich commented Nov 6, 2019

Just checking in, any idea of an eta for node v10 support? We're eagerly awaiting this as well :)

@vesse
Copy link
Owner

vesse commented Nov 7, 2019

Yeah seems that release is not progressing much. I'll try to find time to do a release candidate that depends on the release candidate of ldapjs soon

@vesse
Copy link
Owner

vesse commented Nov 10, 2019

I've published a prerelease versions of ldapauth-fork and passport-ldapauth that depend on the next version of ldapjs and should thus unbind on Node v10 too. Install with npm install passport-ldapauth@next

@seshness
Copy link

Thanks for publishing, @vesse. I tried npm install passport-ldapauth@next locally, but still ended up with an old version of ldapjs that does not unbind.

The next version published on npm maps to 3.0.0-rc.1, which has this transitive dependency on ldapjs:
passport-ldapauth@3.0.0-rc.1 -> ldapauth-fork@^4.3.2 -> ldapjs@^1.0.2

However with 3.0.0-rc.0, this dependency structure looks like
passport-ldapauth@3.0.0-rc.0 -> ldapauth-fork@next -> ldapjs@next

So, at this time, npm install passport-ldapauth@3.0.0-rc.0 transitively installs ldapjs@2.0.0-pre.5.

Should passport-ldapauth@next refer to a newer version of ldapauth-fork, say 5.0.0-rc.5?

@vesse
Copy link
Owner

vesse commented Mar 26, 2020

Oh shoot. I'll try to check these soon. I noticed that it's really easy to mess up the deps when publishing both from master and from the next branch.

@vesse
Copy link
Owner

vesse commented Mar 30, 2020

@seshness should be fixed now:

└─┬ passport-ldapauth@3.0.0-rc.2
...
  ├─┬ ldapauth-fork@5.0.0-rc.5
  │ ├─┬ @types/ldapjs@1.0.6
  │ │ └── @types/node@13.9.5 deduped
  │ ├── @types/node@13.9.5 deduped
  │ ├── bcryptjs@2.4.3
  │ ├─┬ ldapjs@2.0.0-pre.5

@seshness
Copy link

Thank you @vesse! I appreciate it.

@vesse
Copy link
Owner

vesse commented Nov 16, 2020

passport-ldapauth@3.0.0 has now been published after noticing that ldapjs had published their v2 (already a while ago), no need to use the next version anymore. I'll close this now.

@vesse vesse closed this as completed Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants