From 869a39d9e55849c7de7b3ee086cea1017d4f60d5 Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Sun, 15 Aug 2021 17:19:45 +0000 Subject: [PATCH] Check policies for Image Tags in API This patch enforces policy checks required for adding/deleting tags for image in API layer. Partially-Implements: blueprint policy-refactor Change-Id: I7b5bcbce9375df5b0826120df5cdf5d4cb788af2 --- glance/api/v2/image_tags.py | 12 +- .../functional/v2/test_images_api_policy.py | 164 ++++++++++++++++++ .../tests/unit/v2/test_image_tags_resource.py | 2 +- 3 files changed, 175 insertions(+), 3 deletions(-) diff --git a/glance/api/v2/image_tags.py b/glance/api/v2/image_tags.py index 5d37f22414..6b9d4e4a8e 100644 --- a/glance/api/v2/image_tags.py +++ b/glance/api/v2/image_tags.py @@ -20,6 +20,7 @@ import webob.exc from glance.api import policy from glance.api.v2 import images as v2_api +from glance.api.v2 import policy as api_policy from glance.common import exception from glance.common import utils from glance.common import wsgi @@ -44,9 +45,12 @@ class Controller(object): @utils.mutating def update(self, req, image_id, tag_value): - image_repo = self.gateway.get_repo(req.context) + image_repo = self.gateway.get_repo( + req.context, authorization_layer=False) try: image = image_repo.get(image_id) + api_policy.ImageAPIPolicy(req.context, image, + self.policy).modify_image() image.tags.add(tag_value) image_repo.save(image) except exception.NotFound: @@ -71,9 +75,13 @@ class Controller(object): @utils.mutating def delete(self, req, image_id, tag_value): - image_repo = self.gateway.get_repo(req.context) + image_repo = self.gateway.get_repo( + req.context, authorization_layer=False) try: image = image_repo.get(image_id) + api_policy.ImageAPIPolicy(req.context, image, + self.policy).modify_image() + if tag_value not in image.tags: raise webob.exc.HTTPNotFound() image.tags.remove(tag_value) diff --git a/glance/tests/functional/v2/test_images_api_policy.py b/glance/tests/functional/v2/test_images_api_policy.py index 7d7224a352..09aa380bef 100644 --- a/glance/tests/functional/v2/test_images_api_policy.py +++ b/glance/tests/functional/v2/test_images_api_policy.py @@ -845,3 +845,167 @@ class TestImagesPolicy(functional.SynchronousAPIBase): response = self._import_direct(image_id, store_to_import, headers=headers) self.assertEqual(404, response.status_code) + + def _test_image_ownership(self, headers, method): + self.set_policy_rules({ + 'get_image': '', + 'add_image': '', + 'publicize_image': '', + 'communitize_image': '', + 'add_member': '', + }) + + for visibility in ('community', 'public', 'shared'): + path = "/v2/images" + data = { + 'name': '%s-image' % visibility, + 'visibility': visibility, + } + # create image + response = self.api_post(path, json=data) + image = response.json + self.assertEqual(201, response.status_code) + self.assertEqual(visibility, image['visibility']) + + # share the image if visibility is shared + if visibility == 'shared': + path = '/v2/images/%s/members' % image['id'] + data = { + 'member': 'fake-project-id' + } + response = self.api_post(path, + json=data) + self.assertEqual(200, response.status_code) + + # Add/Delete tag + path = '/v2/images/%s/tags/Test_Tag_2' % image['id'] + response = self.api_request(method, path, headers=headers) + self.assertEqual(403, response.status_code) + + def test_image_tag_update(self): + self.start_server() + # Create image + image_id = self._create_and_upload() + + # Make sure we will be able to add tags for the image + path = '/v2/images/%s/tags/Test_Tag' % image_id + response = self.api_put(path) + self.assertEqual(204, response.status_code) + # Make sure tag is added to image + path = '/v2/images/%s' % image_id + response = self.api_get(path) + image = response.json + self.assertEqual(['Test_Tag'], image['tags']) + + # Disable get_image and modify_image should give us 404 Not Found + self.set_policy_rules({ + 'get_image': '!', + 'modify_image': '!' + }) + path = '/v2/images/%s/tags/Test_Tag_2' % image_id + response = self.api_put(path) + self.assertEqual(404, response.status_code) + + # Allow get_image and disable modify_image should give us + # 403 Forbidden + self.set_policy_rules({ + 'get_image': '', + 'modify_image': '!' + }) + path = '/v2/images/%s/tags/Test_Tag_2' % image_id + response = self.api_put(path) + self.assertEqual(403, response.status_code) + + # Adding tag by another project (non-admin user) should return + # 404 Not Found for private image + self.set_policy_rules({ + 'get_image': '', + 'modify_image': '' + }) + # Note for other reviewers, these tests runs by default using + # admin role, to test this scenario we need image + # of current project to be accessed by other projects non-admin + # user. + headers = self._headers({ + 'X-Project-Id': 'fake-project-id', + 'X-Roles': 'member', + }) + path = '/v2/images/%s/tags/Test_Tag_2' % image_id + response = self.api_put(path, headers=headers) + self.assertEqual(404, response.status_code) + + # Adding tag by another project (non-admin user) should return + # 403 Not Found for other than private image + self._test_image_ownership(headers, 'PUT') + + def test_image_tag_delete(self): + self.start_server() + # Create image + image_id = self._create_and_upload() + + # Make sure we will be able to add tags for the image + path = '/v2/images/%s/tags/Test_Tag_1' % image_id + response = self.api_put(path) + self.assertEqual(204, response.status_code) + # add another tag while we can + path = '/v2/images/%s/tags/Test_Tag_2' % image_id + response = self.api_put(path) + self.assertEqual(204, response.status_code) + + # Make sure tags are added to image + path = '/v2/images/%s' % image_id + response = self.api_get(path) + image = response.json + self.assertItemsEqual(['Test_Tag_1', 'Test_Tag_2'], image['tags']) + + # Now delete tag from image + path = '/v2/images/%s/tags/Test_Tag_1' % image_id + response = self.api_delete(path) + self.assertEqual(204, response.status_code) + + # Make sure tag is deleted + path = '/v2/images/%s' % image_id + response = self.api_get(path) + image = response.json + self.assertNotIn('Test_Tag_1', image['tags']) + + # Disable get_image and modify_image should give us 404 Not Found + self.set_policy_rules({ + 'get_image': '!', + 'modify_image': '!' + }) + path = '/v2/images/%s/tags/Test_Tag_2' % image_id + response = self.api_delete(path) + self.assertEqual(404, response.status_code) + + # Allow get_image and disable modify_image should give us + # 403 Forbidden + self.set_policy_rules({ + 'get_image': '', + 'modify_image': '!' + }) + path = '/v2/images/%s/tags/Test_Tag_2' % image_id + response = self.api_delete(path) + self.assertEqual(403, response.status_code) + + # Deleting tag by another project (non-admin user) should return + # 404 Not Found for private image + self.set_policy_rules({ + 'get_image': '', + 'modify_image': '' + }) + # Note for other reviewers, these tests runs by default using + # admin role, to test this scenario we need image + # of current project to be accessed by other projects non-admin + # user. + headers = self._headers({ + 'X-Project-Id': 'fake-project-id', + 'X-Roles': 'member', + }) + path = '/v2/images/%s/tags/Test_Tag_2' % image_id + response = self.api_delete(path, headers=headers) + self.assertEqual(404, response.status_code) + + # Deleting tag by another project (non-admin user) should return + # 403 Not Found for other than private image + self._test_image_ownership(headers, 'DELETE') diff --git a/glance/tests/unit/v2/test_image_tags_resource.py b/glance/tests/unit/v2/test_image_tags_resource.py index 7684197539..c61207d46a 100644 --- a/glance/tests/unit/v2/test_image_tags_resource.py +++ b/glance/tests/unit/v2/test_image_tags_resource.py @@ -65,7 +65,7 @@ class TestImageTagsController(base.IsolatedUnitTest): image_repo = image_data_tests.FakeImageRepo() image_repo.get = fake_get - def get_fake_repo(self): + def get_fake_repo(self, authorization_layer=False): return image_repo self.controller.gateway.get_repo = get_fake_repo