Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

[Andrew Bartlett]

On Sat, 2010-06-19 at 22:12 +0100, Lukasz Zalewski wrote:
> On 13/06/2010 11:14, Lukasz Zalewski wrote:
> > On 13/06/2010 05:56, Andrew Bartlett wrote:
> >> On Sun, 2010-06-13 at 01:39 +0100, Lukasz Zalewski wrote:
> >>> On 12/06/2010 13:25, Andrew Bartlett wrote:
> >>>> On Tue, 2010-06-08 at 20:47 +0100, Lukasz Zalewski wrote:
> >>>>> Hi Jelmer,
> >>>>>
> >>>>> On 08/06/2010 19:39, Jelmer Vernooij wrote:
> >>>>>> Hi Lukasz,
> >>>>>>
> >>>>>> On Mon, 2010-06-07 at 17:29 +0100, Lukasz Zalewski wrote:
> >>>>>>> Attached is a new patch which is an extension to the previous
> >>>>>>> newuser
> >>>>>>> functionality. It also adds the set of group commands (operating
> >>>>>>> on the ldb)
> >>>>>>>
> >>>>>>> Now, new user is created with CN='Firstname Initials.
> >>>>>>> Lastname',<> if
> >>>>>>> any of them exists and --use-username-as-cn is not set (this is
> >>>>>>> the AD
> >>>>>>> way) otherwise CN=username,<> is used
> >>>>>>> If --use-username-as-cn is defined then the user is created with
> >>>>>>> CN=username,<> even if any of (Firstname, Initials,Lastname) exist
> >>>>>>>
> >>>>>>> I also took liberty to add group set of subcommands, i.e.
> >>>>>>> /usr/local/samba/bin/net group
> >>>>>>> Available subcommands:
> >>>>>>> addmembers
> >>>>>>> add
> >>>>>>> removemembers
> >>>>>>> delete
> >>>>>>>
> >>>>>>> These only operate on ldb at the moment.
> >>>>>> Thanks! This looks very nice overall. Some minor comments:
> >>>>> Thx for all of the comments. I have attached modified patches
> >>>>>>
> >>>>>> * PEP8 - please use two empty lines between top-level elements
> >>>>>> (such as
> >>>>>> classes).
> >>>>> I hope this is fixed - I'm quite new to python
> >>>>>>
> >>>>>> * When dealing with transactions it's usually better to delay
> >>>>>> starting
> >>>>>> the transaction as long as possible (where it makes sense, of
> >>>>>> course).
> >>>>>> This makes the code more readable (less indentation). E.g. at the
> >>>>>> bottom
> >>>>>> of your patch you create a ldbmessage after starting a transaction
> >>>>>> rather than just for the self.add call. There's no need to go
> >>>>>> through a
> >>>>>> lot of trouble to do this but if it's easy it's preferable.
> >>>>> newuser and newgroup functions corrected.
> >>>>>>
> >>>>>> * Any reason for using cmp() rather than == when comparing two
> >>>>>> strings ?
> >>>>>> The latter is more readable and more commonly used.
> >>>>> No reason, except old habbit of using strcmp i guess ;)
> >>>>>>
> >>>>>> Cheers,
> >>>>>>
> >>>>>> Jelmer
> >>>>> Let me know if i need to make any more changes
> >>>>
> >>>> There is one more thing that would be really useful. If you look at
> >>>> setup/tests/blackbox_newuser.sh, we have a test for the old version of
> >>>> this script. If you can extend it to cover all the new options you have
> >>>> here, that would help ensure it keeps working.
> >>>>
> >>>> Also make sure that the tree, with this patch in, passes 'make test'
> >>>> just as much as without it.
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Andrew Bartlett
> >>>>
> >>> Hi Andrew,
> >>>
> >>> I have almost finished the test suites, however i have a question - how
> >>> does one pass switches with spaces to testit function, e.g.
> >>> --my-switch="My value with spaces"
> >>> I have tried escaping adding quotes to no success, i.e. its never
> >>> treated as a single parameter
> >>> for example test script fails with the following (i have added an echo
> >>> line to testit function):
> >>> test: group addmembers
> >>> ./bin/net group addmembers --configfile=sh/simple-dc/etc/smb.conf dsg
> >>> "User UT. Tester,User1 UT. Tester"
> >>> failure: group addmembers [
> >>> Usage: net group addmembers<groupname> <listofmembers>
> >>> ]
> >>>
> >>> but when run on the cmd line
> >>> ./bin/net group addmembers --configfile=sh/simple-dc/etc/smb.conf dsg
> >>> "User UT. Tester,User1 UT. Tester"
> >>> all is fine
> >>
> >> Yeah, shell quoting sucks - we really need to redo all these scripts in
> >> python.
> >>
> >> Have you tried using ' or \' rather than "?
> >>
> >> Otherwise, look at all the blackbox scripts in the tree and see if
> >> another already solves this.
> >>
> >> Andrew Bartlett
> >>
> >
> > Hi Andrew,
> > Tried various combinations of ", \" and ' with no success :( As a
> > workaround i have made all the parameters that i could without spaces. I
> > had to comment our two tests (add/remove group members through their cn)
> > as they could not be shortened. I have attached new patch.
> > Please let me know your comments
> >
> > Regards
> >
> > Luk
> 
> Andrew, Jelmer
> Did you get a chance to look at the added test? are they adequate?

I've looked over the tests, and I agree, they are the best that can be
managed.  I've not run it, but assuming it passes 'make test', I'm very
happy for this to be merged. 

Matthias, Can you handle this for me?

Thanks,

Andrew Bartlett
-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100620/25ff201b/attachment.pgp>