diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py index cf883eee7c..ed78d010b3 100644 --- a/keystone/catalog/backends/sql.py +++ b/keystone/catalog/backends/sql.py @@ -125,7 +125,7 @@ class Catalog(sql.Base, catalog.Driver): endpoint_ref = session.query(Endpoint) endpoint_ref = endpoint_ref.filter_by(id=endpoint_id).first() if not endpoint_ref: - raise exception.NotFound('Endpoint not found') + raise exception.EndpointNotFound(endpoint_id=endpoint_id) with session.begin(): session.delete(endpoint_ref) session.flush() diff --git a/keystone/catalog/core.py b/keystone/catalog/core.py index 0fc11bf797..3aeac4ef01 100644 --- a/keystone/catalog/core.py +++ b/keystone/catalog/core.py @@ -19,8 +19,6 @@ import uuid -import webob.exc - from keystone import config from keystone import exception from keystone import identity @@ -131,7 +129,7 @@ class ServiceController(wsgi.Application): def get_service(self, context, service_id): service_ref = self.catalog_api.get_service(context, service_id) if not service_ref: - raise webob.exc.HTTPNotFound() + raise exception.ServiceNotFound(service_id=service_id) return {'OS-KSADM:service': service_ref} def delete_service(self, context, service_id): @@ -168,11 +166,8 @@ class EndpointController(wsgi.Application): endpoint_ref['id'] = endpoint_id service_id = endpoint_ref['service_id'] - try: - service = self.catalog_api.get_service(context, service_id) - except exception.ServiceNotFound: - msg = 'No service exists with id %s' % service_id - raise webob.exc.HTTPBadRequest(msg) + if not self.catalog_api.service_exists(context, service_id): + raise exception.ServiceNotFound(service_id=service_id) new_endpoint_ref = self.catalog_api.create_endpoint( context, endpoint_id, endpoint_ref) diff --git a/keystone/common/ldap/core.py b/keystone/common/ldap/core.py index cb4c321e08..72647c276d 100644 --- a/keystone/common/ldap/core.py +++ b/keystone/common/ldap/core.py @@ -16,6 +16,7 @@ import ldap +from keystone import exception from keystone.common import logging from keystone.common.ldap import fakeldap @@ -140,14 +141,16 @@ class BaseLdap(object): if values['name'] is not None: entity = self.get_by_name(values['name']) if entity is not None: - raise Exception('%s with id %s already exists' - % (self.options_name, values['id'])) + raise exception.Conflict(type=self.options_name, + details='Duplicate name, %s.' % + values['name']) if values['id'] is not None: entity = self.get(values['id']) if entity is not None: - raise Exception('%s with id %s already exists' - % (self.options_name, values['id'])) + raise exception.Conflict(type=self.options_name, + details='Duplicate ID, %s.' % + values['id']) def create(self, values): conn = self.get_connection() diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py index f0fdfdc894..5193e0c416 100644 --- a/keystone/common/sql/core.py +++ b/keystone/common/sql/core.py @@ -43,6 +43,7 @@ Column = sql.Column String = sql.String ForeignKey = sql.ForeignKey DateTime = sql.DateTime +IntegrityError = sql.exc.IntegrityError # Special Fields diff --git a/keystone/common/wsgi.py b/keystone/common/wsgi.py index f2759a5ee7..34bbbd6993 100644 --- a/keystone/common/wsgi.py +++ b/keystone/common/wsgi.py @@ -185,6 +185,9 @@ class Application(BaseApplication): except exception.Error as e: LOG.warning(e) return render_exception(e) + except Exception as e: + logging.exception(e) + return render_exception(exception.UnexpectedError(exception=e)) if result is None: return render_response(status=(204, 'No Content')) diff --git a/keystone/contrib/ec2/core.py b/keystone/contrib/ec2/core.py index 9d1bd368c6..d09cb47aa2 100644 --- a/keystone/contrib/ec2/core.py +++ b/keystone/contrib/ec2/core.py @@ -36,8 +36,6 @@ glance to list images needed to perform the requested task. import uuid -import webob.exc - from keystone import catalog from keystone import config from keystone import exception @@ -114,12 +112,9 @@ class Ec2Controller(wsgi.Application): credentials['host'] = hostname signature = signer.generate(credentials) if not utils.auth_str_equal(credentials.signature, signature): - # TODO(termie): proper exception - msg = 'Invalid signature' - raise webob.exc.HTTPUnauthorized(explanation=msg) + raise exception.Unauthorized(message='Invalid EC2 signature.') else: - msg = 'Signature not supplied' - raise webob.exc.HTTPUnauthorized(explanation=msg) + raise exception.Unauthorized(message='EC2 signature not supplied.') def authenticate(self, context, credentials=None, ec2Credentials=None): @@ -152,8 +147,7 @@ class Ec2Controller(wsgi.Application): creds_ref = self.ec2_api.get_credential(context, credentials['access']) if not creds_ref: - msg = 'Access key not found' - raise webob.exc.HTTPUnauthorized(explanation=msg) + raise exception.Unauthorized(message='EC2 access key not found.') self.check_signature(creds_ref, credentials) @@ -263,7 +257,7 @@ class Ec2Controller(wsgi.Application): :param context: standard context :param user_id: id of user - :raises webob.exc.HTTPForbidden: when token is invalid + :raises exception.Forbidden: when token is invalid """ try: @@ -273,7 +267,7 @@ class Ec2Controller(wsgi.Application): raise exception.Unauthorized() token_user_id = token_ref['user'].get('id') if not token_user_id == user_id: - raise webob.exc.HTTPForbidden() + raise exception.Forbidden() def _is_admin(self, context): """Wrap admin assertion error return statement. @@ -294,9 +288,9 @@ class Ec2Controller(wsgi.Application): :param context: standard context :param user_id: expected credential owner :param credential_id: id of credential object - :raises webob.exc.HTTPForbidden: on failure + :raises exception.Forbidden: on failure """ cred_ref = self.ec2_api.get_credential(context, credential_id) if not user_id == cred_ref['user_id']: - raise webob.exc.HTTPForbidden() + raise exception.Forbidden() diff --git a/keystone/exception.py b/keystone/exception.py index e905dd3850..c76201b081 100644 --- a/keystone/exception.py +++ b/keystone/exception.py @@ -58,26 +58,66 @@ class Unauthorized(Error): class Forbidden(Error): - """You are not authorized to perform the requested action: %(action)s""" + """You are not authorized to perform the requested action.""" code = 403 title = 'Not Authorized' +class ForbiddenAction(Forbidden): + """You are not authorized to perform the requested action: %(action)s""" + + class NotFound(Error): """Could not find: %(target)s""" code = 404 title = 'Not Found' -class NotImplemented(Error): - """The action you have requested has not been implemented.""" - code = 501 - action = 'Not Implemented' +class EndpointNotFound(NotFound): + """Could not find endopint: %(endpoint_id)s""" + + +class RoleNotFound(NotFound): + """Could not find role: %(role_id)s""" + + +class ServiceNotFound(NotFound): + """Could not find service: %(service_id)s""" + + +class TenantNotFound(NotFound): + """Could not find tenant: %(tenant_id)s""" class TokenNotFound(NotFound): """Could not find token: %(token_id)s""" -class ServiceNotFound(NotFound): - """Could not find service: %(service_id)s""" +class UserNotFound(NotFound): + """Could not find user: %(user_id)s""" + + +class Conflict(Error): + """Conflict occurred attempting to store %(type)s. + + %(details)s + + """ + code = 409 + title = 'Conflict' + + +class NotImplemented(Error): + """The action you have requested has not been implemented.""" + code = 501 + action = 'Not Implemented' + + +class UnexpectedError(Error): + """An unexpected error prevented the server from fulfilling your request. + + %(exception)s + + """ + code = 500 + title = 'Internal Server Error' diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py index 18fe050ac3..963ffb9496 100644 --- a/keystone/identity/backends/kvs.py +++ b/keystone/identity/backends/kvs.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. +from keystone import exception from keystone import identity from keystone.common import kvs from keystone.common import utils @@ -151,9 +152,11 @@ class Identity(kvs.Base, identity.Driver): # CRUD def create_user(self, user_id, user): if self.get_user(user_id): - raise Exception('Duplicate id') + msg = 'Duplicate ID, %s.' % user_id + raise exception.Conflict(type='user', details=msg) if self.get_user_by_name(user['name']): - raise Exception('Duplicate name') + msg = 'Duplicate name, %s.' % user['name'] + raise exception.Conflict(type='user', details=msg) user = _ensure_hashed_password(user) self.db.set('user-%s' % user_id, user) self.db.set('user_name-%s' % user['name'], user) @@ -166,7 +169,8 @@ class Identity(kvs.Base, identity.Driver): if 'name' in user: existing = self.db.get('user_name-%s' % user['name']) if existing and user_id != existing['id']: - raise Exception('Duplicate name') + msg = 'Duplicate name, %s.' % user['name'] + raise exception.Conflict(type='user', details=msg) # get the old name and delete it too old_user = self.db.get('user-%s' % user_id) new_user = old_user.copy() @@ -189,9 +193,11 @@ class Identity(kvs.Base, identity.Driver): def create_tenant(self, tenant_id, tenant): if self.get_tenant(tenant_id): - raise Exception('Duplicate id') + msg = 'Duplicate ID, %s.' % tenant_id + raise exception.Conflict(type='tenant', details=msg) if self.get_tenant_by_name(tenant['name']): - raise Exception('Duplicate name') + msg = 'Duplicate name, %s.' % tenant['name'] + raise exception.Conflict(type='tenant', details=msg) self.db.set('tenant-%s' % tenant_id, tenant) self.db.set('tenant_name-%s' % tenant['name'], tenant) return tenant @@ -200,7 +206,8 @@ class Identity(kvs.Base, identity.Driver): if 'name' in tenant: existing = self.db.get('tenant_name-%s' % tenant['name']) if existing and tenant_id != existing['id']: - raise Exception('Duplicate name') + msg = 'Duplicate name, %s.' % tenant['name'] + raise exception.Conflict(type='tenant', details=msg) # get the old name and delete it too old_tenant = self.db.get('tenant-%s' % tenant_id) new_tenant = old_tenant.copy() diff --git a/keystone/identity/backends/ldap/core.py b/keystone/identity/backends/ldap/core.py index f4671a64fb..b4810cb127 100644 --- a/keystone/identity/backends/ldap/core.py +++ b/keystone/identity/backends/ldap/core.py @@ -545,7 +545,7 @@ class RoleApi(common_ldap.BaseLdap, ApiShimMixin): def add_user(self, role_id, user_id, tenant_id=None): user = self.user_api.get(user_id) if user is None: - raise exception.NotFound('User %s not found' % (user_id,)) + raise exception.UserNotFound(user_id=user_id) role_dn = self._subrole_id_to_dn(role_id, tenant_id) conn = self.get_connection() user_dn = self.user_api._id_to_dn(user_id) diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index cb894b5404..1305bbb0a4 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -15,8 +15,10 @@ # under the License. import copy +import functools from keystone import identity +from keystone import exception from keystone.common import sql from keystone.common import utils from keystone.common.sql import migration @@ -35,6 +37,19 @@ def _ensure_hashed_password(user_ref): return user_ref +def handle_conflicts(type='object'): + """Converts IntegrityError into HTTP 409 Conflict.""" + def decorator(method): + @functools.wraps(method) + def wrapper(*args, **kwargs): + try: + return method(*args, **kwargs) + except sql.IntegrityError as e: + raise exception.Conflict(type=type, details=str(e)) + return wrapper + return decorator + + class User(sql.ModelBase, sql.DictBase): __tablename__ = 'user' id = sql.Column(sql.String(64), primary_key=True) @@ -280,6 +295,7 @@ class Identity(sql.Base, identity.Driver): self.create_metadata(user_id, tenant_id, metadata_ref) # CRUD + @handle_conflicts(type='user') def create_user(self, user_id, user): user = _ensure_hashed_password(user) session = self.get_session() @@ -289,6 +305,7 @@ class Identity(sql.Base, identity.Driver): session.flush() return user_ref.to_dict() + @handle_conflicts(type='user') def update_user(self, user_id, user): session = self.get_session() with session.begin(): @@ -311,6 +328,7 @@ class Identity(sql.Base, identity.Driver): session.delete(user_ref) session.flush() + @handle_conflicts(type='tenant') def create_tenant(self, tenant_id, tenant): session = self.get_session() with session.begin(): @@ -319,6 +337,7 @@ class Identity(sql.Base, identity.Driver): session.flush() return tenant_ref.to_dict() + @handle_conflicts(type='tenant') def update_tenant(self, tenant_id, tenant): session = self.get_session() with session.begin(): @@ -340,6 +359,7 @@ class Identity(sql.Base, identity.Driver): session.delete(tenant_ref) session.flush() + @handle_conflicts(type='metadata') def create_metadata(self, user_id, tenant_id, metadata): session = self.get_session() with session.begin(): @@ -349,6 +369,7 @@ class Identity(sql.Base, identity.Driver): session.flush() return metadata + @handle_conflicts(type='metadata') def update_metadata(self, user_id, tenant_id, metadata): session = self.get_session() with session.begin(): @@ -367,6 +388,7 @@ class Identity(sql.Base, identity.Driver): self.db.delete('metadata-%s-%s' % (tenant_id, user_id)) return None + @handle_conflicts(type='role') def create_role(self, role_id, role): session = self.get_session() with session.begin(): @@ -374,6 +396,7 @@ class Identity(sql.Base, identity.Driver): session.flush() return role + @handle_conflicts(type='role') def update_role(self, role_id, role): session = self.get_session() with session.begin(): diff --git a/keystone/identity/core.py b/keystone/identity/core.py index a89f6f4d46..a99671d687 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -20,8 +20,6 @@ import uuid import urllib import urlparse -import webob.exc - from keystone import config from keystone import exception from keystone import policy @@ -287,7 +285,7 @@ class TenantController(wsgi.Application): self.assert_admin(context) tenant = self.identity_api.get_tenant(context, tenant_id) if not tenant: - return webob.exc.HTTPNotFound() + raise exception.TenantNotFound(tenant_id=tenant_id) return {'tenant': tenant} # CRUD Extension @@ -329,16 +327,17 @@ class TenantController(wsgi.Application): break else: msg = 'Marker could not be found' - raise webob.exc.HTTPBadRequest(explanation=msg) + raise exception.ValidationError(message=msg) limit = kwargs.get('limit') if limit is not None: try: limit = int(limit) - assert limit >= 0 + if limit < 0: + raise AssertionError() except (ValueError, AssertionError): msg = 'Invalid limit value' - raise webob.exc.HTTPBadRequest(explanation=msg) + raise exception.ValidationError(message=msg) tenant_refs = tenant_refs[page_idx:limit] @@ -361,7 +360,7 @@ class UserController(wsgi.Application): self.assert_admin(context) user_ref = self.identity_api.get_user(context, user_id) if not user_ref: - raise webob.exc.HTTPNotFound() + raise exception.UserNotFound(user_id=user_id) return {'user': user_ref} def get_users(self, context): @@ -425,7 +424,8 @@ class RoleController(wsgi.Application): """ if tenant_id is None: - raise Exception('User roles not supported: tenant_id required') + raise exception.NotImplemented(message='User roles not supported: ' + 'tenant_id required') roles = self.identity_api.get_roles_for_user_and_tenant( context, user_id, tenant_id) return {'roles': [self.identity_api.get_role(context, x) @@ -436,7 +436,7 @@ class RoleController(wsgi.Application): self.assert_admin(context) role_ref = self.identity_api.get_role(context, role_id) if not role_ref: - raise webob.exc.HTTPNotFound() + raise exception.RoleNotFound(role_id=role_id) return {'role': role_ref} def create_role(self, context, role): @@ -466,7 +466,8 @@ class RoleController(wsgi.Application): """ self.assert_admin(context) if tenant_id is None: - raise Exception('User roles not supported: tenant_id required') + raise exception.NotImplemented(message='User roles not supported: ' + 'tenant_id required') # This still has the weird legacy semantics that adding a role to # a user also adds them to a tenant @@ -485,7 +486,8 @@ class RoleController(wsgi.Application): """ self.assert_admin(context) if tenant_id is None: - raise Exception('User roles not supported: tenant_id required') + raise exception.NotImplemented(message='User roles not supported: ' + 'tenant_id required') # This still has the weird legacy semantics that adding a role to # a user also adds them to a tenant diff --git a/keystone/policy/backends/rules.py b/keystone/policy/backends/rules.py index 76d7767877..24442b06ca 100644 --- a/keystone/policy/backends/rules.py +++ b/keystone/policy/backends/rules.py @@ -97,7 +97,7 @@ def enforce(credentials, action, target): try: common_policy.enforce(match_list, target, credentials) except common_policy.NotAuthorized: - raise exception.Forbidden(action=action) + raise exception.ForbiddenAction(action=action) class Policy(policy.Driver): diff --git a/keystone/service.py b/keystone/service.py index 6d812b7e0e..1cb455ddf7 100644 --- a/keystone/service.py +++ b/keystone/service.py @@ -17,8 +17,6 @@ import uuid import routes -import webob.dec -import webob.exc from keystone import catalog from keystone import exception @@ -277,7 +275,7 @@ class TokenController(wsgi.Application): # If the user is disabled don't allow them to authenticate if not user_ref.get('enabled', True): - raise webob.exc.HTTPForbidden('User has been disabled') + raise exception.Forbidden(message='User has been disabled') except AssertionError as e: raise exception.Unauthorized(e.message) diff --git a/tests/test_exception.py b/tests/test_exception.py index ae7a17aebd..a51efff2ca 100644 --- a/tests/test_exception.py +++ b/tests/test_exception.py @@ -54,9 +54,9 @@ class ExceptionTestCase(test.TestCase): e = exception.Unauthorized() self.assertValidJsonRendering(e) - def test_forbidden(self): + def test_forbidden_action(self): action = uuid.uuid4().hex - e = exception.Forbidden(action=action) + e = exception.ForbiddenAction(action=action) self.assertValidJsonRendering(e) self.assertIn(action, str(e))