Re: [PATCH v2] credential-osxkeychain: support more protocols

2013-05-27 Thread Jeff King
On Mon, May 27, 2013 at 11:46:24PM +0800, Xidorn Quan wrote:

> > We may also eventually want IMAP for git-imap-send, but we have not yet
> > implemented credential-helper support there. We may also want http/socks
> > proxy authentication, but we also have not implemented the git side of
> > that yet. So I think both of those can wait for now.
> 
> Hope the helpers will be implemented soon. IMO, we can add IMAP and
> SOCKS for now since the protocol names are clear, while it is unclear
> what protocol name will be used for HTTP/HTTPS proxy. I guess that
> some may prefer using http/https as protocol name instead of something
> specific. What do you think?

Yes, I think git will need to munge the protocol to indicate to the
helper that it is a proxy (or alternatively, add a new key "proxy=1" or
something). So it would make sense to wait until the git side of the
code materializes so that the helpers knows what to expect.

There was some work done in this area last May, but there are a lot of
corner cases with knowing exactly what the proxy URL is (because in some
cases, curl pulls it from the environment without git's knowledge).
There was a patch series under consideration for libcurl that would let
us supply an "authentication callback" to curl that would be called when
credentials were needed. But I have not kept track of the state of that
patch and whether it ever got merged upstream into libcurl.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] credential-osxkeychain: support more protocols

2013-05-27 Thread Xidorn Quan
On Mon, May 27, 2013 at 11:08 PM, Jeff King  wrote:
[snip]
>
> This looks good to me. Git will ask for "protocol=ftp" when
> accessing the dumb protocol over ftp. And it will ask for smtp via
> git-send-email since 4d31a44 (git-send-email: use git credential to
> obtain password, 2013-02-12).
>
> While we are in the area it may be worth thinking if there are other
> schemes we would want to support. Git might feed any URL scheme that
> curl accepts, so I think we would want to handle FTPS alongside FTP, no?

Good point, I'll add that.

> We may also eventually want IMAP for git-imap-send, but we have not yet
> implemented credential-helper support there. We may also want http/socks
> proxy authentication, but we also have not implemented the git side of
> that yet. So I think both of those can wait for now.

Hope the helpers will be implemented soon. IMO, we can add IMAP and
SOCKS for now since the protocol names are clear, while it is unclear
what protocol name will be used for HTTP/HTTPS proxy. I guess that
some may prefer using http/https as protocol name instead of something
specific. What do you think?

--
Xidorn Quan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] credential-osxkeychain: support more protocols

2013-05-27 Thread Jeff King
On Mon, May 27, 2013 at 10:35:59PM +0800, Xidorn Quan wrote:

> diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c 
> b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> index 3940202..648fadd 100644
> --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> @@ -127,10 +127,14 @@ static void read_credential(void)
>   *v++ = '\0';
>  
>   if (!strcmp(buf, "protocol")) {
> - if (!strcmp(v, "https"))
> + if (!strcmp(v, "ftp"))
> + protocol = kSecProtocolTypeFTP;
> + else if (!strcmp(v, "https"))
>   protocol = kSecProtocolTypeHTTPS;
>   else if (!strcmp(v, "http"))
>   protocol = kSecProtocolTypeHTTP;
> + else if (!strcmp(v, "smtp"))
> + protocol = kSecProtocolTypeSMTP;
>   else /* we don't yet handle other protocols */
>   exit(0);

This looks good to me. Git will ask for "protocol=ftp" when
accessing the dumb protocol over ftp. And it will ask for smtp via
git-send-email since 4d31a44 (git-send-email: use git credential to
obtain password, 2013-02-12).

While we are in the area it may be worth thinking if there are other
schemes we would want to support. Git might feed any URL scheme that
curl accepts, so I think we would want to handle FTPS alongside FTP, no?

We may also eventually want IMAP for git-imap-send, but we have not yet
implemented credential-helper support there. We may also want http/socks
proxy authentication, but we also have not implemented the git side of
that yet. So I think both of those can wait for now.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html