You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Phippard <ma...@gmail.com> on 2012/09/07 15:57:24 UTC
Re: svn commit: r1382028 - in /subversion/trunk/subversion:
include/svn_config.h libsvn_subr/cmdline.c libsvn_subr/config_file.c
On Fri, Sep 7, 2012 at 9:53 AM, <cm...@apache.org> wrote:
> Author: cmpilato
> Date: Fri Sep 7 13:53:05 2012
> New Revision: 1382028
>
> URL: http://svn.apache.org/viewvc?rev=1382028&view=rev
> Log:
> Finish issue #2410 ("Allow client to avoid SSL certificate prompts").
> This adds a runtime configuration knob for explicitly enabling and
> disabling the client certificate path prompt provider.
>
> libsvn_subr/config_file.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/config_file.c?rev=1382028&r1=1382027&r2=1382028&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/config_file.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/config_file.c Fri Sep 7
> 13:53:05 2012
> @@ -1034,6 +1034,13 @@ svn_config_ensure(const char *config_dir
> "# kwallet-svn-application-name-with-pid = yes"
> NL
> #endif
> "###"
> NL
> + "### Set ssl-client-cert-file-prompt to 'yes' to cause the
> client" NL
> + "### to prompt for a path to a client cert file when the server"
> NL
> + "### requests a client cert but no client cert file is found in
> the" NL
> + "### expected place (see the 'ssl-client-cert-file' option in
> the" NL
> + "### 'servers' configuration file). Defaults to 'no'."
> NL
> + "# ssl-client-cert-file-prompt = no"
> NL
> + "###"
> NL
> "### The rest of the [auth] section in this file has been
> deprecated."
>
> NL
> "### Both 'store-passwords' and 'store-auth-creds' can now be"
> NL
>
Shouldn't this default to 'yes' to match the current behavior?
I have only used this feature a few times to test stuff, but I recall being
prompted to type in the path to my cert. So I assume that is what you are
suppressing with this option.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: svn commit: r1382028 - in /subversion/trunk/subversion:
include/svn_config.h libsvn_subr/cmdline.c libsvn_subr/config_file.c
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Sep 7, 2012 at 10:51 AM, C. Michael Pilato <cm...@collab.net>wrote:
>
> I may be wrong, but I kinda get the sense that the GUI clients take their
> pick of which Subversion runtime config options make sense for them to
> honor. At the moment, the only code which honors the state of the new
> configuration variable is the svn_cmdline_* stuff that builds the initial
> auth baton. I can't imagine that TSVN and other GUI clients which such
> slick UI features use that function -- which is strictly aimed at
> command-line utility -- anyway. So can you simply still choose to
> unconditionally add the client-cert prompt provider to the TSVN auth baton
> provider array?
>
Setting aside Stefan's feedback on the default which sounds valid ...
JavaHL clients like Subclipse are using libsvn_client. I know that for
some of the stuff like the auth providers, the JavaHL C++ code is
recreating some of what is in the command line client, so I am not sure if
that covers this.
As a JavaHL users, I expect the runtime configuration options to be applied
for me the same way they are for the command line client. So I would
expect the JavaHL C++ code to be doing whatever is needed so that this
option is honored. Whether the default is yes or no would be a secondary
question. I would expect a user to be able to edit the file to set the
behavior.
If TortoiseSVN will not be impacted by the default, then I would still be
OK with leaving it as you have it.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: svn commit: r1382028 - in /subversion/trunk/subversion: include/svn_config.h
libsvn_subr/cmdline.c libsvn_subr/config_file.c
Posted by Stefan Küng <to...@gmail.com>.
On 07.09.2012 16:51, C. Michael Pilato wrote:
> On 09/07/2012 10:14 AM, Stefan Küng wrote:
>> On 07.09.2012 16:07, Mark Phippard wrote:
>>> On Fri, Sep 7, 2012 at 10:04 AM, C. Michael Pilato <cmpilato@collab.net
>>> <ma...@collab.net>> wrote:
>>>
>>> On 09/07/2012 09:57 AM, Mark Phippard wrote:
>>> > Shouldn't this default to 'yes' to match the current behavior?
>>>
>>> I went with Joe Orton's explicit suggestion (and Karl's implicit
>>> approval
>>> thereof) that we break compatibility and change the default
>>> behavior. See
>>> http://svn.haxx.se/dev/archive-2008-07/0161.shtml. That said, as
>>> much of a
>>> grump as I tend to be about such things, yes, I probably should have
>>> coded
>>> to my conviction and set the default as 'yes'.
>>>
>>>
>>> I do not really have a strong opinion on it. I cannot imagine anyone
>>> likes the current default.
>>
>> Well, actually I think most users do. At least TSVN users:
>> if a cert file is requested, TSVN tells the user and asks for the file. The
>> user then selects the file and TSVN automatically adjusts the config (if the
>> checkbox is set) so that it never asks for that file again.
>> Basically, that saves the user from having to edit the config file manually.
>>
>> Without asking for the file if the server requests one this would be much
>> more complicated: an admin would either have to configure this for every
>> user, or write a lengthy doc on how users have to configure it themselves.
>
> I may be wrong, but I kinda get the sense that the GUI clients take their
> pick of which Subversion runtime config options make sense for them to
> honor. At the moment, the only code which honors the state of the new
> configuration variable is the svn_cmdline_* stuff that builds the initial
> auth baton. I can't imagine that TSVN and other GUI clients which such
> slick UI features use that function -- which is strictly aimed at
> command-line utility -- anyway. So can you simply still choose to
> unconditionally add the client-cert prompt provider to the TSVN auth baton
> provider array?
>
Sure I can do that.
I just wanted to mention that the (previous) option default makes sense
in some situations.
Sorry if I seemed to indicate that the new default would hurt TSVN or
its features. That wasn't my intention.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: svn commit: r1382028 - in /subversion/trunk/subversion: include/svn_config.h
libsvn_subr/cmdline.c libsvn_subr/config_file.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/07/2012 10:14 AM, Stefan Küng wrote:
> On 07.09.2012 16:07, Mark Phippard wrote:
>> On Fri, Sep 7, 2012 at 10:04 AM, C. Michael Pilato <cmpilato@collab.net
>> <ma...@collab.net>> wrote:
>>
>> On 09/07/2012 09:57 AM, Mark Phippard wrote:
>> > Shouldn't this default to 'yes' to match the current behavior?
>>
>> I went with Joe Orton's explicit suggestion (and Karl's implicit
>> approval
>> thereof) that we break compatibility and change the default
>> behavior. See
>> http://svn.haxx.se/dev/archive-2008-07/0161.shtml. That said, as
>> much of a
>> grump as I tend to be about such things, yes, I probably should have
>> coded
>> to my conviction and set the default as 'yes'.
>>
>>
>> I do not really have a strong opinion on it. I cannot imagine anyone
>> likes the current default.
>
> Well, actually I think most users do. At least TSVN users:
> if a cert file is requested, TSVN tells the user and asks for the file. The
> user then selects the file and TSVN automatically adjusts the config (if the
> checkbox is set) so that it never asks for that file again.
> Basically, that saves the user from having to edit the config file manually.
>
> Without asking for the file if the server requests one this would be much
> more complicated: an admin would either have to configure this for every
> user, or write a lengthy doc on how users have to configure it themselves.
I may be wrong, but I kinda get the sense that the GUI clients take their
pick of which Subversion runtime config options make sense for them to
honor. At the moment, the only code which honors the state of the new
configuration variable is the svn_cmdline_* stuff that builds the initial
auth baton. I can't imagine that TSVN and other GUI clients which such
slick UI features use that function -- which is strictly aimed at
command-line utility -- anyway. So can you simply still choose to
unconditionally add the client-cert prompt provider to the TSVN auth baton
provider array?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1382028 - in /subversion/trunk/subversion: include/svn_config.h
libsvn_subr/cmdline.c libsvn_subr/config_file.c
Posted by Stefan Küng <to...@gmail.com>.
On 07.09.2012 16:07, Mark Phippard wrote:
> On Fri, Sep 7, 2012 at 10:04 AM, C. Michael Pilato <cmpilato@collab.net
> <ma...@collab.net>> wrote:
>
> On 09/07/2012 09:57 AM, Mark Phippard wrote:
> > Shouldn't this default to 'yes' to match the current behavior?
>
> I went with Joe Orton's explicit suggestion (and Karl's implicit
> approval
> thereof) that we break compatibility and change the default
> behavior. See
> http://svn.haxx.se/dev/archive-2008-07/0161.shtml. That said, as
> much of a
> grump as I tend to be about such things, yes, I probably should have
> coded
> to my conviction and set the default as 'yes'.
>
>
> I do not really have a strong opinion on it. I cannot imagine anyone
> likes the current default.
Well, actually I think most users do. At least TSVN users:
if a cert file is requested, TSVN tells the user and asks for the file.
The user then selects the file and TSVN automatically adjusts the config
(if the checkbox is set) so that it never asks for that file again.
Basically, that saves the user from having to edit the config file manually.
Without asking for the file if the server requests one this would be
much more complicated: an admin would either have to configure this for
every user, or write a lengthy doc on how users have to configure it
themselves.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: svn commit: r1382028 - in /subversion/trunk/subversion:
include/svn_config.h libsvn_subr/cmdline.c libsvn_subr/config_file.c
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Sep 7, 2012 at 10:04 AM, C. Michael Pilato <cm...@collab.net>wrote:
> On 09/07/2012 09:57 AM, Mark Phippard wrote:
> > Shouldn't this default to 'yes' to match the current behavior?
>
> I went with Joe Orton's explicit suggestion (and Karl's implicit approval
> thereof) that we break compatibility and change the default behavior. See
> http://svn.haxx.se/dev/archive-2008-07/0161.shtml. That said, as much of
> a
> grump as I tend to be about such things, yes, I probably should have coded
> to my conviction and set the default as 'yes'.
>
>
I do not really have a strong opinion on it. I cannot imagine anyone likes
the current default.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: svn commit: r1382028 - in /subversion/trunk/subversion: include/svn_config.h
libsvn_subr/cmdline.c libsvn_subr/config_file.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/07/2012 09:57 AM, Mark Phippard wrote:
> Shouldn't this default to 'yes' to match the current behavior?
I went with Joe Orton's explicit suggestion (and Karl's implicit approval
thereof) that we break compatibility and change the default behavior. See
http://svn.haxx.se/dev/archive-2008-07/0161.shtml. That said, as much of a
grump as I tend to be about such things, yes, I probably should have coded
to my conviction and set the default as 'yes'.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development