Merge "Image not downloaded without restricted property" into stable/juno

5 files changed, 286 insertions, 2 deletions

diff --git a/glance/openstack/common/policy.py b/glance/openstack/common/policy.py
index 0b441f779..fdd36435d 100644
--- a/glance/openstack/common/policy.py
+++ b/glance/openstack/common/policy.py
@@ -774,8 +774,12 @@ class GenericCheck(Check):
             role:compute:admin
         """
 
-        # TODO(termie): do dict inspection via dot syntax
-        match = self.match % target
+        try:
+            match = self.match % target
+        except KeyError:
+            # While doing GenericCheck if key not
+            # present in Target return false
+            return False
 
         try:
             # Try to interpret self.kind as a literal
diff --git a/glance/tests/functional/v1/test_api.py b/glance/tests/functional/v1/test_api.py
index ab5d4be0e..1486fb35c 100644
--- a/glance/tests/functional/v1/test_api.py
+++ b/glance/tests/functional/v1/test_api.py
@@ -713,3 +713,55 @@ class TestApi(functional.FunctionalTest):
         self.assertEqual(response.status, 404)
 
         self.stop_servers()
+
+    def test_download_image_with_no_restricted_property_set_to_image(self):
+        """
+        We test the following sequential series of actions:
+
+        0. POST /images with public image named Image1
+        and no custom properties
+        - Verify 201 returned
+        1. GET image
+        2. DELETE image1
+        - Delete the newly added image
+        """
+        self.cleanup()
+        rules = {"context_is_admin": "role:admin",
+                 "default": "",
+                 "restricted":
+                 "not ('test_key':%(x_test_key)s and role:_member_)",
+                 "download_image": "role:admin or rule:restricted"}
+        self.set_policy_rules(rules)
+        self.start_servers(**self.__dict__.copy())
+
+        image_data = "*" * FIVE_KB
+        headers = minimal_headers('Image1')
+        headers.update({'X-Roles': 'member'})
+        path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
+        http = httplib2.Http()
+        response, content = http.request(path, 'POST', headers=headers,
+                                         body=image_data)
+        self.assertEqual(201, response.status)
+        data = jsonutils.loads(content)
+        image_id = data['image']['id']
+        self.assertEqual(data['image']['checksum'],
+                         hashlib.md5(image_data).hexdigest())
+        self.assertEqual(data['image']['size'], FIVE_KB)
+        self.assertEqual(data['image']['name'], "Image1")
+        self.assertTrue(data['image']['is_public'])
+
+        # 1. GET image
+        path = "http://%s:%d/v1/images/%s" % ("127.0.0.1", self.api_port,
+                                              image_id)
+        http = httplib2.Http()
+        response, content = http.request(path, 'GET')
+        self.assertEqual(200, response.status)
+
+        # 2. DELETE image1
+        path = "http://%s:%d/v1/images/%s" % ("127.0.0.1", self.api_port,
+                                              image_id)
+        http = httplib2.Http()
+        response, content = http.request(path, 'DELETE')
+        self.assertEqual(200, response.status)
+
+        self.stop_servers()
diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py
index 5b1ecef84..14fe3c7ff 100644
--- a/glance/tests/functional/v2/test_images.py
+++ b/glance/tests/functional/v2/test_images.py
@@ -591,6 +591,71 @@ class TestImages(functional.FunctionalTest):
 
         self.stop_servers()
 
+    def test_download_image_with_no_restricted_property_set_to_image(self):
+        rules = {
+            "context_is_admin": "role:admin",
+            "default": "",
+            "restricted":
+            "not ('test_key':%(x_test_key)s and role:_member_)",
+            "download_image": "role:admin or rule:restricted"
+        }
+
+        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
+        path = self._url('/v2/images/%s/file' % image_id)
+        headers = self._headers({'Content-Type': 'application/octet-stream',
+                                 'X-Roles': '_member_'})
+        response = requests.get(path, headers=headers)
+        self.assertEqual(200, 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_download_image_not_allowed_using_restricted_policy(self):
 
         rules = {
diff --git a/glance/tests/unit/test_cache_middleware.py b/glance/tests/unit/test_cache_middleware.py
index 9ad2a8770..afc81c359 100644
--- a/glance/tests/unit/test_cache_middleware.py
+++ b/glance/tests/unit/test_cache_middleware.py
@@ -565,6 +565,76 @@ class TestCacheMiddlewareProcessRequest(base.IsolatedUnitTest):
         actual = cache_filter.process_request(request)
         self.assertTrue(actual)
 
+    def test_v1_process_request_download_without_restricted_property(self):
+        """
+        Test process_request for v1 api where _member_ role able to
+        download the image if restricted property is not added to the image.
+        """
+        image_id = 'test1'
+
+        def fake_get_v1_image_metadata(*args, **kwargs):
+            return {
+                'id': image_id,
+                'name': 'fake_image',
+                'status': 'active',
+                'created_at': '',
+                'min_disk': '10G',
+                'min_ram': '1024M',
+                'protected': False,
+                'locations': '',
+                'checksum': 'c1234',
+                'owner': '',
+                'disk_format': 'raw',
+                'container_format': 'bare',
+                'size': '123456789',
+                'virtual_size': '123456789',
+                'is_public': 'public',
+                'deleted': False,
+                'updated_at': ''
+            }
+
+        request = webob.Request.blank('/v1/images/%s' % image_id)
+        request.context = context.RequestContext(roles=['_member_'])
+        cache_filter = ProcessRequestTestCacheFilter()
+        cache_filter._get_v1_image_metadata = fake_get_v1_image_metadata
+
+        rules = {
+            "restricted":
+            "not ('test_1234':%(x_test_key)s and role:_member_)",
+            "download_image": "role:admin or rule:restricted"
+        }
+        self.set_policy_rules(rules)
+        cache_filter.policy = glance.api.policy.Enforcer()
+        actual = cache_filter.process_request(request)
+        self.assertTrue(actual)
+
+    def test_v2_process_request_download_without_restricted_property(self):
+        """
+        Test process_request for v2 api where member role able to
+        download the image if restricted property is not added to the image.
+        """
+        image_id = 'test1'
+
+        def fake_get_v2_image_metadata(*args, **kwargs):
+            image = ImageStub(image_id)
+            request.environ['api.cache.image'] = image
+            return glance.api.policy.ImageTarget(image)
+
+        request = webob.Request.blank('/v2/images/test1/file')
+        request.context = context.RequestContext(roles=['_member_'])
+        cache_filter = ProcessRequestTestCacheFilter()
+        cache_filter._get_v2_image_metadata = fake_get_v2_image_metadata
+
+        rules = {
+            "restricted":
+            "not ('test_1234':%(x_test_key)s and role:_member_)",
+            "download_image": "role:admin or rule:restricted"
+        }
+        self.set_policy_rules(rules)
+        cache_filter.policy = glance.api.policy.Enforcer()
+        actual = cache_filter.process_request(request)
+        self.assertTrue(actual)
+
 
 class TestCacheMiddlewareProcessResponse(base.IsolatedUnitTest):
     def test_process_v1_DELETE_response(self):
@@ -830,3 +900,84 @@ class TestCacheMiddlewareProcessResponse(base.IsolatedUnitTest):
         resp = webob.Response(request=request)
         actual = cache_filter.process_response(resp)
         self.assertEqual(actual, resp)
+
+    def test_v1_process_response_download_without_restricted_property(self):
+        """
+        Test process_response for v1 api where _member_ role able to
+        download the image if restricted property is not added to the image.
+        """
+        image_id = 'test1'
+
+        def fake_fetch_request_info(*args, **kwargs):
+            return ('test1', 'GET', 'v1')
+
+        def fake_get_v1_image_metadata(*args, **kwargs):
+            return {
+                'id': image_id,
+                'name': 'fake_image',
+                'status': 'active',
+                'created_at': '',
+                'min_disk': '10G',
+                'min_ram': '1024M',
+                'protected': False,
+                'locations': '',
+                'checksum': 'c1234',
+                'owner': '',
+                'disk_format': 'raw',
+                'container_format': 'bare',
+                'size': '123456789',
+                'virtual_size': '123456789',
+                'is_public': 'public',
+                'deleted': False,
+                'updated_at': ''
+            }
+
+        cache_filter = ProcessRequestTestCacheFilter()
+        cache_filter._fetch_request_info = fake_fetch_request_info
+        cache_filter._get_v1_image_metadata = fake_get_v1_image_metadata
+        rules = {
+            "restricted":
+            "not ('test_1234':%(x_test_key)s and role:_member_)",
+            "download_image": "role:admin or rule:restricted"
+        }
+        self.set_policy_rules(rules)
+        cache_filter.policy = glance.api.policy.Enforcer()
+
+        request = webob.Request.blank('/v1/images/%s' % image_id)
+        request.context = context.RequestContext(roles=['_member_'])
+        resp = webob.Response(request=request)
+        actual = cache_filter.process_response(resp)
+        self.assertEqual(actual, resp)
+
+    def test_v2_process_response_download_without_restricted_property(self):
+        """
+        Test process_response for v2 api where member role able to
+        download the image if restricted property is not added to the image.
+        """
+        image_id = 'test1'
+
+        def fake_fetch_request_info(*args, **kwargs):
+            return ('test1', 'GET', 'v2')
+
+        def fake_get_v2_image_metadata(*args, **kwargs):
+            image = ImageStub(image_id)
+            request.environ['api.cache.image'] = image
+            return glance.api.policy.ImageTarget(image)
+
+        cache_filter = ProcessRequestTestCacheFilter()
+        cache_filter._fetch_request_info = fake_fetch_request_info
+        cache_filter._get_v2_image_metadata = fake_get_v2_image_metadata
+
+        rules = {
+            "restricted":
+            "not ('test_1234':%(x_test_key)s and role:_member_)",
+            "download_image": "role:admin or rule:restricted"
+        }
+        self.set_policy_rules(rules)
+        cache_filter.policy = glance.api.policy.Enforcer()
+
+        request = webob.Request.blank('/v2/images/test1/file')
+        request.context = context.RequestContext(roles=['_member_'])
+        resp = webob.Response(request=request)
+        actual = cache_filter.process_response(resp)
+        self.assertEqual(actual, resp)
diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py
index fa7f220e6..bd2182ea8 100644
--- a/glance/tests/unit/v1/test_api.py
+++ b/glance/tests/unit/v1/test_api.py
@@ -2549,6 +2549,18 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         res = req.get_response(self.api)
         self.assertEqual(res.status_int, 403)
 
+    def test_show_image_download_without_restricted_policy(self):
+        rules = {"context_is_admin": "role:admin",
+                 "default": "",
+                 "restricted":
+                 "not ('test_key':%(x_test_key)s and role:_member_)",
+                 "download_image": "role:admin or rule:restricted"}
+        self.set_policy_rules(rules)
+        req = webob.Request.blank("/images/%s" % UUID2)
+        req.headers['X-Auth-Token'] = 'user:tenant:_member_'
+        res = req.get_response(self.api)
+        self.assertEqual(res.status_int, 200)
+
     def test_delete_image(self):
         req = webob.Request.blank("/images/%s" % UUID2)
         req.method = 'DELETE'