Samba4 Patch: newuseradv and newgroupadv scripts for net cmd utlity

[Lukasz Zalewski]

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
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: master-newusergroupcmds.patches3.txt
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100613/738bacc3/attachment.txt>