From 9833226f7e5c86277e703adfd0a2c75c601769b6 Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Fri, 26 Jul 2013 22:21:10 -0600 Subject: [PATCH] Drop webob from auth_token.py This came up as a packaging problem in Fedora. Now that Swift does not need python-webob RPM, some sites install a proxy node without. We add the proper webob dependency to the python-keystoneclient. Still, in the face of parallel installs, and we end patching setup.py and diverge from upstream, and we do not like it. It may be better just stick to WSGI protocol by small amount of hand-rolled code and drop WebOb altogether. Note that we keep WebOb in the testing code. The fix is purely about the stuff that runs inside a WSGI server. Therefore, requirements.txt continues having no WebOb (apparently, was forgotten before), but test-requirements.txt keeps WebOb in, and this patch changes nothing. Initial version of this patch produced an inexplicable drop in testing coverage and Chmouel put his foot down on it. We were unable to figure out why the drop happened. Most likely coverage simply lied about the previous percentages and getting rid of import webob permitted it a better analysis. So, we add some unrelated tests to bump coverage percentages up. Of course we add a targeted test for the new code too. For that one to work, we change how set_middleware() treats its fake_http argument. This version also adds comments rather than docstrings, because they need to source as context. Change-Id: Iba33b11e77b9910211983aa725c69d3886e1c7a7 --- keystoneclient/middleware/auth_token.py | 24 ++++++++++--- tests/test_auth_token_middleware.py | 47 +++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/keystoneclient/middleware/auth_token.py b/keystoneclient/middleware/auth_token.py index 6537a677..cbe00b33 100644 --- a/keystoneclient/middleware/auth_token.py +++ b/keystoneclient/middleware/auth_token.py @@ -153,7 +153,6 @@ import stat import tempfile import time import urllib -import webob.exc from keystoneclient.openstack.common import jsonutils from keystoneclient.common import cms @@ -252,6 +251,19 @@ class ConfigurationError(Exception): pass +class MiniResp(object): + def __init__(self, error_message, env, headers=[]): + # The HEAD method is unique: it must never return a body, even if + # it reports an error (RFC-2616 clause 9.4). We relieve callers + # from varying the error responses depending on the method. + if env['REQUEST_METHOD'] == 'HEAD': + self.body = [''] + else: + self.body = [error_message] + self.headers = list(headers) + self.headers.append(('Content-type', 'text/plain')) + + class AuthProtocol(object): """Auth Middleware that handles authenticating client calls.""" @@ -472,8 +484,9 @@ class AuthProtocol(object): except ServiceError as e: self.LOG.critical('Unable to obtain admin token: %s' % e) - resp = webob.exc.HTTPServiceUnavailable() - return resp(env, start_response) + resp = MiniResp('Service unavailable', env) + start_response('503 Service Unavailable', resp.headers) + return resp.body def _remove_auth_headers(self, env): """Remove headers so a user can't fake authentication. @@ -534,8 +547,9 @@ class AuthProtocol(object): """ headers = [('WWW-Authenticate', 'Keystone uri=\'%s\'' % self.auth_uri)] - resp = webob.exc.HTTPUnauthorized('Authentication required', headers) - return resp(env, start_response) + resp = MiniResp('Authentication required', env, headers) + start_response('401 Unauthorized', resp.headers) + return resp.body def get_admin_token(self): """Return admin token, possibly fetching a new one. diff --git a/tests/test_auth_token_middleware.py b/tests/test_auth_token_middleware.py index 8f396021..9a9ba928 100644 --- a/tests/test_auth_token_middleware.py +++ b/tests/test_auth_token_middleware.py @@ -17,6 +17,7 @@ import datetime import iso8601 import os +import shutil import string import sys import tempfile @@ -671,8 +672,7 @@ class BaseAuthTokenMiddlewareTest(testtools.TestCase): """ conf = conf or self.conf - if 'http_handler' not in conf: - fake_http = fake_http or self.fake_http + if fake_http: conf['http_handler'] = fake_http fake_app = fake_app or self.fake_app self.middleware = auth_token.AuthProtocol(fake_app(expected_env), conf) @@ -1002,6 +1002,21 @@ class AuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest): self.assertIsNotNone(self.middleware.LOG.msg) self.assertIsNotNone(self.middleware.LOG.debugmsg) + def test_request_no_token_http(self): + req = webob.Request.blank('/', environ={'REQUEST_METHOD': 'HEAD'}) + conf = { + 'auth_host': 'keystone.example.com', + 'auth_port': 1234, + 'auth_protocol': 'http', + 'auth_admin_prefix': '/testadmin', + } + self.set_middleware(conf=conf) + body = self.middleware(req.environ, self.start_fake_response) + self.assertEqual(self.response_status, 401) + self.assertEqual(self.response_headers['WWW-Authenticate'], + "Keystone uri='http://keystone.example.com:1234'") + self.assertEqual(body, ['']) + def test_request_blank_token(self): req = webob.Request.blank('/') req.headers['X-Auth-Token'] = '' @@ -1197,6 +1212,34 @@ class AuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest): datetime.timedelta(seconds=24)) +class CertDownloadMiddlewareTest(BaseAuthTokenMiddlewareTest): + def setUp(self): + super(CertDownloadMiddlewareTest, self).setUp() + self.base_dir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.base_dir) + super(CertDownloadMiddlewareTest, self).tearDown() + + # Usually we supply a signed_dir with pre-installed certificates, + # so invocation of /usr/bin/openssl succeeds. This time we give it + # an empty directory, so it fails. + def test_request_no_token_dummy(self): + cert_dir = os.path.join(self.base_dir, 'certs') + os.mkdir(cert_dir) + conf = { + 'auth_host': 'keystone.example.com', + 'auth_port': 1234, + 'auth_protocol': 'http', + 'auth_admin_prefix': '/testadmin', + 'signing_dir': cert_dir, + } + self.set_middleware(fake_http=self.fake_http, conf=conf) + self.assertRaises(cms.subprocess.CalledProcessError, + self.middleware.verify_signed_token, + self.token_dict['signed_token_scoped']) + + class v2AuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest): """v2 token specific tests.