LDAP groups #55

wastez · 2021-10-21T07:58:28Z

Hello,

Thank you for this tool, it is great.
I've one question.
Would it be possible to manage groups over the ldap (not only the admin group)?
This would be better to manage.

wastez · 2021-10-21T19:53:55Z

Or have i missed something? Couldn't find this feature.

thomas-pike · 2021-10-23T12:01:00Z

No, it's not currently supported. Likely such a change would need some refactoring of the scripts/ldap_update.php file and of the LDAP config settings. In principle it's fairly easy though.

wastez · 2021-10-23T12:10:40Z

That would be really great if you could do this.
We would donate you some money for your work.

Resolves: #55

thomas-pike · 2021-10-31T17:50:13Z

The sync_groups branch has an experimental implementation of this. Try it out with git pull and git checkout sync_groups. Configure the groups you want to sync by adding multiple lines like the following to the [ldap] section of the config:

sync_groups[] = name_of_first_group
sync_groups[] = name_of_second_group
...

wastez · 2021-10-31T21:28:08Z

As far as i can say everything is working.
Using it as live system will show us more.

wastez · 2021-10-31T21:49:53Z

Thank you again for your great work and your great support, it's really great.

If you wish a donation for your work just give me a account where it can be transfered.

thomas-pike · 2021-10-31T22:04:17Z

You're welcome. It's good to know this tool is still finding use out there.

No payment necessary, but thank you very much for offering.

wastez · 2021-11-02T09:59:30Z

Hello,

FIrst issue, i'm not able to add a group to add a ldap group as a server administrator.
The Oops screen appears.

wastez · 2021-11-02T10:06:00Z

ErrorException: Undefined index: mail in /srv/keys/model/user.php:316\n1635847436: Stack trace:\n1635847436: #0 /srv/keys/model/user.php(316): exception_error_handler()\n1635847436: #1 /srv/keys/model/userdirectory.php(99): User->get_details_from_ldap()\n1635847436: #2 /srv/keys/views/server.php(45): UserDirectory->get_user_by_uid()\n1635847436: #3 /srv/keys/requesthandler.php(65): require('/srv/keys/views...')\n1635847436: #4 /srv/keys/public_html/init.php(18): require('/srv/keys/reque...')\n1635847436: #5 {main}, referer: https://server-hostname.test.com/servers/client-hostname.test.com

wastez · 2021-11-02T10:13:23Z

Looks like you have to edit your code in keys/requesthandler.php for LDAP groups.

wastez · 2021-11-02T10:43:21Z

And if user is in the ldap group by first login:

[Tue Nov 02 11:49:06.978827 2021] [php7:notice] [pid 659] [client 172.24.196.58:61213] 1635850146: InvalidArgumentException: Entity must be in directory before it can be added to a group in /srv/keys/model/group.php:118\n1635850146: Stack trace:\n1635850146: #0 /srv/keys/model/user.php(352): Group->add_member()\n1635850146: #1 /srv/keys/model/userdirectory.php(99): User->get_details_from_ldap()\n1635850146: #2 /srv/keys/requesthandler.php(24): UserDirectory->get_user_by_uid()\n1635850146: #3 /srv/keys/public_html/init.php(18): require('/srv/keys/reque...')\n1635850146: #4 {main}
[Tue Nov 02 11:49:10.542667 2021] [php7:notice] [pid 661] [client 172.24.196.58:61222] 1635850150: InvalidArgumentException: Entity must be in directory before it can be added to a group in /srv/keys/model/group.php:118\n1635850150: Stack trace:\n1635850150: #0 /srv/keys/model/user.php(352): Group->add_member()\n1635850150: #1 /srv/keys/model/userdirectory.php(99): User->get_details_from_ldap()\n1635850150: #2 /srv/keys/requesthandler.php(24): UserDirectory->get_user_by_uid()\n1635850150: #3 /srv/keys/public_html/init.php(18): require('/srv/keys/reque...')\n1635850150: #4 {main}, referer: https://server.test.com/

wastez · 2021-11-03T15:16:12Z

In both cases i know where the problem is, if the logs are to less just contact me.
For the ldap group problem on the the first login i wrote a dirty fix to solve it in the meantime, but as i said i'm not good in php.

Relates to: #55

thomas-pike · 2021-11-07T11:20:43Z

Apologies for the trouble, clearly I needed to do more testing, which I have done now. I've pushed a fix for the LDAP group problem.

As for your other problem, I believe it is unrelated to the group changes, and is caused by a user in your LDAP directory that doesn't have a mail attribute (I'm guessing that you have user_email = mail in your [ldap] config, and this code is currently assuming that all users have at minimum the user_id, user_name and user_email attributes. I'm not sure what the preferred behaviour should be for a user that doesn't have all of those...

wastez · 2021-11-07T12:10:46Z

Hello,

No thats sureley not the problem, users without mail can't be added via ldap.
A default group is working, (with the same users inside) but not a ldap group.
Could it be that that this if is accepted also on ldap groups, which isn't accepted when using a default group?
if($ldapuser = reset($ldapusers)) {

Edit:
I double checked the users in the mysql db, every user have a email.

wastez · 2021-11-07T12:22:12Z

And your group fix still have a problem if the user is still added to the group:
[Sun Nov 07 13:19:49.046414 2021] [php7:notice] [pid 47254] [client X.X.X.X:59434] 1636287589: ErrorException: Trying to get property 'uid' of non-object in /srv/keys/model/group.php:123\n1636287589: Stack trace:\n1636287589: #0 /srv/keys/model/group.php(123): exception_error_handler()\n1636287589: #1 /srv/keys/model/user.php(356): Group->add_member()\n1636287589: #2 /srv/keys/model/userdirectory.php(99): User->get_details_from_ldap()\n1636287589: #3 /srv/keys/requesthandler.php(24): UserDirectory->get_user_by_uid()\n1636287589: #4 /srv/keys/public_html/init.php(18): require('/srv/keys/reque...')\n1636287589: #5 {main}, referer: https://testserver.test.com/users/x-test03

With a new refresh it is working but the first access on the site fails.
After the refresh the add member isn't executed again, so it is working after refresh.

thomas-pike · 2021-11-07T12:40:30Z

No thats sureley not the problem, users without mail can't be added via ldap. A default group is working, (with the same users inside) but not a ldap group. Could it be that that this if is accepted also on ldap groups, which isn't accepted when using a default group? if($ldapuser = reset($ldapusers)) {

Edit: I double checked the users in the mysql db, every user have a email.

If this was the error you were getting:

ErrorException: Undefined index: mail in /srv/keys/model/user.php:316

then I do not see how it can be anything other than a user in LDAP missing a mail attribute. Especially since the previous 2 lines, fetching the user_id and user_name lines for the same user succeeded without error.

wastez · 2021-11-07T12:42:02Z

Maybe i've the problem, my config is looking like that:
user_id = sAMAccountName
user_name = cn
user_email = mail

I'm using a samba ad, the users have no uid attribute, they have one but they are empty.

thomas-pike · 2021-11-07T12:43:08Z

And your group fix still have a problem if the user is still added to the group:

Yes, this is still problematic it seems. My testing did not reveal this possibly because my very limited test setup does not have outgoing email enabled. This is going to be tricky to solve properly as there are circular dependencies going on here.

wastez · 2021-11-07T12:45:15Z

Wouldn't it be better you take the relations from the config.ini?
As wroting it hardcoded.

thomas-pike · 2021-11-07T12:49:34Z

Wouldn't it be better you take the relations from the config.ini? As wroting it hardcoded.

This is getting very confusing having 2 different issues in the same ticket. If you're referring to the LDAP attributes used, those are indeed using the values from config.ini.

Lines 314-316 of model/user.php are fetching the attributes from LDAP based on the values of user_id, user_name and user_email specified in your config.ini. You are getting an error on line 316, which means it has already successfully retreived both the user_id (in your case sAMAccountName) and the user_name (in your case cn) from LDAP, but you are encountering the error on line 316, which is trying to fetch the user_email (in your case mail) from LDAP. Since this is the line you are having the error on, I am unable to draw any conclusion other than you have a user in LDAP that is missing its mail attribute.

wastez · 2021-11-07T12:52:20Z

No, the problem is not the mail, every user has a mail, also in the db.
Also the mail is available in the users attribute on my samba ad.
But the users doesn't have a uid in samba ad.
As i said my php is not good. (i'm no coder)

a user look like this in the db:
| 981 | x-test03 | x-test03 | x-test03@test.com| NULL | LDAP | 1 | 0 | 0 | 0 | 0x3130386236643362613064656337666539316432366335396533323833363931333366393835643662363137306236386338383333333364383864303835633439663039376562306231613166326134343232376133373133356566356230613838306161383064386631353134386431666239393965613038666266306334 |

thomas-pike · 2021-11-07T12:58:18Z

Sorry, but again I repeat: if you are encountering the error on line 316 of model/user.php and the error is that the mail attribute is undefined, then there is no other explanation than that a user is lacking the mail attribute in LDAP (or access is being denied to the mail attribute in LDAP). It is literally impossible to be anything else. This is not about what's in the database. This is about what's in LDAP.

wastez · 2021-11-07T12:59:46Z

Right now i`m talking about adding groups as admin to servers and this is the error message:

[Sun Nov 07 13:19:49.046414 2021] [php7:notice] [pid 47254] [client x.x.x.x:59434] 1636287589: ErrorException: Trying to get property 'uid' of non-object in /srv/keys/model/group.php:123\n1636287589: Stack trace:\n1636287589: #0 /srv/keys/model/group.php(123): exception_error_handler()\n1636287589: #1 /srv/keys/model/user.php(356): Group->add_member()\n1636287589: #2 /srv/keys/model/userdirectory.php(99): User->get_details_from_ldap()\n1636287589: #3 /srv/keys/requesthandler.php(24): UserDirectory->get_user_by_uid()\n1636287589: #4 /srv/keys/public_html/init.php(18): require('/srv/keys/reque...')\n1636287589: #5 {main}, referer: https://testserver.test.com/users/x-test03

thomas-pike · 2021-11-07T13:01:16Z

Right, and that is a completely different error to the one I was talking about.

That comment was referring to this error that you mentioned earlier:

ErrorException: Undefined index: mail in /srv/keys/model/user.php:316\n1635847436: Stack trace:\n1635847436: #0 /srv/keys/model/user.php(316): exception_error_handler()\n1635847436: #1 /srv/keys/model/userdirectory.php(99): User->get_details_from_ldap()\n1635847436: #2 /srv/keys/views/server.php(45): UserDirectory->get_user_by_uid()\n1635847436: #3 /srv/keys/requesthandler.php(65): require('/srv/keys/views...')\n1635847436: #4 /srv/keys/public_html/init.php(18): require('/srv/keys/reque...')\n1635847436: #5 {main}, referer: https://server-hostname.test.com/servers/client-hostname.test.com

wastez · 2021-11-07T13:29:34Z

I now know what the problem is.
The group have a empty mail attribute field.
I entered a group to test and now it was working.

But the group normaly doens't have a mail address.

thomas-pike · 2021-11-07T13:30:20Z

That would indicate that a group is being treated as a user. That is concerning.

wastez · 2021-11-07T13:31:42Z

Exactly, so your if($ldapuser = reset($ldapusers)) { seems to also be entered on groups.

thomas-pike · 2021-11-07T13:32:58Z

It should not be getting even that far. It would mean that User::get_details_from_ldap() is being called for a group, and that should not be the case.

wastez · 2021-11-07T13:33:55Z

Before you didn't have this problem, because you are not using ldap groups.

thomas-pike · 2021-11-07T13:41:54Z

As far as I can tell it shouldn't matter. If the entity is an object of type Group, it should not have a ->get_details_from_ldap() method to call. Of course there is the slim possibility that something is extremely broken in the code here that means a group is being treated as a user, but I don't think that can be the case.

Do you have any record in the user table in the database whose name matches the name of a group in LDAP?

thomas-pike · 2021-11-07T13:45:57Z

And a follow-up question: are your groups in LDAP in separate subtrees from your users? ie. are your dn_user and dn_group config settings distinctly separate?

wastez · 2021-11-07T13:45:58Z

No of course no record in user table matches the group name in ldap.

We have naming conventions on our ldap, so thats impossible.
But i also dopple checked it right now.

wastez · 2021-11-07T13:46:18Z

No its the same subtree.

wastez · 2021-11-07T13:47:08Z

So you just difference it by subtree? That would explain the problem.

thomas-pike · 2021-11-07T13:53:42Z

Typically groups would be in a separate subtree from users, eg. as in the example config:

; LDAP subtree containing USER entries
dn_user = "ou=users,dc=example,dc=com"
; LDAP subtree containing GROUP entries
dn_group = "ou=groups,dc=example,dc=com"

If they are in the same subtree then it could potentially cause problems if there were a naming conflict. But you say that you have separate conventions for groups, so I'm not sure that's possible.

It seems to be that it is trying to add a new user to the database, and for some reason it has fetched the details of a group from LDAP. I can't really explain that at the moment.

thomas-pike · 2021-11-07T14:00:29Z

Your stacktrace is showing the following path through the code:

/srv/keys/views/server.php(45): $entity = $user_dir->get_user_by_uid($_POST['user_name']);
/srv/keys/model/userdirectory.php(99): $user->get_details_from_ldap();
/srv/keys/model/user.php(316): <- Error encountered here

It hasn't reached any group-related code at this point. It is trying to fetch details of the user specified in the form, a user that is not yet in the database.

It is at this point that whatever it has fetched from LDAP is lacking the mail attribute.

wastez · 2021-11-07T14:03:27Z

Yes, right now, the group and users are in the same subtree.
But as i said naming conventions doesn't allow to have the same name in groups and in users.

wastez · 2021-11-07T14:06:55Z

And yes the group was now added as user.

thomas-pike · 2021-11-07T14:10:56Z

Ok, so we've got to the bottom of this. You are trying to add a group as an administrator of a server. Line 45 of views/server.php tries first to find a user with the name given. It searches the user subtree and finds a record that matches, assumes it to be a user and tries to treat it as such. Normally it would not find any such entry, and would proceed to line 48 of views/server.php where it tries to find a group with the name, but since you have both users and groups in the same subtree it finds a group at the point where it's expecting a user.

This problem is therefore unrelated to the changes made in this ticket to support fetching LDAP groups for users. I'm not sure there's any good solution for it either.

thomas-pike · 2021-11-07T14:12:45Z

The only proper fix for that issue I can think of is having 2 separate fields in the UI, one for adding a user as an admin, and one for adding a group. Even so, it would still encounter errors if you accidentally used the wrong field.

wastez · 2021-11-07T14:13:44Z

And you can't add a filter for users and for groups in the ldap search?

thomas-pike · 2021-11-07T14:15:01Z

Possibly (edit: in fact probably indeed the best solution). That would require adding more settings to the config file for user_query and group_query. I'll make a separate ticket for that.

thomas-pike · 2021-11-07T14:18:46Z

Filed #60 for the user/group mismatch issue.

wastez · 2021-11-07T14:45:39Z

Just for your information:
I tried to use different subtrees on our testsystem.
With our old database the sync throws a error:

PHP Fatal error:  Uncaught mysqli_sql_exception: Incorrect integer value: '' for column 'admin' at row 1 in /srv/keys/model/record.php:155
Stack trace:
#0 /srv/keys/model/record.php(155): mysqli_stmt->execute()
#1 /srv/keys/model/user.php(46): Record->update()
#2 /srv/keys/model/user.php(335): User->update()
#3 /srv/keys/scripts/ldap_update.php(42): User->get_details_from_ldap()
#4 {main}
  thrown in /srv/keys/model/record.php on line 155

In a new Database there was no problem.
But switching the group folder with the existing database results in an error.
But if you add a filter now this problem should be obsolete.

thomas-pike · 2021-11-07T15:06:50Z

#60 has been resolved, you can now add user_filter and group_filter in your config.ini. See config-sample.ini for suggested values.

I also noticed the Incorrect integer value: '' for column 'admin' error and added a fix for that.

thomas-pike · 2021-11-07T15:09:36Z

That should just leave one remaining bug: first login group membership triggering an error due to $active_user not being set.

wastez · 2021-11-07T19:31:47Z

Any chance to get this solved? Or is this a bigger change?

…first time Relates to: #55

thomas-pike · 2021-11-07T20:09:49Z

There's a tentative fix for this on the set_active_user branch. Please feel free to try it out.

wastez · 2021-11-07T20:14:43Z

Login is working as it should now.
Any suggestions what to test?
Do you know what could be affected?

wastez · 2021-11-07T20:20:42Z

I tested a lot right now.
It really looks good.

thomas-pike · 2021-11-07T20:36:04Z

As long as it is working for you, this should be a reasonably low-risk change. I'll merge it now.

wastez · 2021-11-07T20:37:22Z

Ok fine.
Everthing is working and i tried a lot things, so everything should be ok.

thomas-pike added the enhancement label Oct 24, 2021

thomas-pike self-assigned this Oct 31, 2021

thomas-pike added a commit that referenced this issue Oct 31, 2021

Enable syncing of more LDAP groups

7b300d8

Resolves: #55

thomas-pike closed this as completed in 946c6ff Oct 31, 2021

thomas-pike reopened this Nov 7, 2021

thomas-pike added a commit that referenced this issue Nov 7, 2021

Move responsibility for adding new users in to the user directory

1788054

Relates to: #55

thomas-pike added a commit that referenced this issue Nov 7, 2021

Set $active_user immediately after creating user when logging in for …

4641ce0

…first time Relates to: #55

thomas-pike closed this as completed Nov 7, 2021

LDAP groups #55

LDAP groups #55

Comments

wastez commented Oct 21, 2021 • edited Loading

wastez commented Oct 21, 2021

thomas-pike commented Oct 23, 2021

wastez commented Oct 23, 2021

thomas-pike commented Oct 31, 2021

wastez commented Oct 31, 2021

wastez commented Oct 31, 2021

thomas-pike commented Oct 31, 2021

wastez commented Nov 2, 2021

wastez commented Nov 2, 2021 • edited Loading

wastez commented Nov 2, 2021 • edited Loading

wastez commented Nov 2, 2021 • edited Loading

wastez commented Nov 3, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021 • edited Loading

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021 • edited Loading

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021 • edited Loading

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021 • edited Loading

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021

thomas-pike commented Nov 7, 2021 • edited Loading

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021

wastez commented Nov 7, 2021

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

thomas-pike commented Nov 7, 2021 • edited Loading

wastez commented Nov 7, 2021

wastez commented Nov 7, 2021

thomas-pike commented Nov 7, 2021

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021

thomas-pike commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021 • edited Loading

thomas-pike commented Nov 7, 2021

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021

wastez commented Nov 7, 2021

thomas-pike commented Nov 7, 2021

wastez commented Nov 7, 2021

wastez commented Oct 21, 2021 •

edited

Loading

wastez commented Nov 2, 2021 •

edited

Loading

wastez commented Nov 2, 2021 •

edited

Loading

wastez commented Nov 2, 2021 •

edited

Loading

wastez commented Nov 3, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

thomas-pike commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

thomas-pike commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

thomas-pike commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

thomas-pike commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading

thomas-pike commented Nov 7, 2021 •

edited

Loading

thomas-pike commented Nov 7, 2021 •

edited

Loading

wastez commented Nov 7, 2021 •

edited

Loading