[PATCHES] Add Unix attributes to a user or group

Andrew Bartlett abartlet at samba.org
Thu Sep 22 19:22:36 UTC 2016


On Thu, 2016-09-22 at 15:22 +0100, Rowland Penny wrote:
> On Thu, 22 Sep 2016 08:36:53 -0500
> Andrew Bartlett <abartlet at samba.org> wrote:
> 
> > 
> > On Thu, 2016-09-22 at 13:56 +0100, Rowland Penny wrote:
> > > 
> > > Hi, these patches allow RFC2307 attributes to be added to a user
> > > or group created on ADUC.
> > > 
> > > The first patch for samdb.py actually does the
> > > addition/modification
> > > 
> > > The second & third patches will add the same attributes that
> > > windows
> > > adds via the Unix Attributes tab in ADUC (note: this tab does not
> > > exist
> > > on win10).
> > > 
> > > the fourth patch allows adding or modifying user attributes,
> > > either
> > > single or multiple attributes, these will be prompted for.
> > 
> > Thanks Rowland.  
> > 
> > I do very much appreciate your efforts to improve samba-tool.
> > 
> > As you have probably come to expect, my first request is to please
> > write up the matching automated tests. 
> 
> I am quite prepared to update the 'samba-tool user create' test, but
> only after the test is updated to actually test what samba-tool does
> now when a user is created with rfc2307 attributes.

That's fine, but I don't think we can proceed with this patch until
then.

I realise this is asking you to do more work, but this is one of the
few leavers we have for expanding our testing.  And yes, I have often
had to start a whole new testsuite when wanting to tweak an existing
area. 

It is doubly important for python code, because we don't even have a
compiler to check it.

> > 
> > 
> > We need tests that run the various options (because python errors
> > are
> > only discovered when code is run, so we must cover all the
> > codepaths),
> > and we need tests that confirm that the values are correctly
> > modified
> > in the database by comparing with the results in the LDB entries.
> > 
> > Regarding 'nisadd', I'm assuming we are specifying the invalid
> > unixUserPassword out of some caution that someone will foolishly
> > use
> > this for real NIS, and missing might become an empty password?  Can
> > you check if this really happens? 
> 
> This was discussed when Marc altered 'samba-tool user create', this
> is exactly what ADUC does.

Can you get me the archive link so I can't familiarise/remind myself?

> > 
> >  I also think we should avoid
> > making more reference to NIS than we need to, it is old, outdated
> > and
> > no longer relevant to what we are doing, it just happens to be
> > where
> > some of the schema came from.  For that same reason, the --nis-
> > domain
> > option should be omitted, and if we must fill it in for the windows
> > GUI, then we should just use our workgroup name or the value given
> > for whole domain if that is stored somewhere (check how provision
> > specifies it).
> 
> Just what do you think 'nis-domain' is ??? would it be better if I
> used
> --NETBiosName or --workgroup or something else???

To be clear, I'm asking for it to be omitted.  Behind the UI it should
be set in the same way as during provision. 

> > 
> > 
> > Given all that, I think we should make this just part of a generic
> > 'user modify' that fills out any required parts for rfc2307 when
> > those
> > elements are specified, just like 'samba-tool user create'.  (I
> > realise that --nis-domain is specified there, but I would like that
> > removed in the long term).
> 
> It does exactly what samba-tool user create does when you add the
> relevant RFC2307 attribute.
> 
> What are you going to replace --nis-domain with?? 
> If you want, I can set --nis-domain automatically by sourcing it from
> CN=<WORKGROUP>,CN=ypservers,CN=ypServ30,CN=RpcServices,CN=System,DC=e
> xample,DC=com

That is what I was asking, yes.

> 
> > 
> > For 'samba-tool user modaddrs' can you explain a little more what
> > this
> > brings over ldbedit?  I ask as the tool is restricted to ldap
> > attribute names, does not show the existing values, and does not
> > throw an error for invalid attribute names (it seems to just
> > continue).
> 
> You and I may be comfortable using ldb tools to manage sam.ldb, but
> others aren't. I understand this patch isn't perfect, but it is
> somewhere to start from, if anybody has any ideas how to improve it,
> I
> am very willing to listen.

I gave my suggestion here:

> > 
> > 
> > Perhaps if you want to make it a friendly ldbedit, we should have
> > it
> > display the object in that tool, just avoiding the search
> > expression
> > steps?
> > 

Beyond that, I would suggest finding some other tool in free software
that has a similar task, and make a study of that UI, so we can learn
the lessons of others.  We should work a little to get it right, we
will probably keep it for quite some time.

> > 
> > I realise this is a lot of criticism, and I'm asking for a
> > significant
> > amount more work, but we do need to take the time to get our user
> > interfaces correct, as once released, we should avoid changing
> > them.
> 
> OK, I will withdraw the last patch and try and make it more friendly,
> but we need the the other three, or have you missed the fact that
> win10
> no longer has the Unix Attribute tabs ?
> 
> I will repeat it again, these patches only do what the Unix
> attributes
> tab does on ADUC.

I'm not objecting to the concept, just the implementation and testing. 

I'm very much looking forward to refined patches that address the
concerns I raised and add the tests I asked for.

I do realise this is a high bar, and ever since Samba started the bar
has been rising.  I'll fill you in privately on some history, but we
got here because all the alternatives where worse, and the only reason
we have a working AD DC at all is because of comprehensive testing:

That automated testing carried us though the dark times when only a few
of us carried the AD DC forward, because subtle changes elsewhere in
the codebase couldn't silently break things. 

When I'm working with Catalyst's team at work, we find we spend
something close to 75% of our time testing, or re-working and re-
writing the code as a result of that testing.  It is a tremendous cost,
but the alternative is unthinkable (think many engineers going in
circles trying to squash regressions caused by other engineers
squashing regressions). 

I hope this clarifies things,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba




More information about the samba-technical mailing list