[MERGE][bug #203186] Ignore passwords for ssh in authentication.conf and warn user

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
|>>>>> "john" == John Arbash Meinel <john at arbash-meinel.com> writes:
|
|     john> Vincent Ladeuil wrote:
|     john> ~         if credentials is not None:
|     john> ~             password = credentials['password']
|     john> +            if scheme is 'ssh':
|     john> +                trace.warning('password ignored in section [%s],'
|     john> +                              ' use an ssh agent instead'
|     john> +                              % credentials['name'])
|     john> +                password = None
|     john> ~         else:
|     john> ~             password = None
|     john> ~         # Prompt user only if we could't find a password
|     john> ~         if password is None:
|     john> ~             if prompt is None:
|     john> -                # Create a default prompt suitable for most of the
cases
|     john> +                # Create a default prompt suitable for most cases
|     john> ~                 prompt = '%s' % scheme.upper() + '
%(user)s@%(host)s password'
|     john> ~             # Special handling for optional fields in the prompt
|     john> ~             if port is not None:
|
|     john> ^- Doesn't that 'if' check need to be:
|
|     john> if password is not None and scheme is 'ssh':
|
| Doh. I shouldn't code while sleeping.
|
| Are you ok with the message or do you have a better proposal ?

It is ok. Though if you are going to say "do XX" instead, I would sort of like
to have a "bzr help XX" topic available. Even if it is just a rough sketch of
what to do.


|
|     john> You should add a test that we *don't* issue a warning
|     john> if there is a user but no password.
|
| Done.
|
|     john> We also could use symbol_versioning.warn() which
|     john> already has some code to trap it rather than trying to
|     john> use the TestUIFactory.
|
| Using symbol_versioning looks so strange in that context, nothing
| is related to symbol versioning here, I don't think I get your
| point.

Just that the symbol_versioning.warn() is meant to be overridden by the test
suite for trapping warnings. (We use it for the callDeprecated stuff). I agree
it has nothing to do with symbol_versioning.

|
|     john> TestUI is ok, though.
|
| What is TestUI ? At first I thought it was some class I didn't
| know about, but grepping the sources hits only TestUIFactory >-/
|
| Anyway, the attached patch addresses the points I understood :)
|
|       Vincent


TestUIFactory is what I meant, I just was being lazy about typing.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkgtsQUACgkQJdeBCYSNAAPtVwCgp9d/UR029VaeZ5v1E2T89ak5
rY4An3V9dwrn9PP/gJp+MiEj6Vy+Z4+V
=KMZD
-----END PGP SIGNATURE-----