[PATCH] samba-tool: Easily edit a users object in AD V3 with test

[Rowland Penny]

On Tue, 04 Jul 2017 14:51:39 +0200
Daniele Dario <d.dario76 at gmail.com> wrote:

> Hi Rowland,
> 
> On mar, 2017-07-04 at 13:25 +0100, Rowland Penny via samba-technical
> wrote:
> > On Tue, 4 Jul 2017 14:11:23 +0300
> > Alexander Bokovoy <ab at samba.org> wrote:
> > 
> > > On ti, 04 heinä 2017, Rowland Penny via samba-technical wrote:
> > > > On Tue, 4 Jul 2017 12:39:00 +0300
> > > > Alexander Bokovoy <ab at samba.org> wrote:
> > > > 
> > > > > On ti, 04 heinä 2017, Rowland Penny via samba-technical wrote:
> > > > > > On Mon, 3 Jul 2017 18:07:49 +0300
> > > > > > Alexander Bokovoy <ab at samba.org> wrote:
> > > > > > 
> > > > > > > Looks like it works! Great!
> > > > > > 
> > > > > > OK, one step forward, three steps back ;-)
> > > > > > 
> > > > > > How do you create a new file ?
> > > > > > 
> > > > > > I have created the edit.sh file in my local git, but when I
> > > > > > run:
> > > > > > 
> > > > > > git commit -s -a
> > > > > > 
> > > > > > I get:
> > > > > > 
> > > > > > On branch master
> > > > > > Your branch is ahead of 'origin/master' by 1 commit.
> > > > > >   (use "git push" to publish your local commits)
> > > > > > Untracked files:
> > > > > > 	python/samba/tests/samba_tool/edit.sh
> > > > > > 
> > > > > > nothing added to commit but untracked files present
> > > > > > 
> > > > > > I don't think I should run 'git push' as, from reading the
> > > > > > manpage, it looks like this will try to create 'edit.sh' in
> > > > > > the Samba git.
> > > > > git add <file>
> > > > > https://www.atlassian.com/git/tutorials/saving-changes
> > > > 
> > > > I did wonder about that, but 'man git commit' said this:
> > > > 
> > > >        The command git commit -a first looks at your working
> > > > tree, notices that you have modified hello.c and removed
> > > > goodbye.c, and performs necessary git add and git rm for you.
> > > > 
> > > > Looks like the manpage lies, because I was using '-a' and it
> > > > didn't add the file ;-)
> > > > 
> > > > I followed the suggestion and it worked, so I have attached my
> > > > new patches for editing users and the test for this.
> > > Ok, I have only one comment I'd like to be addressed before we
> > > accept this changeset:
> > > 
> > > +# create editor.sh
> > > +echo "#!/usr/bin/env bash" > /tmp/editor.sh
> > > +echo "user_ldif=\"\$1\"" >> /tmp/editor.sh
> > > +echo "SED=\$(which sed)" >> /tmp/editor.sh
> > > +echo "\$SED -i -e 's/userAccountControl: 512/userAccountControl:
> > > 514/' \$user_ldif" >> /tmp/editor.sh +
> > > +chmod +x /tmp/editor.sh
> > > +
> > > 
> > > Please don't use a static path here. It would prevent multiple
> > > parallel 'make test' runs on the same machine by different users.
> > > And also would allow an attacker (whoever that be) to rewrite your
> > > editor.sh and inject an execution into your 'make test' run.
> > > 
> > > Instead, create a temporary file:
> > > 
> > > tmpeditor=$(mktemp --suffix .sh samba-tool-editor-XXXXXXXX)
> > > 
> > > And put the content into it:
> > > 
> > > cat >$tmpeditor <<-'EOF'
> > > 	#!/usr/bin/env bash
> > > 	user_ldif="$1"
> > > 	SED=$(which sed)
> > > 	$SED -i -e 's/userAccountControl: 512/userAccountControl:
> > > 514/' $user_ldif EOF
> > > 
> > > chmod +x $tmpeditor
> > > 
> > > <<-'EOF' means to start here document which ends with EOF, strip
> > > leading tabs, do not perform interpolation within the document.
> > > This allows you to keep the content of the script clean from shell
> > > expansion protection.
> > > 
> > > 
> > 
> > OK, here are the new patches ;-)
> > 
> > Rowland
> > 
> 
> I'm not sure but you create the edit.sh script as
> 
> +# create editor.sh
> +tmpeditor=$(mktemp --suffix .sh samba-tool-editor-XXXXXXXX)
> +
> +cat >$tmpeditor <<-'EOF'
> +#!/usr/bin/env bash
> +user_ldif="$1"
> +SED=$(which sed)
> +$SED -i -e 's/userAccountControl: 512/userAccountControl: 514/'
> $user_ldif
> +EOF
> +
> +chmod +x $tmpeditor
> 
> but when you run the test you do
> 
> +# Edit test user
> +subunit_start_test "Edit_User"
> +output=$(${STpath}/source4/scripting/bin/samba-tool user edit
> sambatool1 --editor=/tmp/editor.sh \
> +-H "ldap://$SERVER" "-U$USERNAME" "--password=$PASSWORD")
> 
> Isn't it wrong the --editor part?
> 
> I'd say it would be --editor=$tmpeditor
> 
> Daniele.
> 

Note to self: must engage brain ;-)

Yes you are probably correct.
I say probably, because I didn't change that line and it worked, I have
just changed it and now it doesn't work.

I will try and see why it doesn't work and get back with V4 patches :-(

Rowland