[PATCH] samba-tool group addmembers

Wed Jun 7 10:23:21 UTC 2017

On Wed, 7 Jun 2017 12:56:35 +0300
Alexander Bokovoy <ab at samba.org> wrote:

> On ke, 07 kesä 2017, Rowland Penny via samba-technical wrote:
> > On Wed, 7 Jun 2017 12:02:53 +0300
> > Alexander Bokovoy <ab at samba.org> wrote:
> > 
> > > On ke, 07 kesä 2017, Rowland Penny via samba-technical wrote:
> > > > 
> > > > Hi, if you try to add a member to a group and the member exists
> > > > as a sAMAccountName and a CN, you get this:
> > > > 
> > > > root at dc1:~# samba-tool group addmembers group12 rowland
> > > > ERROR(exception): Failed to add members "rowland" to group
> > > > "group12"
> > > > - Unable to find "rowland". Operation cancelled. File
> > > > "/usr/local/samba/lib/python2.7/site-packages/samba/netcmd/group.py",
> > > > line 239, in run add_members_operation=True) File
> > > > "/usr/local/samba/lib/python2.7/site-packages/samba/samdb.py",
> > > > line 278, in add_remove_group_members raise Exception('Unable
> > > > to find "%s". Operation cancelled.' % member)
> > > > 
> > > > The user 'rowland' exists here:
> > > > 
> > > > dn: CN=Rowland Penny,CN=Users,DC=samdom,DC=example,DC=com
> > > > sAMAccountName: rowland
> > > > 
> > > > and here:
> > > > 
> > > > dn: CN=rowland,OU=SUDOers,DC=samdom,DC=example,DC=com
> > > > 
> > > > The problem isn't that 'rowland' doesn't exist, it is that he
> > > > exists more than once ;-)
> > > > 
> > > > Another user had the same problem, but he created the users with
> > > > '--use-username-as-cn'
> > > > 
> > > > This patch fixed the problem for me and the other user, it just
> > > > changes the search for the user to use only the sAMAccountName
> > > > attribute.
> > > > 
> > > > Rowland
> > > 
> > > > From 8191910cc59e045b94b3779b2b9cddca1b75c230 Mon Sep 17
> > > > 00:00:00 2001 From: Rowland Penny <rpenny at samba.org>
> > > > Date: Wed, 7 Jun 2017 09:23:10 +0100
> > > > Subject: [PATCH] samba-tool: You cannot add members to a group
> > > > if the member exists as a sAMAccountName and a CN.
> > > > 
> > > > Signed-off-by: Rowland Penny <rpenny at samba.org>
> > > > ---
> > > >  python/samba/netcmd/group.py | 2 ++
> > > >  python/samba/samdb.py        | 4 ++--
> > > >  2 files changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/python/samba/netcmd/group.py
> > > > b/python/samba/netcmd/group.py index 11f8773..b9d6add 100644
> > > > --- a/python/samba/netcmd/group.py
> > > > +++ b/python/samba/netcmd/group.py
> > > > @@ -199,6 +199,8 @@ This command adds one or more members to an
> > > > existing Active Directory group. The 
> > > >  When a member is added to a group the member may inherit
> > > > permissions and rights from the group.  Likewise, when
> > > > permission or rights of a group are changed, the changes may
> > > > reflect in the members through inheritance. +The member names
> > > > specified on the command must be the sAMaccountName. + Example1:
> > > >  samba-tool group addmembers supergroup Group1,Group2,User1 -H
> > > > ldap://samba.samdom.example.com -Uadministrator%passw0rd 
> > > > diff --git a/python/samba/samdb.py b/python/samba/samdb.py
> > > > index 19dd8e9..b4d6768 100644
> > > > --- a/python/samba/samdb.py
> > > > +++ b/python/samba/samdb.py
> > > > @@ -267,8 +267,8 @@ changetype: modify
> > > >  
> > > >              for member in members:
> > > >                  targetmember =
> > > > self.search(base=self.domain_dn(), scope=ldb.SCOPE_SUBTREE,
> > > > -
> > > > expression="(|(sAMAccountName=%s)(CN=%s))" % (
> > > > -                    ldb.binary_encode(member),
> > > > ldb.binary_encode(member)), attrs=[])
> > > > +
> > > > expression="(sAMAccountName=%s)" % (
> > > > +                    ldb.binary_encode(member)), attrs=[])
> > > >  
> > > >                  if len(targetmember) != 1:
> > > >                      raise Exception('Unable to find "%s".
> > > > Operation cancelled.' % member) -- 
> > > > 2.1.4
> > > > 
> > > I think instead of removing CN=%s from the filter it would be
> > > better to limit the search by objectclass filtering to
> > > 'objectclass=user'.
> > > 
> > > E.g. change expression into
> > > 
> > >   expression="(&(|(sAMAccountName=%s)(CN=%s))(objectclass=user))"
> > > % ...
> > > 
> > > 
> > 
> > I don't think this will work, the other user created users with
> > '--use-username-as-cn' and it failed in the same way for him until
> > he tried my patch, my supposition is that the user is seen twice.
> > Once by 'sAMAccountName' and again by 'CN'. There is also the
> > problem of groups, or do you suggest we use something like:
> > 
> > expression="(&(|(sAMAccountName=%s)(CN=%s))(|(objectclass=user)(objectclass=group)))"
> > % ...
> Yes. At least in python/samba/netcmd/group.py it is clear that members
> of a group could be user, group, or machine account. This means all of
> those object classes need to be in the filter. Luckily, that is all
> covered by (|(objectclass=user)(objectclass=group)).
> 
> Leaving out CN=%s would fail hierarchical groups (adding a group as a
> member of a group). Your problem with sudoers is effectively on
> matching more than needed objects. If cn=rowland,cn=SUDOers,.. is not
> a user or a group, then it will not match this new filter and
> everything will be fine.
> 

So, by leaving out 'CN=%s', I will not be able to add a group to a
group ?

Not by my testing:

root at member1:~# samba-tool group listmembers unixgroup
suser1
User20
User21
user22
rowland
ldap01

Now add a group to a group:

root at member1:~# samba-tool group addmembers unixgroup testgroup
Added members to group unixgroup

root at member1:~# samba-tool group listmembers unixgroup
suser1
User20
User21
TestGroup
user22
rowland
ldap01

And before you ask, add a computer to a group:
root at member1:~# samba-tool group addmembers unixgroup DEVSTATION$
Added members to group unixgroup

root at member1:~# samba-tool group listmembers unixgroup
suser1
User20
User21
TestGroup
user22
DEVSTATION$
rowland
ldap01

Extracts from the AD objects:

dn: CN=Unixgroup,CN=Users,DC=samdom,DC=example,DC=com
sAMAccountName: Unixgroup
member: CN=TestGroup,CN=Users,DC=samdom,DC=example,DC=com
member: CN=DEVSTATION,CN=Computers,DC=samdom,DC=example,DC=com

dn: CN=TestGroup,CN=Users,DC=samdom,DC=example,DC=com
sAMAccountName: TestGroup
memberOf: CN=Unixgroup,CN=Users,DC=samdom,DC=example,DC=com

dn: CN=DEVSTATION,CN=Computers,DC=samdom,DC=example,DC=com
sAMAccountName: DEVSTATION$
memberOf: CN=Unixgroup,CN=Users,DC=samdom,DC=example,DC=com

Still think we need 'CN=%s' ??

Rowland