[MERGE][bug #300347] Don't require "user@" in HTTP(S) URLs that need auth (was Re: [MERGE] [bug #256612])

[John Arbash Meinel]

John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
+    def test_user_from_auth_conf(self):
+        if self._testing_pycurl():
+            raise tests.TestNotApplicable(
+                'pycurl does not support authentication.conf')
+        user =' joe'

^- This looks like a typo, and you really want:
   user = 'joe'

Though perhaps you wanted to test user names that start with a space?
   user = ' joe'

+        password = 'foo'
+        self.server.add_user(user, password)
+        # Create a minimal config file with the right password
+        conf = config.AuthenticationConfig()
+        conf._get_config().update(
+            {'httptest': {'scheme': 'http', 'port': self.server.port,
+                          'user': user, 'password': password}})
+        conf._save()
+        t = self.get_user_transport()
+        # Issue a request to the server to connect
+        self.assertEqual('contents of a\n',t.get('a').read())

^- a space here after ',' would be good.
Doing "get_user_transport(user=None) would be a bit clearer *to me* that 
you
are ensuring the URL didn't include a known user. Especially since the 
name of
the function is "..._user_..." I went around and checked that it 
actually was
doing the right thing. It may not be true for others, though.

+        # Only one 'Authentication Required' error should occur
+        self.assertEqual(1, self.server.auth_required_errors)
+

I find this rather confusing:
     def get_user_password(self, auth):
         """Ask user for a password if none is already available."""
         auth_conf = config.AuthenticationConfig()
         user = auth['user']
         password = auth['password']
         realm = auth['realm']

What is "auth" versus "auth_conf". Namely, it is what changed in this 
part of
the patch:

          if user is None:
-            user = auth.get_user(auth['protocol'], auth['host'],
-                                 port=auth['port'], path=auth['path'],
-                                 realm=realm)
-            if user is None:
-                # Default to local user
-                user = getpass.getuser()
-
-        if password is None:
+            user = auth_conf.get_user(auth['protocol'], auth['host'],
+                                      port=auth['port'], 
path=auth['path'],
+                                      realm=realm)
+        if user is not None and password is None:

Perhaps just documenting what the "auth" parameter is, and then maybe 
adding a
comment about how "auth_conf" differs from "auth". (Then again, maybe 
I'm only
confused because I don't know the code well enough.)

Or perhaps the confusion is because whatever "auth" is, it somehow has 
the
property that:

auth['user'] can be None, but "auth.get_user(...)" can return something 
useful,
but apparently not as useful as "auth_conf.get_user(...)".


For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3Cm23agkctos.fsf_-_%40free.fr%3E
Project: Bazaar