Download forbidden when get_image_location is set.

When using v2 an attempt to download an image would return a 403 if the
get_image_location policy was set.

Note: We had been returning both 404 and 204 when no data was
available. There was no way to detect the 404 case without trying to
access the image locations so I've standardized on 204.

Change-Id: I658b08a35d3a8cb8a7096baf716ccb3d6e7d9abf
Closes-bug: 1501672
This commit is contained in:
Stuart McLaren 2015-09-30 16:54:12 +00:00
parent 04776634a2
commit b47f625443
3 changed files with 19 additions and 14 deletions

View File

@ -199,8 +199,6 @@ class ImageDataController(object):
msg = _('The requested image has been deactivated. ' msg = _('The requested image has been deactivated. '
'Image data download is forbidden.') 'Image data download is forbidden.')
raise exception.Forbidden(message=msg) raise exception.Forbidden(message=msg)
if not image.locations:
raise exception.ImageDataNotFound()
except exception.ImageDataNotFound as e: except exception.ImageDataNotFound as e:
raise webob.exc.HTTPNoContent(explanation=e.msg) raise webob.exc.HTTPNoContent(explanation=e.msg)
except exception.NotFound as e: except exception.NotFound as e:
@ -248,7 +246,7 @@ class ResponseSerializer(wsgi.JSONResponseSerializer):
response.app_iter = iter(image.get_data(offset=offset, response.app_iter = iter(image.get_data(offset=offset,
chunk_size=chunk_size)) chunk_size=chunk_size))
except glance_store.NotFound as e: except glance_store.NotFound as e:
raise webob.exc.HTTPNotFound(explanation=e.msg) raise webob.exc.HTTPNoContent(explanation=e.msg)
except glance_store.RemoteServiceUnavailable as e: except glance_store.RemoteServiceUnavailable as e:
raise webob.exc.HTTPServiceUnavailable(explanation=e.msg) raise webob.exc.HTTPServiceUnavailable(explanation=e.msg)
except (glance_store.StoreGetNotSupported, except (glance_store.StoreGetNotSupported,

View File

@ -397,7 +397,12 @@ class ImageProxy(glance.domain.proxy.Image):
def get_data(self, offset=0, chunk_size=None): def get_data(self, offset=0, chunk_size=None):
if not self.image.locations: if not self.image.locations:
raise store.NotFound(_("No image data could be found")) # NOTE(mclaren): This is the only set of arguments
# which work with this exception currently, see:
# https://bugs.launchpad.net/glance-store/+bug/1501443
# When the above glance_store bug is fixed we can
# add a msg as usual.
raise store.NotFound(image=None)
err = None err = None
for loc in self.image.locations: for loc in self.image.locations:
try: try:

View File

@ -122,10 +122,12 @@ class TestImagesController(base.StoreClearingUnitTest):
request, str(uuid.uuid4())) request, str(uuid.uuid4()))
def test_download_no_location(self): 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() request = unit_test_utils.get_fake_request()
self.image_repo.result = FakeImage('abcd') self.image_repo.result = FakeImage('abcd')
self.assertRaises(webob.exc.HTTPNoContent, self.controller.download, image = self.controller.download(request, unit_test_utils.UUID2)
request, unit_test_utils.UUID2) self.assertEqual('abcd', image.image_id)
def test_download_non_existent_image(self): def test_download_non_existent_image(self):
request = unit_test_utils.get_fake_request() request = unit_test_utils.get_fake_request()
@ -139,7 +141,7 @@ class TestImagesController(base.StoreClearingUnitTest):
self.assertRaises(webob.exc.HTTPForbidden, self.controller.download, self.assertRaises(webob.exc.HTTPForbidden, self.controller.download,
request, str(uuid.uuid4())) request, str(uuid.uuid4()))
def test_download_get_image_location_forbidden(self): def test_download_ok_when_get_image_location_forbidden(self):
class ImageLocations(object): class ImageLocations(object):
def __len__(self): def __len__(self):
raise exception.Forbidden() raise exception.Forbidden()
@ -148,8 +150,8 @@ class TestImagesController(base.StoreClearingUnitTest):
image = FakeImage('abcd') image = FakeImage('abcd')
self.image_repo.result = image self.image_repo.result = image
image.locations = ImageLocations() image.locations = ImageLocations()
self.assertRaises(webob.exc.HTTPForbidden, self.controller.download, image = self.controller.download(request, unit_test_utils.UUID1)
request, str(uuid.uuid4())) self.assertEqual('abcd', image.image_id)
def test_upload(self): def test_upload(self):
request = unit_test_utils.get_fake_request() request = unit_test_utils.get_fake_request()
@ -493,11 +495,11 @@ class TestImageDataSerializer(test_utils.BaseTestCase):
self.serializer.download, self.serializer.download,
response, image) response, image)
def test_download_not_found(self): def test_download_no_content(self):
"""Test image download returns HTTPNotFound. """Test image download returns HTTPNoContent
Make sure that serializer returns 404 not found error in case of Make sure that serializer returns 204 no content error in case of
image is not available at specified location. image data is not available at specified location.
""" """
with mock.patch.object(glance.api.policy.ImageProxy, with mock.patch.object(glance.api.policy.ImageProxy,
'get_data') as mock_get_data: 'get_data') as mock_get_data:
@ -508,7 +510,7 @@ class TestImageDataSerializer(test_utils.BaseTestCase):
response.request = request response.request = request
image = FakeImage(size=3, data=iter('ZZZ')) image = FakeImage(size=3, data=iter('ZZZ'))
image.get_data = mock_get_data image.get_data = mock_get_data
self.assertRaises(webob.exc.HTTPNotFound, self.assertRaises(webob.exc.HTTPNoContent,
self.serializer.download, self.serializer.download,
response, image) response, image)