-
Notifications
You must be signed in to change notification settings - Fork 22
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
Implement LDAP auth #37
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #37 +/- ##
============================================
- Coverage 6.35% 5.48% -0.88%
- Complexity 184 211 +27
============================================
Files 18 20 +2
Lines 472 547 +75
============================================
Hits 30 30
- Misses 442 517 +75
Continue to review full report at Codecov.
|
Screenshot used in README.md was not correct and confusing for the initial user.
comments and checks missing
@NotAProfessionalDeveloper - great contribution! If using ldapProxyUser, I think you will need to include ... |
Thanks will fix. Unbind before binding again is unnecessary.
…On April 21, 2020 7:32:52 PM GMT+02:00, Simon Weller ***@***.***> wrote:
@NotAProfessionalDeveloper - great contribution!
If using ldapProxyUser, I think you will need to include
$ldapConn = ldap_connect($this->ldapUrl); prior to attempting the user
bind, or the ldap_bind will fail.
...
if ($info['count'] == 1) {
$ldapConn = ldap_connect($this->ldapUrl);
if (ldap_bind($ldapConn, $info[0]['dn'], utf8_encode($password))) {
ldap_unbind($ldapConn);
...
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#37 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
Unbinding frees the connection descriptor, and anyway unbinding is not necessary to bind again, so removing it
@NotAProfessionalDeveloper please rebase and add a test |
I like this PR, but it needs to be reworked to match the current code and quality standards incl. functional test. If you want to finish this PR, let us know, for now, closing this PR. |
Seems the original author abandoned this PR, so I wonder if we can take on it: we have RL project we shall use LDAP authentication. I'll try to contact the author to see if there is still any interest in that PR. |
unfortunately commits email is in shape of [email protected] and I have no other idea how to reach him, so I'll just make another PR |
Fixes #14
The following enhancements and modifications implement a way to authenticate against an LDAP server.
We assume that a successful LDAP authentication means the user is allowed to access the application independent of RBAC later in the application. This initial authorization can be varied by the object filter (or the base DN). Currently there is only one keyword value pair to define such a filter to match the username (Common Name/CN) as such.
But we still need the DB! A User record is automatically created in the DB (if it does not exist) when the user first logs in, since we still need to remember some metadata. It is updated from LDAP, if the concerning attributes are configured and LDAP exposes that information. Otherwise, local data is used (name, email, role), which can be set using User Preferences dialog. This data gets overwritten as soon as the directory contains data.
So far, the identity of the user was defined by an email address, not a username. In a directory setting, that changes. However, the User model doesn't account for a username, only an email address (and a real name). I could have:
As a consequence I've introduced a UserLDAP model, together with an AuthLDAP controller, and also a UserPreferences view, since the single form currently used via the controller no longer cuts it. There is now also a setupUserModelLDAP() trait.
Concerning roles: I'm not sure how to handle that. Currently I'm not assigning any role at all. And there doesn't seem to be anything that is not accessible in my App. In any case we give the option to retrieve the role from LDAP.
UserAdmin will not be needed much with directory authentication, except for an admin to modify attributes not provided by LDAP. In any case, username and password cannot be changed in the application. And also not in LDAP via the application. A password field doesn't even exist anymore, so we might need to update UserAdmin to account for the new User model...RoleAdmin remains viable of course. RegisterForm is also not used in this case.
The Login form got a minor update because we need to change the caption and error message depending on the authentication type.
There is still an issue with potential user enumeration during Login:
Usage example: