Remove dead 403->404 code
The db.ImageRepo.save() operation covers up a Forbidden error with NotFound in the case of trying to update an image that the user does not own. This is actually never hit in reality as the authorization layer would have caught it before, and returned Forbidden. The API is the layer that should be deciding to hide images for which the user cannot see, to avoid things like being able to show an image, but get back a 404 on update. In order to do that, we need the lower layers to report the situation faithfully and let the upper layers decide how to expose that to the user. Specifically, for the policy refactor, we have tests that assert the Forbidden behavior, and after disabling the authorization layer, will break (and rightfully so) because they get NotFound when they hit the lower-layer check. Because it is hidden so deep, even the API can't distinguish between the two. I imagine this check was added long ago to provide the "if you can't see it, return NotFound instead of Forbidden, which would expose the fact that it exists" behavior which is desired. However, the authorization layer means we never get here anymore. This patch is provided without any test changes to prove that it does not actually alter the real behavior. The test_permissions functional test asserts the proper behavior before and after this change, as well as after subsequent refactor patches. Change-Id: I0084350ebb09cc1cb3752b45165e49f166bfdf91
This commit is contained in:
parent
2fef2e6c4f
commit
429f16124b
@ -182,17 +182,13 @@ class ImageRepo(object):
|
||||
if (image_values['size'] is not None
|
||||
and image_values['size'] > CONF.image_size_cap):
|
||||
raise exception.ImageSizeLimitExceeded
|
||||
try:
|
||||
new_values = self.db_api.image_update(self.context,
|
||||
image.image_id,
|
||||
image_values,
|
||||
purge_props=True,
|
||||
from_state=from_state,
|
||||
atomic_props=(
|
||||
IMAGE_ATOMIC_PROPS))
|
||||
except (exception.ImageNotFound, exception.Forbidden):
|
||||
msg = _("No image found with ID %s") % image.image_id
|
||||
raise exception.ImageNotFound(msg)
|
||||
new_values = self.db_api.image_update(self.context,
|
||||
image.image_id,
|
||||
image_values,
|
||||
purge_props=True,
|
||||
from_state=from_state,
|
||||
atomic_props=(
|
||||
IMAGE_ATOMIC_PROPS))
|
||||
self.db_api.image_tag_set_all(self.context, image.image_id,
|
||||
image.tags)
|
||||
image.updated_at = new_values['updated_at']
|
||||
|
@ -804,7 +804,7 @@ def image_update(context, image_id, image_values, purge_props=False,
|
||||
try:
|
||||
image = DATA['images'][image_id]
|
||||
except KeyError:
|
||||
raise exception.ImageNotFound()
|
||||
raise exception.ImageNotFound(image_id)
|
||||
|
||||
location_data = image_values.pop('locations', None)
|
||||
if location_data is not None:
|
||||
|
Loading…
x
Reference in New Issue
Block a user