diff --git a/keystone/common/wsgi.py b/keystone/common/wsgi.py index 34caec2c4d..c52df414ed 100644 --- a/keystone/common/wsgi.py +++ b/keystone/common/wsgi.py @@ -32,6 +32,7 @@ import webob import webob.dec import webob.exc +from keystone import exception from keystone.common import utils @@ -153,16 +154,13 @@ class Application(BaseApplication): @webob.dec.wsgify def __call__(self, req): arg_dict = req.environ['wsgiorg.routing_args'][1] - action = arg_dict['action'] - del arg_dict['action'] + action = arg_dict.pop('action') del arg_dict['controller'] logging.debug('arg_dict: %s', arg_dict) + # allow middleware up the stack to provide context & params context = req.environ.get('openstack.context', {}) - # allow middleware up the stack to override the params - params = {} - if 'openstack.params' in req.environ: - params = req.environ['openstack.params'] + params = req.environ.get('openstack.params', {}) params.update(arg_dict) # TODO(termie): do some basic normalization on methods @@ -170,7 +168,12 @@ class Application(BaseApplication): # NOTE(vish): make sure we have no unicode keys for py2.6. params = self._normalize_dict(params) - result = method(context, **params) + + try: + result = method(context, **params) + except exception.Error as e: + logging.warning(e) + return render_exception(e) if result is None or type(result) is str or type(result) is unicode: return result @@ -435,3 +438,22 @@ class ExtensionRouter(Router): conf.update(local_config) return cls(app) return _factory + + +def render_exception(error): + """Forms a WSGI response based on the current error.""" + resp = webob.Response() + resp.status = '%s %s' % (error.code, error.title) + resp.headerlist = [('Content-Type', 'application/json')] + + body = { + 'error': { + 'code': error.code, + 'title': error.title, + 'message': str(error), + } + } + + resp.body = json.dumps(body) + + return resp diff --git a/keystone/exception.py b/keystone/exception.py new file mode 100644 index 0000000000..cefc260431 --- /dev/null +++ b/keystone/exception.py @@ -0,0 +1,54 @@ +import re + + +class Error(StandardError): + """Base error class. + + Child classes should define an HTTP status code, title, and a doc string. + + """ + code = None + title = None + + def __init__(self, message=None, **kwargs): + """Use the doc string as the error message by default.""" + message = message or self.__doc__ % kwargs + super(Error, self).__init__(message) + + def __str__(self): + """Cleans up line breaks and indentation from doc strings.""" + string = super(Error, self).__str__() + string = re.sub('[ \n]+', ' ', string) + string = string.strip() + return string + + +class ValidationError(Error): + """Expecting to find %(attribute)s in %(target)s. + + The server could not comply with the request since it is either malformed + or otherwise incorrect. + + The client is assumed to be in error. + + """ + code = 400 + title = 'Bad Request' + + +class Unauthorized(Error): + """The request you have made requires authentication.""" + code = 401 + title = 'Not Authorized' + + +class Forbidden(Error): + """You are not authorized to perform the requested action: %(action)s""" + code = 403 + title = 'Not Authorized' + + +class NotFound(Error): + """Could not find: %(target)s""" + code = 404 + title = 'Not Found' diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 80ca31d121..5982a7ab00 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -10,6 +10,7 @@ import webob.exc from keystone import catalog from keystone import config +from keystone import exception from keystone import policy from keystone import token from keystone.common import manager @@ -233,7 +234,9 @@ class TenantController(wsgi.Application): """ token_ref = self.token_api.get_token(context=context, token_id=context['token_id']) - assert token_ref is not None + + if token_ref is None: + raise exception.Unauthorized() user_ref = token_ref['user'] tenant_ids = self.identity_api.get_tenants_for_user( diff --git a/keystone/service.py b/keystone/service.py index eae882d017..e2698cf7c1 100644 --- a/keystone/service.py +++ b/keystone/service.py @@ -10,6 +10,7 @@ import webob.dec import webob.exc from keystone import catalog +from keystone import exception from keystone import identity from keystone import policy from keystone import token @@ -196,6 +197,10 @@ class TokenController(wsgi.Application): old_token_ref = self.token_api.get_token(context=context, token_id=token) + + if old_token_ref is None: + raise exception.Unauthorized() + user_ref = old_token_ref['user'] tenants = self.identity_api.get_tenants_for_user(context, @@ -247,6 +252,10 @@ class TokenController(wsgi.Application): token_ref = self.token_api.get_token(context=context, token_id=token_id) + + if token_ref is None: + raise exception.NotFound(target='token') + if belongs_to: assert token_ref['tenant']['id'] == belongs_to diff --git a/tests/test_exception.py b/tests/test_exception.py new file mode 100644 index 0000000000..2c4b948406 --- /dev/null +++ b/tests/test_exception.py @@ -0,0 +1,53 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 +import uuid +import json + +from keystone.common import wsgi +from keystone import exception +from keystone import test + + +class ExceptionTestCase(test.TestCase): + def setUp(self): + pass + + def tearDown(self): + pass + + def assertValidJsonRendering(self, e): + resp = wsgi.render_exception(e) + self.assertEqual(resp.status_int, e.code) + self.assertEqual(resp.status, '%s %s' % (e.code, e.title)) + + j = json.loads(resp.body) + self.assertIsNotNone(j.get('error')) + self.assertIsNotNone(j['error'].get('code')) + self.assertIsNotNone(j['error'].get('title')) + self.assertIsNotNone(j['error'].get('message')) + self.assertNotIn('\n', j['error']['message']) + self.assertNotIn(' ', j['error']['message']) + self.assertTrue(type(j['error']['code']) is int) + + def test_validation_error(self): + target = uuid.uuid4().hex + attribute = uuid.uuid4().hex + e = exception.ValidationError(target=target, attribute=attribute) + self.assertValidJsonRendering(e) + self.assertIn(target, str(e)) + self.assertIn(attribute, str(e)) + + def test_unauthorized(self): + e = exception.Unauthorized() + self.assertValidJsonRendering(e) + + def test_forbidden(self): + action = uuid.uuid4().hex + e = exception.Forbidden(action=action) + self.assertValidJsonRendering(e) + self.assertIn(action, str(e)) + + def test_not_found(self): + target = uuid.uuid4().hex + e = exception.NotFound(target=target) + self.assertValidJsonRendering(e) + self.assertIn(target, str(e)) diff --git a/tests/test_keystoneclient.py b/tests/test_keystoneclient.py index 7f00b2a03e..036e62d8c5 100644 --- a/tests/test_keystoneclient.py +++ b/tests/test_keystoneclient.py @@ -125,6 +125,8 @@ class KeystoneClientTests(object): self.assertEquals(tenants[0].id, self.tenant_bar['id']) def test_authenticate_and_delete_token(self): + from keystoneclient import exceptions as client_exceptions + client = self.get_client() token = client.auth_token token_client = self._client(token=token) @@ -133,10 +135,26 @@ class KeystoneClientTests(object): client.tokens.delete(token_client.auth_token) - # FIXME(dolph): this should raise unauthorized - # from keystoneclient import exceptions as client_exceptions - # with self.assertRaises(client_exceptions.Unauthorized): - self.assertRaises(Exception, token_client.tenants.list) + self.assertRaises(client_exceptions.Unauthorized, + token_client.tenants.list) + + def test_authenticate_no_password(self): + from keystoneclient import exceptions as client_exceptions + + user_ref = self.user_foo.copy() + user_ref['password'] = None + self.assertRaises(client_exceptions.AuthorizationFailure, + self.get_client, + user_ref) + + def test_authenticate_no_username(self): + from keystoneclient import exceptions as client_exceptions + + user_ref = self.user_foo.copy() + user_ref['name'] = None + self.assertRaises(client_exceptions.AuthorizationFailure, + self.get_client, + user_ref) # TODO(termie): I'm not really sure that this is testing much def test_endpoints(self):