Rev 4309: Handle servers proposing several authentication schemes. in file:///home/vila/src/bzr/bugs/366107-http-mutiple-auth-schemes/

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon May 4 15:48:23 BST 2009


At file:///home/vila/src/bzr/bugs/366107-http-mutiple-auth-schemes/

------------------------------------------------------------
revno: 4309
revision-id: v.ladeuil+lp at free.fr-20090504144821-39dvqkikmd3zqkdg
parent: v.ladeuil+lp at free.fr-20090428103449-s4s63f02963t3k6i
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 366107-http-mutiple-auth-schemes
timestamp: Mon 2009-05-04 16:48:21 +0200
message:
  Handle servers proposing several authentication schemes.
  
  * bzrlib/transport/http/_urllib2_wrappers.py:
  (AbstractAuthHandler.auth_required): Several schemes can be
  proposed by the server, try to match each one in turn.
  (BasicAuthHandler.auth_match): Delete dead code.
  
  * bzrlib/tests/test_http.py:
  (load_tests): Separate proxy and http authentication tests as they
  require different server setups.
  (TestAuth.create_transport_readonly_server): Simplified by using
  parameter provided by load_tests.
  (TestAuth.test_changing_nonce): Adapt to new parametrization.
  (TestProxyAuth.create_transport_readonly_server): Deleted.
  
  * bzrlib/tests/http_utils.py:
  (DigestAndBasicAuthRequestHandler, HTTPBasicAndDigestAuthServer,
  ProxyBasicAndDigestAuthServer): Add a test server proposing both
  basic and digest auth schemes but accepting only digest as valid.
-------------- next part --------------
=== modified file 'bzrlib/tests/http_utils.py'
--- a/bzrlib/tests/http_utils.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/http_utils.py	2009-05-04 14:48:21 +0000
@@ -297,8 +297,6 @@
 
     def authorized(self):
         tcs = self.server.test_case_server
-        if tcs.auth_scheme != 'digest':
-            return False
 
         auth_header = self.headers.get(tcs.auth_header_recv, None)
         if auth_header is None:
@@ -319,6 +317,24 @@
         self.send_header(tcs.auth_header_sent,header)
 
 
+class DigestAndBasicAuthRequestHandler(DigestAuthRequestHandler):
+    """Implements a digest and basic authentication of a request.
+
+    I.e. the server proposes both schemes and the client should choose the best
+    one it can handle, which, in that case, should be digest, the only scheme
+    accepted here.
+    """
+
+    def send_header_auth_reqed(self):
+        tcs = self.server.test_case_server
+        self.send_header(tcs.auth_header_sent,
+                         'Basic realm="%s"' % tcs.auth_realm)
+        header = 'Digest realm="%s", ' % tcs.auth_realm
+        header += 'nonce="%s", algorithm="%s", qop="auth"' % (tcs.auth_nonce,
+                                                              'MD5')
+        self.send_header(tcs.auth_header_sent,header)
+
+
 class AuthServer(http_server.HttpServer):
     """Extends HttpServer with a dictionary of passwords.
 
@@ -410,6 +426,7 @@
 
         return response_digest == auth['response']
 
+
 class HTTPAuthServer(AuthServer):
     """An HTTP server requiring authentication"""
 
@@ -447,6 +464,18 @@
         self.init_http_auth()
 
 
+class HTTPBasicAndDigestAuthServer(DigestAuthServer, HTTPAuthServer):
+    """An HTTP server requiring basic or digest authentication"""
+
+    def __init__(self, protocol_version=None):
+        DigestAuthServer.__init__(self, DigestAndBasicAuthRequestHandler,
+                                  'basicdigest',
+                                  protocol_version=protocol_version)
+        self.init_http_auth()
+        # We really accept Digest only
+        self.auth_scheme = 'digest'
+
+
 class ProxyBasicAuthServer(ProxyAuthServer):
     """A proxy server requiring basic authentication"""
 
@@ -465,3 +494,15 @@
         self.init_proxy_auth()
 
 
+class ProxyBasicAndDigestAuthServer(DigestAuthServer, ProxyAuthServer):
+    """An proxy server requiring basic or digest authentication"""
+
+    def __init__(self, protocol_version=None):
+        DigestAuthServer.__init__(self, DigestAndBasicAuthRequestHandler,
+                                  'basicdigest',
+                                  protocol_version=protocol_version)
+        self.init_proxy_auth()
+        # We really accept Digest only
+        self.auth_scheme = 'digest'
+
+

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2009-04-11 15:39:28 +0000
+++ b/bzrlib/tests/test_http.py	2009-05-04 14:48:21 +0000
@@ -113,17 +113,34 @@
                                             protocol_scenarios)
     tests.multiply_tests(tp_tests, tp_scenarios, result)
 
+    # proxy auth: each auth scheme on all http versions on all implementations.
+    tppa_tests, remaining_tests = tests.split_suite_by_condition(
+        remaining_tests, tests.condition_isinstance((
+                TestProxyAuth,
+                )))
+    proxy_auth_scheme_scenarios = [
+        ('basic', dict(_auth_server=http_utils.ProxyBasicAuthServer)),
+        ('digest', dict(_auth_server=http_utils.ProxyDigestAuthServer)),
+        ('basicdigest',
+         dict(_auth_server=http_utils.ProxyBasicAndDigestAuthServer)),
+        ]
+    tppa_scenarios = tests.multiply_scenarios(tp_scenarios,
+                                              proxy_auth_scheme_scenarios)
+    tests.multiply_tests(tppa_tests, tppa_scenarios, result)
+
     # auth: each auth scheme on all http versions on all implementations.
     tpa_tests, remaining_tests = tests.split_suite_by_condition(
         remaining_tests, tests.condition_isinstance((
                 TestAuth,
                 )))
     auth_scheme_scenarios = [
-        ('basic', dict(_auth_scheme='basic')),
-        ('digest', dict(_auth_scheme='digest')),
+        ('basic', dict(_auth_server=http_utils.HTTPBasicAuthServer)),
+        ('digest', dict(_auth_server=http_utils.HTTPDigestAuthServer)),
+        ('basicdigest',
+         dict(_auth_server=http_utils.HTTPBasicAndDigestAuthServer)),
         ]
     tpa_scenarios = tests.multiply_scenarios(tp_scenarios,
-        auth_scheme_scenarios)
+                                             auth_scheme_scenarios)
     tests.multiply_tests(tpa_tests, tpa_scenarios, result)
 
     # activity: activity on all http versions on all implementations
@@ -1451,6 +1468,8 @@
     _auth_header = 'Authorization'
     _password_prompt_prefix = ''
     _username_prompt_prefix = ''
+    # Set by load_tests
+    _auth_server = None
 
     def setUp(self):
         super(TestAuth, self).setUp()
@@ -1459,16 +1478,7 @@
                                   ('b', 'contents of b\n'),])
 
     def create_transport_readonly_server(self):
-        if self._auth_scheme == 'basic':
-            server = http_utils.HTTPBasicAuthServer(
-                protocol_version=self._protocol_version)
-        else:
-            if self._auth_scheme != 'digest':
-                raise AssertionError('Unknown auth scheme: %r'
-                                     % self._auth_scheme)
-            server = http_utils.HTTPDigestAuthServer(
-                protocol_version=self._protocol_version)
-        return server
+        return self._auth_server(protocol_version=self._protocol_version)
 
     def _testing_pycurl(self):
         return pycurl_present and self._transport == PyCurlTransport
@@ -1628,8 +1638,9 @@
         self.assertEqual(1, self.server.auth_required_errors)
 
     def test_changing_nonce(self):
-        if self._auth_scheme != 'digest':
-            raise tests.TestNotApplicable('HTTP auth digest only test')
+        if self._auth_server not in (http_utils.HTTPDigestAuthServer,
+                                     http_utils.ProxyDigestAuthServer):
+            raise tests.TestNotApplicable('HTTP/proxy auth digest only test')
         if self._testing_pycurl():
             raise tests.KnownFailure(
                 'pycurl does not handle a nonce change')
@@ -1667,18 +1678,6 @@
                                   ('b-proxied', 'contents of b\n'),
                                   ])
 
-    def create_transport_readonly_server(self):
-        if self._auth_scheme == 'basic':
-            server = http_utils.ProxyBasicAuthServer(
-                protocol_version=self._protocol_version)
-        else:
-            if self._auth_scheme != 'digest':
-                raise AssertionError('Unknown auth scheme: %r'
-                                     % self._auth_scheme)
-            server = http_utils.ProxyDigestAuthServer(
-                protocol_version=self._protocol_version)
-        return server
-
     def get_user_transport(self, user, password):
         self._install_env({'all_proxy': self.get_user_url(user, password)})
         return self._transport(self.server.get_url())

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2009-04-27 03:27:46 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2009-05-04 14:48:21 +0000
@@ -1035,32 +1035,39 @@
                 # Let's be ready for next round
                 self._retry_count = None
                 return None
-        server_header = headers.get(self.auth_required_header, None)
-        if server_header is None:
+        server_headers = headers.getheaders(self.auth_required_header)
+        if not server_headers:
             # The http error MUST have the associated
             # header. This must never happen in production code.
             raise KeyError('%s not found' % self.auth_required_header)
 
         auth = self.get_auth(request)
         auth['modified'] = False
-        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']):
-                # We already tried that, give up
-                return None
-
-            if self.requires_username and auth.get('user', None) is None:
-                # Without a known user, we can't authenticate
-                return None
-
-            # Housekeeping
-            request.connection.cleanup_pipe()
-            response = self.parent.open(request)
-            if response:
-                self.auth_successful(request, response)
-            return response
+        # FIXME: the auth handler should be selected at a single place instead
+        # of letting all handlers try to match all headers.
+        for server_header in server_headers:
+            # Several schemes can be proposed by the server, try to match each
+            # one in turn
+            matching_handler = self.auth_match(server_header, auth)
+            if matching_handler:
+                # 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']):
+                    # We already tried that, give up
+                    return None
+
+                if self.requires_username and auth.get('user', None) is None:
+                    # Without a known user, we can't authenticate
+                    return None
+
+                # Housekeeping
+                request.connection.cleanup_pipe()
+                # Retry the request with an authentication header added
+                response = self.parent.open(request)
+                if response:
+                    self.auth_successful(request, response)
+                return response
         # We are not qualified to handle the authentication.
         # Note: the authentication error handling will try all
         # available handlers. If one of them authenticates
@@ -1260,9 +1267,6 @@
 
         match, realm = self.extract_realm(raw_auth)
         if match:
-            if scheme != 'basic':
-                return False
-
             # Put useful info into auth
             self.update_auth(auth, 'scheme', scheme)
             self.update_auth(auth, 'realm', realm)



More information about the bazaar-commits mailing list