Rev 2375: Take jam's review comments into account. Fix typos, give better in file:///v/home/vila/src/bugs/72792/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Sun Apr 15 16:57:10 BST 2007
At file:///v/home/vila/src/bugs/72792/
------------------------------------------------------------
revno: 2375
revision-id: v.ladeuil+lp at free.fr-20070415155708-frrm29cd9vvvd8do
parent: v.ladeuil+lp at free.fr-20070413163858-3v6pox4wls7j9ljb
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 72792
timestamp: Sun 2007-04-15 17:57:08 +0200
message:
Take jam's review comments into account. Fix typos, give better
explanations, add a test, complete another.
* bzrlib/transport/http/_urllib2_wrappers.py:
(HTTPBasicAuthHandler.http_error_401): Better explanation.
* bzrlib/transport/http/_urllib.py:
(HttpTransport_urllib.__init__): _auth renamed to _auth_scheme.
* bzrlib/tests/test_http.py:
(TestHTTPBasicAuth.test_unknown_user): New test.
* bzrlib/tests/HTTPTestUtil.py:
(BasicAuthHTTPServer): New class. Be explicit about use
requirements: basic authentication is mandatory.
modified:
bzrlib/tests/HTTPTestUtil.py HTTPTestUtil.py-20050914180604-247d3aafb7a43343
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
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/tests/HTTPTestUtil.py'
--- a/bzrlib/tests/HTTPTestUtil.py 2007-04-13 08:11:01 +0000
+++ b/bzrlib/tests/HTTPTestUtil.py 2007-04-15 15:57:08 +0000
@@ -310,14 +310,18 @@
class BasicAuthRequestHandler(TestingHTTPRequestHandler):
- """Requires a basic authentification to process requests."""
+ """Requires a basic authentication to process requests.
+
+ This is intended to be used with a server that always and
+ only use basic authentication.
+ """
def do_GET(self):
tcs = self.server.test_case_server
- if tcs.auth == 'basic':
+ if tcs.auth_scheme == 'basic':
auth_header = self.headers.get('Authorization')
authorized = False
- if auth_header:
+ if auth_header and auth_header.lower().startswith('basic '):
coded_auth = auth_header[len('Basic '):]
user, password = coded_auth.decode('base64').split(':')
authorized = tcs.authorized(user, password)
@@ -330,17 +334,36 @@
TestingHTTPRequestHandler.do_GET(self)
+
class AuthHTTPServer(HttpServer):
- """AuthHTTPServer extends HttpServer with a dictionary of passwords"""
-
- def __init__(self, request_handler, auth):
+ """AuthHTTPServer extends HttpServer with a dictionary of passwords.
+
+ This is used as a base class for various schemes.
+
+ Note that no users are defined by default, so add_user should
+ be called before issuing the first request.
+ """
+
+ def __init__(self, request_handler, auth_scheme):
HttpServer.__init__(self, request_handler)
- # No authentification is done by default
- self.auth = auth
+ self.auth_scheme = auth_scheme
self.password_of = {}
def add_user(self, user, password):
+ """Declare a user with an associated password.
+
+ password can be empty, use an empty string ('') in that
+ case, not None.
+ """
self.password_of[user] = password
def authorized(self, user, password):
- return self.password_of[user] == password
+ expected_password = self.password_of.get(user, None)
+ return expected_password is not None and password == expected_password
+
+
+class BasicAuthHTTPServer(AuthHTTPServer):
+ """An HTTP server requiring basic authentication"""
+
+ def __init__(self):
+ AuthHTTPServer.__init__(self, BasicAuthRequestHandler, 'basic')
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py 2007-04-13 16:16:54 +0000
+++ b/bzrlib/tests/test_http.py 2007-04-15 15:57:08 +0000
@@ -20,16 +20,17 @@
# TODO: Should be renamed to bzrlib.transport.http.tests?
# TODO: What about renaming to bzrlib.tests.transport.http ?
+from cStringIO import StringIO
import os
import select
import socket
import threading
-from cStringIO import StringIO
import bzrlib
from bzrlib import (
errors,
osutils,
+ ui,
urlutils,
)
from bzrlib.tests import (
@@ -44,10 +45,9 @@
HttpServer_urllib,
)
from bzrlib.tests.HTTPTestUtil import (
- AuthHTTPServer,
BadProtocolRequestHandler,
BadStatusRequestHandler,
- BasicAuthRequestHandler,
+ BasicAuthHTTPServer,
FakeProxyRequestHandler,
ForbiddenRequestHandler,
HTTPServerRedirecting,
@@ -71,7 +71,6 @@
)
from bzrlib.transport.http._urllib import HttpTransport_urllib
-import bzrlib.ui
class FakeManager(object):
@@ -1153,11 +1152,9 @@
_transport = HttpTransport_urllib
_auth_header = 'Authorization'
- _auth_type = 'basic'
- _request_handler_class = BasicAuthRequestHandler
def create_transport_readonly_server(self):
- return AuthHTTPServer(self._request_handler_class, self._auth_type)
+ return BasicAuthHTTPServer()
def setUp(self):
super(TestHTTPBasicAuth, self).setUp()
@@ -1165,11 +1162,11 @@
('b', 'contents of b\n'),])
self.server = self.get_readonly_server()
- self.old_factory = bzrlib.ui.ui_factory
+ self.old_factory = ui.ui_factory
self.addCleanup(self.restoreUIFactory)
def restoreUIFactory(self):
- bzrlib.ui.ui_factory = self.old_factory
+ ui.ui_factory = self.old_factory
def get_user_url(self, user=None, password=None):
"""Build an url embedding user and password"""
@@ -1192,6 +1189,11 @@
t = self._transport(self.get_user_url('joe', 'foo'))
self.assertEqual('contents of a\n', t.get('a').read())
+ def test_unknown_user(self):
+ self.server.add_user('joe', 'foo')
+ t = self._transport(self.get_user_url('bill', 'foo'))
+ self.assertRaises(errors.InvalidHttpResponse, t.get, 'a')
+
def test_wrong_pass(self):
self.server.add_user('joe', 'foo')
t = self._transport(self.get_user_url('joe', 'bar'))
@@ -1200,11 +1202,13 @@
def test_prompt_for_password(self):
self.server.add_user('joe', 'foo')
t = self._transport(self.get_user_url('joe'))
- bzrlib.ui.ui_factory = TestUIFactory(stdin='foo\n',
- stdout=StringIOWrapper())
+ ui.ui_factory = TestUIFactory(stdin='foo\n', stdout=StringIOWrapper())
self.assertEqual('contents of a\n',t.get('a').read())
# stdin should be empty
- self.assertEqual('', bzrlib.ui.ui_factory.stdin.readline())
+ self.assertEqual('', ui.ui_factory.stdin.readline())
# And we shouldn't prompt again for a different request
# against the same transport.
self.assertEqual('contents of b\n',t.get('b').read())
+ t2 = t.clone()
+ # And neither against a clone
+ self.assertEqual('contents of b\n',t2.get('b').read())
=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py 2007-04-13 16:16:54 +0000
+++ b/bzrlib/transport/http/_urllib.py 2007-04-15 15:57:08 +0000
@@ -50,21 +50,22 @@
if from_transport is not None:
super(HttpTransport_urllib, self).__init__(base, from_transport)
self._connection = from_transport._connection
- self._auth = from_transport._auth
+ self._auth_scheme = from_transport._auth_scheme
self._user = from_transport._user
self._password = from_transport._password
self._opener = from_transport._opener
else:
- # urllib2 will be confused if it find
- # authentification infos in the urls. So we handle
- # them separatly. Note that we don't need to do that
- # when cloning (as above) since the cloned base is
- # already clean.
+ # urllib2 will be confused if it find authentication
+ # info in the urls. So we handle them separatly.
+ # Note: we don't need to when cloning because it was
+ # already done.
clean_base, user, password = self._extract_auth(base)
super(HttpTransport_urllib, self).__init__(clean_base,
from_transport)
self._connection = None
- self._auth = None # We have to wait the 401 to know
+ # auth_scheme will be set once we authenticate
+ # successfully after a 401 error.
+ self._auth_scheme = None
self._user = user
self._password = password
self._opener = self._opener_class()
@@ -94,7 +95,7 @@
pm.add_password(None, authuri, self._user, self._password)
def _extract_auth(self, url):
- """Extracts authentification information from url.
+ """Extracts authentication information from url.
Get user and password from url of the form: http://user:pass@host/path
:returns: (clean_url, user, password)
@@ -130,8 +131,8 @@
# We will issue our first request, time to ask for a
# password if needed
self._ask_password()
- # Ensure authentification info is provided
- request.set_auth(self._auth, self._user, self._password)
+ # Ensure authentication info is provided
+ request.set_auth(self._auth_scheme, self._user, self._password)
mutter('%s: [%s]' % (request.method, request.get_full_url()))
if self._debuglevel > 0:
@@ -144,7 +145,7 @@
# to connect to the server
self._connection = request.connection
# And get auth parameters too
- self._auth = request.auth
+ self._auth_scheme = request.auth_scheme
self._user = request.user
self._password = request.password
=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py 2007-04-13 16:16:54 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py 2007-04-15 15:57:08 +0000
@@ -29,7 +29,7 @@
connection even for requests that urllib2 doesn't expect to contain body data.
And a custom Request class that lets us track redirections, and
-handle authentification schemes.
+handle authentication schemes.
We also create a Request hierarchy, to make it clear what type of
request is being made.
@@ -159,7 +159,7 @@
self.set_auth(None, None, None) # Until the first 401
def set_auth(self, auth_scheme, user, password=None):
- self.auth = auth_scheme
+ self.auth_scheme = auth_scheme
self.user = user
self.password = password
@@ -167,7 +167,7 @@
return self.method
-# The urlib2.xxxAuthHandler handle the authentification of the
+# The urlib2.xxxAuthHandler handle the authentication of the
# requests, to do that, they need an urllib2 PasswordManager *at
# build time*. We also need one to reuse the passwords already
# typed by the user.
@@ -693,10 +693,10 @@
class HTTPBasicAuthHandler(urllib2.HTTPBasicAuthHandler):
- """Custom basic authentification handler.
+ """Custom basic authentication handler.
- Send the authentification preventively to avoid the the
- roundtrip associated with the 401 error.
+ Send the authentication preventively to avoid the roundtrip
+ associated with the 401 error.
"""
def get_auth(self, user, password):
@@ -705,7 +705,7 @@
return auth
def set_auth(self, request):
- """Add the authentification header if needed.
+ """Add the authentication header if needed.
All required informations should be part of the request.
"""
@@ -714,8 +714,8 @@
self.get_auth(request.user, request.password))
def http_request(self, request):
- """Insert an authentification header if information is available"""
- if request.auth == 'basic' and request.password is not None:
+ """Insert an authentication header if information is available"""
+ if request.auth_scheme == 'basic' and request.password is not None:
self.set_auth(request)
return request
@@ -727,7 +727,14 @@
code, msg,
headers)
if response is not None:
- req.auth = 'basic'
+ # We capture the auth_scheme to be able to send the
+ # authentication header with the next requests
+ # without waiting for a 401 error.
+ # The urllib2.HTTPBasicAuthHandler will return a
+ # response *only* if the basic authentication
+ # succeeds. If another scheme is used or the
+ # authentication fails, the response will be None.
+ req.auth_scheme = 'basic'
return response
More information about the bazaar-commits
mailing list