Julian Foad wrote on Tue, 1 Jul 2008 at 19:51 +0100:
> On Tue, 2008-07-01 at 23:39 +0530, Senthil Kumaran S wrote:
> > Daniel Shahaf wrote:
> > >> +# Issue 2242, auth cache picking up password from wrong username entry
> > >> +def basic_auth_test(sbox):
> > >> + "basic auth test"
> > >> +
> > >> + sbox.build(read_only = True)
> > >
> > > Creates a working copy.
> >
> > Will add the comment.
>
> I don't think Daniel meant you should add a comment there. (It's obvious
> what it does so it would be a redundant comment.) I think he was just
> preparing the way for his next remark that contrasts with this one.
>
Correct, thanks Julian.
(It's much easier when someone else writes my text for me and I just
have to say "Yes".)
> > >> + wc_dir = sbox.wc_dir
> > >> +
> > >> + # Checkout with jrandom
> > >> + exit_code, output, errput = svntest.main.run_command(
> > >> + svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
> > >> + '--username', 'jrandom', '--password', 'rayjandom')
> > >> +
> > >
> > > Tries to checkout onto existing working copy. It happens to work, but
> > > I'm not sure that we guarantee that 'checkout' will behave as 'update'
> > > if its target already exists. In which case, we need s/checkout/update/
> > > here.
> >
> > Yes checkout works like update here. It says checked out revision 'n'.
I know that it "works". I question whether its working here is
a feature that we can rely on, or an incident of implementation that may
be declared a bug and removed. (Briefly: does our API guarantee that
'svn co URL dir; svn co URL dir;' will work as it currently does?)
> > The intention here is to check the auth. Even an 'svn ls' will do
> > but I chose this. The previous checkout is called with
> > '--no-auth-cache' as per 'run_svn' in main.py, so here we need to
> > authenticate again if we dont find an entry in ~/.subversion/auth.
> >
--username and --password override reading from the cache, so there
shouldn't be a problem.
> > > Though, actually, you can do without a working copy for this test, e.g.
> > > by doing s/checkout $URL/info $URL/.
> >
> > Yes could be, but I thought it will be comfortable to use a working copy here,
> > in case if we want to extend this test case in future.
> >
Makes sense.
> > >> + exit_code, output, errput = svntest.main.run_command(
> > >> + svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
> > >> + '--username', 'jrandom', '--non-interactive')
> > >> +
> > >
> > > We need --config-dir here or we would be affected by the user's
> > > ~/.subversion/, right? (where, possibly, the creds from a previous run of
> > > the test are cached, etc.)
> >
> > Yes exactly, that is the intention of using '--non-interactive' where we intend
> > to use ~/.subversion
I'm sorry, but I don't follow. I think --config-dir should be added to
all the run_command calls here; I don't understand how --non-interactive
or its absence come into play.
> > and also I am not using 'run_svn' here because it passes
> > the password in command line as default according to:
> >
Agreed.
> > <snip>
> > def _with_auth(args):
> > assert '--password' not in args
> > args = args + ('--password', wc_passwd,
> > '--no-auth-cache' )
> > if '--username' in args:
> > return args
> > else:
> > return args + ('--username', wc_author )
> > </snip>
> >
> > >> + # Checkout with jconstant
> > >> + exit_code, output, errput = svntest.main.run_command(
> > >> + svntest.main.svn_binary, None, 1, 'co', sbox.repo_url, wc_dir,
> > >> + '--username', 'jconstant', '--password', 'rayjandom')
> > >> +
> > >
> > > Why aren't you passing --non-interactive? Is it due to the plaintext
> > > passwords logic?
> > >
> > > if (svn_cstring_casecmp(store_plaintext_passwords,
> > > SVN_CONFIG_ASK) == 0)
> > > {
> > > if (non_interactive)
> > > /* In non-interactive mode, the default behaviour is
> > > * to not store the password, because it is usually
> > > * passed on the command line. */
> > > may_save_password = FALSE;
> >
> > Yes the above code holds true, where I register the new password for this new
> > user in the auth cache for the following checkouts.
> >
Assuming I understood you correctly (I think what you just said means
"Yes" ;)), I would prefer to see --non-interactive passed and
store-plaintext-passwords=True set in the config directory. (Are you
running with keyring or kwallet? I'm surprising the above run_command()
works without --non-interactive.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-01 22:10:53 CEST