Stefan Sperling wrote on Fri, 18 Jul 2008 at 17:37 +0200:
> On Fri, Jul 18, 2008 at 05:20:59PM +0300, Daniel Shahaf wrote:
> > Stefan Sperling wrote on Fri, 18 Jul 2008 at 15:20 +0200:
> > > [[[
> > > Fix unnecessary plaintext password saving prompt when the username
> > > is supplied on the command line and the password is already cached.
> > >
> > > * subversion/libsvn_subr/simple_providers.c
> > > (svn_auth__simple_first_creds_helper): Start out with may_save = FALSE.
> > > The old code did set creds->may_save to TRUE whenever a username
> > > was supplied on the command line, regardless of whether a password
> > > was already cached or not. This unconditionally triggered the whole
> > > auth caching logic (including the 'may I save your password in
> > > plaintext?' prompt) to be run again, even if a password was already
> > > cached.
> > >
> > > Patch by: stylesen
> > > Found by: arfrever
> > > Approved by: stsp
> > > ]]]
> > >
> > > Please commit if you agree with my log message.
> > >
> >
> > After this patch, svn_auth__simple_first_creds_helper() will never set
> > 'may_save' to TRUE except when the auth cache exists and is of a older
> > format that contains a password but not a passtype:
> >
> > /* If the auth data didn't contain a password type,
> > force a write to upgrade the format of the auth
> > data file. */
> > if (password && passtype && !have_passtype)
> > may_save = TRUE;
> >
> > Don't we need to set may_save to 'TRUE' in some cases as well?
> >
> > Daniel
> > (who didn't reply in the morning, because he thinks he's missing
> > something obvious, again)
>
> No, we don't need to set may_save to TRUE in any other case.
>
> All that svn_auth__simple_first_creds_helper() (part of the
> "simple file" provider) does is look whether there are some credentials
> cached already. If there are any, there's not much point in caching
> them again, right? So creds->may_save can be as FALSE as it wants to be.
>
> If the creds don't authenticate the next provider will be tried
> anyway, because the simple file provider has no 'next_credentials'
> method (see the definition of svn_auth_provider_t in svn_auth.h
> if you don't know the difference between 'first_credentials' and
> 'next_credentials').
>
> If svn_auth__simple_first_creds_helper() cannot find any creds in the
> auth cache, it says "I have no credentials" (by setting *credentials
> to NULL) and, again, the next provider is tried.
>
> Among the next providers is likely to be the 'prompt provider', which
> prompts the user interactively and creates its own creds object.
>
> There are two invocations of the prompt provider in simple_providers.c,
> both of which use '! no_auth_cache' to set the 'may_save' parameter
> while prompting the user for a username and password. Both calls,
> if successful, create an entirely new credentials object:
>
> One is in simple_prompt_first_creds:
> SVN_ERR(prompt_for_simple_creds((svn_auth_cred_simple_t **) credentials_p,
> pb, parameters, realmstring, TRUE,
> ! no_auth_cache, pool));
>
> The other one is in simple_prompt_next_creds:
> SVN_ERR(prompt_for_simple_creds((svn_auth_cred_simple_t **) credentials_p,
> pb, parameters, realmstring, FALSE,
> ! no_auth_cache, pool));
>
> Here's the signature of prompt_for_simple_creds for reference:
>
> static svn_error_t *
> prompt_for_simple_creds(svn_auth_cred_simple_t **cred_p,
> simple_prompt_provider_baton_t *pb,
> apr_hash_t *parameters,
> const char *realmstring,
> svn_boolean_t first_time,
> svn_boolean_t may_save,
> apr_pool_t *pool)
>
> The may_save parameter is copied straight into the returned credentials
> objects.
Not always:
if (def_username && def_password)
{
*cred_p = apr_palloc(pool, sizeof(**cred_p));
(*cred_p)->username = apr_pstrdup(pool, def_username);
(*cred_p)->password = apr_pstrdup(pool, def_password);
(*cred_p)->may_save = TRUE;
}
ignores the 'may_save' parameter. (Here, def_{username,password}
are the values of --username and --password.)
> So unless the user passes --no-auth-cache, the credentials
> returned from these calls have their may_save member set to TRUE,
> and they will be cached in the usual way.
>
> Did this relieve your concerns?
>
Yes, and thank you for the long explanation.
Daniel
> Stefan
>
---------------------------------------------------------------------
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-18 18:11:35 CEST