uhm-coe / authorizer

Authorizer is a WordPress plugin that uses Google, CAS, LDAP, or an OAuth2 provider for logins, and can prevent public access to a WordPress site. It also blocks repeated failed login attempts.
GNU General Public License v3.0
64 stars 36 forks source link

Feature Request: use user login as the directory user login #111

Closed paulraines68 closed 2 years ago

paulraines68 commented 2 years ago

I don't think Authorizer needs access to the LDAP server outside of the actual user authentication step. I am requesting an option where instead of given a static separate Directory User login and password, one can click a check box that says to use the given credentials of the user in the login form to authenticate to LDAP.

figureone commented 2 years ago

This is difficult because the standard unique identifier for WordPress accounts is an email address, so we need to fetch LDAP attributes for the user logging in to find the email address and then link it with a WordPress account.

Typically, you do ldap_bind() using a service account, and then do ldap_search() with the uid of the user logging in to find their email address (and other attributes), and then finally do another ldap_bind() using the uid and password of the user logging in to confirm their credentials are valid.

If we drop the first two, then all we know is an LDAP user with uid successfully authenticated, but nothing about which WordPress account they should be authenticated as.

All that said, we did release a feature (upon request) for CAS logins to use the CAS and WordPress usernames as unique identifiers, instead of email addresses. We could do something similar with LDAP, with the caveat that it is less secure. See the note by the "CAS users linked by username" option in Authorizer Settings:

Note: The default (and most secure) behavior is to associate WordPress accounts with CAS accounts by the email they have in common. However, some uncommon CAS server configurations don't contain email addresses for users. Enable this option if your CAS server doesn't have an attribute containing an email, or if you have WordPress accounts that don't have emails.

paulraines68 commented 2 years ago

Using the given login dialog user/password for the intial ldap_bind in place of a dedicated service account would still work as long as the LDAP permissions on the server allow all users the ability to do the ldap_search for the email address over all accounts, right? Our LDAP server allows that though the userPassword field is hidden to all but the admin account.

figureone commented 2 years ago

Got it, I think we can make it attempt to bind as the user logging in, instead of a service account, if the service account credentials are left blank.

We'll do some testing on that and then hopefully get a release out soon.

figureone commented 2 years ago

Aloha, version 3.3.2 is out now with the change. Please test and let us know if it the ldap bind works without a directory user (instead using the credentials of the user logging in to do the bind). https://github.com/uhm-coe/authorizer/commit/54a206e0b8bbd8da5d300b2634ac579b4cbfd8a2

paulraines68 commented 2 years ago

The bind works and connects but the search fails for reasons I cannot understand.

For example this works fine:

$ ldapsearch  -H ldaps://ldap.example.org:636 -x -W -D "cn=per2,cn=Users,dc=example,dc=org" -b "cn=Users,dc=example,dc=org" "(|(cn=per2)(mail=per2))"
Enter LDAP Password:

# per2, Users, example.org
dn: CN=per2,CN=Users,DC=example,DC=org
objectClass: top
objectClass: person
...
...
...
# search result
search: 2
result: 0 Success

# numResponses: 2
# numEntries: 12, Users, example.org

But the same settings in Authorizor when doing the test gives:

[2:39 pm] Attempting to authenticate via LDAP.
Connected to LDAP host ldaps://ldap.example.org:636.
Using LDAP search filter: (|(cn=per2)(mail=per2))
Failed to find user per2 in cn=Users,dc=example,dc=org. Trying next search base.
Failed: no LDAP user per2 found.
paulraines68 commented 2 years ago

I should add that the test does work when I give it a directory user/password in the settings. So maybe the bind is failing but the test results don't show it that way. Hmm. Yes, giving the wrong password in the test gives the same output as about like the bind connected even though it must have failed as I gave the wrong password.

paulraines68 commented 2 years ago

Okay, the issue was the anonymous bind was succeeding but no searches were allowed under that bind. So I had to add code to class-authentication.php to prevent the attempt of @ldap_bind if $bind_rdn == null. Now it works like I need.

So I can just hack every release to do this for my site if you don't want to deal with it any further. Or you could add an option to allow admins to prevent anonymous bind attempts.

Thanks for you work

figureone commented 2 years ago

Got it, we can introduce a separate checkbox option to skip the anonymous bind attempt. In the meantime, another alternative to editing the code is to just add an invalid username into the LDAP Directory User field. It will try to bind as that user, fail, and then fall back to binding as the logging in user. This will cause a bit more LDAP traffic, but you won't have to worry about reapplying the custom code changes.

figureone commented 2 years ago

We added the [username] wildcard to the LDAP Directory User field so you can avoid the anonymous bind and go straight to binding as the user logging in. Example: Screen Shot 2022-04-29 at 10 01 45 AM

This will be included in version 3.3.3, due out shortly! https://github.com/uhm-coe/authorizer/commit/1639e8cf3fb2c9872e4b69daa39863248ffd8215

figureone commented 2 years ago

Version 3.3.3 is now available, please let us know if you run into any issues!

paulraines68 commented 1 year ago

Just want to add a comment to this issue that if one uses this uid=[username] method then logins by email address do not work.

One "hack" in the code could be that if you see a '@' in the username, you switch to mail=[username] (assuming 'mail' is the mail attribute given in the settings, for doing the initial bind/search.

paulraines68 commented 1 year ago

Sorry, forget it. Bind login will not work like that. So I need to figure out how to get the login forms from saying one can use an email address.

figureone commented 1 year ago

The current code actually will remove an ampersand and anything after it from the username field when the user logs in: https://github.com/uhm-coe/authorizer/blob/master/src/authorizer/class-authentication.php#L1018-L1019

So as long as your LDAP users' emails match their uids (e.g., username and username@example.com), then it should work if the user provides their email in the login form.

paulraines68 commented 1 year ago

That is an excellent hack. Unfortunately our emails and usernames don't match but I cannot think of any other way to handle it cleanly. So I need to figure out how to do hooks to change the login dialog to remove the "email" part so as not to confuse users. Thanks

figureone commented 1 year ago

Sorry, it's been a minute since I reviewed the LDAP code here. I forgot that the default search filter should actually handle both emails and usernames, as long as you provide both attributes in Authorizer settings ("LDAP attribute containing username" and "LDAP attribute containing email address"): https://github.com/uhm-coe/authorizer/blob/master/src/authorizer/class-authentication.php#L1251-L1262 That code creates a search filter that checks whether the user-supplied login matches either the uid attribute OR the email attribute: (|(uid=username)(mail=username)).

Can you test that out using an email address as the login? You can use the "LDAP test connection" fields and the "Test" button and it should give you some feedback on success/failure.

paulraines68 commented 1 year ago

Since the bind login at 1174 will have failed before it gets to that code, I am afraid that cannot fix the situation

If one is is doing the initial bind either anonymously or with a dedicated directory user/password, then bind will work and a user can give either username or password. But if using uid=[username],dc=domain,dc=org for directory user then only username will work. For users already approved, it is possible for your code to look up usernames in the Wordpress DB from a given email. But for new, unapproved users that will obviously not work.

figureone commented 1 year ago

Thanks, I had forgotten the context here. Yeah I can't think of a good way to support either username or email address for the bind user.

I have some insight on changing those labels since Authorizer also modifies wp-login.php some. Unfortunately there's not many filters to hook into there, so we instead just used jQuery to modify the labels. Something like this should work:

add_filter( 'login_footer', function () {
    ?>
    <script>
        jQuery( 'label[for="user_login"]' ).html( 'Username' );
    </script>
    <?php
} );