Rev 4310: Tighten multiple auth schemes handling. in file:///home/vila/src/bzr/bugs/366107-http-mutiple-auth-schemes/

[Vincent Ladeuil]

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

------------------------------------------------------------
revno: 4310
revision-id: v.ladeuil+lp at free.fr-20090504152126-13juy7bmikof23xc
parent: v.ladeuil+lp at free.fr-20090504144821-39dvqkikmd3zqkdg
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 366107-http-mutiple-auth-schemes
timestamp: Mon 2009-05-04 17:21:26 +0200
message:
  Tighten multiple auth schemes handling.
  
  * bzrlib/transport/http/_urllib2_wrappers.py:
  (AbstractAuthHandler): Add a 'scheme' attribute to identify the
  handlers.
  (AbstractAuthHandler.auth_required): Once the most secured scheme
  is known to be proposed by the server, the other handlers should
  not be tried.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2009-04-27 23:26:00 +0000
+++ b/NEWS	2009-05-04 15:21:26 +0000
@@ -58,6 +58,9 @@
 * Non-recursive ``bzr ls`` now works properly when a path is specified.
   (Jelmer Vernooij, #357863)
 
+* Correctly handle http servers proposing multiple authentication schemes.
+  (Vincent Ladeuil, #366107)
+
 Documentation
 *************
 

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2009-05-04 14:48:21 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2009-05-04 15:21:26 +0000
@@ -978,6 +978,9 @@
       successful and the request authentication parameters have been updated.
     """
 
+    scheme = None
+    """The scheme as it appears in the server header (lower cased)"""
+
     _max_retry = 3
     """We don't want to retry authenticating endlessly"""
 
@@ -1044,7 +1047,8 @@
         auth = self.get_auth(request)
         auth['modified'] = False
         # FIXME: the auth handler should be selected at a single place instead
-        # of letting all handlers try to match all headers.
+        # of letting all handlers try to match all headers, but the current
+        # design doesn't allow a simple implementation.
         for server_header in server_headers:
             # Several schemes can be proposed by the server, try to match each
             # one in turn
@@ -1057,6 +1061,20 @@
                     # We already tried that, give up
                     return None
 
+                # Only the most secure scheme proposed by the server should be
+                # used, since the handlers use 'handler_order' to describe that
+                # property, the first handler tried takes precedence, the
+                # others should not attempt to authenticate if the best one
+                # failed.
+                best_scheme = auth.get('best_scheme', None)
+                if best_scheme is None:
+                    # At that point, if current handler should doesn't succeed
+                    # the credentials are wrong (or incomplete), but we know
+                    # that the associated scheme should be used.
+                    best_scheme = auth['best_scheme'] = self.scheme
+                if  best_scheme != self.scheme:
+                    continue
+
                 if self.requires_username and auth.get('user', None) is None:
                     # Without a known user, we can't authenticate
                     return None
@@ -1199,13 +1217,13 @@
     NTLM support may also be added.
     """
 
+    scheme = 'negotiate'
     handler_order = 480
-
     requires_username = False
 
     def auth_match(self, header, auth):
         scheme, raw_auth = self._parse_auth_header(header)
-        if scheme != 'negotiate':
+        if scheme != self.scheme:
             return False
         self.update_auth(auth, 'scheme', scheme)
         resp = self._auth_match_kerberos(auth)
@@ -1244,8 +1262,8 @@
 class BasicAuthHandler(AbstractAuthHandler):
     """A custom basic authentication handler."""
 
+    scheme = 'basic'
     handler_order = 500
-
     auth_regexp = re.compile('realm="([^"]*)"', re.I)
 
     def build_auth_header(self, auth, request):
@@ -1262,7 +1280,7 @@
 
     def auth_match(self, header, auth):
         scheme, raw_auth = self._parse_auth_header(header)
-        if scheme != 'basic':
+        if scheme != self.scheme:
             return False
 
         match, realm = self.extract_realm(raw_auth)
@@ -1304,6 +1322,7 @@
 class DigestAuthHandler(AbstractAuthHandler):
     """A custom digest authentication handler."""
 
+    scheme = 'digest'
     # Before basic as digest is a bit more secure and should be preferred
     handler_order = 490
 
@@ -1315,7 +1334,7 @@
 
     def auth_match(self, header, auth):
         scheme, raw_auth = self._parse_auth_header(header)
-        if scheme != 'digest':
+        if scheme != self.scheme:
             return False
 
         # Put the requested authentication info into a dict