bzr asks for password twice while checking out or updating .htaccess protected http folder

[Fabio Varesano]

I'm using bzr 2.1.2. I have an http server which has the repository folder .htaccess protected.. I use that as checkout/update location. Each time I co or up I have to insert the password twice.

mgz on irc suggested to use authentication.conf.. This probably fixed that, but I'm not going to insert my password in plain text on my production webserver.

I run bzr up -Dhttp and the results are at http://pastebin.com/m7GsGSjB

Thanks,

Fabio

[Vincent Ladeuil (vila)]

ALERT: Your password is trivially accessible from the pastebin, you'd better change it right now.

[Fabio Varesano (fabio-varesano)]

Thanks Vincent!

[Martin Packman (gz)]

One workaround I suggested on irc was using authentication.conf to store username and password rather than entering on the terminal. Another option is to use a url in the form nosmart+http://... and use the --remember flag to keep that as the parent branch location.

The basic issue is that an http request first probes for bzr+http even for a normal http url. If the server doesn't support this, an exception is thrown and the connection and credentials are not recorded. Therefore when the plain request is attempted next, it prompts for a second time. I'm not sure if always preserving the connection and credentials regardless of response is correct, but that does seem to fix this bug.

A fun variation of this problem is trying `bzr branch bzr+http://...` from a smartless server, it prompts for credentials twice and always fails.

[Vincent Ladeuil (vila)]

I agree with Martin's analysis, having encountered the issue in tests.

The fix can be trivial (record the connection and its credentials in a 'finally' clause) but its fallouts are unclear:
some exceptions mean that the connection has been closed by the server in which case the credentials may
or may not be valid and the next connection may or not reuse them. Now, it may be worth checking that the
*credentials* have been validated and record them even if the connection has already been lost.

[Martin Packman (gz)]

Wrapping in try/finally is what I tried, which indeed doesn't really feel like right thing. Also problematic are that this is in bzrlib.transport.http._urllib so presumably pycurl will need some kind of change as well, and this looks a pain to test. Think it really wants a blackbox style test to ensure the console interactions are sane, but doing that across http libraries and authentication methods, which avoiding leaking server threads, looks complicated.

[Vincent Ladeuil (vila)]

>>>>> Martin [gz] <email address hidden> writes:

    > Wrapping in try/finally is what I tried, which indeed doesn't really
    > feel like right thing. Also problematic are that this is in
    > bzrlib.transport.http._urllib so presumably pycurl will need some kind
    > of change as well,

Well, pycurl there is kind of a blackbox and you need to provide the
credentials in the url. pycurl just can't prompt either.

    > and this looks a pain to test.

Nope, all the hard work has already been done in test_http.py, all the
tests auth are parametrized for all http implementations (and then
some).

    > Think it really wants a blackbox style test to ensure the console
    > interactions are sane, but doing that across http libraries and
    > authentication methods, which avoiding leaking server threads,
    > looks complicated.

Either it's fixed after the leaking-tests proposal is landed or before
but in both cases taking care of leaks is out of scope ;)

It would be simpler to fix it after the landing though, so I've added
the relevant tags.

Status confimed and importance medium are appropriate IMHO, but patches
welcome !

[Vincent Ladeuil (vila)]

Also, the -Dhttp behavior should be fixed to never display credentials in debug output however they are encrypted.
Devs would always be able to get them back by modifying the debug code if and when the need arise.