From 675729be2e3073204e6acc1627394f53f3e3cf51 Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Thu, 16 Apr 2015 13:47:31 +1000 Subject: [PATCH] Base use webob Why are we manually managing environment vs headers? We already have a dependency on webob, let's actually use it and simplify our code a little. This is a very basic patch which just allows us to use webob requests and responses to prove there is no difference. From this we can do additional cleanups in later reviews. Change-Id: I35c773fb4a15a1c8391c4129300ef2f0592d79ee --- keystonemiddleware/auth_token/__init__.py | 72 ++++++++----------- keystonemiddleware/auth_token/_utils.py | 25 ------- .../auth_token/test_auth_token_middleware.py | 1 - .../tests/unit/auth_token/test_utils.py | 31 -------- 4 files changed, 31 insertions(+), 98 deletions(-) delete mode 100644 keystonemiddleware/auth_token/_utils.py delete mode 100644 keystonemiddleware/tests/unit/auth_token/test_utils.py diff --git a/keystonemiddleware/auth_token/__init__.py b/keystonemiddleware/auth_token/__init__.py index 87c80435..e680a0c8 100644 --- a/keystonemiddleware/auth_token/__init__.py +++ b/keystonemiddleware/auth_token/__init__.py @@ -212,6 +212,7 @@ from keystoneclient import session from oslo_config import cfg from oslo_serialization import jsonutils import six +import webob.dec from keystonemiddleware.auth_token import _auth from keystonemiddleware.auth_token import _base @@ -221,7 +222,6 @@ from keystonemiddleware.auth_token import _identity from keystonemiddleware.auth_token import _revocations from keystonemiddleware.auth_token import _signing_dir from keystonemiddleware.auth_token import _user_plugin -from keystonemiddleware.auth_token import _utils from keystonemiddleware.i18n import _, _LC, _LE, _LI, _LW @@ -527,19 +527,8 @@ class AuthProtocol(object): else: return CONF[group][name] - def _call_app(self, env, start_response): - # NOTE(jamielennox): We wrap the given start response so that if an - # application with a 'delay_auth_decision' setting fails, or otherwise - # raises Unauthorized that we include the Authentication URL headers. - def _fake_start_response(status, response_headers, exc_info=None): - if status.startswith('401'): - response_headers.extend(self._reject_auth_headers) - - return start_response(status, response_headers, exc_info) - - return self._app(env, _fake_start_response) - - def __call__(self, env, start_response): + @webob.dec.wsgify + def __call__(self, request): """Handle incoming request. Authenticate send downstream on success. Reject request if @@ -556,8 +545,8 @@ class AuthProtocol(object): env.get('HTTP_X_SERVICE_ROLES'))) return msg - self._token_cache.initialize(env) - self._remove_auth_headers(env) + self._token_cache.initialize(request.environ) + self._remove_auth_headers(request.environ) try: user_auth_ref = None @@ -565,58 +554,61 @@ class AuthProtocol(object): try: self._LOG.debug('Authenticating user token') - user_token_info = self._get_user_token_from_header(env) + user_token_info = self._get_user_token_from_header( + request.environ) user_auth_ref, user_token_info = self._validate_token( - user_token_info, env) - env['keystone.token_info'] = user_token_info + user_token_info, request.environ) + request.environ['keystone.token_info'] = user_token_info user_headers = self._build_user_headers(user_auth_ref) - self._add_headers(env, user_headers) + self._add_headers(request.environ, user_headers) except exc.InvalidToken: if self._delay_auth_decision: self._LOG.info( _LI('Invalid user token - deferring reject ' 'downstream')) - self._add_headers(env, {'X-Identity-Status': 'Invalid'}) + self._add_headers(request.environ, + {'X-Identity-Status': 'Invalid'}) else: self._LOG.info( _LI('Invalid user token - rejecting request')) - return self._reject_request(env, start_response) + self._reject_request() try: self._LOG.debug('Authenticating service token') - serv_token = self._get_service_token_from_header(env) + serv_token = self._get_service_token_from_header( + request.environ) if serv_token is not None: serv_auth_ref, serv_token_info = self._validate_token( - serv_token, env) + serv_token, request.environ) serv_headers = self._build_service_headers(serv_auth_ref) - self._add_headers(env, serv_headers) + self._add_headers(request.environ, serv_headers) except exc.InvalidToken: if self._delay_auth_decision: self._LOG.info( _LI('Invalid service token - deferring reject ' 'downstream')) - self._add_headers(env, + self._add_headers(request.environ, {'X-Service-Identity-Status': 'Invalid'}) else: self._LOG.info( _LI('Invalid service token - rejecting request')) - return self._reject_request(env, start_response) + self._reject_request() - env['keystone.token_auth'] = _user_plugin.UserAuthPlugin( - user_auth_ref, serv_auth_ref) + p = _user_plugin.UserAuthPlugin(user_auth_ref, serv_auth_ref) + request.environ['keystone.token_auth'] = p except exc.ServiceError as e: self._LOG.critical(_LC('Unable to obtain admin token: %s'), e) - return self._do_503_error(env, start_response) + raise webob.exc.HTTPServiceUnavailable() - self._LOG.debug("Received request from %s", _fmt_msg(env)) + self._LOG.debug("Received request from %s", _fmt_msg(request.environ)) - return self._call_app(env, start_response) + response = request.get_response(self._app) - def _do_503_error(self, env, start_response): - resp = _utils.MiniResp('Service unavailable', env) - start_response('503 Service Unavailable', resp.headers) - return resp.body + if response.status_int == 401: + response.headers.extend(self._reject_auth_headers) + + return response def _init_auth_headers(self): """Initialize auth header list. @@ -683,7 +675,7 @@ class AuthProtocol(object): header_val = 'Keystone uri=\'%s\'' % self._auth_uri return [('WWW-Authenticate', header_val)] - def _reject_request(self, env, start_response): + def _reject_request(self): """Redirect client to auth server. :param env: wsgi request environment @@ -691,10 +683,8 @@ class AuthProtocol(object): :returns: HTTPUnauthorized http response """ - resp = _utils.MiniResp('Authentication required', - env, self._reject_auth_headers) - start_response('401 Unauthorized', resp.headers) - return resp.body + raise webob.exc.HTTPUnauthorized(body='Authentication required', + headers=self._reject_auth_headers) def _token_hashes(self, token): """Generate a list of hashes that the current token may be cached as. diff --git a/keystonemiddleware/auth_token/_utils.py b/keystonemiddleware/auth_token/_utils.py deleted file mode 100644 index ce24e500..00000000 --- a/keystonemiddleware/auth_token/_utils.py +++ /dev/null @@ -1,25 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - - -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.encode()] - self.headers = list(headers) - self.headers.append(('Content-type', 'text/plain')) diff --git a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py index ca4f1d86..c6372784 100644 --- a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py +++ b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py @@ -985,7 +985,6 @@ class CommonAuthTokenMiddlewareTest(object): self.assertEqual(401, resp.status_int) self.assertEqual("Keystone uri='https://keystone.example.com:1234'", resp.headers['WWW-Authenticate']) - self.assertEqual('', resp.body) def test_request_blank_token(self): resp = self.call_middleware(headers={'X-Auth-Token': ''}) diff --git a/keystonemiddleware/tests/unit/auth_token/test_utils.py b/keystonemiddleware/tests/unit/auth_token/test_utils.py deleted file mode 100644 index af590f32..00000000 --- a/keystonemiddleware/tests/unit/auth_token/test_utils.py +++ /dev/null @@ -1,31 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import testtools - -from keystonemiddleware.auth_token import _utils - - -class TokenEncodingTest(testtools.TestCase): - - def test_messages_encoded_as_bytes(self): - """Test that string are passed around as bytes for PY3.""" - msg = "This is an error" - - class FakeResp(_utils.MiniResp): - def __init__(self, error, env): - super(FakeResp, self).__init__(error, env) - - fake_resp = FakeResp(msg, dict(REQUEST_METHOD='GET')) - # On Py2 .encode() don't do much but that's better than to - # have a ifdef with six.PY3 - self.assertEqual(msg.encode(), fake_resp.body[0])