[PATCH] allow http authentication information to be specified in the configuration (2)

[Dirkjan Ochtman]

Somewhat superficial review:

On 22/04/2009 16:20, Sune Foldager wrote:
> New version with support for schemas in the prefix and some fixes in the documentation.

The URI RFC calls it a scheme, not schema. We should probably comply 
with that.

 > # HG changeset patch
> # User Sune Foldager <cryo at cyanite.org>
> # Date 1240409959 -7200
> # Node ID 6f75c801af5647f3667d358d002f3f1ff35eb2c1
> # Parent  28a72f620cdef2883d1332a5edcc14b05224fd7b
> allow http authentication information to be specified in the configuration
>
> diff --git a/doc/hgrc.5.txt b/doc/hgrc.5.txt
> --- a/doc/hgrc.5.txt
> +++ b/doc/hgrc.5.txt
> @@ -100,6 +100,45 @@
>   Mercurial "hgrc" file, the purpose of each section, its possible
>   keys, and their possible values.
>
> +[[auth]]
> +auth:
> +  Authentication credentials for HTTP authentication.
> +  Each line has the following format:
> +
> +<name>.<argument>  =<value>
> +
> +  where<name>  is used to group arguments into authentication entries.

Weird spacing? Might be due to email client.

> +  Example:
> +
> +    foo.prefix = hg.intevation.org/mercurial
> +    foo.credentials = foo:bar
> +    foo.schemas = http https

So change the option name here, too.

> +
> +  Supported arguments:
> +
> +  prefix;;
> +    Either '*' or an URI prefix with or without the schema part. The
> +    authentication entry with the longest matching prefix is used
> +    (where '*' matches everything and counts as a match of length 1).
> +    If the prefix doesn't include a schema, the match is performed against
> +    the URI with its schema stripped as well, and the schemas argument,
> +    q.v., is then subsequently consulted.
> +    If this argument is not given, the authentication entry is skipped.
> +  credentials;;
> +    Username-password pair to authenticate with, in the same format used
> +    in URIs. Password is optional and if left out, the user will be
> +    prompted for it.
> +    If this argument is not given, the authentication entry is skipped.
> +  schemas;;
> +    Space separated list of URI-schemas to use this authentication entry
> +    with. Only used if the prefix doesn't include a schema.
> +    Supported schemas are http and https. They will match static-http
> +    and static-https respectively, as well.
> +    Default: https.
> +
> +  If no suitable authentication entry is found, the user is prompted for
> +  credentials as usual if required by the remote.
> +
>   [[decode]]
>   decode/encode::
>     Filters for transforming files on checkout/checkin. This would
> diff --git a/mercurial/url.py b/mercurial/url.py
> --- a/mercurial/url.py
> +++ b/mercurial/url.py
> @@ -105,24 +105,65 @@
>               self, realm, authuri)
>           user, passwd = authinfo
>           if user and passwd:
> +            self._writedebug(user, passwd)
>               return (user, passwd)
>
> -        if not self.ui.interactive:
> -            raise util.Abort(_('http authorization required'))
> +        user, passwd = self._readauthtoken(authuri)
> +        if not user or not passwd:
> +            if not self.ui.interactive:
> +                raise util.Abort(_('http authorization required'))
>
> -        self.ui.write(_("http authorization required\n"))
> -        self.ui.status(_("realm: %s\n") % realm)
> -        if user:
> -            self.ui.status(_("user: %s\n") % user)
> -        else:
> -            user = self.ui.prompt(_("user:"), default=None)
> +            self.ui.write(_("http authorization required\n"))
> +            self.ui.status(_("realm: %s\n") % realm)
> +            if user:
> +                self.ui.status(_("user: %s\n") % user)
> +            else:
> +                user = self.ui.prompt(_("user:"), default=None)
>
> -        if not passwd:
> -            passwd = self.ui.getpass()
> +            if not passwd:
> +                passwd = self.ui.getpass()
>
>           self.add_password(realm, authuri, user, passwd)
> +        self._writedebug(user, passwd)
>           return (user, passwd)
>
> +    def _writedebug(self, user, passwd):
> +        self.ui.debug(_('http auth: user %s, password %s\n') %
> +                 (user, passwd and '*' * len(passwd) or 'not set'))

I usually prefer it if lines like those are split into an intermediate 
variable, then the actual call (but that's somewhat personal).

> +
> +    def _readauthconfig(self):
> +        config = dict()
> +        for key, val in self.ui.configitems('auth'):
> +            group, setting = key.split('.', 1)
> +            if group in config:
> +                d = config[group]
> +            else:
> +                config[group] = d = dict()
> +            d[setting] = val
> +        return config

This could use some dict.setdefault(). That should make it much more 
compact, meaning you should probably merge it with the method below 
(which I think makes more sense from an API standpoint, too).

> +
> +    def _readauthtoken(self, uri):
> +        config = self._readauthconfig()
> +        schema, hostpath = uri.split('://', 1)
> +        bestlen = 0
> +        bestauth = None
> +
> +        for auth in config.itervalues():
> +            prefix = auth.get('prefix')
> +            if not prefix: continue
> +            p = prefix.split('://', 1)
> +            if len(p)>  1:
> +                schemas, prefix = [p[0]], p[1]
> +            else:
> +                schemas = (auth.get('schemas') or 'https').split()
> +            if (prefix == '*' or hostpath.startswith(prefix)) and len(prefix)  >  bestlen and schema in schemas:

I *think* this line is a little long. Might be another spacing issue?

> +                bestlen = len(prefix)
> +                bestauth = auth.get('credentials')
> +
> +        if bestauth:
> +            return netlocsplit(bestauth + '@dummy')[2:4]
> +        return None, None
> +
>   class proxyhandler(urllib2.ProxyHandler):
>       def __init__(self, ui):
>           proxyurl = ui.config("http_proxy", "host") or os.getenv('http_proxy')
> @@ -285,9 +326,6 @@
>       passmgr = passwordmgr(ui)
>       if authinfo is not None:
>           passmgr.add_password(*authinfo)
> -        user, passwd = authinfo[2:4]
> -        ui.debug(_('http auth: user %s, password %s\n') %
> -                 (user, passwd and '*' * len(passwd) or 'not set'))
>
>       handlers.extend((urllib2.HTTPBasicAuthHandler(passmgr),
>                        httpdigestauthhandler(passmgr)))

Cheers,

Dirkjan