-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add LDAP auth module #6226
Add LDAP auth module #6226
Conversation
feec168
to
3017ef2
Compare
Codecov Report
@@ Coverage Diff @@
## master #6226 +/- ##
==========================================
- Coverage 93.85% 93.78% -0.07%
==========================================
Files 167 168 +1
Lines 11302 11344 +42
==========================================
+ Hits 10607 10639 +32
- Misses 695 705 +10
Continue to review full report at Codecov.
|
I don't know why Codecov thinks coverage in files which I did not touch has decreased. 😐 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments from me.
For codecov we are more focus on diff, the red parts that aren't tested.
3017ef2
to
b2ed290
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you just double check if you are destroying / unbinding the client in the correct places? If everything is good we can get this merged.
There was one more place where I forgot the destroy/unbind. Should be all fixed now. |
This PR adds a auth module for LDAP servers. There are some people who have rolled their own implementations of this (you can find them by searching for 'ldap' in the issues) so there is definitely a need for it. The PR adds a dependency on the ldapjs package.
I tried to make it as generic as possible, so it should work with all kinds of LDAP servers and configurations. This means that it takes a lot of config options, but I'll open a PR in the
docs
repo shortly where I describe how to set it up.