Rev 2916: Make hhtp proxy aware of AuthenticationConfig (for password). in http://code.launchpad.net/%7Ev-ladeuil/bzr/auth.ring

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon Oct 22 22:19:33 BST 2007


At http://code.launchpad.net/%7Ev-ladeuil/bzr/auth.ring

------------------------------------------------------------
revno: 2916
revision-id:v.ladeuil+lp at free.fr-20071022211913-119445k3qco40sx0
parent: v.ladeuil+lp at free.fr-20071022151824-eol757lk393ofc38
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: auth.ring
timestamp: Mon 2007-10-22 23:19:13 +0200
message:
  Make hhtp proxy aware of AuthenticationConfig (for password).
  
  * bzrlib/transport/http/_urllib2_wrappers.py:
  (Request.__init__): Update the auth dict content documentation.
  (extract_credentials, extract_authentication_uri): Deleted.
  (PasswordManager): Deleted.
  (ProxyHandler.set_proxy): Query AuthenticationConfig for the
  password. Simplified.
  (AbstractAuthHandler): Looong forgotten feedback review about
  _retry_count. The behaviour is not changed.
  (AbstractAuthHandler.get_password): Change signature now that the
  auth dict contains all relevant information. Simplified.
  (DigestAuthHandler.auth_match): Delete gratuitous differences with
  BasicAuthHandler.
  (Opener.__init__): Get rid of PasswordManager.
  
  * bzrlib/transport/http/_urllib.py:
  (HttpTransport_urllib._perform): Replace authuri by its components
  in the auth dict.
  
  * bzrlib/transport/ftp.py: 
  Register 'aftp' for urlparse (revealed by the host empty test in
  _split_url).
  
  * bzrlib/transport/__init__.py:
  (ConnectedTransport._split_url): Check that host is not empty. Was
  first needed to make a proxy test pass, but revealed more bugs in
  the test suite.
  
  * bzrlib/tests/test_transport_implementations.py:
  (TransportTests.test__reuse_for): Fix bug revealed by the host
  empty test in _split_url.
  
  * bzrlib/tests/test_transport.py:
  (TestConnectedTransport.test_connection_sharing_propagate_credentials): 
  Fix bug revealed by the host empty test in _split_url.
  
  * bzrlib/tests/test_smart_transport.py:
  (HTTPTunnellingSmokeTest._test_bulk_data): Fix bug revealed by the
  host empty test in _split_url.
  
  * bzrlib/tests/test_http.py:
  (TestHttpTransportUrls.test_invalid_http_urls): ConnectionError
  can't be raised here.
  (TestHttpProxyWhiteBox._proxied_request): No more PasswordManager,
  yeah !
  (TestProxyHttpServer): Delete FIXME, tests have been added a long
  time ago.
  
  * bzrlib/config.py:
  (AuthenticationConfig.get_user): Fix silly copy/paste.
  (AuthenticationConfig.get_password): Fix oversight.
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
  bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/transport/ftp.py        ftp.py-20051116161804-58dc9506548c2a53
  bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
  bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2007-10-22 15:18:24 +0000
+++ b/bzrlib/config.py	2007-10-22 21:19:13 +0000
@@ -1076,7 +1076,8 @@
 
         :return: The found user.
         """
-        credentials = self.get_credentials(scheme, host, port, user, path)
+        credentials = self.get_credentials(scheme, host, port, user=None,
+                                           path=path)
         if credentials is not None:
             user = credentials['user']
         else:
@@ -1104,6 +1105,8 @@
         credentials = self.get_credentials(scheme, host, port, user, path)
         if credentials is not None:
             password = credentials['password']
+        else:
+            password = None
         if password is None:
             # Prompt user only if we could't find a password
             if prompt is None:

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2007-10-22 15:18:24 +0000
+++ b/bzrlib/tests/test_http.py	2007-10-22 21:19:13 +0000
@@ -79,7 +79,6 @@
     )
 from bzrlib.transport.http._urllib import HttpTransport_urllib
 from bzrlib.transport.http._urllib2_wrappers import (
-    PasswordManager,
     ProxyHandler,
     Request,
     )
@@ -205,7 +204,7 @@
     def test_invalid_http_urls(self):
         """Trap invalid construction of urls"""
         t = self._transport('http://bazaar-vcs.org/bzr/bzr.dev/')
-        self.assertRaises((errors.InvalidURL, errors.ConnectionError),
+        self.assertRaises(errors.InvalidURL,
                           self._transport,
                           'http://http://bazaar-vcs.org/bzr/bzr.dev/')
 
@@ -798,7 +797,7 @@
             osutils.set_or_unset_env(name, value)
 
     def _proxied_request(self):
-        handler = ProxyHandler(PasswordManager())
+        handler = ProxyHandler()
         request = Request('GET','http://baz/buzzle')
         handler.set_proxy(request, 'http')
         return request
@@ -833,10 +832,6 @@
     # FIXME: We don't have an https server available, so we don't
     # test https connections.
 
-    # FIXME: Once the test suite is better fitted to test
-    # authorization schemes, test proxy authorizations too (see
-    # bug #83954).
-
     def setUp(self):
         TestCaseWithTwoWebservers.setUp(self)
         self.build_tree_contents([('foo', 'contents of foo\n'),

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2007-10-12 05:26:46 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2007-10-22 21:19:13 +0000
@@ -2045,8 +2045,10 @@
 
         http_transport = self.get_readonly_transport()
         medium = http_transport.get_smart_medium()
-        #remote_transport = RemoteTransport('fake_url', medium)
-        remote_transport = remote.RemoteTransport('/', medium=medium)
+        # Since we provide the medium, the url below will be mostly ignored
+        # during the test, as long as the path is '/'.
+        remote_transport = remote.RemoteTransport('bzr://fake_host/',
+                                                  medium=medium)
         self.assertEqual(
             [(0, "c")], list(remote_transport.readv("data-file", [(0,1)])))
 

=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py	2007-10-08 02:22:17 +0000
+++ b/bzrlib/tests/test_transport.py	2007-10-22 21:19:13 +0000
@@ -663,11 +663,13 @@
         self.assertEquals(t.relpath('sftp://host.com/dev/%path/sub'), 'sub')
 
     def test_connection_sharing_propagate_credentials(self):
-        t = ConnectedTransport('foo://user@host.com/abs/path')
+        t = ConnectedTransport('ftp://user@host.com/abs/path')
+        self.assertEquals('user', t._user)
+        self.assertEquals('host.com', t._host)
         self.assertIs(None, t._get_connection())
         self.assertIs(None, t._password)
         c = t.clone('subdir')
-        self.assertEquals(None, c._get_connection())
+        self.assertIs(None, c._get_connection())
         self.assertIs(None, t._password)
 
         # Simulate the user entering a password

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2007-10-08 04:47:49 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2007-10-22 21:19:13 +0000
@@ -1110,7 +1110,7 @@
 
         def new_url(scheme=None, user=None, password=None,
                     host=None, port=None, path=None):
-            """Build a new url from t.base chaging only parts of it.
+            """Build a new url from t.base changing only parts of it.
 
             Only the parameters different from None will be changed.
             """
@@ -1123,7 +1123,11 @@
             if path     is None: path     = t._path
             return t._unsplit_url(scheme, user, password, host, port, path)
 
-        self.assertIsNot(t, t._reuse_for(new_url(scheme='foo')))
+        if t._scheme == 'ftp':
+            scheme = 'sftp'
+        else:
+            scheme = 'ftp'
+        self.assertIsNot(t, t._reuse_for(new_url(scheme=scheme)))
         if t._user == 'me':
             user = 'you'
         else:

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-10-09 08:43:47 +0000
+++ b/bzrlib/transport/__init__.py	2007-10-22 21:19:13 +0000
@@ -1352,6 +1352,9 @@
             except ValueError:
                 raise errors.InvalidURL('invalid port number %s in url:\n%s' %
                                         (port, url))
+        if host == '':
+            raise errors.InvalidURL('Host empty in: %s' % url)
+
         host = urllib.unquote(host)
         path = urllib.unquote(path)
 

=== modified file 'bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py	2007-10-22 15:18:24 +0000
+++ b/bzrlib/transport/ftp.py	2007-10-22 21:19:13 +0000
@@ -48,14 +48,16 @@
 from bzrlib.trace import mutter, warning
 from bzrlib.transport import (
     AppendBasedFileStream,
+    ConnectedTransport,
     _file_streams,
+    register_urlparse_netloc_protocol,
     Server,
-    ConnectedTransport,
     )
 from bzrlib.transport.local import LocalURLServer
 import bzrlib.ui
 
-_have_medusa = False
+
+register_urlparse_netloc_protocol('aftp')
 
 
 class FtpPathError(errors.PathError):

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2007-10-22 15:18:24 +0000
+++ b/bzrlib/transport/http/_urllib.py	2007-10-22 21:19:13 +0000
@@ -75,17 +75,15 @@
             request.connection = connection
             (auth, proxy_auth) = self._get_credentials()
         else:
-            # First request, intialize credentials
+            # First request, intialize credentials.
+            # scheme and realm will be set by the _urllib2_wrappers.AuthHandler
             user = self._user
             password = self._password
-            authuri = self._remote_path('.')
-            auth = {'user': user, 'password': password, 'authuri': authuri}
-
-            if user and password is not None: # '' is a valid password
-                # Make the (user, password) available to urllib2
-                # We default to a realm of None to catch them all.
-                self._opener.password_manager.add_password(None, authuri,
-                                                           user, password)
+            auth = {'host': self._host, 'port': self._port,
+                    'user': user, 'password': password,
+                    'protocol': self._unqualified_scheme,
+                    'path': self._path}
+            # Proxy initialization will be done by first proxied request
             proxy_auth = {}
         # Ensure authentication info is provided
         request.auth = auth

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2007-10-22 15:18:24 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2007-10-22 21:19:13 +0000
@@ -34,11 +34,6 @@
 
 DEBUG = 0
 
-# TODO: It may be possible to share the password_manager across
-# all transports by prefixing the realm by the protocol used
-# (especially if other protocols do not use realms). See
-# PasswordManager below.
-
 # FIXME: Oversimplifying, two kind of exceptions should be
 # raised, once a request is issued: URLError before we have been
 # able to process the response, HTTPError after that. Process the
@@ -62,11 +57,11 @@
 from bzrlib import (
     config,
     errors,
+    transport,
     ui,
     )
 
 
-
 # We define our own Response class to keep our httplib pipe clean
 class Response(httplib.HTTPResponse):
     """Custom HTTPResponse, to avoid the need to decorate.
@@ -194,7 +189,7 @@
         # Unless told otherwise, redirections are not followed
         self.follow_redirections = False
         # auth and proxy_auth are dicts containing, at least
-        # (scheme, url, realm, user, password).
+        # (scheme, host, port, realm, user, password, protocol, path).
         # The dict entries are mostly handled by the AuthHandler.
         # Some authentication schemes may add more entries.
         self.auth = {}
@@ -241,54 +236,6 @@
         urllib2.Request.set_proxy(self, proxy, type)
 
 
-def extract_credentials(url):
-    """Extracts credentials information from url.
-
-    Get user and password from url of the form: http://user:pass@host/path
-    :returns: (clean_url, user, password)
-    """
-    scheme, netloc, path, query, fragment = urlparse.urlsplit(url)
-
-    if '@' in netloc:
-        auth, netloc = netloc.split('@', 1)
-        if ':' in auth:
-            user, password = auth.split(':', 1)
-        else:
-            user, password = auth, None
-        user = urllib.unquote(user)
-        if password is not None:
-            password = urllib.unquote(password)
-    else:
-        user = None
-        password = None
-
-    # Build the clean url
-    clean_url = urlparse.urlunsplit((scheme, netloc, path, query, fragment))
-
-    return clean_url, user, password
-
-def extract_authentication_uri(url):
-    """Extract the authentication uri from any url.
-
-    In the context of bzr, we simplify the authentication uri
-    to the host only. For the transport lifetime, we allow only
-    one user by realm on a given host. I.e. handling several
-    users for different paths for the same realm should be done
-    at a higher level.
-    """
-    scheme, host, path, query, fragment = urlparse.urlsplit(url)
-    return '%s://%s' % (scheme, host)
-
-
-# The AuthHandler classes handle the authentication of the requests, to do
-# that, they need a PasswordManager *at build time*. We also need one to reuse
-# the passwords entered by the user.
-class PasswordManager(urllib2.HTTPPasswordMgrWithDefaultRealm):
-
-    def __init__(self):
-        urllib2.HTTPPasswordMgrWithDefaultRealm.__init__(self)
-
-
 class ConnectionHandler(urllib2.BaseHandler):
     """Provides connection-sharing by pre-processing requests.
 
@@ -738,9 +685,8 @@
     handler_order = 100
     _debuglevel = DEBUG
 
-    def __init__(self, password_manager, proxies=None):
+    def __init__(self, proxies=None):
         urllib2.ProxyHandler.__init__(self, proxies)
-        self.password_manager = password_manager
         # First, let's get rid of urllib2 implementation
         for type, proxy in self.proxies.items():
             if self._debuglevel > 0:
@@ -783,7 +729,7 @@
 
     def proxy_bypass(self, host):
         """Check if host should be proxied or not"""
-        no_proxy = self.get_proxy_env_var('no', None)
+        no_proxy = self.get_proxy_env_var('no', default_to=None)
         if no_proxy is None:
             return False
         hhost, hport = urllib.splitport(host)
@@ -815,35 +761,26 @@
         # grok user:password at host:port as well as
         # http://user:password@host:port
 
-        # FIXME: query AuthenticationConfig too
+        (scheme, user, password,
+         host, port, path) = transport.ConnectedTransport._split_url(proxy)
 
-        # Extract credentials from the url and store them in the
-        # password manager so that the proxy AuthHandler can use
-        # them later.
-        proxy, user, password = extract_credentials(proxy)
         if request.proxy_auth == {}:
-            # No proxy auth parameter are available, we are
-            # handling the first proxied request, intialize.
-            # scheme and realm will be set by the AuthHandler
-            authuri = extract_authentication_uri(proxy)
-            request.proxy_auth = {'user': user, 'password': password,
-                                  'authuri': authuri}
-            if user and password is not None: # '' is a valid password
-                # We default to a realm of None to catch them all.
-                self.password_manager.add_password(None, authuri,
-                                                   user, password)
-        orig_type = request.get_type()
-        scheme, r_scheme = urllib.splittype(proxy)
-        if self._debuglevel > 0:
-            print 'scheme: %s, r_scheme: %s' % (scheme, r_scheme)
-        host, XXX = urllib.splithost(r_scheme)
-        if host is None:
-            raise errors.InvalidURL(proxy,
-                                    'Invalid syntax in proxy env variable')
-        host = urllib.unquote(host)
-        request.set_proxy(host, type)
-        if self._debuglevel > 0:
-            print 'set_proxy: proxy set to %s://%s' % (type, host)
+            # No proxy auth parameter are available, we are handling the first
+            # proxied request, intialize.  scheme (the authentication scheme)
+            # and realm will be set by the AuthHandler
+            request.proxy_auth = {
+                                  'host': host, 'port': port,
+                                  'user': user, 'password': password,
+                                  'protocol': scheme,
+                                   # We ignore path since we connect to a proxy
+                                  'path': None}
+        if port is None:
+            phost = host
+        else:
+            phost = host + ':%d' % port
+        request.set_proxy(phost, type)
+        if self._debuglevel > 0:
+            print 'set_proxy: proxy set to %s://%s' % (type, phost)
         return request
 
 
@@ -885,7 +822,7 @@
       successful and the request authentication parameters have been updated.
     """
 
-    _max_retry = 2
+    _max_retry = 3
     """We don't want to retry authenticating endlessly"""
 
     # The following attributes should be defined by daughter
@@ -893,10 +830,10 @@
     # - auth_required_header:  the header received from the server
     # - auth_header: the header sent in the request
 
-    def __init__(self, password_manager):
-        self.password_manager = password_manager
-        self.find_user_password = password_manager.find_user_password
-        self.add_password = password_manager.add_password
+    def __init__(self):
+        # We want to know when we enter into an try/fail cycle of
+        # authentications so we initialize to None to indicate that we aren't
+        # in such a cycle by default.
         self._retry_count = None
 
     def update_auth(self, auth, key, value):
@@ -915,8 +852,8 @@
         """
         # Don't try  to authenticate endlessly
         if self._retry_count is None:
-            # The retry being recusrsive calls, None identify the first try
-            self._retry_count = 0
+            # The retry being recusrsive calls, None identify the first retry
+            self._retry_count = 1
         else:
             self._retry_count += 1
             if self._retry_count > self._max_retry:
@@ -938,8 +875,8 @@
         if self.auth_match(server_header, auth):
             # auth_match may have modified auth (by adding the
             # password or changing the realm, for example)
-            if request.get_header(self.auth_header, None) is not None \
-                    and not auth['modified']:
+            if (request.get_header(self.auth_header, None) is not None
+                and not auth['modified']):
                 # We already tried that, give up
                 return None
 
@@ -1001,29 +938,20 @@
         # It may happen that we need to reconnect later, let's be ready
         self._retry_count = None
 
-    def get_password(self, user, authuri, realm=None):
+    # FIXME: Rename get_credentials or something and query the auth config for
+    # user too.
+    def get_password(self, auth):
         """Ask user for a password if none is already available."""
-        user_found, password = self.find_user_password(realm, authuri)
-        if user_found != user:
-            # FIXME: write a test for that case
-            password = None
+        user = auth['user']
+        realm = auth['realm']
+        password = auth['password']
 
         if password is None:
-            scheme, netloc, path, query, fragment = urlparse.urlsplit(authuri)
-            if ':' in netloc:
-                host, port = netloc.rsplit(':', 1)
-                # port is an int here invalid values have been handled by the
-                # upper layers
-                port = int(port)
-            else:
-                host = netloc
-                port = None
-            auth = config.AuthenticationConfig()
-            password = auth.get_password(
-                scheme, host, user, port=port, path=path, realm=realm,
+            auth_conf = config.AuthenticationConfig()
+            password = auth_conf.get_password(
+                auth['protocol'], auth['host'], user, port=auth['port'],
+                path=auth['path'], realm=realm,
                 prompt=self.password_prompt)
-            if password is not None:
-                self.add_password(realm, authuri, user, password)
         return password
 
     def http_request(self, request):
@@ -1064,8 +992,7 @@
             self.update_auth(auth, 'scheme', scheme)
             self.update_auth(auth, 'realm', realm)
             if auth.get('password',None) is None:
-                password = self.get_password(auth['user'], auth['authuri'],
-                                             auth['realm'])
+                password = self.get_password(auth)
                 self.update_auth(auth, 'password', password)
         return match is not None
 
@@ -1125,16 +1052,16 @@
             return False
 
         realm = req_auth.get('realm', None)
+        # Put useful info into auth
+        self.update_auth(auth, 'scheme', scheme)
+        self.update_auth(auth, 'realm', realm)
         if auth.get('password',None) is None:
-            auth['password'] = self.get_password(auth['user'],
-                                                 auth['authuri'],
-                                                 realm)
-        # Put useful info into auth
+            password = self.get_password(auth)
+            self.update_auth(auth, 'password', password)
+
         try:
-            self.update_auth(auth, 'scheme', scheme)
             if req_auth.get('algorithm', None) is not None:
                 self.update_auth(auth, 'algorithm', req_auth.get('algorithm'))
-            self.update_auth(auth, 'realm', realm)
             nonce = req_auth['nonce']
             if auth.get('nonce', None) != nonce:
                 # A new nonce, never used
@@ -1316,14 +1243,13 @@
                  connection=ConnectionHandler,
                  redirect=HTTPRedirectHandler,
                  error=HTTPErrorProcessor,):
-        self.password_manager = PasswordManager()
         self._opener = urllib2.build_opener( \
             connection, redirect, error,
-            ProxyHandler(self.password_manager),
-            HTTPBasicAuthHandler(self.password_manager),
-            HTTPDigestAuthHandler(self.password_manager),
-            ProxyBasicAuthHandler(self.password_manager),
-            ProxyDigestAuthHandler(self.password_manager),
+            ProxyHandler(),
+            HTTPBasicAuthHandler(),
+            HTTPDigestAuthHandler(),
+            ProxyBasicAuthHandler(),
+            ProxyDigestAuthHandler(),
             HTTPHandler,
             HTTPSHandler,
             HTTPDefaultErrorHandler,



More information about the bazaar-commits mailing list