[PATCH] git-svn: enable platform-specific authentication

[PATCH] git-svn: enable platform-specific authentication

[Gustav Munkby]

Use the platform-specific authentication providers that are
exposed to subversion bindings starting with subversion 1.6.

Signed-off-by: Gustav Munkby <grddev@gmail.com>
---
 git-svn.perl |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 0fd2fd2..3f7c3c8 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4930,6 +4930,9 @@ BEGIN {
 
 sub _auth_providers () {
 	[
+	  $SVN::Core::VERSION lt '1.6' ? () :
+	    @{SVN::Core::auth_get_platform_specific_client_providers(
+	      undef,undef)},
 	  SVN::Client::get_simple_provider(),
 	  SVN::Client::get_ssl_server_trust_file_provider(),
 	  SVN::Client::get_simple_prompt_provider(
-- 
1.7.5.1

Re: [PATCH] git-svn: enable platform-specific authentication

[Eric Wong]

Gustav Munkby <grddev@gmail.com> wrote:
> Use the platform-specific authentication providers that are
> exposed to subversion bindings starting with subversion 1.6.

This came up several months ago, I understand there were some issues
with the SVN Perl bindings.  Cc-ing interested parties.

>  sub _auth_providers () {
>  	[
> +	  $SVN::Core::VERSION lt '1.6' ? () :
> +	    @{SVN::Core::auth_get_platform_specific_client_providers(
> +	      undef,undef)},

I think it needs to take into account the config from
SVN::Core::config_get_config, otherwise people with non-standard SVN
configurations could get locked out.  I seem to recall this was the
broken part in the SVN Perl bindings, but one of the Cc-ed parties would
know for sure.

-- 
Eric Wong

Re: [PATCH] git-svn: enable platform-specific authentication

[Matthijs Kooijman]

[-- Attachment #1: Type: text/plain, Size: 3890 bytes --]

Hey folks,

> > Use the platform-specific authentication providers that are
> > exposed to subversion bindings starting with subversion 1.6.
> 
> This came up several months ago, I understand there were some issues
> with the SVN Perl bindings.  Cc-ing interested parties.
I missed the CC, sorry for that.

> >  sub _auth_providers () {
> >  	[
> > +	  $SVN::Core::VERSION lt '1.6' ? () :
> > +	    @{SVN::Core::auth_get_platform_specific_client_providers(
> > +	      undef,undef)},
> 
> I think it needs to take into account the config from
> SVN::Core::config_get_config, otherwise people with non-standard SVN
> configurations could get locked out.  I seem to recall this was the
> broken part in the SVN Perl bindings, but one of the Cc-ed parties would
> know for sure.

Indeed, but a proposed patch by Eric for this did not work. I solved the
problem quite some time ago, but apparently I never sent out the
solution (I think I got distracted by trying to get a passphrase prompt
to unlock locked keychains). I couldn't find my fixes anymore either,
but I think I've managed to reproduce them just now.

Some basic testing shows below patch works, but I think it might need
some more testing and work. At least the below patch allows for example
to disable the gnome-keyring provider from a different svn config
directory by passing --config-dir /some/path to git-svn (which is not
possible using above patch passing undef, which will only read from
~/.subversion).

Using strace, I did notice that git-svn still reads stuff
from ~/.subversion/auth/svn.ssl.server/ and
.subversion/auth/svn.simple/, but I couldn't exactly find why this is
right away. In any case, it also happens without this patch applied, so
I guess it's a completely separate issue.

As for the actual patch, notice that config_get_config returns a hash
that consists again of a "config" and "servers" patch. Previous attempts
at this patch passed the entire hash to
auth_get_platform_specific_client_providers, but it only wants the
"client" part. It's a bit confusing until you realize that the
config_get_config return value represents your ~/.subversion directory,
which again contains a "config" and "servers" file.

I'm not 100% sure this patch is correct as it is now. I hope to get
another look at my "automatically unlock keychain" work tomorrow,
in case there are some hints about flaws in this patch there. In the
meanwhile, feedback on this patch is welcome.

Gr.

Matthijs

diff --git a/git-svn.perl b/git-svn.perl
index da3fea8..6dc5196 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4916,7 +4916,7 @@ BEGIN {
 }
 
 sub _auth_providers () {
-       [
+       my @rv = (
          SVN::Client::get_simple_provider(),
          SVN::Client::get_ssl_server_trust_file_provider(),
          SVN::Client::get_simple_prompt_provider(
@@ -4932,7 +4932,23 @@ sub _auth_providers () {
            \&Git::SVN::Prompt::ssl_server_trust),
          SVN::Client::get_username_prompt_provider(
            \&Git::SVN::Prompt::username, 2)
-       ]
+       );
+
+       # earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
+       # this function
+       if ($SVN::Core::VERSION gt '1.6.12') {
+               my $config = SVN::Core::config_get_config($config_dir);
+               my ($p, @a);
+              # config_get_config returns all config files from
+              # ~/.subversion, auth_get_platform_specific_client_providers
+              # just wants the config "file".
+               @a = ($config->{'config'}, undef);
+               $p = SVN::Core::auth_get_platform_specific_client_providers(@a);
+              # Insert the return value from
+              # auth_get_platform_specific_providers
+               unshift @rv, @$p;
+       }
+       \@rv;
 }
 
 sub escape_uri_only {


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] git-svn: enable platform-specific authentication

[Matthijs Kooijman]

[-- Attachment #1: Type: text/plain, Size: 4514 bytes --]

Hey folks,

I sent the below patch a few months ago, and not having it applied in
git-svn bit me again just now. Did any of you get a chance to have a
look at it?

I'm still not 100% sure if this patch is correct for all the corner
cases, but it works like a charm in the regular case.

Perhaps it should just be included as is?

Gr.

Matthijs

On Tue, Aug 09, 2011 at 11:06:38PM +0200, Matthijs Kooijman wrote:
> Hey folks,
> 
> > > Use the platform-specific authentication providers that are
> > > exposed to subversion bindings starting with subversion 1.6.
> > 
> > This came up several months ago, I understand there were some issues
> > with the SVN Perl bindings.  Cc-ing interested parties.
> I missed the CC, sorry for that.
> 
> > >  sub _auth_providers () {
> > >  	[
> > > +	  $SVN::Core::VERSION lt '1.6' ? () :
> > > +	    @{SVN::Core::auth_get_platform_specific_client_providers(
> > > +	      undef,undef)},
> > 
> > I think it needs to take into account the config from
> > SVN::Core::config_get_config, otherwise people with non-standard SVN
> > configurations could get locked out.  I seem to recall this was the
> > broken part in the SVN Perl bindings, but one of the Cc-ed parties would
> > know for sure.
> 
> Indeed, but a proposed patch by Eric for this did not work. I solved the
> problem quite some time ago, but apparently I never sent out the
> solution (I think I got distracted by trying to get a passphrase prompt
> to unlock locked keychains). I couldn't find my fixes anymore either,
> but I think I've managed to reproduce them just now.
> 
> Some basic testing shows below patch works, but I think it might need
> some more testing and work. At least the below patch allows for example
> to disable the gnome-keyring provider from a different svn config
> directory by passing --config-dir /some/path to git-svn (which is not
> possible using above patch passing undef, which will only read from
> ~/.subversion).
> 
> Using strace, I did notice that git-svn still reads stuff
> from ~/.subversion/auth/svn.ssl.server/ and
> .subversion/auth/svn.simple/, but I couldn't exactly find why this is
> right away. In any case, it also happens without this patch applied, so
> I guess it's a completely separate issue.
> 
> As for the actual patch, notice that config_get_config returns a hash
> that consists again of a "config" and "servers" patch. Previous attempts
> at this patch passed the entire hash to
> auth_get_platform_specific_client_providers, but it only wants the
> "client" part. It's a bit confusing until you realize that the
> config_get_config return value represents your ~/.subversion directory,
> which again contains a "config" and "servers" file.
> 
> I'm not 100% sure this patch is correct as it is now. I hope to get
> another look at my "automatically unlock keychain" work tomorrow,
> in case there are some hints about flaws in this patch there. In the
> meanwhile, feedback on this patch is welcome.
> 
> Gr.
> 
> Matthijs
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index da3fea8..6dc5196 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -4916,7 +4916,7 @@ BEGIN {
>  }
>  
>  sub _auth_providers () {
> -       [
> +       my @rv = (
>           SVN::Client::get_simple_provider(),
>           SVN::Client::get_ssl_server_trust_file_provider(),
>           SVN::Client::get_simple_prompt_provider(
> @@ -4932,7 +4932,23 @@ sub _auth_providers () {
>             \&Git::SVN::Prompt::ssl_server_trust),
>           SVN::Client::get_username_prompt_provider(
>             \&Git::SVN::Prompt::username, 2)
> -       ]
> +       );
> +
> +       # earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
> +       # this function
> +       if ($SVN::Core::VERSION gt '1.6.12') {
> +               my $config = SVN::Core::config_get_config($config_dir);
> +               my ($p, @a);
> +              # config_get_config returns all config files from
> +              # ~/.subversion, auth_get_platform_specific_client_providers
> +              # just wants the config "file".
> +               @a = ($config->{'config'}, undef);
> +               $p = SVN::Core::auth_get_platform_specific_client_providers(@a);
> +              # Insert the return value from
> +              # auth_get_platform_specific_providers
> +               unshift @rv, @$p;
> +       }
> +       \@rv;
>  }
>  
>  sub escape_uri_only {
> 



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] git-svn: enable platform-specific authentication

[Nikolaus Demmel]

Matthijs Kooijman wrote
> 
> I sent the below patch a few months ago, and not having it applied in
> git-svn bit me again just now. Did any of you get a chance to have a
> look at it?
> 
> I'm still not 100% sure if this patch is correct for all the corner
> cases, but it works like a charm in the regular case.
> 
> Perhaps it should just be included as is?
> 

Hi,

is this patch also meant to deal with / fix the handling the keychain as an
authentication handler on OS X?

Is there anything I could do to help getting this moving forward? I could
try test it on OS X, if noone else can.

Cheers,
Nikolaus


--
View this message in context: http://git.661346.n2.nabble.com/PATCH-git-svn-enable-platform-specific-authentication-tp6376961p7298038.html
Sent from the git mailing list archive at Nabble.com.