Conversation
|
I review the code and I managed to get it to work. So thanks for your contribution! Like you described, it has some limitations. Second, I guess you have been working with a ldap server that allows for 'username@domain' bind. This is mostly common for I guess a windows AD environment. To make the code work with other LDAP servers the line: should be changed to:
Where 'domain' will be changed to 'Base DN' this will make the code compatible with both active directory as well as with a linux ldap server. Also an inclusion of an simple checkbox to check to either disable or enable TLS would be a minor change but a big plus. So I would prefer some minor changes before we accept this pull request
|
| $newSalt = Util::randomString(20); | ||
| $newHash = Encryption::passwordHash($newPass, $newSalt); | ||
| $user = new User(null, $username, $email, $newHash, $newSalt, 1, 1, 0, time(), 3600, $group->getId(), 0, "", "", "", ""); | ||
| $user = new User(null, $username, $email, $newHash, $newSalt, 1, 0,1, 0, time(), 3600, $group->getId(), 0, "", "", "", ""); |
There was a problem hiding this comment.
LINT: missing space after comma
| @@ -0,0 +1,2 @@ | |||
| Order deny,allow | |||
| Deny from all No newline at end of file | |||
There was a problem hiding this comment.
Is this supposed to be part of this commit pull request?
| (77, 1, 'hcErrorIgnore', 'DeviceGetFanSpeed'), | ||
| (78, 8, 'ldap_server', ''), | ||
| (79, 8, 'ldap_domain', ''); | ||
|
|
| {{ENDIF}} | ||
| </td> | ||
| </tr> | ||
| <tr> |
There was a problem hiding this comment.
LINT: Indentation does not match outer indentation
|
Any chance we can get this merged and a new release put out?? :) pretty please |
|
My apologies! I didn't mean to drop this on your doorstep and bail. I have had other work commitments that took me in a different direction and I really don't have the bandwidth to finish what I started here. I just wanted to to share a partial solution for an on-going feature request in hopes that it could help aid in development. |
|
@gentoo9ball Could you test the branch of @dru1d-foofus? This help me to determine if the code is ready. |
4 participants
Potentially Fixes #316
I have been working to add LDAP authentication functionality for our particular environment and noticed there was an outstanding issue/feature request from 2018. We use starttls here and that might not be required for every LDAP configuration; however, I didn't have time to really expand upon those edge cases.
There have also been changes made to the UserAPI for enabling and disabling LDAP. I updated the .tex files, but didn't not generate new .pdfs or anything.
I'm not the most skilled developer and there will probably be bugs/better ways to accomplish the task, but I'm hoping this will help get the ball rolling.