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/20 23:42:56 UTC
Re: svn commit: r30723 - in branches/dont-save-plaintext-passwords-by-default: . subversion/include subversion/libsvn_subr subversion/svn
Hmm, why not just have the "global" servers section be the normal
place to configure this? Why use the other config file at all?
--dave
On Sun, Apr 20, 2008 at 4:20 PM, <st...@tigris.org> wrote:
> Author: stsp
> Date: Sun Apr 20 16:20:34 2008
> New Revision: 30723
>
> Log:
> On the dont-save-plaintext-passwords-by-default branch,
> implement support for specifying the 'store-plaintext-passwords'
> option on a per-server basis.
>
> * subversion/include/svn_path.h
> (svn_path_url_get_hostname_part): Declare.
>
> * subversion/include/svn_config.h: Add comment about
> SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS to servers
> category options section.
>
> * subversion/include/svn_cmdline.h
> (svn_cmdline_setup_auth_baton2): Declare. (Yes, it's back in a new guise.)
> (svn_cmdline_setup_auth_baton): Deprecate.
>
> * subversion/libsvn_subr/config_file.c
> (svn_config_ensure): Document 'store-plaintext-passwords' option
> in default servers file. Hint at the per-server config option in
> the default config file.
>
> * subversion/libsvn_subr/cmdline.c
> (find_store_plaintext_passwords_option): New function.
> (svn_cmdline_setup_auth_baton2): Expands svn_cmdline_setup_auth_baton.
> Now accepts multiple config categories in a hash table, instead of a
> single category, so it can look at the settings set in both the
> 'config' and 'servers' files. Of course, check both these files
> to determine whether storing plaintext passwords is allowed.
> Also takes a list of URLs needed to match server groups defined
> in the 'servers' file. The 'store-plaintext-passwords' option in the
> 'servers' file overrides the on in the 'config' file, but the [global]
> section of the 'servers' file is ignored -- we don't want contradicting
> global settings for this option.
> (svn_cmdline_setup_auth_baton): Call svn_cmdline_setup_auth_baton2.
>
> * subversion/libsvn_subr/path.c
> (svn_path_url_get_hostname_part): New function.
>
> * subversion/svn/main.c
> (main): Gather URLs supplied on the command line for use by
> svn_cmdline_setup_auth_baton2, and call this function instead
> of its older brother.
>
> * TODO.branch: Tick off TODO item.
>
> Modified:
> branches/dont-save-plaintext-passwords-by-default/TODO.branch
> 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/include/svn_path.h
> branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c
> branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config_file.c
> branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/path.c
> branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c
>
> Modified: branches/dont-save-plaintext-passwords-by-default/TODO.branch
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/TODO.branch?pathrev=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/TODO.branch Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/TODO.branch Sun Apr 20 16:20:34 2008 (r30723)
> @@ -2,12 +2,6 @@ TODO items on this branch:
>
> - properly handle ^C during the prompt
>
> -- Add the 'store-plaintext-passwords' option to ~/.subversion/servers,
> - in addition to the global one in ~/.subversion/config.
> - The use case being that some people may want to save plaintext
> - passwords for some servers, but not for others.
> - (Is this also possible on a per-repository basis?)
> -
> - During the prompt, tell users where exactly the configuration file is,
> if possible.
>
>
> 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=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_cmdline.h Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_cmdline.h Sun Apr 20 16:20:34 2008 (r30723)
> @@ -269,12 +269,40 @@ svn_cmdline_auth_plaintext_prompt(svn_bo
> /** 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.
> + * values of the command line options of the same names. @a cfg_hash
> + * is a hash table as returned by svn_config_get_config, @a urls is a
> + * list of any URLs that were supplied as command line arguments (used to
> + * filter settings from SVN_CONFIG_CATEGORY_SERVERS), 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.
> + *
> + * @since New in 1.6.
> + */
> +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,
> + apr_hash_t *cfg_hash,
> + apr_array_header_t *urls,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *pool);
> +
> +/**
> + * Like svn_cmdline_setup_auth_baton2, but with a @a cfg parameter
> + * of type svn_config_t* instead of the cfg_hash parameter, and
> + * no list of urls either.
> + *
> + * Users of this function make it impossible to specify the
> + * 'store-plaintext-passwords' option on a per-server basis.
> *
> * @since New in 1.4.
> + * @deprecated Provided for backward compatibility with the 1.5 API.
> */
> 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=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_config.h Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_config.h Sun Apr 20 16:20:34 2008 (r30723)
> @@ -74,6 +74,8 @@ typedef struct svn_config_t svn_config_t
> #define SVN_CONFIG_OPTION_SSL_CLIENT_CERT_PASSWORD "ssl-client-cert-password"
> #define SVN_CONFIG_OPTION_SSL_PKCS11_PROVIDER "ssl-pkcs11-provider"
> #define SVN_CONFIG_OPTION_HTTP_LIBRARY "http-library"
> +/* SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS is also valid for servers
> + * (but not in SECTION_GLOBAL). It is defined below */
>
> #define SVN_CONFIG_CATEGORY_CONFIG "config"
> #define SVN_CONFIG_SECTION_AUTH "auth"
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_path.h
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_path.h?pathrev=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_path.h Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/include/svn_path.h Sun Apr 20 16:20:34 2008 (r30723)
> @@ -481,6 +481,10 @@ const char *svn_path_url_add_component(c
> const char *component,
> apr_pool_t *pool);
>
> +/** Return the hostname part of @a url, allocated in @a pool.
> + * May return NULL on invalid input. */
> +const char *svn_path_url_get_hostname_part(const char *url, apr_pool_t *pool);
> +
> /**
> * Convert @a iri (Internationalized URI) to an URI.
> * The return value may be the same as @a iri if it was already
>
> 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=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/cmdline.c Sun Apr 20 16:20:34 2008 (r30723)
> @@ -42,6 +42,7 @@
> #include "svn_nls.h"
> #include "svn_auth.h"
> #include "utf_impl.h"
> +#include "svn_config.h"
>
> #include "svn_private_config.h"
>
> @@ -352,22 +353,48 @@ svn_cmdline_handle_exit_error(svn_error_
> return EXIT_FAILURE;
> }
>
> +/** An implementation of svn_config_enumerator2_t. */
> +static svn_boolean_t
> +find_store_plaintext_passwords_option(const char *name,
> + const char *value,
> + void *baton,
> + apr_pool_t *pool)
> +{
> + svn_boolean_t *found = (svn_boolean_t *)baton;
> +
> + if (! *found)
> + *found = strcmp(name, SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS) == 0;
> +
> + return TRUE;
> +}
> +
> svn_error_t *
> -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_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,
> + apr_hash_t *cfg_hash,
> + apr_array_header_t *urls,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *pool)
> {
> svn_boolean_t store_password_val = TRUE;
> const char *store_plaintext_password_val;
> svn_boolean_t store_auth_creds_val = TRUE;
> svn_auth_provider_object_t *provider;
> + int i;
> + apr_pool_t *subpool;
> +
> + /* The 'config' and 'servers' config files, respectively. */
> + svn_config_t *cfg_config = apr_hash_get(cfg_hash,
> + SVN_CONFIG_CATEGORY_CONFIG,
> + APR_HASH_KEY_STRING);
> + svn_config_t *cfg_servers = apr_hash_get(cfg_hash,
> + SVN_CONFIG_CATEGORY_SERVERS,
> + APR_HASH_KEY_STRING);
>
> /* The whole list of registered providers */
> apr_array_header_t *providers
> @@ -463,7 +490,7 @@ svn_cmdline_setup_auth_baton(svn_auth_ba
> config_dir);
>
> /* Determine whether storing passwords in any form is allowed. */
> - SVN_ERR(svn_config_get_bool(cfg, &store_password_val,
> + SVN_ERR(svn_config_get_bool(cfg_config, &store_password_val,
> SVN_CONFIG_SECTION_AUTH,
> SVN_CONFIG_OPTION_STORE_PASSWORDS,
> TRUE));
> @@ -471,18 +498,76 @@ svn_cmdline_setup_auth_baton(svn_auth_ba
> if (! store_password_val)
> svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_DONT_STORE_PASSWORDS, "");
>
> - /* Determine whether storing passwords in plaintext has been
> + /**
> + * Determine whether storing passwords in plaintext has been
> * explicitly allowed or denied, or whether we should prompt
> - * the user about it. */
> - SVN_ERR(svn_config_get_yes_no_prompt
> - (cfg, &store_plaintext_password_val, SVN_CONFIG_SECTION_AUTH,
> - SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS, SVN_CONFIG_PROMPT));
> + * the user about it. Check the setting from the 'servers' file
> + * first (ignoring its [global] section), and if not provided
> + * there, try the 'config' file.
> + */
> +
> + store_plaintext_password_val = NULL;
> + if (urls)
> + {
> + /* Check server groups in turn, first match wins. */
> + subpool = svn_pool_create(pool);
> + for (i = 0; i < urls->nelts; i++)
> + {
> + const char *server_group;
> + const char *url;
> + const char *hostname;
> +
> + svn_pool_clear(subpool);
> +
> + url = APR_ARRAY_IDX(urls, i, const char *);
> +
> + hostname = svn_path_url_get_hostname_part(url, subpool);
> + if (hostname == NULL)
> + continue;
> +
> + server_group = svn_config_find_group(cfg_servers, hostname,
> + SVN_CONFIG_SECTION_GROUPS,
> + subpool);
> + if (server_group)
> + {
> + /* Check if the group has 'store-plaintext-passwords'
> + * defined. We don't care about the actual value just yet. */
> + svn_boolean_t found = FALSE;
> + int n;
> +
> + n = svn_config_enumerate2(cfg_servers, server_group,
> + find_store_plaintext_passwords_option,
> + &found, subpool);
> + if (found && n > 0)
> + {
> + /* It's defined, grab the value. */
> + SVN_ERR(svn_config_get_yes_no_prompt
> + (cfg_servers,
> + &store_plaintext_password_val,
> + server_group,
> + SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS,
> + SVN_CONFIG_PROMPT));
> + break;
> + }
> + else
> + /* No luck with this group, try next one. */
> + continue;
> + }
> + }
> + svn_pool_destroy(subpool);
> + }
> +
> + if (! store_plaintext_password_val)
> + /* No luck in 'servers' file, try 'config' file. */
> + SVN_ERR(svn_config_get_yes_no_prompt
> + (cfg_config, &store_plaintext_password_val, SVN_CONFIG_SECTION_AUTH,
> + SVN_CONFIG_OPTION_STORE_PLAINTEXT_PASSWORDS, SVN_CONFIG_PROMPT));
>
> svn_auth_set_parameter(*ab, SVN_AUTH_PARAM_STORE_PLAINTEXT_PASSWORDS,
> store_plaintext_password_val);
>
> /* Determine whether we are allowed to write to the auth/ area. */
> - SVN_ERR(svn_config_get_bool(cfg, &store_auth_creds_val,
> + SVN_ERR(svn_config_get_bool(cfg_config, &store_auth_creds_val,
> SVN_CONFIG_SECTION_AUTH,
> SVN_CONFIG_OPTION_STORE_AUTH_CREDS,
> TRUE));
> @@ -494,6 +579,33 @@ svn_cmdline_setup_auth_baton(svn_auth_ba
> }
>
> svn_error_t *
> +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)
> +{
> + apr_hash_t *cfg_hash;
> +
> + /* Argh, this makes us read the config again, even if the
> + * client has already done so. But there's no other way
> + * to be backward compatible. */
> + SVN_ERR(svn_config_get_config(&cfg_hash, config_dir, pool));
> +
> + SVN_ERR(svn_cmdline_setup_auth_baton2(ab, non_interactive,
> + auth_username, auth_password,
> + config_dir, no_auth_cache,
> + cfg_hash, NULL, cancel_func,
> + cancel_baton, pool));
> + return SVN_NO_ERROR;
> +}
> +
> +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_file.c
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config_file.c?pathrev=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config_file.c Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/config_file.c Sun Apr 20 16:20:34 2008 (r30723)
> @@ -766,7 +766,7 @@ svn_config_ensure(const char *config_dir
> {
> apr_file_t *f;
> const char *contents =
> - "### This file specifies server-specific protocol parameters," NL
> + "### This file specifies server-specific parameters," NL
> "### including HTTP proxy information, and HTTP timeout settings." NL
> "###" NL
> "### The currently defined server options are:" NL
> @@ -793,6 +793,17 @@ svn_config_ensure(const char *config_dir
> "### http-library Which library to use for http/https"
> NL
> "### connections (neon or serf)" NL
> + "### store-plaintext-passwords Specifies whether passwords used" NL
> + "### to authenticate against a" NL
> + "### Subversion server may be cached" NL
> + "### on disk unencrypted." NL
> + "###" NL
> + "### store-plaintext-passwords may be either 'yes', 'no', or 'prompt'." NL
> + "### It defaults to 'prompt', which means that Subversion will ask" NL
> + "### you before saving a password to disk in unencrypted form." NL
> + "### This option can be set globally in the 'config' file, and will" NL
> + "### be ignored if it occurs in the [global] section of this file." NL
> + "### The global setting is overridden by settings in this file." NL
> "###" NL
> "### HTTP timeouts, if given, are specified in seconds. A timeout" NL
> "### of 0, i.e. zero, causes a builtin default to be used." NL
> @@ -822,12 +833,13 @@ svn_config_ensure(const char *config_dir
> "# http-auth-types = basic;digest;negotiate" NL
> #endif
> "# neon-debug-mask = 130" NL
> + "# store-plaintext-passwords = no" NL
> "" NL
> "### Information for the second group:" NL
> "# [othergroup]" NL
> "# http-proxy-host = proxy2.some-domain-name.com" NL
> "# http-proxy-port = 9000" NL
> - "# No username and password, so use the defaults below." NL
> + "# No username and password for the proxy, so use the defaults below." NL
> "" NL
> "### You can set default parameters in the 'global' section." NL
> "### These parameters apply if no corresponding parameter is set in" NL
> @@ -914,6 +926,9 @@ svn_config_ensure(const char *config_dir
> "### 'prompt', which means that Subversion will ask you before" NL
> "### saving a password to disk in unencrypted form. Note that" NL
> "### this option has no effect if 'store-passwords' is set to 'no'." NL
> + "### This option can also be set on a per-server basis in the" NL
> + "### 'servers' configuration file in your config directory." NL
> + "### Values for specific servers override the value specified here." NL
> "# store-plaintext-passwords = no" NL
> "### Set store-auth-creds to 'no' to avoid storing any subversion" NL
> "### credentials in the auth/ area of your config directory." NL
>
> Modified: branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/path.c
> URL: http://svn.collab.net/viewvc/svn/branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/path.c?pathrev=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/path.c Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/libsvn_subr/path.c Sun Apr 20 16:20:34 2008 (r30723)
> @@ -1140,6 +1140,18 @@ svn_path_url_add_component(const char *u
> return svn_path_join(url, svn_path_uri_encode(component, pool), pool);
> }
>
> +const char *
> +svn_path_url_get_hostname_part(const char *url, apr_pool_t *pool)
> +{
> + apr_array_header_t *parts = svn_cstring_split(url, "/", TRUE, pool);
> +
> + if (parts->nelts < 2)
> + return NULL;
> +
> + return APR_ARRAY_IDX(parts, 1, const char *);
> +}
> +
> +
> svn_error_t *
> svn_path_get_absolute(const char **pabsolute,
> const char *relative,
>
> 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=30723&r1=30722&r2=30723
> ==============================================================================
> --- branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c Sun Apr 20 13:23:38 2008 (r30722)
> +++ branches/dont-save-plaintext-passwords-by-default/subversion/svn/main.c Sun Apr 20 16:20:34 2008 (r30723)
> @@ -1049,6 +1049,7 @@ main(int argc, const char *argv[])
> svn_boolean_t descend = TRUE;
> svn_boolean_t interactive_conflicts = FALSE;
> apr_hash_t *changelists;
> + apr_array_header_t *urls;
>
> /* Initialize the app. */
> if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
> @@ -1914,17 +1915,62 @@ main(int argc, const char *argv[])
> apr_signal(SIGXFSZ, SIG_IGN);
> #endif
>
> + /* Any URL targets supplied?
> + * If any, svn_cmdline_setup_auth_baton2 wants them, so it can
> + * properly read settings from the 'servers' config file. */
> + urls = apr_array_make(pool, 0, sizeof(const char *));
> + {
> + apr_array_header_t *targets;
> + apr_pool_t *subpool;
> + int ind;
> +
> + /* XXX: Work around the fact that svn_opt_args_to_target_array3
> + * modifies the argument index. This is awfully ugly, but
> + * better than duplicating its URL-specific code here... */
> + ind = os->ind;
> + err = svn_opt_args_to_target_array3(&targets, os, opt_state.targets, pool);
> + os->ind = ind;
> + if (err)
> + {
> + if (err->apr_err != SVN_ERR_RESERVED_FILENAME_SPECIFIED)
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + else
> + /* This only means that a .svn directory was in the targets list. */
> + svn_error_clear(err);
> + }
> +
> + subpool = svn_pool_create(pool);
> + for (i = 0; i < targets->nelts; i++)
> + {
> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> + const char *truepath;
> + svn_opt_revision_t dummy;
> +
> + svn_pool_clear(subpool);
> +
> + /* Strip peg revisions from paths since they might break
> + * regexes defining server groups in the 'servers' config file. */
> + if ((err = svn_opt_parse_path(&dummy, &truepath, target, subpool)))
> + svn_handle_error2(err, stderr, TRUE, "svn: ");
> +
> + if (svn_path_is_url(truepath))
> + APR_ARRAY_PUSH(urls, const char *) = truepath;
> + }
> + svn_pool_destroy(subpool);
> + }
> +
> /* Set up Authentication stuff. */
> - 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)))
> + 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,
> + ctx->config,
> + urls,
> + 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: r30723 - in branches/dont-save-plaintext-passwords-by-default: . subversion/include subversion/libsvn_subr subversion/svn
Posted by Travis <sv...@castle.fastmail.fm>.
On Apr 22, 2008, at 4:41 AM, Stefan Sperling wrote:
> On Mon, Apr 21, 2008 at 10:50:25PM +0200, Stefan Sperling wrote:
>> On Mon, Apr 21, 2008 at 10:44:19AM -0700, David Glasser wrote:
>>> On Mon, Apr 21, 2008 at 6:31 AM, C. Michael Pilato
>>> <cm...@collab.net> wrote:
>>>> Stefan Sperling wrote:
>>>>
>>>>> On Sun, Apr 20, 2008 at 04:42:56PM -0700, David Glasser wrote:
>>>>>
>>>>>> Hmm, why not just have the "global" servers section be the normal
>>>>>> place to configure this? Why use the other config file at all?
>>>>>>
>>>>>
>>>>> Because most users will look at the 'config' file first, I guess.
>>>>> Also, there, it's right next to 'store-passwords'.
>>>>>
>>>>> The docstrings reference each other, so I think it's sorta OK...
>>>>>
>>>>> The other idea I had was to have the option be valid in both
>>>>> the config file and the servers file [global] section, and
>>>>> have svn print a warning when the two disagree and fall back
>>>>> to 'prompt'. But I discarded that as overkill.
>>>>>
>>>>
>>>> That we ever had the store-passwords option in the 'config'
>>>> file instead of
>>>> in the 'servers' file (bound to RA-related things) might have
>>>> been a
>>>> mistake. Let's evaluate the correct location for this new
>>>> option without
>>>> concern for the dubious decisions of the past.
>>>
>>> +1
>>
>> Right, so if we stored 'store-plaintext-passwords' in 'servers' only,
>> we should also move 'store-passwords' and 'store-auth-creds' to
>> 'servers'.
>> That is, the whole [auth] section of the 'config' file will go to the
>> 'servers' file, with a comment in 'config' informing users about
>> the move.
>>
>> Agreed?
> ...
> Does anyone oppose moving the whole [auth] section from 'config' to
> 'servers' for consistency?
Yay, please do. Having it in 'config' always struck me as wrong
because there's no reason I necessarily want to have the same
settings when accessing all servers. I might want to prevent store-
auth-creds for some specific servers (high security, very infrequent
use) but allow it for my workaday projects (it's almost impossible to
do without when using the command-line client).
Also, simply looking for the settings, it's more natural to look in
'server' for the [auth] settings since those are all about
credentials for communication with a given server, like the http-
proxy-* and ssl* settings. For my apache/dav served repositories,
the repository itself has no authentication/authorization information.
-Travis
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30723 - in branches/dont-save-plaintext-passwords-by-default: . subversion/include subversion/libsvn_subr subversion/svn
Posted by Karl Fogel <kf...@red-bean.com>.
Stefan Sperling <st...@elego.de> writes:
>> Might be good to do this deprecation/move on a separate branch, merge
>> that to trunk, then incorporate into the dont-save-plaintext branch and
>> DTRT. They're kind of separate changes, I think.
>
> I've already done the move in my branch WC now, basically about to
> commit the change, and I'm too lazy right now to untangle these two
> admittedly separate topics. I hope that is OK? :)
Sure, no big deal IMHO.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30723 - in
branches/dont-save-plaintext-passwords-by-default: .
subversion/include subversion/libsvn_subr subversion/svn
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Apr 22, 2008 at 02:33:18PM -0400, Karl Fogel wrote:
> Stefan Sperling <st...@elego.de> writes:
> > I will deprecate the old location in the comments, like this:
> >
> > "[auth]" NL
> > "### Set store-passwords to 'no' to avoid storing passwords in the" NL
> > "### auth/ area of your config directory. It defaults to 'yes'," NL
> > "### but Subversion will never save your password to disk in" NL
> > "### plaintext unless you tell it to (see below)." NL
> > "### Note that this option only prevents saving of *new* passwords;" NL
> > "### it doesn't invalidate existing passwords. (To do that, remove" NL
> > "### the cache files by hand as described in the Subversion book.)" NL
> > + "### NOTE: This option can now be specified in the 'servers' file" NL
> > + "### in your config directory. This location for this option has" NL
> > + "### been deprecated. Anything specified here is overridden by" NL
> > + "### settings specified in the 'servers' file." NL
> > "# store-passwords = no" NL
>
> +1, but put the deprecation notice at the top of the block, so people
> see it and then know that everything they read after that is about
> something deprecated anyway.
Yes, that's a good idea.
> > No, the code enforces this. But note that having all the [auth]
> > settings in 'servers' makes much more sense anyway, because
> > there, they can be configured on a per-server basis.
> >
> > It also aligns much more naturally with the layering, since
> > authentication is done only if the RA layer is entered anyway,
> > which has always been getting its configuration from 'servers'.
> >
> > The [auth] section in 'config' was a mistake, it would have
> > been much more natural to put it in 'servers' in the first place.
>
> Okay, I'm convinced.
>
> Might be good to do this deprecation/move on a separate branch, merge
> that to trunk, then incorporate into the dont-save-plaintext branch and
> DTRT. They're kind of separate changes, I think.
I've already done the move in my branch WC now, basically about to
commit the change, and I'm too lazy right now to untangle these two
admittedly separate topics. I hope that is OK? :)
--
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: r30723 - in branches/dont-save-plaintext-passwords-by-default: . subversion/include subversion/libsvn_subr subversion/svn
Posted by Karl Fogel <kf...@red-bean.com>.
Stefan Sperling <st...@elego.de> writes:
> I will deprecate the old location in the comments, like this:
>
> "[auth]" NL
> "### Set store-passwords to 'no' to avoid storing passwords in the" NL
> "### auth/ area of your config directory. It defaults to 'yes'," NL
> "### but Subversion will never save your password to disk in" NL
> "### plaintext unless you tell it to (see below)." NL
> "### Note that this option only prevents saving of *new* passwords;" NL
> "### it doesn't invalidate existing passwords. (To do that, remove" NL
> "### the cache files by hand as described in the Subversion book.)" NL
> + "### NOTE: This option can now be specified in the 'servers' file" NL
> + "### in your config directory. This location for this option has" NL
> + "### been deprecated. Anything specified here is overridden by" NL
> + "### settings specified in the 'servers' file." NL
> "# store-passwords = no" NL
+1, but put the deprecation notice at the top of the block, so people
see it and then know that everything they read after that is about
something deprecated anyway.
> No, the code enforces this. But note that having all the [auth]
> settings in 'servers' makes much more sense anyway, because
> there, they can be configured on a per-server basis.
>
> It also aligns much more naturally with the layering, since
> authentication is done only if the RA layer is entered anyway,
> which has always been getting its configuration from 'servers'.
>
> The [auth] section in 'config' was a mistake, it would have
> been much more natural to put it in 'servers' in the first place.
Okay, I'm convinced.
Might be good to do this deprecation/move on a separate branch, merge
that to trunk, then incorporate into the dont-save-plaintext branch and
DTRT. They're kind of separate changes, I think.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30723 - in
branches/dont-save-plaintext-passwords-by-default: .
subversion/include subversion/libsvn_subr subversion/svn
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Apr 22, 2008 at 10:10:45AM -0400, Karl Fogel wrote:
> Stefan Sperling <st...@elego.de> writes:
> > Does anyone oppose moving the whole [auth] section from 'config' to
> > 'servers' for consistency?
>
> As long as we maintain compatibility...
I will deprecate the old location in the comments, like this:
"[auth]" NL
"### Set store-passwords to 'no' to avoid storing passwords in the" NL
"### auth/ area of your config directory. It defaults to 'yes'," NL
"### but Subversion will never save your password to disk in" NL
"### plaintext unless you tell it to (see below)." NL
"### Note that this option only prevents saving of *new* passwords;" NL
"### it doesn't invalidate existing passwords. (To do that, remove" NL
"### the cache files by hand as described in the Subversion book.)" NL
+ "### NOTE: This option can now be specified in the 'servers' file" NL
+ "### in your config directory. This location for this option has" NL
+ "### been deprecated. Anything specified here is overridden by" NL
+ "### settings specified in the 'servers' file." NL
"# store-passwords = no" NL
> But why is there a connection between which file the "[auth]" section
> lives in (a user-visible question) and which URLs we use that data for
> (an internal code-flow issue)?
Because 'config' is read before the subcommand is run, which
then figures out whether to contact a server, and which one.
While reading 'servers', we need to know which server we're contacting,
because we may need to match a server group:
[groups]
server1 = *.tigris.org
[server1]
store-plaintext-passwords = no
[global]
store-plaintext-passwords = yes
> The choice of which config file to use should be made based on what will
> make most sense for users, who don't know or care how our code is
> organized internally. It sounds like these two issues are maybe getting
> mixed together, in your comments above?
No, the code enforces this. But note that having all the [auth]
settings in 'servers' makes much more sense anyway, because
there, they can be configured on a per-server basis.
It also aligns much more naturally with the layering, since
authentication is done only if the RA layer is entered anyway,
which has always been getting its configuration from 'servers'.
The [auth] section in 'config' was a mistake, it would have
been much more natural to put it in 'servers' in the first place.
--
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: r30723 - in branches/dont-save-plaintext-passwords-by-default: . subversion/include subversion/libsvn_subr subversion/svn
Posted by Karl Fogel <kf...@red-bean.com>.
Stefan Sperling <st...@elego.de> writes:
> Actually, storing 'store-plaintext-passwords' in 'servers' is *required*
> to fix the following bug I just realised exists (and is really silly
> once you realise it's there, I do really stupid things sometimes):
>
> In svn/main.c on the branch, we're currently grabbing URLs from the
> command line so we can use those to match server groups in the 'servers'
> file. The approach is totally naive and wrong. We're not even checking
> (and we can't, in main.c) which of the URLs we're actually connecting to
> (they might be part of, say, a log message argument), and on top of that,
> the URL we're connecting to might not even be retrieved from the command
> line! "svn ci ..." causes auth data to be cached for repos that require
> auth only for write operations, like our own repo does. Doh!
>
> So this is clearly an issue of "layer violation". 'store-plaintext-passwords'
> has to move out of 'config' and svn_cmdline_setup_auth_baton, and go into
> 'servers', and be evaluated by RA layers. This probably means that quite a
> bit of code will need to be refactored/rewritten on the branch :/
>
> Does anyone oppose moving the whole [auth] section from 'config' to
> 'servers' for consistency?
As long as we maintain compatibility...
But why is there a connection between which file the "[auth]" section
lives in (a user-visible question) and which URLs we use that data for
(an internal code-flow issue)?
The choice of which config file to use should be made based on what will
make most sense for users, who don't know or care how our code is
organized internally. It sounds like these two issues are maybe getting
mixed together, in your comments above?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r30723 - in
branches/dont-save-plaintext-passwords-by-default: .
subversion/include subversion/libsvn_subr subversion/svn
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Apr 21, 2008 at 10:50:25PM +0200, Stefan Sperling wrote:
> On Mon, Apr 21, 2008 at 10:44:19AM -0700, David Glasser wrote:
> > On Mon, Apr 21, 2008 at 6:31 AM, C. Michael Pilato <cm...@collab.net> wrote:
> > > Stefan Sperling wrote:
> > >
> > > > On Sun, Apr 20, 2008 at 04:42:56PM -0700, David Glasser wrote:
> > > >
> > > > > Hmm, why not just have the "global" servers section be the normal
> > > > > place to configure this? Why use the other config file at all?
> > > > >
> > > >
> > > > Because most users will look at the 'config' file first, I guess.
> > > > Also, there, it's right next to 'store-passwords'.
> > > >
> > > > The docstrings reference each other, so I think it's sorta OK...
> > > >
> > > > The other idea I had was to have the option be valid in both
> > > > the config file and the servers file [global] section, and
> > > > have svn print a warning when the two disagree and fall back
> > > > to 'prompt'. But I discarded that as overkill.
> > > >
> > >
> > > That we ever had the store-passwords option in the 'config' file instead of
> > > in the 'servers' file (bound to RA-related things) might have been a
> > > mistake. Let's evaluate the correct location for this new option without
> > > concern for the dubious decisions of the past.
> >
> > +1
>
> Right, so if we stored 'store-plaintext-passwords' in 'servers' only,
> we should also move 'store-passwords' and 'store-auth-creds' to 'servers'.
> That is, the whole [auth] section of the 'config' file will go to the
> 'servers' file, with a comment in 'config' informing users about the move.
>
> Agreed?
Actually, storing 'store-plaintext-passwords' in 'servers' is *required*
to fix the following bug I just realised exists (and is really silly
once you realise it's there, I do really stupid things sometimes):
In svn/main.c on the branch, we're currently grabbing URLs from the
command line so we can use those to match server groups in the 'servers'
file. The approach is totally naive and wrong. We're not even checking
(and we can't, in main.c) which of the URLs we're actually connecting to
(they might be part of, say, a log message argument), and on top of that,
the URL we're connecting to might not even be retrieved from the command
line! "svn ci ..." causes auth data to be cached for repos that require
auth only for write operations, like our own repo does. Doh!
So this is clearly an issue of "layer violation". 'store-plaintext-passwords'
has to move out of 'config' and svn_cmdline_setup_auth_baton, and go into
'servers', and be evaluated by RA layers. This probably means that quite a
bit of code will need to be refactored/rewritten on the branch :/
Does anyone oppose moving the whole [auth] section from 'config' to
'servers' for consistency?
--
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: r30723 - in
branches/dont-save-plaintext-passwords-by-default: .
subversion/include subversion/libsvn_subr subversion/svn
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Apr 21, 2008 at 10:44:19AM -0700, David Glasser wrote:
> On Mon, Apr 21, 2008 at 6:31 AM, C. Michael Pilato <cm...@collab.net> wrote:
> > Stefan Sperling wrote:
> >
> > > On Sun, Apr 20, 2008 at 04:42:56PM -0700, David Glasser wrote:
> > >
> > > > Hmm, why not just have the "global" servers section be the normal
> > > > place to configure this? Why use the other config file at all?
> > > >
> > >
> > > Because most users will look at the 'config' file first, I guess.
> > > Also, there, it's right next to 'store-passwords'.
> > >
> > > The docstrings reference each other, so I think it's sorta OK...
> > >
> > > The other idea I had was to have the option be valid in both
> > > the config file and the servers file [global] section, and
> > > have svn print a warning when the two disagree and fall back
> > > to 'prompt'. But I discarded that as overkill.
> > >
> >
> > That we ever had the store-passwords option in the 'config' file instead of
> > in the 'servers' file (bound to RA-related things) might have been a
> > mistake. Let's evaluate the correct location for this new option without
> > concern for the dubious decisions of the past.
>
> +1
Right, so if we stored 'store-plaintext-passwords' in 'servers' only,
we should also move 'store-passwords' and 'store-auth-creds' to 'servers'.
That is, the whole [auth] section of the 'config' file will go to the
'servers' file, with a comment in 'config' informing users about the move.
Agreed?
--
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: r30723 - in branches/dont-save-plaintext-passwords-by-default: . subversion/include subversion/libsvn_subr subversion/svn
Posted by David Glasser <gl...@davidglasser.net>.
On Mon, Apr 21, 2008 at 6:31 AM, C. Michael Pilato <cm...@collab.net> wrote:
> Stefan Sperling wrote:
>
> > On Sun, Apr 20, 2008 at 04:42:56PM -0700, David Glasser wrote:
> >
> > > Hmm, why not just have the "global" servers section be the normal
> > > place to configure this? Why use the other config file at all?
> > >
> >
> > Because most users will look at the 'config' file first, I guess.
> > Also, there, it's right next to 'store-passwords'.
> >
> > The docstrings reference each other, so I think it's sorta OK...
> >
> > The other idea I had was to have the option be valid in both
> > the config file and the servers file [global] section, and
> > have svn print a warning when the two disagree and fall back
> > to 'prompt'. But I discarded that as overkill.
> >
>
> That we ever had the store-passwords option in the 'config' file instead of
> in the 'servers' file (bound to RA-related things) might have been a
> mistake. Let's evaluate the correct location for this new option without
> concern for the dubious decisions of the past.
+1
--dave
--
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: r30723 - in branches/dont-save-plaintext-passwords-by-default:
. subversion/include subversion/libsvn_subr subversion/svn
Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Sperling wrote:
> On Sun, Apr 20, 2008 at 04:42:56PM -0700, David Glasser wrote:
>> Hmm, why not just have the "global" servers section be the normal
>> place to configure this? Why use the other config file at all?
>
> Because most users will look at the 'config' file first, I guess.
> Also, there, it's right next to 'store-passwords'.
>
> The docstrings reference each other, so I think it's sorta OK...
>
> The other idea I had was to have the option be valid in both
> the config file and the servers file [global] section, and
> have svn print a warning when the two disagree and fall back
> to 'prompt'. But I discarded that as overkill.
That we ever had the store-passwords option in the 'config' file instead of
in the 'servers' file (bound to RA-related things) might have been a
mistake. Let's evaluate the correct location for this new option without
concern for the dubious decisions of the past.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r30723 - in
branches/dont-save-plaintext-passwords-by-default: .
subversion/include subversion/libsvn_subr subversion/svn
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Apr 20, 2008 at 04:42:56PM -0700, David Glasser wrote:
> Hmm, why not just have the "global" servers section be the normal
> place to configure this? Why use the other config file at all?
Because most users will look at the 'config' file first, I guess.
Also, there, it's right next to 'store-passwords'.
The docstrings reference each other, so I think it's sorta OK...
The other idea I had was to have the option be valid in both
the config file and the servers file [global] section, and
have svn print a warning when the two disagree and fall back
to 'prompt'. But I discarded that as overkill.
--
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