LDAP groups
Closed this issue · 60 comments
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.
Or have i missed something? Couldn't find this feature.
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.
That would be really great if you could do this.
We would donate you some money for your work.
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
...
As far as i can say everything is working.
Using it as live system will show us more.
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.
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.
Hello,
FIrst issue, i'm not able to add a group to add a ldap group as a server administrator.
The Oops screen appears.
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
Looks like you have to edit your code in keys/requesthandler.php for LDAP groups.
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/
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.
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...
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.
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.
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.
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.
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.
Wouldn't it be better you take the relations from the config.ini?
As wroting it hardcoded.
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.
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 |
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.
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
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
Sorry this message was from the first logon:
[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
Yes, and as I said above:
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.
The problem occurs whether or not outgoing emails are enabled. It is triggered as you say by first login. I had added the user to the database in a different way (by granting them access to a server account), thus triggering a somewhat different code path.
Granting them to a server account is working without problems.
But as soon i add an admin to an server i get this:
[Sun Nov 07 14:15:43.534998 2021] [php7:notice] [pid 51043] [client x.x.x.x:60947] 1636290943: ErrorException: Undefined index: mail in /srv/keys/model/user.php:316\n1636290943: Stack trace:\n1636290943: #0 /srv/keys/model/user.php(316): exception_error_handler()\n1636290943: #1 /srv/keys/model/userdirectory.php(99): User->get_details_from_ldap()\n1636290943: #2 /srv/keys/views/server.php(45): UserDirectory->get_user_by_uid()\n1636290943: #3 /srv/keys/requesthandler.php(65): require('/srv/keys/views...')\n1636290943: #4 /srv/keys/public_html/init.php(18): require('/srv/keys/reque...')\n1636290943: #5 {main}, referer: https://testserver.test.com/servers/x-l-samba03.l.x
But the clue is every user which is used in the group have in ldap a mail attribute.
And it also appears if email is disabled in config.ini
These are 2 very separate issues, and as I said, it is very confusing having both issues in the same ticket. And again, I refer you to this comment: #55 (comment)
It does not matter whether outgoing email is enabled. This is about fetching attributes from LDAP and nothing else.
I know what you wrote, and in LDAP every users which is in the group has the mail attribute and it is filled with the correct mail address.
Then we are at an impasse with that issue, because there is no other explanation. But I will continue to try to fix the other issue.
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.
That would indicate that a group is being treated as a user. That is concerning.
Exactly, so your if($ldapuser = reset($ldapusers)) { seems to also be entered on groups.
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.
Before you didn't have this problem, because you are not using ldap groups.
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?
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?
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.
No its the same subtree.
So you just difference it by subtree? That would explain the problem.
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.
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.
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.
And yes the group was now added as user.
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.
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.
And you can't add a filter for users and for groups in the ldap search?
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.
Filed #60 for the user/group mismatch issue.
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.
#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.
That should just leave one remaining bug: first login group membership triggering an error due to $active_user not being set.
Any chance to get this solved? Or is this a bigger change?
There's a tentative fix for this on the set_active_user branch. Please feel free to try it out.
Login is working as it should now.
Any suggestions what to test?
Do you know what could be affected?
I tested a lot right now.
It really looks good.
As long as it is working for you, this should be a reasonably low-risk change. I'll merge it now.
Ok fine.
Everthing is working and i tried a lot things, so everything should be ok.