From 95d0747b8063ab1389920768344ee3bf32abdb3f Mon Sep 17 00:00:00 2001
From: Darja Shakhray <dshakhray@mirantis.com>
Date: Mon, 28 Sep 2015 21:10:05 +0300
Subject: [PATCH] Fix error when downloading image status is not active

Added check image_status when downloading image.

Change-Id: I39d49ea8a828863fdf4d5efc98126a9009e4b098
Closes-bug: #1472449
(cherry picked from commit d4d94b290ceb9147dd285822e201dd85ce812ef0)
---
 glance/api/v2/image_data.py                      |  7 +++---
 glance/tests/functional/test_cache_middleware.py |  4 ++--
 glance/tests/functional/v2/test_images.py        |  2 +-
 glance/tests/unit/v2/test_image_data_resource.py | 29 +++++++++++++++++++++---
 4 files changed, 33 insertions(+), 9 deletions(-)

Index: glance-11.0.2.dev5/glance/api/v2/image_data.py
===================================================================
--- glance-11.0.2.dev5.orig/glance/api/v2/image_data.py
+++ glance-11.0.2.dev5/glance/api/v2/image_data.py
@@ -209,9 +209,10 @@ class ImageDataController(object):
         image_repo = self.gateway.get_repo(req.context)
         try:
             image = image_repo.get(image_id)
-            if image.status == 'deactivated':
-                msg = _('The requested image has been deactivated. '
-                        'Image data download is forbidden.')
+            if (image.status != 'active' and (image.status != 'deactivated'
+                                              or not req.context.is_admin)):
+                msg = _('The requested image is in status %s. '
+                        'Image data download is forbidden.') % image.status
                 raise exception.Forbidden(message=msg)
         except exception.ImageDataNotFound as e:
             raise webob.exc.HTTPNoContent(explanation=e.msg)
Index: glance-11.0.2.dev5/glance/tests/functional/test_cache_middleware.py
===================================================================
--- glance-11.0.2.dev5.orig/glance/tests/functional/test_cache_middleware.py
+++ glance-11.0.2.dev5/glance/tests/functional/test_cache_middleware.py
@@ -386,12 +386,12 @@ class BaseCacheMiddlewareTest(object):
         response, content = http.request(path, 'GET')
         self.assertEqual(403, response.status)
 
-        # Download the image with v2. Ensure it is forbidden
+        # Download the image with v2.
         path = "http://%s:%d/v2/images/%s/file" % ("127.0.0.1", self.api_port,
                                                    image_id)
         http = httplib2.Http()
         response, content = http.request(path, 'GET')
-        self.assertEqual(403, response.status)
+        self.assertEqual(200, response.status)
 
         # Reactivate the image using v2
         path = "http://%s:%d/v2/images/%s/actions/reactivate"
Index: glance-11.0.2.dev5/glance/tests/functional/v2/test_images.py
===================================================================
--- glance-11.0.2.dev5.orig/glance/tests/functional/v2/test_images.py
+++ glance-11.0.2.dev5/glance/tests/functional/v2/test_images.py
@@ -438,7 +438,7 @@ class TestImages(functional.FunctionalTe
         path = self._url('/v2/images/%s/file' % image_id)
         headers = self._headers()
         response = requests.get(path, headers=headers)
-        self.assertEqual(204, response.status_code)
+        self.assertEqual(403, response.status_code)
 
         def _verify_image_checksum_and_status(checksum, status):
             # Checksum should be populated and status should be active
Index: glance-11.0.2.dev5/glance/tests/unit/v2/test_image_data_resource.py
===================================================================
--- glance-11.0.2.dev5.orig/glance/tests/unit/v2/test_image_data_resource.py
+++ glance-11.0.2.dev5/glance/tests/unit/v2/test_image_data_resource.py
@@ -105,6 +105,7 @@ class TestImagesController(base.StoreCle
     def test_download(self):
         request = unit_test_utils.get_fake_request()
         image = FakeImage('abcd',
+                          status='active',
                           locations=[{'url': 'http://example.com/image',
                                       'metadata': {}, 'status': 'active'}])
         self.image_repo.result = image
@@ -112,7 +113,7 @@ class TestImagesController(base.StoreCle
         self.assertEqual('abcd', image.image_id)
 
     def test_download_deactivated(self):
-        request = unit_test_utils.get_fake_request()
+        request = unit_test_utils.get_fake_request(is_admin=False)
         image = FakeImage('abcd',
                           status='deactivated',
                           locations=[{'url': 'http://example.com/image',
@@ -121,11 +122,33 @@ class TestImagesController(base.StoreCle
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.download,
                           request, str(uuid.uuid4()))
 
+        request = unit_test_utils.get_fake_request(is_admin=True)
+        image = FakeImage('abcd',
+                          status='deactivated',
+                          locations=[{'url': 'http://example.com/image',
+                                      'metadata': {}, 'status': 'active'}])
+        self.image_repo.result = image
+        image = self.controller.download(request, unit_test_utils.UUID1)
+        self.assertEqual('abcd', image.image_id)
+
+    def test_download_is_not_active(self):
+        state = ['queued', 'deleted', 'saving', 'killed', 'pending_delete']
+        for st in state:
+            request = unit_test_utils.get_fake_request()
+            image = FakeImage('abcd',
+                              status=st,
+                              locations=[{'url': 'http://example.com/image',
+                                          'metadata': {}, 'status': 'active'}])
+            self.image_repo.result = image
+            self.assertRaises(webob.exc.HTTPForbidden,
+                              self.controller.download,
+                              request, str(uuid.uuid4()))
+
     def test_download_no_location(self):
         # NOTE(mclaren): NoContent will be raised by the ResponseSerializer
         # That's tested below.
         request = unit_test_utils.get_fake_request()
-        self.image_repo.result = FakeImage('abcd')
+        self.image_repo.result = FakeImage('abcd', status='active')
         image = self.controller.download(request, unit_test_utils.UUID2)
         self.assertEqual('abcd', image.image_id)
 
@@ -147,7 +170,7 @@ class TestImagesController(base.StoreCle
                 raise exception.Forbidden()
 
         request = unit_test_utils.get_fake_request()
-        image = FakeImage('abcd')
+        image = FakeImage('abcd', status='active')
         self.image_repo.result = image
         image.locations = ImageLocations()
         image = self.controller.download(request, unit_test_utils.UUID1)
