Add LDAPS-support to LDAP-Authcontroller #7014
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7014 +/- ##
==========================================
- Coverage 93.81% 93.80% -0.02%
==========================================
Files 169 169
Lines 12407 12415 +8
==========================================
+ Hits 11640 11646 +6
- Misses 767 769 +2
Continue to review full report at Codecov.
|
src/Adapters/Auth/ldap.js
Outdated
| 'LDAP: Wrong username or password' | ||
| ) | ||
| ); | ||
| let error; |
There was a problem hiding this comment.
NIT: Could you please move this line to inside the if currently in line 27?
There was a problem hiding this comment.
can't move it in the if clause. the if clause (inside the bind callback) is executed twice when the cert mismatch happens. first time the mismatch error is detected, second time the client destruction destroyes the correct error message. error has to be outside to stay set to the first mismatch error after the second client destroy error happened.
There was a problem hiding this comment.
It does not make sense. The var is not used outside the scope and inside the scope it is only assigned and then used to reject the promise. You can also do the following just to make sure:
reject(error);
client.destroy(ldapError);
return;There was a problem hiding this comment.
You are right. The return reject (error) was the part that confuses the execution flow. With the rejection and the return splitted I could also remove the case undefined.
…s to LDAP-Authcontroller
I added LDAPS Support to the LDAP-Authcontroller, because plain LDAP is not my first choice when it comes to authentication ;)
The certificates are for testing purposes only and expires in 100 years.