[PATCH] DRAFT: Add new test which covers force user bug
Michael Adam
obnox at samba.org
Thu Jan 7 08:43:17 UTC 2016
Hi Robin,
Thanks a lot for working on the testsuite!!
Some comments:
- Have you run a full 'make test' with your patch?
(You should, in this case, since it modifies the test
environment...)
- I don't quite understand the patch yet:
See comments inline below.
On 2016-01-04 at 14:35 +0100, Robin Hack wrote:
> Patch is here :).
>
> On Mon, Jan 4, 2016 at 2:28 PM, Robin Hack <rhack at redhat.com> wrote:
>
> > Hi Jeremy.
> >
> > Sorry for such a big delay. I was on vacation. And thank you very much for
> > heads up.
> >
> > I looked at issue which you described and I hope that I fixed it.
> >
> > In previous case I used $unix_gids[0] for $unix_name user.
> > I use newly generated gid now.
> >
> > Could you please check and review my changes again and test them? I ran
> > $ make test TESTS=samba3.local.nss
> > and issue seems gone. I'm not sure if other issues will not raise.
> >
> > Sorry for issues with my patch. I didn't realized how fragile it can be :(.
> >
> >
> > Have nice day
> > Robin Hack
> From cae478de1f577b181eab68f0bef7d6e49ba1c1a7 Mon Sep 17 00:00:00 2001
> From: Robin Hack <rhack at redhat.com>
> Date: Fri, 4 Dec 2015 14:50:56 +0100
> Subject: [PATCH] samba3.blackbox.smbclient.forceuser: Add new test for force
> user option.
>
> Test covers commit
> a85d6eb355e8277fe69db6e5837a7f65f57d9c1d
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=9878
> RH BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1077651
>
> Signed-off-by: Robin Hack <rhack at redhat.com>
> ---
> selftest/target/Samba3.pm | 31 ++++++++++++++++
> source3/script/tests/test_forceuser.sh | 66 ++++++++++++++++++++++++++++++++++
> source3/selftest/tests.py | 6 ++--
> 3 files changed, 101 insertions(+), 2 deletions(-)
> create mode 100755 source3/script/tests/test_forceuser.sh
>
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index 94612b2..ccb2751 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -790,11 +790,36 @@ $ret->{USERNAME} = KTEST\\Administrator
> sub setup_maptoguest($$)
> {
> my ($self, $path) = @_;
> + my $prefix_abs = abs_path($path);
>
> print "PROVISIONING maptoguest...";
>
> + my @dirs = ();
> +
> + mkdir($prefix_abs, 0777);
> + my $share_dir="$prefix_abs/share";
> + push(@dirs, $share_dir);
> +
> + my $force_user_not_work_dir = "$share_dir/force_user_not_work";
> + push(@dirs, $force_user_not_work_dir);
> +
> + my $unix_name = ($ENV{USER} or $ENV{LOGNAME} or `PATH=/usr/ucb:$ENV{PATH} whoami`);
> + chomp $unix_name;
> +
> +
> my $options = "
> map to guest = bad user
> +
> +# BUG: https://bugzilla.samba.org/show_bug.cgi?id=9878
> +# BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1077651
These bugs are not at all related to map to guest.
But your test and comments seem to suggest that.
I think you should either remove the references to map-to-guest
and do it (maybe additionally?) with a non-map-to-guest share,
or else you should remove the above BZ references.
> +[force_user_not_work]
> + path = $force_user_not_work_dir
> + comment = force user test share with map to guest set to bad user
> + valid users = \@$unix_name
> + force user = $unix_name
> + force group = everyone
> + write list = $unix_name
> + create mask = 0644
> ";
>
> my $vars = $self->provision($path,
> @@ -804,6 +829,9 @@ map to guest = bad user
>
> $vars or return undef;
>
> + mkdir($_, 0777) foreach(@dirs);
> + chmod(0777, $_) foreach(@dirs);
> +
> if (not $self->check_or_start($vars, "yes", "no", "yes")) {
> return undef;
> }
> @@ -1231,6 +1259,7 @@ sub provision($$$$$$$$)
> my ($uid_pdbtest_wkn);
> my ($gid_nobody, $gid_nogroup, $gid_root, $gid_domusers, $gid_domadmins);
> my ($gid_userdup, $gid_everyone);
> + my ($gid_same_name);
>
> if ($unix_uid < 0xffff - 5) {
> $max_uid = 0xffff;
> @@ -1258,6 +1287,7 @@ sub provision($$$$$$$$)
> $gid_domadmins = $max_gid - 5;
> $gid_userdup = $max_gid - 6;
> $gid_everyone = $max_gid - 7;
> + $gid_same_name = $max_gid - 8;
>
> ##
> ## create conffile
> @@ -1551,6 +1581,7 @@ pdbtest_wkn:x:$uid_pdbtest_wkn:$gid_everyone:pdbtest_wkn gecos:$prefix_abs:/bin/
> }
> print GROUP "nobody:x:$gid_nobody:
> nogroup:x:$gid_nogroup:nobody
> +$unix_name:x:$gid_same_name:
> $unix_name-group:x:$unix_gids[0]:
> domusers:X:$gid_domusers:
> domadmins:X:$gid_domadmins:
> diff --git a/source3/script/tests/test_forceuser.sh b/source3/script/tests/test_forceuser.sh
> new file mode 100755
> index 0000000..46fe251
> --- /dev/null
> +++ b/source3/script/tests/test_forceuser.sh
> @@ -0,0 +1,66 @@
> +#!/bin/sh
> +#
> +# Blackbox test for share with force user settings
> +#
> +
> +if [ $# -lt 6 ]; then
> +cat <<EOF
> +Usage: test_forceuser.sh SERVER DOMAIN USERNAME PASSWORD LOCAL_PATH SMBCLIENT <smbclient arguments>
> +EOF
> +exit 1;
> +fi
> +
> +SERVER="$1"
> +DOMAIN="$2"
> +USERNAME="$3"
> +PASSWORD="$4"
> +LOCAL_PATH="$5"
> +SMBCLIENT="$6"
> +SMBCLIENT="$VALGRIND ${SMBCLIENT}"
> +shift 6
> +ADDARGS="$*"
> +failed=0
> +
> +
> +incdir=`dirname $0`/../../../testprogs/blackbox
> +. $incdir/subunit.sh
> +
> +
> +run_cmd_nooutput() {
> + CMD="$1"
> +
> + out=`eval ${CMD} &> TESTOUT`
> + if [ $? != 0 ] ; then
> + cat TESTOUT
> + rm -f TESTOUT
> + echo "command failed"
> + false
> + return
> + fi
> +
> + rm -f TESTOUT
> + true
> + return
> +}
> +
> +test_force_user_not_work()
> +{
> + SMB_SHARE="force_user_not_work"
> + run_cmd_nooutput "${SMBCLIENT} //${SERVER}/${SMB_SHARE} -U$USERNAME%$PASSWORD -c 'put TESTFILE'"
> +}
> +
> +# Setup
> +pushd "${LOCAL_PATH}" > /dev/null
> +rm -f TESTFILE
> +echo "hello samba!" > TESTFILE
> +
> +# Test
> +testit "force user not works when map to guest is set" \
> + test_force_user_not_work || failed=`expr $failed + 1`
I do not quite understand the logic here yet:
The above seems to test that the smbclient command succeeds.
But the name and comments suggest that it tests failure...
So I guess you should at least adapt the message and function
name? And the share name?
But again -- what is the real relationship yo 'map to guest'?
Thanks -- Michael
> +# Cleanup
> +rm -f TESTFILE
> +popd > /dev/null
> +
> +# Results
> +testok $0 $failed
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index 9c68943..0909da2 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -154,8 +154,10 @@ plantestsuite("samba3.ntlm_auth.krb5(ktest:local)", "ktest:local", [os.path.join
> for env in ["maptoguest", "simpleserver"]:
> plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) local creds" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', smbclient3, configuration + " --option=clientntlmv2auth=no --option=clientlanmanauth=yes"])
>
> -env = "maptoguest"
> -plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) bad username" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', 'notmy$USERNAME', '$PASSWORD', smbclient3, configuration + " --option=clientntlmv2auth=no --option=clientlanmanauth=yes"])
> +for env in ["maptoguest"]:
> + plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) bad username" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', 'notmy$USERNAME', '$PASSWORD', smbclient3, configuration + " --option=clientntlmv2auth=no --option=clientlanmanauth=yes"])
> + plantestsuite("samba3.blackbox.smbclient.forceuser (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_forceuser.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', smbclient3])
> +
>
> # plain
> for env in ["nt4_dc"]:
> --
> 1.9.3
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160107/1255a637/signature.sig>
More information about the samba-technical
mailing list