Nested groups assignment

dnutan · July 24, 2017, 2:35pm

I cannot recall if this was discussed in the past.

Users and Groups panel:

A group can be member of itself
Group2 can be member of Group1 at the same time Group1 is member of Group2 (is this a problem?)

Tested only with local AD.

bwdjames · July 24, 2017, 3:17pm

In theory, I don’t see an issue with this until you create a nest group loop (watch the server grind to a halt when this occurs).

davidep · July 24, 2017, 3:31pm

They both don’t make any sense and should be forbidden!

pike · July 24, 2017, 5:29pm

Bad. A lot.

[quote=“dnutan, post:1, topic:7441”]
Group2 can be member of Group1 at the same time Group1 is member of Group2 (is this a problem?)
[/quote]Bad * bad * bad = cube-bad.

bwdjames · July 24, 2017, 6:41pm

Yeah - I didn’t read the original question properly but the 2nd half of my comments remain the same - creating nested group loop is a bad thing

alefattorini · July 25, 2017, 10:03am

Our UI is so powerful that permits magic things like those! I agree with the “cube-bad” definition @pike

pike · July 25, 2017, 11:17am

I don’t think that lack of controls could be… so powerful.

davidep · July 26, 2017, 7:36am

@dnutan, do you want to open a bug?

giacomo · July 27, 2017, 10:35am

I’m not sure this is a bug, see this commit:

AD supports nested groups, which AFAIK is used internally for “domain admins” and “administrators”.

Removing this function is just a matter of deleting 3 lines (

github.com

NethServer/nethserver-sssd/blob/master/root/usr/share/nethesis/NethServer/Module/Account/Group/Modify.php#L73


{
    static $groups;
    if( ! isset($groups)) {
        $groups = array_keys($this->getGroupProvider()->getGroups());
    }
    return $groups;
}


private function enumerateMembers()
{
    if($this->getGroupProvider()->isAD()) {
        return array_unique(array_merge($this->getUsers(), $this->getGroups()));
    }
    return $this->getUsers();
}


public function readMembers()
{
    return $this->getGroupProvider()->getGroupMembers($this->parameters['groupname']);
}

). Should I proceed?

/cc @dev_team

dnutan · July 27, 2017, 10:57am

Having nested groups is ok.

The bug lays in the UI:

Management -> Users and Groups -> Groups tab -> Edit a group
Add members search field should filter out the group that is being edited.

At least it wasn’t on a local AD the last time I tried.

davidep · August 24, 2017, 3:54pm

I ran a simple experiments with sssd:

create a “cyclic group” g1 → g2 → g1
create a “self loop” g1 → g1

both cases seem supported by sssd. Group members are returned consistently by

[root@vm7 nethserver]# getent group g1
g1@dpnet.nethesis.it:*:107401105:first.user@dpnet.nethesis.it,admin@dpnet.nethesis.it
[root@vm7 nethserver]# getent group g2
g2@dpnet.nethesis.it:*:107401106:first.user@dpnet.nethesis.it,admin@dpnet.nethesis.it

Furthermore, cycles seem to be allowed in AD and RSAT tools. Some (poorly implemented) third party apps could not cope with them. There are some scripts around that analyze the LDAP db and report cycles.

In the end, I think we can retain the current implementation and allow group cycles because the underlying implementation allows them.

What do you think? /cc @dnutan

dnutan · August 24, 2017, 6:35pm

Yes. Have searched for it and as you said AD accepts circular nested groups after all.