Merge "Prevent '500' error when using forbidden marker"

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,
@@ -368,6 +369,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
@@ -959,6 +970,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

@@ -442,6 +442,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(),
@@ -602,6 +621,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(),