Groups now ignore subgroups for type-checking to know which tabs should appear
Groups now ignore subgroups for type-checking to know which tabs should appear
Description
While investigating whether or not the dev version (on demo-dev.fusiondirectory.org) addressed the bug in issue #5657, I found that groups are still behaving in ways that are undesirable. 1) "Groups of groups" cannot have mail attributes assigned to them, and 2) existing groups that have mail attributes assigned have them deleted when a group is assigned as a member.
Distribution Name and Version
Unsure - generated issue on demo-dev.fusiondirectory.org.
FusionDirectory Version
1.3-dev
PHP version used
Unsure - generated issue on demo-dev.fusiondirectory.org.
Origin of php packages
Unsure - generated issue on demo-dev.fusiondirectory.org.
Steps to Reproduce
These are my steps to reproduce on the demo-dev.fusiondirectory.org server, starting from the "clean" setup, as the admin user.
- Add "IMAP/POP3 generic service" to the demo-dev server object.
- Create a group called "innergroup", assigning any user as a group member. Click OK to return to the group menu, then click on the new group to edit.
- Assign mail settings to this group, using innermail@test.org as the email address.
- This validates that issue #5657 is apparently addressed.
- Create a group called "outergroup", assigning innergroup as the group member. Click OK to return to the group menu, then click on the new group to edit.
- Notice the lack of "Mail" and "Partage" tabs, confirming the first part of this new issue.
- Change the group membership to only include a single user, removing innergroup. Click "Apply".
- Assign mail settings to this group, using outermail@test.org as the email address. Click OK to return to the group menu, then back into "outergroup" to edit. See that the mail settings are still there.
- Re-add innergroup to the member objects. As soon as it is selected, the "Mail" and "Partage" tabs disappear.
- Click OK or Apply to receive the message: "The object has changed since opened in FusionDirectory. All changes that may be done by others will get lost if you save this entry!" Close the modal window and click Cancel.
- Re-open outergroup, and check the mail settings to confirm that these settings have been deleted without an opportunity to abort the operation.
Expected behavior:
Groups of groups should be allowed to have email address assigned. Mail tab should stay present. At a minimum, mail settings should not be erased without warning and a way to back out.
Actual behavior:
Mail settings 1) cannot be applied or 2) get deleted, as discussing in reproduction steps.
Reproduces how often: 100% of the time when reproduction steps are followed.
Activity
Problem is the following: FD looks at which types are in a group, and activates tabs which are common to all these types (with some nuances). If there is a group in the group, it’s no longer considered a group of users and the mail tab goes away.
Possible fixes are:
- Look into the groups in the group to see which types are in there
- This can be slow if there are lots of groups in there. Infinite loops could happen
- Ignore group in the type list, so a group with users and groups will still be considered a user group. (even if the subgroup might contain something else)
- Keep the present behavior. (quite limiting)
By Côme Chilliet on 2017-10-12T14:43:49 (imported from GitLab)
Keeping the present behavior would be bad, at least in that it deletes the mail attributes prior to saving any other changes (as in Step 10 of the reproduction steps). At a minimum, I'd recommend fixing that.
As to other the possible fixes, ignoring group in the type list seems like a more logical solution. I don't know what downstream effects might have, but it would certainly be a worthwhile addition for certain LDAP directories.
By David Cortijo on 2017-10-12T22:11:13 (imported from GitLab)
created branch
5713-groups-of-groups-mail-problems-deletion-without-warning-and-mail-settings-prohibitedBy Côme Chilliet on 2017-10-16T08:07:14 (imported from GitLab)
mentioned in commit e4c3332f
By Côme Chilliet on 2017-10-16T08:11:42 (imported from GitLab)
added To Be Tested label
By Côme Chilliet on 2017-10-16T08:26:15 (imported from GitLab)
I tested against the current demo-dev build (at Oct 17, 2017 1:55 PM UTC).
A group of groups (without users) can't have Mail attributes assigned. That was somewhat expected.
However, if the last user is removed from a group with users and groups, it still triggers the "The object has changed since opened in FusionDirectory" warning, and deletes the mail settings without an opportunity to abort the operation.
Fundamentally, that's the biggest problem with this entire loop - changing the type of group causes attributes to be deleted in LDAP before a warning that they're going to be deleted. That's where I have my biggest concern.
By David Cortijo on 2017-10-17T13:58:35 (imported from GitLab)
I have done additional testing in 1.2 to figure out what behavior is actually triggering here, as well as to see if I can work around the problem until 1.3 is formally released.
I have a CHECK hook that I've tested just for ogroup, as well as tested tied to ogroup, mailGroup, and partageGroup.
Hook script is written in perl, and currently called group-premodify-audit.pl, which is called with the following placeholders as command-line variables: %dn% %mail% %displayName% %description% %J[;]|gosaMailAlternateAddress% %J[;]|member% %callerUID% %callerDN% [pluginName*]
This is specified statically for the hook, so the command line for the ogroup hook is /home/members/fd-hooks/group-premodify-audit.pl %dn% %mail% %displayName% %description% %J[;]|gosaMailAlternateAddress% %J[;]|member% %callerUID% %callerDN% ogroup, for example.
Each CHECK hook is firing properly, and one of the things I do in the hook is grab the LDIF of the object prior to it changing, as follows (irrelevant lines are omitted).
my $callerUID = $ARGV[6]; my $callerDN = $ARGV[7]; my $callPlugin = $ARGV[8];
my $fdTemp = "/home/members/fd-tmp"; my {fdTemp}/{callPlugin}.ldif"; my $auditDN = "ou=locks,ou=fusiondirectory,dc=test,dc=org"; my $oldObjectDN =
/usr/bin/ldapsearch -o ldif-wrap=no -LLL -x -H ldap://localhost -b "$auditDN" "(fdUserDn=$callerDN)" fdObjectDn |/bin/grep ^fdObjectDn |awk -F": " '{for (i=2; i<NF; i++) printf \$i " "; print \$NF}'| /usr/bin/base64 -d;open (my $ldifHandle, '>', $tempFile) or die "Could not open temporary tracking file. Please report this error to an administrator immediately! $!"; my $ldifOut =
/usr/bin/ldapsearch -o ldif-wrap=no -LLL -x -H ldap://localhost -b "$oldObjectDN" -s base; print $ldifHandle $ldifOut; close $ldifHandle;When I assign a group as a member to an existing group (with mail and partage attributes), the LDIF data shows no indication of having mail attributes for all three plugin CHECK hooks. Other validations in the CHECK hook (looking for members that have "ou=groups" in the DN) are properly detecting problems and failing the CHECK.*
*(Please note that this functionality works perfectly for all other cases, including tracing a group object when the DN has changed (which we have other requirements for). It only fails expectations when we run into Group of Groups.)
After displaying the CHECK hook error(s), I then see the same "The object has changed since opened in FusionDirectory" message, and the application deletes the mail settings as indicated in my prior update.
When I go to the Audit tab in FusionDirectory immediately after this, I can see that remove for both plugin/mailGroup and plugin/partageGroup have fired, despite the CHECK for the change failing.
What this is clearly showing is that the "remove" processes fire before the CHECK does. This is validated by both:
- The LDIF data showing a change prior to the CHECK completing, and
- The fact that a failed CHECK doesn't prevent the remove.
I suspect that this is one of the fundamental issues going on under the hood - the removals in LDAP are firing before CHECK, with no way to prevent it from happening, rather than operating during the PRE or POST stages.
I'm not yet familiar enough with the inner workings of FusionDirectory to confirm that this is causing the set of issues we're seeing overall, but I wanted to share my findings thusfar.
By David Cortijo on 2017-10-23T18:18:01 (imported from GitLab)
Edited by bmortieradded technical discussion label
By bmortier on 2017-11-27T21:00:00 (imported from GitLab)
I don’t think check should be fired in this case, check is not called when you remove a tab from an object.
It should not remove email tab data from the LDAP until you hit save, you should be able to cancel after removing the last member and not modify anything. I’ll try to understand where this message you get is coming from.
By Côme Chilliet on 2018-02-06T08:35:13 (imported from GitLab)
created branch
5713-groups-of-groups-mail-problems-deletion-without-warning-and-mail-settings-prohibitedBy Côme Chilliet on 2018-02-06T10:57:21 (imported from GitLab)
mentioned in commit d9b516ce
By Côme Chilliet on 2018-02-06T11:00:38 (imported from GitLab)
There was indeed a problem with the workflow resulting in this «the object changed» warning upon save. The MR should fix that.
But it will still remove user tabs if you remove all users, I don’t have a clean solution for this as checking group types recursively is complicated.
By Côme Chilliet on 2018-02-06T11:06:54 (imported from GitLab)
removed technical discussion label
By bmortier on 2018-02-06T15:47:44 (imported from GitLab)
I just confirmed on demo-dev.fusiondirectory.org that tabs are no longer removed if I cancel out of the operation. Since we can prevent the removal of the tabs without an explicit save, that will help immensely. I have additional "Check" hooks that currently prevent any "groups of groups" in our system, so I can work around that part of the problem.
In terms of my specific needs, at the end of the day, I'd like to be able to create a single "outer" group that has 2+ "inner" group members, and be able to assign an email address to that outer group.
I guess what I don't understand is why FD won't allow a group without users as direct members to have an email address, since the current build will allow a group of "users + groups" to have one. I suppose that there could be a setting that allows Mail and Partage tabs to always appear for Group objects, overriding the current behavior or the evaluation of the group members, but that sounds like a lot of trouble to make happen, and probably not worth the effort.
For my specific implementation, once 1.3 is released, I could always set up a "dummy" user that would unlock the desired "group of groups" implementation if no 'real' users are supposed to be directly part of the group - it would be a bit of an ugly workaround, but it should theoretically get the job done unless I'm missing something here.
By David Cortijo on 2018-02-12T22:03:22 (imported from GitLab)
