Patchwork [V2] url: added authuri when login information is requested (issue3209)

login
register
mail settings
Submitter Lucas Moscovicz
Date Jan. 17, 2014, 12:47 a.m.
Message ID <c9b8a862e29a5b80adce.1389919655@devrs047.prn2.facebook.com>
Download mbox | patch
Permalink /patch/3362/
State Superseded
Commit 7d589d923b8a1ae7b920a88cf66a79d3faa330b5
Headers show

Comments

Lucas Moscovicz - Jan. 17, 2014, 12:47 a.m.
# HG changeset patch
# User Lucas Moscovicz <lmoscovicz@fb.com>
# Date 1389833180 28800
#      Wed Jan 15 16:46:20 2014 -0800
# Node ID c9b8a862e29a5b80adce2d314f458f71b97e6dae
# Parent  f694cd81b600b65d23dcdc7a02cfd6a57dd1d018
url: added authuri when login information is requested (issue3209)

When users are using a revset they can get multiple password prompts.
This prompts have no extra information about which password is being requested
so I added the authuri to the prompt to make it recognizable.

As in:
$ hg log -r "outgoing('https://bitbucket.org/mg/test') -
outgoing('https://bitbucket.org/nesneros/test')"
http authorization required
realm: Bitbucket.org HTTP
user: interrupted!

I changed it to describe the url when prompting for password.
As in:
$ hg log -r "outgoing('https://bitbucket.org/mg/test') -
outgoing('https://bitbucket.org/nesneros/test')"
http authorization required for https://bitbucket.org/mg/test
realm: Bitbucket.org HTTP
user: interrupted!
Mads Kiilerich - Jan. 17, 2014, 1:08 a.m.
On 01/17/2014 01:47 AM, Lucas Moscovicz wrote:
> # HG changeset patch
> # User Lucas Moscovicz <lmoscovicz@fb.com>
> # Date 1389833180 28800
> #      Wed Jan 15 16:46:20 2014 -0800
> # Node ID c9b8a862e29a5b80adce2d314f458f71b97e6dae
> # Parent  f694cd81b600b65d23dcdc7a02cfd6a57dd1d018
> url: added authuri when login information is requested (issue3209)
>
> When users are using a revset they can get multiple password prompts.
> This prompts have no extra information about which password is being requested
> so I added the authuri to the prompt to make it recognizable.
>
> As in:
> $ hg log -r "outgoing('https://bitbucket.org/mg/test') -
> outgoing('https://bitbucket.org/nesneros/test')"
> http authorization required
> realm: Bitbucket.org HTTP
> user: interrupted!
>
> I changed it to describe the url when prompting for password.
> As in:
> $ hg log -r "outgoing('https://bitbucket.org/mg/test') -
> outgoing('https://bitbucket.org/nesneros/test')"
> http authorization required for https://bitbucket.org/mg/test
> realm: Bitbucket.org HTTP
> user: interrupted!

Nice - and nice tests.

Some very minor comments:

> diff --git a/mercurial/url.py b/mercurial/url.py
> --- a/mercurial/url.py
> +++ b/mercurial/url.py
> @@ -32,10 +32,13 @@
>                   user, passwd = auth.get('username'), auth.get('password')
>                   self.ui.debug("using auth.%s.* for authentication\n" % group)
>           if not user or not passwd:
> +            u = util.url(authuri)
> +            u.query=None

Are you sure we never end up with a http://user:pass@dom/ URL with 
(invalid?) credentials here? It might deserve a comment so someone else 
don't come back later and add a hidepassword() here.

>               if not self.ui.interactive():
> -                raise util.Abort(_('http authorization required'))
> +                raise util.Abort(_('http authorization required for %s') %
> +                str(u))

That is odd indentation. (The str is not necessary ... but not do any 
harm.)

>   
> -            self.ui.write(_("http authorization required\n"))
> +            self.ui.write(_("http authorization required for %s\n") % str(u))
>               self.ui.write(_("realm: %s\n") % realm)
>               if user:
>                   self.ui.write(_("user: %s\n") % user)
> diff --git a/tests/test-http.t b/tests/test-http.t
> --- a/tests/test-http.t
> +++ b/tests/test-http.t
> @@ -156,12 +156,24 @@
>     >    --config server.preferuncompressed=True
>     $ cat pid >> $DAEMON_PIDS
>   
> +  $ cat << EOF > get_pass.py
> +  > import getpass
> +  > def new_getpass(arg):
> +  >   return "pass"
> +  > getpass.getpass = new_getpass
> +  > EOF

Mercurial do usually not use '_' in names.

/Mads

Patch

diff --git a/mercurial/url.py b/mercurial/url.py
--- a/mercurial/url.py
+++ b/mercurial/url.py
@@ -32,10 +32,13 @@ 
                 user, passwd = auth.get('username'), auth.get('password')
                 self.ui.debug("using auth.%s.* for authentication\n" % group)
         if not user or not passwd:
+            u = util.url(authuri)
+            u.query=None
             if not self.ui.interactive():
-                raise util.Abort(_('http authorization required'))
+                raise util.Abort(_('http authorization required for %s') %
+                str(u))
 
-            self.ui.write(_("http authorization required\n"))
+            self.ui.write(_("http authorization required for %s\n") % str(u))
             self.ui.write(_("realm: %s\n") % realm)
             if user:
                 self.ui.write(_("user: %s\n") % user)
diff --git a/tests/test-http.t b/tests/test-http.t
--- a/tests/test-http.t
+++ b/tests/test-http.t
@@ -156,12 +156,24 @@ 
   >    --config server.preferuncompressed=True
   $ cat pid >> $DAEMON_PIDS
 
+  $ cat << EOF > get_pass.py
+  > import getpass
+  > def new_getpass(arg):
+  >   return "pass" 
+  > getpass.getpass = new_getpass
+  > EOF
+
   $ hg id http://localhost:$HGPORT2/
-  abort: http authorization required
+  abort: http authorization required for http://localhost:$HGPORT2/
   [255]
-  $ hg id http://user@localhost:$HGPORT2/
-  abort: http authorization required
+  $ hg id http://localhost:$HGPORT2/
+  abort: http authorization required for http://localhost:$HGPORT2/
   [255]
+  $ hg id --config ui.interactive=true --config extensions.getpass=get_pass.py http://user@localhost:$HGPORT2/
+  http authorization required for http://localhost:$HGPORT2/
+  realm: mercurial
+  user: user
+  password: 5fed3813f7f5
   $ hg id http://user:pass@localhost:$HGPORT2/
   5fed3813f7f5
   $ echo '[auth]' >> .hg/hgrc
@@ -183,7 +195,7 @@ 
   5 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
   $ hg id http://user2@localhost:$HGPORT2/
-  abort: http authorization required
+  abort: http authorization required for http://localhost:$HGPORT2/
   [255]
   $ hg id http://user:pass2@localhost:$HGPORT2/
   abort: HTTP Error 403: no