Add support for group-based authorization

[olivierbeytrison]

A little enhancement to allow for real group-based authorization.

Including documentation update.

Feel free to edit if needed. Works well in production on our side.

Thanks for the great plugin!

[asyncnomi]

Thanks for this PR I was looking for that exact feature.

However there is a little issue concerning the bind user.

For the first request to find the user as filled into the form, the bind user is indeed correctly use, but for the group check (using mode "group"), the bind user is no longer used.

In my case the login user can't search for groups into the ldap. Resulting in "Error in group authentification".

I fixed the issue by adding:

if (defined('LDAPAUTH_SEARCH_USER') && defined('LDAPAUTH_SEARCH_PASS')) {
	if (!@ldap_bind($ldapConnection, LDAPAUTH_SEARCH_USER, LDAPAUTH_SEARCH_PASS)) {
		die('Couldn\'t bind search user ' . LDAPAUTH_SEARCH_USER);
	}
}

before

$group_result = ldap_search($ldapConnection, LDAPAUTH_GROUP_BASE, $group_filter, array(LDAPAUTH_GROUP_ATTR));

The additional condition "empty($ldapSuccess)" previously used is removed cause it's not empty anymore.

I'm a newbie concerning LDAP and php integration of php-ldap. This solution is surely not be the best approach, maybe an additional variable must be declare to trigger this behavious, or maybe there is another solution to make the binding remains.

[olivierbeytrison]

Hi @asyncnomi

Thanks for your valid input! This issue didn't appear on my side as this is an internal ldap auth server. so any valid user as read access to the entire tree.

I think the best approach would be to check the group-based membership before doing the ldap bind as the user that logs on. This is a single ldap query and this would avoid having to bind again after the auth.

I'll update the code and update the MR

[olivierbeytrison]

hmm finally it was easier to re-bind before doing the group check. I added some verfications at the begining of the plugins and made it clear in the documentation that LDAPAUTH_SEARCH_USER/PASS are required for group-based authorization

[asyncnomi]

Thank you, I can confirm that the script works on my side !

[mattv8]

Thank you for your contribution! I apologize for the delay in review, since I'm only getting around to this now. Let me test to check for any issues on my end. What version of Yourls are you testing with?

[asyncnomi]

Hey, in my case it has been tested on commit 41ff6adf5404e762fc1700e3e7091595b6069501

[mattv8]

Hey, in my case it has been tested on commit 41ff6adf5404e762fc1700e3e7091595b6069501

This one? https://github.com/YOURLS/YOURLS/tree/41ff6adf5404e762fc1700e3e7091595b6069501

[asyncnomi]

Exactly !

[mattv8]

Okay thanks. Since this branch is only 2 months old, I'll test against the latest release: 1.9.2. Thanks for confirming!

[asyncnomi]

@mattv8 Tested against the last 1.10.0 release, still works fine !