Re: [BUG?] google code http auth weirdness
On Sat, Mar 16, 2013 at 06:11:32PM -0700, Shawn O. Pearce wrote: > > That seems kind of crazy to me. It's generating an HTTP 200 just to tell > > us the credentials are wrong. Which kind of makes sense; it's the only > > way to convince the git client to show a custom message when it aborts > > (rather than producing its own client-side "authorization failed" > > message). > > Actually the reason here wasn't to use a custom message. It was to > avoid the client from entering into the old /info/refs fallback path > that existed between 703e6e76 (Git 1.6.6.2) and 6ac964a62 (Git > 1.7.12.3). During these versions non-200 responses from the smart > request usually led to a useless error in the client. I suggested to > the Google Code team when they implemented Git support to use a 200 > response with the ERR header to give the end-user something easier to > understand than what the Git client have otherwise printed. OK, that makes sense. I do wonder, though, if the loss of the custom message will be bad for Google Code. The current message is: fatal: remote error: Invalid username/password. You may need to use your generated googlecode.com password; see https://code.google.com/hosting/settings which is an important clue (this being the first time I had used Google Code to authenticate, I was quite surprised that the password was different from the web page's password, and that pointer helped me figure it out more quickly). > > 1. Always return a 401 for bad credentials. This means it would lose > > the custom message. But I think that is a good indication that the > > client should be putting more effort into showing the body of the > > 401. Probably not all the time, as we do not want to spew HTML at > > the user, but we could perhaps relay error bodies if the > > content-type is "application/x-git-error" or something. > > > > The downside is that even if we make such a client change and the > > the Google Code server change sit's behavior, people on older git > > clients will lose the benefit of the message. > > I don't think this is the way to handle errors in the protocol. I > prefer the existing approach of sending a 200 OK with the ERR header > to indicate the message to show to the client. It works since 1.6.6 > when smart HTTP was introduced, and it has a very specific meaning. > The 200 is stating the transport worked, and the ERR is stating the > Git operation did not. :-) Sorry, I should have been more clear. I mean only to use it for transport errors, not git errors. So we should not in general be converting 200 responses into something else (with the exception of the 401, because it does have a transport meaning). But when we _do_ issue an unsuccessful HTTP code, we should include a message that can be relayed to the user. > Where we have an issue is on a recoverable authentication error. > Recoverable authentication errors should use 401 so the client can try > again. I don't know how older (e.g. 1.6.6) clients behave here with a > 401 response. I guess I should crack out a 1.6.6 build and test. Before 42653c0 (Prompt for a username when an HTTP request 401s, 2010-04-01), in v1.7.1.1, I think you just get something like: error: 401 Authorization failed fatal: HTTP request failed > I don't work on Google Code, but I have asked the team to consider > making at least this change. Thanks for looking into it. -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: [BUG?] google code http auth weirdness
On Fri, Mar 15, 2013 at 4:59 AM, Jeff King wrote:
> I tried pushing to a repository at Google Code for the first time today,
> and I encountered some weird behavior with respect to asking for
> credentials.
>
> If I use the url "https://code.google.com/r/repo/";, everything works; I
> get prompted for my username/password.
>
> But if I instead use the url "https://myu...@code.google.com/r/repo/";,
> it doesn't work. I get:
>
> $ git push
> fatal: remote error: Invalid username/password.
> You may need to use your generated googlecode.com password; see
> https://code.google.com/hosting/settings
>
> without any prompt at all. I bisected it to 986bbc0 (http: don't always
> prompt for password, 2011-11-04), but I think that is a red herring. It
> worked before that commit because we always asked for the password,
> before we even talked to the server.
>
> After that commit, we should be reacting to an HTTP 401. But it seems that
> Google Code's 401 behavior is odd. When t5540 checks this, the
> conversation goes something like:
>
> 1. We do a GET with no "Authorization" header.
>
> 2. The server returns a 401, asking for credentials.
>
> 3. Curl repeats the GET request, putting the username into the
> Authorization header.
>
> 4. The server returns a 401, again, as our credential is not
> sufficient.
>
> 5. Curl returns the 401 to git; git prompts for the credential, feeds
> it to curl, and then repeats the request. It works.
>
> But with Google Code, the first three steps are identical. But then for
> step 4, the server returns this:
>
> < HTTP/1.1 200 OK
> < Content-Type: application/x-git-receive-pack-advertisement
> < X-Content-Type-Options: nosniff
> < Date: Fri, 15 Mar 2013 11:43:14 GMT
> < Server: git_frontend
> < Content-Length: 175
> < X-XSS-Protection: 1; mode=block
> <
> * Connection #0 to host code.google.com left intact
> packet: git< # service=git-receive-pack
> packet: git<
> packet: git< ERR Invalid username/password [...]
>
> That seems kind of crazy to me. It's generating an HTTP 200 just to tell
> us the credentials are wrong. Which kind of makes sense; it's the only
> way to convince the git client to show a custom message when it aborts
> (rather than producing its own client-side "authorization failed"
> message).
Actually the reason here wasn't to use a custom message. It was to
avoid the client from entering into the old /info/refs fallback path
that existed between 703e6e76 (Git 1.6.6.2) and 6ac964a62 (Git
1.7.12.3). During these versions non-200 responses from the smart
request usually led to a useless error in the client. I suggested to
the Google Code team when they implemented Git support to use a 200
response with the ERR header to give the end-user something easier to
understand than what the Git client have otherwise printed.
But in hindsight maybe I should have told them to always return 401
and let the client handle the error.
FWIW this same "misfeature" probably exists in Gerrit Code Review and
does exist on {gerrit,android}.googlesource.com because it also came
from me.
> But it takes the retry decision out of the client's hands. And
> in this case, it is wrong; the failed credential is a result of curl
> trying the username only, and git never even gets a chance to provide
> the real credential.
>
> Technically this did work before 986bbc0, so it could be considered a
> regression in git, but I really think that Google Code is in the wrong
> here. It should either:
>
> 1. Always return a 401 for bad credentials. This means it would lose
> the custom message. But I think that is a good indication that the
> client should be putting more effort into showing the body of the
> 401. Probably not all the time, as we do not want to spew HTML at
> the user, but we could perhaps relay error bodies if the
> content-type is "application/x-git-error" or something.
>
> The downside is that even if we make such a client change and the
> the Google Code server change sit's behavior, people on older git
> clients will lose the benefit of the message.
I don't think this is the way to handle errors in the protocol. I
prefer the existing approach of sending a 200 OK with the ERR header
to indicate the message to show to the client. It works since 1.6.6
when smart HTTP was introduced, and it has a very specific meaning.
The 200 is stating the transport worked, and the ERR is stating the
Git operation did not. :-)
Where we have an issue is on a recoverable authentication error.
Recoverable authentication errors should use 401 so the client can try
again. I don't know how older (e.g. 1.6.6) clients behave here with a
401 response. I guess I should crack out a 1.6.6 build and test.
> 2. Treat a credential with a non-empty username and an empty password
> specially, and return a 401. This covers the specific case of
> https://user@host/, but continu
[BUG?] google code http auth weirdness
I tried pushing to a repository at Google Code for the first time today, and I encountered some weird behavior with respect to asking for credentials. If I use the url "https://code.google.com/r/repo/";, everything works; I get prompted for my username/password. But if I instead use the url "https://myu...@code.google.com/r/repo/";, it doesn't work. I get: $ git push fatal: remote error: Invalid username/password. You may need to use your generated googlecode.com password; see https://code.google.com/hosting/settings without any prompt at all. I bisected it to 986bbc0 (http: don't always prompt for password, 2011-11-04), but I think that is a red herring. It worked before that commit because we always asked for the password, before we even talked to the server. After that commit, we should be reacting to an HTTP 401. But it seems that Google Code's 401 behavior is odd. When t5540 checks this, the conversation goes something like: 1. We do a GET with no "Authorization" header. 2. The server returns a 401, asking for credentials. 3. Curl repeats the GET request, putting the username into the Authorization header. 4. The server returns a 401, again, as our credential is not sufficient. 5. Curl returns the 401 to git; git prompts for the credential, feeds it to curl, and then repeats the request. It works. But with Google Code, the first three steps are identical. But then for step 4, the server returns this: < HTTP/1.1 200 OK < Content-Type: application/x-git-receive-pack-advertisement < X-Content-Type-Options: nosniff < Date: Fri, 15 Mar 2013 11:43:14 GMT < Server: git_frontend < Content-Length: 175 < X-XSS-Protection: 1; mode=block < * Connection #0 to host code.google.com left intact packet: git< # service=git-receive-pack packet: git< packet: git< ERR Invalid username/password [...] That seems kind of crazy to me. It's generating an HTTP 200 just to tell us the credentials are wrong. Which kind of makes sense; it's the only way to convince the git client to show a custom message when it aborts (rather than producing its own client-side "authorization failed" message). But it takes the retry decision out of the client's hands. And in this case, it is wrong; the failed credential is a result of curl trying the username only, and git never even gets a chance to provide the real credential. Technically this did work before 986bbc0, so it could be considered a regression in git, but I really think that Google Code is in the wrong here. It should either: 1. Always return a 401 for bad credentials. This means it would lose the custom message. But I think that is a good indication that the client should be putting more effort into showing the body of the 401. Probably not all the time, as we do not want to spew HTML at the user, but we could perhaps relay error bodies if the content-type is "application/x-git-error" or something. The downside is that even if we make such a client change and the the Google Code server change sit's behavior, people on older git clients will lose the benefit of the message. 2. Treat a credential with a non-empty username and an empty password specially, and return a 401. This covers the specific case of https://user@host/, but continues to show the custom message when the user provides a wrong password. It does mean that the custom message will not be shown if the user actually enters a blank password at the prompt (but it will still be shown if they get prompted for both username and password and leave both blank). So it's a little hacky, but I think it would work OK in practice. What do you think? -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
