Merge "Wrapped unexpected exceptions (bug 955411)"

14 files changed, 121 insertions, 55 deletions

diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py
index cf883eee..ed78d010 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 0fc11bf7..3aeac4ef 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 cb4c321e..72647c27 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 f0fdfdc8..5193e0c4 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 f2759a5e..34bbbd69 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 9d1bd368..d09cb47a 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 e905dd38..c76201b0 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 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 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 TokenNotFound(NotFound):
-    """Could not find token: %(token_id)s"""
+class UnexpectedError(Error):
+    """An unexpected error prevented the server from fulfilling your request.
 
+    %(exception)s
 
-class ServiceNotFound(NotFound):
-    """Could not find service: %(service_id)s"""
+    """
+    code = 500
+    title = 'Internal Server Error'
diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py
index 18fe050a..963ffb94 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 f4671a64..b4810cb1 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 cb894b54..1305bbb0 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 a89f6f4d..a99671d6 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 76d77678..24442b06 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 6d812b7e..1cb455dd 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 ae7a17ae..a51efff2 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))