Fixes v2 return status on unauthorized download

If a user is not allowed to download an image because of a policy,
glance v2 API responds with a  HTTP 200 status and no data instead
of HTTP 403 for no cache only.
The problem is that get_data implementation for notification
proxy is a generator, this situation delays the other proxies
get_data calls (including the policy proxy) for the first time
data is retrieved.
Hence, there is a delay in enforcing policy, so 200 is sent before
the API gets the chance to catch the policy exception.

DocImpact
Closes-Bug: #1326781
Change-Id: I1e50a069a6b7f9eed7160cd5908a5fa30274e227

glance/notifier.py

@@ -223,9 +223,9 @@ class ImageProxy(glance.domain.proxy.Image):
             'receiver_user_id': self.context.user,
         }
 
-    def get_data(self):
+    def _get_chunk_data_iterator(self, data):
         sent = 0
-        for chunk in self.image.get_data():
+        for chunk in data:
             yield chunk
             sent += len(chunk)
 
@@ -242,6 +242,13 @@ class ImageProxy(glance.domain.proxy.Image):
                      " notification: %(err)s") % {'err': err})
             LOG.error(msg)
 
+    def get_data(self):
+        # Due to the need of evaluating subsequent proxies, this one
+        # should return a generator, the call should be done before
+        # generator creation
+        data = self.image.get_data()
+        return self._get_chunk_data_iterator(data)
+
     def set_data(self, data, size=None):
         payload = format_image_notification(self.image)
         self.notifier.info('image.prepare', payload)

glance/tests/functional/v2/test_images.py

@@ -19,6 +19,7 @@ import tempfile
 import uuid
 
 import requests
+import six
 
 from glance.openstack.common import jsonutils
 from glance.tests import functional
@@ -451,6 +452,64 @@ class TestImages(functional.FunctionalTest):
 
         self.stop_servers()
 
+    def test_download_policy_when_cache_is_not_enabled(self):
+
+        rules = {'context_is_admin': 'role:admin', 'default': '',
+                 'download_image': '!'}
+        self.set_policy_rules(rules)
+        self.start_servers(**self.__dict__.copy())
+
+        # Create an image
+        path = self._url('/v2/images')
+        headers = self._headers({'content-type': 'application/json',
+                                 'X-Roles': 'member'})
+        data = jsonutils.dumps({'name': 'image-1', 'disk_format': 'aki',
+                                'container_format': 'aki'})
+        response = requests.post(path, headers=headers, data=data)
+        self.assertEqual(201, response.status_code)
+
+        # Returned image entity
+        image = jsonutils.loads(response.text)
+        image_id = image['id']
+        expected_image = {
+            'status': 'queued',
+            'name': 'image-1',
+            'tags': [],
+            'visibility': 'private',
+            'self': '/v2/images/%s' % image_id,
+            'protected': False,
+            'file': '/v2/images/%s/file' % image_id,
+            'min_disk': 0,
+            'min_ram': 0,
+            'schema': '/v2/schemas/image',
+        }
+        for key, value in six.iteritems(expected_image):
+            self.assertEqual(image[key], value, key)
+
+        # Upload data to image
+        path = self._url('/v2/images/%s/file' % image_id)
+        headers = self._headers({'Content-Type': 'application/octet-stream'})
+        response = requests.put(path, headers=headers, data='ZZZZZ')
+        self.assertEqual(204, response.status_code)
+
+        # Get an image should fail
+        path = self._url('/v2/images/%s/file' % image_id)
+        headers = self._headers({'Content-Type': 'application/octet-stream'})
+        response = requests.get(path, headers=headers)
+        self.assertEqual(403, response.status_code)
+
+        # Image Deletion should work
+        path = self._url('/v2/images/%s' % image_id)
+        response = requests.delete(path, headers=self._headers())
+        self.assertEqual(204, response.status_code)
+
+        # This image should be no longer be directly accessible
+        path = self._url('/v2/images/%s' % image_id)
+        response = requests.get(path, headers=self._headers())
+        self.assertEqual(404, response.status_code)
+
+        self.stop_servers()
+
     def test_permissions(self):
         # Create an image that belongs to TENANT1
         path = self._url('/v2/images')

glance/tests/unit/test_notifier.py

@@ -197,6 +197,12 @@ class TestImageNotifications(utils.BaseTestCase):
         self.assertIsInstance(images[0], glance.notifier.ImageProxy)
         self.assertEqual(images[0].image, 'images_from_list')
 
+    def test_image_get_data_should_call_next_image_get_data(self):
+        with mock.patch.object(self.image, 'get_data') as get_data_mock:
+            self.image_proxy.get_data()
+
+            self.assertTrue(get_data_mock.called)
+
     def test_image_get_data_notification(self):
         self.image_proxy.size = 10
         data = ''.join(self.image_proxy.get_data())