threads / patch / 27392

patchgit-svn: enable platform-specific authentication

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

## tl;dr

5 messages between May 18, 2011 and Feb 19, 2012. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Gustav Munkby· May 18, 2011, 08:45 UTC · lore

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(-)
Show changes to git-svn.perl +3 −0
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
Eric Wong· May 18, 2011, 19:57 UTC · re: Gustav Munkby · lore

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

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.

Show 5 quoted lines
>  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
Matthijs Kooijman· Aug 9, 2011, 21:06 UTC · re: Eric Wong · lore

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

Hey folks,
Show 5 quoted lines
> > 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.
Show 11 quoted lines
> >  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
Show changes to git-svn.perl +18 −2
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 {
Matthijs Kooijman· Jan 3, 2012, 20:44 UTC · re: Matthijs Kooijman · lore

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

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:
Show 96 quoted lines
> 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 {
> 
Nikolaus Demmel· Feb 19, 2012, 04:06 UTC · re: Matthijs Kooijman · lore

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

Matthijs Kooijman wrote
Show 10 quoted lines
> 
> 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.

← back to recent threads