Prevent '500' error when using forbidden marker

If an existing image id was used as a marker, but
the user didn't have access to that image a '500'
error occurred. Return '400' instead.

Fixes bug 1178307.

Change-Id: Ib632297d944a19e18694fff154307a3bc6d5b625

glance/db/sqlalchemy/api.py

@@ -296,11 +296,15 @@ def _image_get(context, image_id, session=None, force_show_deleted=False):
         image = query.one()
 
     except sa_orm.exc.NoResultFound:
-        raise exception.NotFound("No image found with ID %s" % image_id)
+        msg = (_("No image found with ID %s") % image_id)
+        LOG.debug(msg)
+        raise exception.NotFound(msg)
 
     # Make sure they can look at it
     if not is_image_visible(context, image):
-        raise exception.Forbidden("Image not visible to you")
+        msg = (_("Forbidding request, image %s not visible") % image_id)
+        LOG.debug(msg)
+        raise exception.Forbidden(msg)
 
     return image
 

glance/registry/api/v1/images.py

@@ -70,7 +70,7 @@ class Controller(object):
         try:
             return self.db_api.image_get_all(context, filters=filters,
                                              **params)
-        except exception.NotFound as e:
+        except (exception.NotFound, exception.Forbidden) as e:
             msg = _("Invalid marker. Image could not be found.")
             raise exc.HTTPBadRequest(explanation=msg)
 

glance/tests/unit/v1/test_registry_api.py

@@ -119,6 +119,7 @@ class TestRegistryAPI(base.IsolatedUnitTest):
              'min_disk': 0,
              'min_ram': 0,
              'size': 13,
+             'owner': '123',
              'locations': ["file:///%s/%s" % (self.test_dir, UUID1)],
              'properties': {'type': 'kernel'}},
             {'id': UUID2,
@@ -349,6 +350,16 @@ class TestRegistryAPI(base.IsolatedUnitTest):
         self.assertEquals(res.status_int, 400)
         self.assertTrue('marker' in res.body)
 
+    def test_get_index_forbidden_marker(self):
+        """
+        Tests that the /images registry API returns a 400
+        when a forbidden marker is provided
+        """
+        self.context = glance.context.RequestContext(is_admin=False)
+        req = webob.Request.blank('/images?marker=%s' % UUID1)
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 400)
+
     def test_get_index_limit(self):
         """
         Tests that the /images registry API returns list of
@@ -940,6 +951,26 @@ class TestRegistryAPI(base.IsolatedUnitTest):
         res = req.get_response(self.api)
         self.assertEquals(res.status_int, 400)
 
+    def test_get_details_malformed_marker(self):
+        """
+        Tests that the /images/detail registry API returns a 400
+        when a malformed marker is provided
+        """
+        req = webob.Request.blank('/images/detail?marker=4')
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 400)
+        self.assertTrue('marker' in res.body)
+
+    def test_get_details_forbidden_marker(self):
+        """
+        Tests that the /images/detail registry API returns a 400
+        when a forbidden marker is provided
+        """
+        self.context = glance.context.RequestContext(is_admin=False)
+        req = webob.Request.blank('/images/detail?marker=%s' % UUID1)
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 400)
+
     def test_get_details_filter_name(self):
         """
         Tests that the /images/detail registry API returns list of

glance/tests/unit/v1/test_registry_client.py

@@ -439,6 +439,25 @@ class TestRegistryV1Client(base.IsolatedUnitTest):
                           self.client.get_images,
                           marker=_gen_uuid())
 
+    def test_get_image_index_forbidden_marker(self):
+        """Test exception is raised when marker is forbidden"""
+        UUID5 = _gen_uuid()
+        extra_fixture = {'id': UUID5,
+                         'status': 'saving',
+                         'is_public': False,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #125',
+                         'size': 19,
+                         'owner': '0123',
+                         'checksum': None}
+
+        db_api.image_create(self.context, extra_fixture)
+        self.context = context.RequestContext(is_admin=False)
+        self.assertRaises(exception.Invalid,
+                          self.client.get_images,
+                          marker=UUID5)
+
     def test_get_image_index_limit(self):
         """Test correct number of images returned with limit param."""
         extra_fixture = {'id': _gen_uuid(),
@@ -599,6 +618,25 @@ class TestRegistryV1Client(base.IsolatedUnitTest):
                           self.client.get_images_detailed,
                           marker=_gen_uuid())
 
+    def test_get_image_details_forbidden_marker(self):
+        """Test exception is raised when marker is forbidden"""
+        UUID5 = _gen_uuid()
+        extra_fixture = {'id': UUID5,
+                         'status': 'saving',
+                         'is_public': False,
+                         'disk_format': 'vhd',
+                         'container_format': 'ovf',
+                         'name': 'new name! #125',
+                         'size': 19,
+                         'owner': '0123',
+                         'checksum': None}
+
+        db_api.image_create(self.context, extra_fixture)
+        self.context = context.RequestContext(is_admin=False)
+        self.assertRaises(exception.Invalid,
+                          self.client.get_images_detailed,
+                          marker=UUID5)
+
     def test_get_image_details_by_name(self):
         """Tests that a detailed call can be filtered by name"""
         extra_fixture = {'id': _gen_uuid(),