search

operasoftware / ssh-key-authority

A tool for managing SSH key access to any number of servers.
Apache License 2.0
456 stars 71 forks source link

LDAP groups #55

Closed wastez closed 3 years ago

wastez commented 3 years ago

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 commented 3 years ago

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

thomas-pike commented 3 years ago

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 commented 3 years ago

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

thomas-pike commented 3 years ago

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 commented 3 years ago

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

wastez commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago 
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 commented 3 years ago

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

wastez commented 3 years ago

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 commented 3 years ago

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.

thomas-pike commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

thomas-pike commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

thomas-pike commented 3 years ago

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.

thomas-pike commented 3 years ago

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.

wastez commented 3 years ago

Granting them to a server account is working without problems.

wastez commented 3 years ago

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

thomas-pike commented 3 years ago

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: https://github.com/operasoftware/ssh-key-authority/issues/55#issuecomment-962606227

It does not matter whether outgoing email is enabled. This is about fetching attributes from LDAP and nothing else.

wastez commented 3 years ago

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.

thomas-pike commented 3 years ago

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.

wastez commented 3 years ago

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 commented 3 years ago

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

wastez commented 3 years ago

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

thomas-pike commented 3 years ago

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 commented 3 years ago

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

thomas-pike commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

No its the same subtree.

wastez commented 3 years ago

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

thomas-pike commented 3 years ago

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 commented 3 years ago

Your stacktrace is showing the following path through the code:

  1. /srv/keys/views/server.php(45): $entity = $user_dir->get_user_by_uid($_POST['user_name']);
  2. /srv/keys/model/userdirectory.php(99): $user->get_details_from_ldap();
  3. /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 commented 3 years ago

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 commented 3 years ago

And yes the group was now added as user.

thomas-pike commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

thomas-pike commented 3 years ago

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.