Rev 4350: Correctly handle http servers proposing multiple authentication in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Mon May 11 08:36:36 BST 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4350
revision-id: pqm at pqm.ubuntu.com-20090511073632-ti658145fo07a4vw
parent: pqm at pqm.ubuntu.com-20090511052540-r3l0iwm1uo716iv0
parent: v.ladeuil+lp at free.fr-20090511064430-0n4ebsalx2vn5n79
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2009-05-11 08:36:32 +0100
message:
Correctly handle http servers proposing multiple authentication
schemes
removed:
bzrlib/tests/test_http_implementations.py test_http_implementa-20071218210003-65nh81gglcfvurw6-1
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/tests/__init__.py selftest.py-20050531073622-8d0e3c8845c97a64
bzrlib/tests/http_utils.py HTTPTestUtil.py-20050914180604-247d3aafb7a43343
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
------------------------------------------------------------
revno: 4349.1.1
revision-id: v.ladeuil+lp at free.fr-20090511064430-0n4ebsalx2vn5n79
parent: pqm at pqm.ubuntu.com-20090511052540-r3l0iwm1uo716iv0
parent: v.ladeuil+lp at free.fr-20090504152126-13juy7bmikof23xc
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: integration
timestamp: Mon 2009-05-11 08:44:30 +0200
message:
Correctly handle http servers proposing multiple authentication schemes
removed:
bzrlib/tests/test_http_implementations.py test_http_implementa-20071218210003-65nh81gglcfvurw6-1
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/tests/__init__.py selftest.py-20050531073622-8d0e3c8845c97a64
bzrlib/tests/http_utils.py HTTPTestUtil.py-20050914180604-247d3aafb7a43343
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
------------------------------------------------------------
revno: 4307.4.3
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.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
------------------------------------------------------------
revno: 4307.4.2
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.
modified:
bzrlib/tests/http_utils.py HTTPTestUtil.py-20050914180604-247d3aafb7a43343
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
------------------------------------------------------------
revno: 4307.4.1
revision-id: v.ladeuil+lp at free.fr-20090428103449-s4s63f02963t3k6i
parent: pqm at pqm.ubuntu.com-20090428004234-6j7ndsmvsx3hsrqo
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 366107-http-mutiple-auth-schemes
timestamp: Tue 2009-04-28 12:34:49 +0200
message:
Remove never used test module.
* bzrlib/tests/__init__.py:
(test_suite): Remove never used bzrlib.tests.test_http_implementations.
removed:
bzrlib/tests/test_http_implementations.py test_http_implementa-20071218210003-65nh81gglcfvurw6-1
modified:
bzrlib/tests/__init__.py selftest.py-20050531073622-8d0e3c8845c97a64
=== modified file 'NEWS'
--- a/NEWS 2009-05-11 04:33:27 +0000
+++ b/NEWS 2009-05-11 06:44:30 +0000
@@ -66,6 +66,9 @@
bzr+ssh:// clients (which have bug #354036). Instead, the server now
causes those clients to send the missing records. (Andrew Bennetts)
+* Correctly handle http servers proposing multiple authentication schemes.
+ (Vincent Ladeuil, #366107)
+
* End-Of-Line content filters are now loaded correctly.
(Ian Clatworthy, Brian de Alwis, #355280)
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2009-05-08 17:29:33 +0000
+++ b/bzrlib/tests/__init__.py 2009-05-11 06:44:30 +0000
@@ -3414,7 +3414,6 @@
'bzrlib.tests.test_help',
'bzrlib.tests.test_hooks',
'bzrlib.tests.test_http',
- 'bzrlib.tests.test_http_implementations',
'bzrlib.tests.test_http_response',
'bzrlib.tests.test_https_ca_bundle',
'bzrlib.tests.test_identitymap',
=== 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())
=== removed file 'bzrlib/tests/test_http_implementations.py'
--- a/bzrlib/tests/test_http_implementations.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_http_implementations.py 1970-01-01 00:00:00 +0000
@@ -1,51 +0,0 @@
-# Copyright (C) 2007 Canonical Ltd
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-
-"""Tests for HTTP transports and servers implementations.
-
-(transport, server) implementations tested here are supplied by
-HTTPTestProviderAdapter. Note that a server is characterized by a request
-handler class.
-
-Transport implementations are normally tested via
-test_transport_implementations. The tests here are about the variations in HTTP
-protocol implementation to guarantee the robustness of our transports.
-"""
-
-import errno
-import SimpleHTTPServer
-import socket
-
-import bzrlib
-from bzrlib import (
- config,
- errors,
- osutils,
- tests,
- transport,
- ui,
- urlutils,
- )
-from bzrlib.tests import (
- http_server,
- http_utils,
- )
-from bzrlib.transport.http import (
- _urllib,
- _urllib2_wrappers,
- )
-
-
=== 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 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"""
@@ -1035,32 +1038,54 @@
# 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, 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
+ 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
+
+ # 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
+
+ # 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
@@ -1192,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)
@@ -1237,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):
@@ -1255,14 +1280,11 @@
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)
if match:
- if scheme != 'basic':
- return False
-
# Put useful info into auth
self.update_auth(auth, 'scheme', scheme)
self.update_auth(auth, 'realm', realm)
@@ -1300,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
@@ -1311,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
More information about the bazaar-commits
mailing list