[PATCH] AuthenticationConfig patch to provide expected keys to credential store plug-ins

[Vincent Ladeuil]

>>>>> "jfroy" == Jean-Francois Roy <bahamut at macstorm.org> writes:

    jfroy> In addition, the netrc credential store plug-in as well as
    jfroy> its tests are designed under the assumption the credentials
    jfroy> dictionary will contain a "host" key, when it fact this would
    jfroy> never be the case.

This is the case for the default section only since all sections in
.netrc are by host.

    jfroy> In other words, without this patch, the netrc credential
    jfroy> store plug-in bundled with Bazaar is inoperative.

I don't think so, but anyway, that's not the point :)

    jfroy> Relevant tests have been updated

You missed the ones you pointed out above: the netrc_credential_store
ones :-)

Since they were the root cause of the problem you're addressing here
(they test the lower level API, masking the problem to the upper
levels), it would be best to update them by using an
AuthenticationConfig object on top of the NetrcCredentialStore one and
use get_password instead of decode_password directly.

Or even better *add* a new single test that reproduce the problem
without your patch and pass with your patch.

<snip/>

    jfroy> === modified file 'NEWS'
    jfroy> --- NEWS	2009-03-05 09:12:17 +0000
    jfroy> +++ NEWS	2009-03-05 18:28:58 +0000
    jfroy> @@ -110,6 +110,12 @@
    jfroy>        when needed.
    jfroy>        (Vincent Ladeuil)
 
    jfroy> +    * Corrected a disconnect between ``AuthenticationConfig`` and credential 
    jfroy> +      provider plug-ins where the credentials dictionary passed to 
    jfroy> +      providers would not contain expected keys, as tested by the netrc 
    jfroy> +      provider plug-in tests.
    jfroy> +      (Jean-Francois Roy)
    jfroy> +

Documenting what didn't work is good but documenting what is working
*now* is better :)

How about:

authentication plugins now receive all the parameters from the request
itself (aka host, port, realm, path, etc), only the parameters defined
in the authentication section were provided previously.

    jfroy> === modified file 'bzrlib/config.py'
    jfroy> --- bzrlib/config.py	2009-01-17 01:30:58 +0000
    jfroy> +++ bzrlib/config.py	2009-03-05 18:15:54 +0000

<snip/>

    jfroy> @@ -1057,12 +1060,26 @@
 
<snip/>

    jfroy> +            # Delete all extra keys from the credentials dictionary
    jfroy> +            del credentials['scheme']
    jfroy> +            del credentials['host']
    jfroy> +            del credentials['port']
    jfroy> +            del credentials['path']
    jfroy> +            del credentials['realm']

I understand you comply with the doc string here, I wonder if we should
left these keys in place an update the doc string instead :)

BB:tweak

Note to other reviewer, I'd be happy to merge that patch myself and
handle any needed tweak.

     Vincent