You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Glasser <gl...@davidglasser.net> on 2008/04/19 17:58:13 UTC
Re: svn commit: r30701 - in branches/dont-save-plaintext-passwords-by-default/subversion: include libsvn_subr svn
It might be cleaner to, instead of making the option be
boolean-with-special-meaning-for-undefined, have it just be a
three-value option: yes, no, prompt (with prompt as the default, and
accepting all the synonyms of yes/no like svn_config_get_bool). This
way you could also (if it's in .subversion/servers) make the default
be "yes" but have it be "prompt" for specific servers.
--dave
On Sat, Apr 19, 2008 at 5:00 AM, <st...@tigris.org> wrote:
> Author: stsp
> Date: Sat Apr 19 05:00:42 2008
> New Revision: 30701
>
> Log:
> On the dont-save-plaintext-passwords-by-default branch, drop the command
> line option --store-plaintext-pw in favour of an interactive prompt.
>
> * subversion/include/svn_config.h
> (svn_config_get_bool2): New function.
> (svn_config_get_bool): Adjust docstring in light of svn_config_get_bool2.
>
> * subversion/include/svn_cmdline.h
> (svn_cmdline_setup_auth_baton2): Remove declaration of removed function.
> (svn_cmdline_setup_auth_baton): Restore old docstring.
>
> * subversion/include/svn_auth.h
> (svn_auth_plaintext_prompt_func_t): New callback.
> (SVN_AUTH_PARAM_DONT_STORE_PLAINTEXT_PASSWORDS): New constant.
> (svn_auth_get_simple_provider2): Declare.
> (svn_auth_get_simple_provider): Deprecate.
>
> * subversion/libsvn_subr/config.c
> (svn_config_get_bool2): Extension of svn_config_get_bool.
> Can indicate whether the default value supplied was used,
> or whether the boolean value returned was actually retrieved
> from the configuration file.
> (svn_config_get_bool): Call svn_config_get_bool2.
>
> * subversion/libsvn_subr/cmdline.c
> (svn_cmdline_setup_auth_baton2): Remove. This was needed to pass
> the --store-plaintext-pw pcommand line option down to the providers.
> (svn_cmdline_setup_auth_baton): Restore, and create the simple provider
> with svn_auth_get_simple_provider2. Use svn_config_get_bool2 to determine
> whether store-plaintext-passwords is explicitly defined in the
> configuration file. Also sprinkle some new comments, and remove
> a rather obsolete comment, which seems to predate even the
> store-passwords configuration option.
>
> * subversion/libsvn_subr/prompt.c
> (svn_cmdline_auth_plaintext_prompt): New function.
>
> * subversion/libsvn_subr/simple_providers.c
> (simple_provider_baton_t): New type.
> (simple_save_creds_helper): Store encrypted passwords by default.
> For plaintext passwords, check the configuration file settings.
> If the configuration file does not define any explicit preference,
> use the svn_auth_plaintext_prompt_func_t callback to prompt the
> user about how to proceed.
> (svn_auth_get_simple_provider2): New revision of
> svn_auth_get_simple_provider, takes an svn_auth_plaintext_prompt_func_t
> callback, and a baton for it.
> (svn_auth_get_simple_provider): Call svn_auth_get_simple_provider2.
>
> * subversion/svn/main.c
> (svn_cl__longopt_t, svn_cl__options): Remove opt_store_plaintext_passwords.
> (svn_cl__options): Remove the --store-plaintext-pw option.
> (main): Remove handling of opt_store_plaintext_passwords, and call
> svn_cmdline_setup_auth_baton again.
>
> Modified:
> branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_auth.h
> branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_cmdline.h
> branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_config.h
> branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c
> branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config.c
> branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/prompt.c
> branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/simple_providers.c
> branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_auth.h
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_auth.h?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_auth.h Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_auth.h Sat Apr 19 05:00:42 2008 (r30701)
> @@ -457,6 +457,32 @@ typedef svn_error_t *(*svn_auth_ssl_clie
> svn_boolean_t may_save,
> apr_pool_t *pool);
>
> +/** Called only by providers which save passwords unencrypted.
> + * In this callback, clients should ask the user whether storing
> + * a password to disk in plaintext is allowed, and return the answer
> + * in @a may_save_plaintext.
> + *
> + * @a baton is an implementation-specific closure.
> + *
> + * All allocations should be done in @a pool.
> + *
> + * This callback is only called if the store-plaintext-passwords
> + * directive in the configuration file is undefined.
> + *
> + * If this callback is NULL it is not called. This matches the
> + * deprecated behaviour of storing unencrypted passwords by default,
> + * and is only done this way for backward compatibility reasons.
> + * Client developers are highly encouraged to provide this callback
> + * to ensure their users are made aware of the fact that their password
> + * is going to be stored unencrypted. In the future, providers may
> + * default to not storing the password unencrypted if this callback is NULL.
> + *
> + * @since New in 1.6
> + */
> +typedef svn_error_t *(*svn_auth_plaintext_prompt_func_t)
> + (svn_boolean_t *may_save_plaintext,
> + void *baton,
> + apr_pool_t *pool);
>
>
> /** Initialize an authentication system.
> @@ -525,10 +551,19 @@ const void * svn_auth_get_parameter(svn_
> "dont-store-passwords"
>
> /** @brief The application wants providers to save passwords to disk in
> - * plain text. Property value is irrelevant; only property's existence
> + * plaintext. Property value is irrelevant; only property's existence
> * matters. */
> #define SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS SVN_AUTH_PARAM_PREFIX \
> - "store-plaintext-passwords"
> + "store-plaintext-passwords"
> +
> +/** @brief The application does not want providers to save passwords to
> + * disk in plaintext. Property value is irrelevant; only property's existence
> + * matters. This overrides SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS.
> + *
> + * We use two parameters because we need to know whether there is an
> + * explicit setting in the configuration file or not. */
> +#define SVN_AUTH_PARAM_DONT_STORE_PLAINTEXT_PASSWORDS SVN_AUTH_PARAM_PREFIX \
> + "dont-store-plaintext-passwords"
>
> /** @brief The application doesn't want any providers to save credentials
> * to disk. Property value is irrelevant; only property's existence
> @@ -556,7 +591,6 @@ const void * svn_auth_get_parameter(svn_
> * ~/.subversion. */
> #define SVN_AUTH_PARAM_CONFIG_DIR SVN_AUTH_PARAM_PREFIX "config-dir"
>
> -
> /** Get an initial set of credentials.
> *
> * Ask @a auth_baton to set @a *credentials to a set of credentials
> @@ -647,8 +681,15 @@ void svn_auth_get_username_prompt_provid
>
> /** Create and return @a *provider, an authentication provider of type @c
> * svn_auth_cred_simple_t that gets/sets information from the user's
> - * ~/.subversion configuration directory. Allocate @a *provider in
> - * @a pool.
> + * ~/.subversion configuration directory.
> + *
> + * If the provider is going to save the password unencrypted,
> + * it calls @a plaintext_prompt_func before saving the
> + * password.
> + *
> + * @a plaintext_prompt_baton is passed to @a plaintext_prompt_func.
> + *
> + * Allocate @a *provider in @a pool.
> *
> * If a default username or password is available, @a *provider will
> * honor them as well, and return them when
> @@ -656,6 +697,18 @@ void svn_auth_get_username_prompt_provid
> * SVN_AUTH_PARAM_DEFAULT_USERNAME and @c
> * SVN_AUTH_PARAM_DEFAULT_PASSWORD).
> *
> + * @since New in 1.6.
> + */
> +void svn_auth_get_simple_provider2
> + (svn_auth_provider_object_t **provider,
> + svn_auth_plaintext_prompt_func_t plaintext_prompt_func,
> + void* plaintext_prompt_baton,
> + apr_pool_t *pool);
> +
> +/** Like svn_auth_get_simple_provider2, but without the ability to
> + * call the svn_auth_plaintext_prompt_func_t callback.
> + *
> + * @deprecated Provided for backwards compatibility with the 1.5 API.
> * @since New in 1.4.
> */
> void svn_auth_get_simple_provider(svn_auth_provider_object_t **provider,
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_cmdline.h
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_cmdline.h?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_cmdline.h Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_cmdline.h Sat Apr 19 05:00:42 2008 (r30701)
> @@ -253,37 +253,28 @@ svn_cmdline_auth_ssl_client_cert_pw_prom
> svn_boolean_t may_save,
> apr_pool_t *pool);
>
> -/** Initialize auth baton @a ab with the standard set of authentication
> - * providers used by the command line client. @a non_interactive,
> - * @a username, @a password, @a config_dir, and @a no_auth_cache are the
> - * values of the command line options of the same names.
> - * @a store_plaintext_passwords indicates whether storing passwords to
> - * disk in plaintext is allowed.
> - * @a cfg is the @c SVN_CONFIG_CATEGORY_CONFIG configuration,
> - * and @a cancel_func and @a cancel_baton control the cancellation of
> - * the prompting providers that are initialized.
> - * @a pool is used for all allocations.
> +/** An implementation of @c svn_auth_plaintext_prompt_func_t that
> + * prompts the user whether storing unencypted passwords to disk
> + * is OK on the command line.
> *
> * @since New in 1.6.
> + *
> + * Does not use @a baton.
> */
> svn_error_t *
> -svn_cmdline_setup_auth_baton2(svn_auth_baton_t **ab,
> - svn_boolean_t non_interactive,
> - const char *username,
> - const char *password,
> - const char *config_dir,
> - svn_boolean_t no_auth_cache,
> - svn_boolean_t store_plaintext_passwords,
> - svn_config_t *cfg,
> - svn_cancel_func_t cancel_func,
> - void *cancel_baton,
> - apr_pool_t *pool);
> +svn_cmdline_auth_plaintext_prompt(svn_boolean_t *may_save_plaintext,
> + void *baton,
> + apr_pool_t *pool);
>
> -/**
> - * Like svn_cmdline_setup_auth_baton2, but with store_plaintext_passwords
> - * set to TRUE.
> +/** Initialize auth baton @a ab with the standard set of authentication
> + * providers used by the command line client. @a non_interactive,
> + * @a username, @a password, @a config_dir, and @a no_auth_cache are the
> + * values of the command line options of the same names. @a cfg is the
> + * @c SVN_CONFIG_CATEGORY_CONFIG configuration, and @a cancel_func and
> + * @a cancel_baton control the cancellation of the prompting providers
> + * that are initialized. @a pool is used for all allocations.
> *
> - * @deprecated Provided for backward compatibility with the 1.5 API.
> + * @since New in 1.4.
> */
> svn_error_t *
> svn_cmdline_setup_auth_baton(svn_auth_baton_t **ab,
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_config.h
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_config.h?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_config.h Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_config.h Sat Apr 19 05:00:42 2008 (r30701)
> @@ -214,6 +214,19 @@ void svn_config_set(svn_config_t *cfg,
> * Parses the option as a boolean value. The recognized representations
> * are 'TRUE'/'FALSE', 'yes'/'no', 'on'/'off', '1'/'0'; case does not
> * matter. Returns an error if the option doesn't contain a known string.
> + *
> + * @a *default_value_was_used is set to TRUE if the option was not found
> + * in the configuration file.
> + *
> + * @since New in 1.6
> + */
> +svn_error_t *svn_config_get_bool2(svn_config_t *cfg, svn_boolean_t *valuep,
> + const char *section, const char *option,
> + svn_boolean_t default_value,
> + svn_boolean_t *default_value_was_used);
> +
> +/** Like svn_config_get_bool2(), but without the ability to determine
> + * whether a parameter was unspecified.
> */
> svn_error_t *svn_config_get_bool(svn_config_t *cfg, svn_boolean_t *valuep,
> const char *section, const char *option,
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c Sat Apr 19 05:00:42 2008 (r30701)
> @@ -353,22 +353,22 @@ svn_cmdline_handle_exit_error(svn_error_
> }
>
> svn_error_t *
> -svn_cmdline_setup_auth_baton2(svn_auth_baton_t **ab,
> - svn_boolean_t non_interactive,
> - const char *auth_username,
> - const char *auth_password,
> - const char *config_dir,
> - svn_boolean_t no_auth_cache,
> - svn_boolean_t store_plaintext_passwords,
> - svn_config_t *cfg,
> - svn_cancel_func_t cancel_func,
> - void *cancel_baton,
> - apr_pool_t *pool)
> +svn_cmdline_setup_auth_baton(svn_auth_baton_t **ab,
> + svn_boolean_t non_interactive,
> + const char *auth_username,
> + const char *auth_password,
> + const char *config_dir,
> + svn_boolean_t no_auth_cache,
> + svn_config_t *cfg,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *pool)
> {
> svn_boolean_t store_password_val = TRUE;
> svn_boolean_t store_plaintext_password_val = FALSE;
> svn_boolean_t store_auth_creds_val = TRUE;
> svn_auth_provider_object_t *provider;
> + svn_boolean_t default_value_was_used = FALSE;
>
> /* The whole list of registered providers */
> apr_array_header_t *providers
> @@ -384,7 +384,9 @@ svn_cmdline_setup_auth_baton2(svn_auth_b
> svn_auth_get_keychain_simple_provider(&provider, pool);
> APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
> #endif
> - svn_auth_get_simple_provider(&provider, pool);
> + svn_auth_get_simple_provider2(&provider,
> + svn_cmdline_auth_plaintext_prompt,
> + NULL, pool);
> APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
> svn_auth_get_username_provider(&provider, pool);
> APR_ARRAY_PUSH(providers, svn_auth_provider_object_t *) = provider;
> @@ -461,6 +463,7 @@ svn_cmdline_setup_auth_baton2(svn_auth_b
> svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_CONFIG_DIR,
> config_dir);
>
> + /* Determine whether storing passwords in any form is allowed. */
> SVN_ERR(svn_config_get_bool(cfg, &store_password_val,
> SVN_CONFIG_SECTION_AUTH,
> SVN_CONFIG_OPTION_STORE_PASSWORDS,
> @@ -469,17 +472,26 @@ svn_cmdline_setup_auth_baton2(svn_auth_b
> if (! store_password_val)
> svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_DONT_STORE_PASSWORDS, "");
>
> - SVN_ERR(svn_config_get_bool(cfg, &store_plaintext_password_val,
> - SVN_CONFIG_SECTION_AUTH,
> - SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS,
> - FALSE));
> + /* Determine whether storing passwords in plaintext has been
> + * explicitly allowed or denied. */
> + SVN_ERR(svn_config_get_bool2(cfg, &store_plaintext_password_val,
> + SVN_CONFIG_SECTION_AUTH,
> + SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS,
> + FALSE, /* <-- arbitrary */
> + &default_value_was_used));
>
> - if (store_plaintext_passwords || store_plaintext_password_val)
> - svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS, "");
> + if (! default_value_was_used)
> + {
> + if (store_plaintext_password_val)
> + svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS,
> + "");
> + else
> + svn_auth_set_parameter(*ab,
> + SVN_AUTH_PARAM_DONT_STORE_PLAINTEXT_PASSWORDS,
> + "");
> + }
>
> - /* There are two different ways the user can disable disk caching
> - of credentials: either via --no-auth-cache, or in the config
> - file ('store-auth-creds = no'). */
> + /* Determine whether we are allowed to write to the auth/ area. */
> SVN_ERR(svn_config_get_bool(cfg, &store_auth_creds_val,
> SVN_CONFIG_SECTION_AUTH,
> SVN_CONFIG_OPTION_STORE_AUTH_CREDS,
> @@ -492,24 +504,6 @@ svn_cmdline_setup_auth_baton2(svn_auth_b
> }
>
> svn_error_t *
> -svn_cmdline_setup_auth_baton(svn_auth_baton_t **ab,
> - svn_boolean_t non_interactive,
> - const char *username,
> - const char *password,
> - const char *config_dir,
> - svn_boolean_t no_auth_cache,
> - svn_config_t *cfg,
> - svn_cancel_func_t cancel_func,
> - void *cancel_baton,
> - apr_pool_t *pool)
> -{
> - return svn_cmdline_setup_auth_baton2(ab, non_interactive, username,
> - password, config_dir, no_auth_cache,
> - TRUE, cfg, cancel_func, cancel_baton,
> - pool);
> -}
> -
> -svn_error_t *
> svn_cmdline__getopt_init(apr_getopt_t **os,
> int argc,
> const char *argv[],
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config.c
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config.c?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config.c Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config.c Sat Apr 19 05:00:42 2008 (r30701)
> @@ -609,15 +609,19 @@ svn_config_set(svn_config_t *cfg,
>
>
> svn_error_t *
> -svn_config_get_bool(svn_config_t *cfg, svn_boolean_t *valuep,
> - const char *section, const char *option,
> - svn_boolean_t default_value)
> +svn_config_get_bool2(svn_config_t *cfg, svn_boolean_t *valuep,
> + const char *section, const char *option,
> + svn_boolean_t default_value,
> + svn_boolean_t *default_value_was_used)
> {
> const char *tmp_value;
>
> svn_config_get(cfg, &tmp_value, section, option, NULL);
> if (tmp_value == NULL)
> - *valuep = default_value;
> + {
> + *valuep = default_value;
> + *default_value_was_used = TRUE;
> + }
> else if (0 == svn_cstring_casecmp(tmp_value, SVN_CONFIG_TRUE)
> || 0 == svn_cstring_casecmp(tmp_value, "yes")
> || 0 == svn_cstring_casecmp(tmp_value, "on")
> @@ -636,6 +640,15 @@ svn_config_get_bool(svn_config_t *cfg, s
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_config_get_bool(svn_config_t *cfg, svn_boolean_t *valuep,
> + const char *section, const char *option,
> + svn_boolean_t default_value)
> +{
> + svn_boolean_t dummy;
> + return svn_config_get_bool2(cfg, valuep, section, option,
> + default_value, &dummy);
> +}
>
>
> void
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/prompt.c
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/prompt.c?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/prompt.c Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/prompt.c Sat Apr 19 05:00:42 2008 (r30701)
> @@ -376,6 +376,48 @@ svn_cmdline_auth_ssl_client_cert_pw_prom
> return SVN_NO_ERROR;
> }
>
> +/* This implements 'svn_auth_plaintext_prompt_func_t'. */
> +svn_error_t *
> +svn_cmdline_auth_plaintext_prompt(svn_boolean_t *may_save_plaintext,
> + void *baton,
> + apr_pool_t *pool)
> +{
> + const char *answer = NULL;
> + svn_boolean_t answered = FALSE;
> + const char *prompt_string = _("Store password unencrypted (yes/no)? ");
> +
> + SVN_ERR(svn_cmdline_printf(pool, "\n"));
> + SVN_ERR(svn_cmdline_printf(pool, "-------------------------------------"
> + "----------------------------------\n"));
> + SVN_ERR(svn_cmdline_printf(pool, _("ATTENTION! Your password is going to "
> + "be stored to disk unencrypted!")));
> + SVN_ERR(svn_cmdline_printf(pool, "\n"));
> + SVN_ERR(svn_cmdline_printf(pool, "-------------------------------------"
> + "----------------------------------\n"));
> + SVN_ERR(svn_cmdline_printf(pool, _("You can get rid of this warning by "
> + "editing your configuration file.")));
> + SVN_ERR(svn_cmdline_printf(pool, "\n"));
> +
> + do
> + {
> + SVN_ERR(prompt(&answer, prompt_string, FALSE, NULL, pool));
> + if (strcmp(answer, _("yes")) == 0)
> + {
> + *may_save_plaintext = TRUE;
> + answered = TRUE;
> + }
> + else if (strcmp(answer, _("no")) == 0)
> + {
> + *may_save_plaintext = FALSE;
> + answered = TRUE;
> + }
> + else
> + prompt_string = _("Please type 'yes' or 'no': ");
> + }
> + while (! answered);
> +
> + return SVN_NO_ERROR;
> +}
>
>
> /** Generic prompting. **/
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/simple_providers.c
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/simple_providers.c?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/simple_providers.c Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/simple_providers.c Sat Apr 19 05:00:42 2008 (r30701)
> @@ -44,6 +44,14 @@
> #define SVN_AUTH__WINCRYPT_PASSWORD_TYPE "wincrypt"
> #define SVN_AUTH__KEYCHAIN_PASSWORD_TYPE "keychain"
>
> +/* Baton type for the simple provider. */
> +typedef struct
> +{
> + svn_auth_plaintext_prompt_func_t plaintext_prompt_func;
> + void *plaintext_prompt_baton;
> +
> +} simple_provider_baton_t;
> +
>
> /* A function that stores PASSWORD (or some encrypted version thereof)
> either directly in CREDS, or externally using REALMSTRING and USERNAME
> @@ -242,11 +250,17 @@ simple_save_creds_helper(svn_boolean_t *
> apr_hash_get(parameters,
> SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS,
> APR_HASH_KEY_STRING);
> - svn_boolean_t non_interactive = apr_hash_get(parameters,
> + const char *dont_store_plaintext_passwords =
> + apr_hash_get(parameters,
> + SVN_AUTH_PARAM_DONT_STORE_PLAINTEXT_PASSWORDS,
> + APR_HASH_KEY_STRING);
> + svn_boolean_t non_interactive = apr_hash_get(parameters,
> SVN_AUTH_PARAM_NON_INTERACTIVE,
> APR_HASH_KEY_STRING) != NULL;
> + simple_provider_baton_t *b = (simple_provider_baton_t*)provider_baton;
> *saved = FALSE;
>
> + /* This is --no-auth-cache */
> if (! creds->may_save)
> return SVN_NO_ERROR;
>
> @@ -261,24 +275,57 @@ simple_save_creds_helper(svn_boolean_t *
> svn_string_create(creds->username, pool));
>
> /* Don't store passwords in any form if the user has told
> - * us not to do so. If we are allowed to store passwords,
> - * default to only storing encrypted passwords, unless the user
> - * has explicitly allowed us to store plain-text passwords. */
> - if (! dont_store_passwords
> - && (strcmp(passtype, SVN_AUTH__WINCRYPT_PASSWORD_TYPE) == 0
> - || strcmp(passtype, SVN_AUTH__KEYCHAIN_PASSWORD_TYPE) == 0
> - || store_plaintext_passwords))
> - {
> - svn_boolean_t password_stored;
> - password_stored = password_set(creds_hash, realmstring, creds->username,
> - creds->password, non_interactive, pool);
> - if (password_stored && passtype)
> + * us not to do so. */
> + if (! dont_store_passwords)
> + {
> + svn_boolean_t may_save_password = FALSE;
> +
> + /* If the password is going to be stored encrypted, go right
> + * ahead and store it to disk. Else determine whether saving
> + * in plaintext is OK. */
> + if (strcmp(passtype, SVN_AUTH__WINCRYPT_PASSWORD_TYPE) == 0
> + || strcmp(passtype, SVN_AUTH__KEYCHAIN_PASSWORD_TYPE) == 0)
> + {
> + may_save_password = TRUE;
> + }
> + else
> + {
> + if (! dont_store_plaintext_passwords && ! store_plaintext_passwords)
> + {
> + /* We have no information from the configuration file whether
> + * saving unencrypted is allowed. Prompt the user. */
> + svn_boolean_t may_save_plaintext;
> + /* TODO: At some point, we might want to default to not
> + * storing if the callback is NULL. */
> + if (b->plaintext_prompt_func)
> + SVN_ERR((*b->plaintext_prompt_func)(&may_save_plaintext,
> + b->plaintext_prompt_baton,
> + pool));
> + may_save_password = may_save_plaintext;
> + }
> + else
> + {
> + /* dont_store_plaintext_passwords overrides
> + * store_plaintext_passwords */
> + may_save_password = (dont_store_plaintext_passwords
> + ? FALSE : TRUE);
> + }
> + }
> +
> + if (may_save_password)
> {
> - /* Store the password type with the auth data, so that we
> - know which provider owns the password. */
> - apr_hash_set(creds_hash, SVN_AUTH__AUTHFILE_PASSTYPE_KEY,
> - APR_HASH_KEY_STRING,
> - svn_string_create(passtype, pool));
> + svn_boolean_t password_stored;
> + password_stored = password_set(creds_hash, realmstring,
> + creds->username, creds->password,
> + non_interactive, pool);
> + if (password_stored && passtype)
> + {
> + /* Store the password type with the auth data, so that we
> + know which provider owns the password. */
> + apr_hash_set(creds_hash, SVN_AUTH__AUTHFILE_PASSTYPE_KEY,
> + APR_HASH_KEY_STRING,
> + svn_string_create(passtype, pool));
> + }
> }
> }
>
> @@ -331,18 +378,32 @@ static const svn_auth_provider_t simple_
> simple_save_creds
> };
>
> -
> /* Public API */
> void
> -svn_auth_get_simple_provider(svn_auth_provider_object_t **provider,
> - apr_pool_t *pool)
> +svn_auth_get_simple_provider2
> + (svn_auth_provider_object_t **provider,
> + svn_auth_plaintext_prompt_func_t plaintext_prompt_func,
> + void* plaintext_prompt_baton,
> + apr_pool_t *pool)
> {
> svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
> + simple_provider_baton_t *pb = apr_pcalloc(pool, sizeof(*pb));
> +
> + pb->plaintext_prompt_func = plaintext_prompt_func;
> + pb->plaintext_prompt_baton = plaintext_prompt_baton;
>
> po->vtable = &simple_provider;
> + po->provider_baton = pb;
> *provider = po;
> }
>
> +void
> +svn_auth_get_simple_provider(svn_auth_provider_object_t **provider,
> + apr_pool_t *pool)
> +{
> + svn_auth_get_simple_provider2(provider, NULL, NULL, pool);
> +}
> +
>
> /*-----------------------------------------------------------------------*/
> /* Prompt provider */
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c?pathrev=30701&r1=30700&r2=30701
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c Fri Apr 18 19:59:25 2008 (r30700)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c Sat Apr 19 05:00:42 2008 (r30701)
> @@ -101,8 +101,7 @@ typedef enum {
> opt_parents,
> opt_accept,
> opt_show_revs,
> - opt_reintegrate,
> - opt_store_plaintext_passwords
> + opt_reintegrate
> } svn_cl__longopt_t;
>
> /* Option codes and descriptions for the command line client.
> @@ -279,11 +278,6 @@ const apr_getopt_option_t svn_cl__option
> {"reintegrate", opt_reintegrate, 0,
> N_("lump-merge all of source URL's unmerged changes")},
>
> - {"store-plaintext-pw", opt_store_plaintext_passwords, 0,
> - N_("cache passwords on disk even if they are\n"
> - " "
> - "going to be stored in plain text")},
> -
> /* Long-opt Aliases
> *
> * These have NULL desriptions, but an option code that matches some
> @@ -315,7 +309,7 @@ const apr_getopt_option_t svn_cl__option
> willy-nilly to every invocation of 'svn') . */
> const int svn_cl__global_options[] =
> { opt_auth_username, opt_auth_password, opt_no_auth_cache, opt_non_interactive,
> - opt_config_dir, opt_store_plaintext_passwords, 0
> + opt_config_dir, 0
> };
>
> /* Options for giving a log message. (Some of these also have other uses.)
> @@ -1505,9 +1499,6 @@ main(int argc, const char *argv[])
> case opt_reintegrate:
> opt_state.reintegrate = TRUE;
> break;
> - case opt_store_plaintext_passwords:
> - opt_state.store_plaintext_passwords = TRUE;
> - break;
> default:
> /* Hmmm. Perhaps this would be a good place to squirrel away
> opts that commands like svn diff might need. Hmmm indeed. */
> @@ -1924,17 +1915,16 @@ main(int argc, const char *argv[])
> #endif
>
> /* Set up Authentication stuff. */
> - if ((err = svn_cmdline_setup_auth_baton2(&ab,
> - opt_state.non_interactive,
> - opt_state.auth_username,
> - opt_state.auth_password,
> - opt_state.config_dir,
> - opt_state.no_auth_cache,
> - opt_state.store_plaintext_passwords,
> - cfg,
> - ctx->cancel_func,
> - ctx->cancel_baton,
> - pool)))
> + if ((err = svn_cmdline_setup_auth_baton(&ab,
> + opt_state.non_interactive,
> + opt_state.auth_username,
> + opt_state.auth_password,
> + opt_state.config_dir,
> + opt_state.no_auth_cache,
> + cfg,
> + ctx->cancel_func,
> + ctx->cancel_baton,
> + pool)))
> svn_handle_error2(err, stderr, TRUE, "svn: ");
>
> ctx->auth_baton = ab;
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
>
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30701 - in
branches/dont-save-plaintext-passwords-by-default/subversion:
include libsvn_subr svn
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Apr 21, 2008 at 02:30:54AM -0400, Karl Fogel wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
> > It might be cleaner to, instead of making the option be
> > boolean-with-special-meaning-for-undefined, have it just be a
> > three-value option: yes, no, prompt (with prompt as the default, and
> > accepting all the synonyms of yes/no like svn_config_get_bool). This
> > way you could also (if it's in .subversion/servers) make the default
> > be "yes" but have it be "prompt" for specific servers.
>
> I believe I've usually seen "ask" used for this, instead of "prompt".
>
> ("Prompt" is a term of art, and non-native English speakers are less
> likely to be familiar with it than they are with "ask".)
Indeed. Done in r30727.
Thanks!
--
Stefan Sperling <st...@elego.de> Software Monkey
German law requires the following banner :(
elego Software Solutions GmbH HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12 Tel: +49 30 23 45 86 96
13355 Berlin Fax: +49 30 23 45 86 95
http://www.elego.de CEO: Olaf Wagner
Store password unencrypted (yes/no)? No
Re: svn commit: r30701 - in branches/dont-save-plaintext-passwords-by-default/subversion: include libsvn_subr svn
Posted by Karl Fogel <kf...@red-bean.com>.
"David Glasser" <gl...@davidglasser.net> writes:
> It might be cleaner to, instead of making the option be
> boolean-with-special-meaning-for-undefined, have it just be a
> three-value option: yes, no, prompt (with prompt as the default, and
> accepting all the synonyms of yes/no like svn_config_get_bool). This
> way you could also (if it's in .subversion/servers) make the default
> be "yes" but have it be "prompt" for specific servers.
I believe I've usually seen "ask" used for this, instead of "prompt".
("Prompt" is a term of art, and non-native English speakers are less
likely to be familiar with it than they are with "ask".)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30701 - in
branches/dont-save-plaintext-passwords-by-default/subversion:
include libsvn_subr svn
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Apr 19, 2008 at 10:58:13AM -0700, David Glasser wrote:
> It might be cleaner to, instead of making the option be
> boolean-with-special-meaning-for-undefined, have it just be a
> three-value option: yes, no, prompt (with prompt as the default, and
> accepting all the synonyms of yes/no like svn_config_get_bool).
Yes, indeed. I'll do that.
> This way you could also (if it's in .subversion/servers) make the default
> be "yes" but have it be "prompt" for specific servers.
Good idea. My idea for some reason was to have the global option
override the server-specific ones, but that is actually backwards,
as you just showed.
Thanks!
--
Stefan Sperling <st...@elego.de> Software Developer
elego Software Solutions GmbH HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12 Tel: +49 30 23 45 86 96
13355 Berlin Fax: +49 30 23 45 86 95
http://www.elego.de Geschaeftsfuehrer: Olaf Wagner