Improve access check on images

Makes sure that users can delete only their own images, snapshots.
Enable listing of all images, both private which are owned and the public
ones. Only list the private images/snapshots for the owner and admin users.
Fixes bug 863305

Change-Id: I7326ec4a99158c8db5319f2397c99c5a89be2cb5

Authors

@@ -74,6 +74,7 @@ Kevin Bringard <kbringard@attinteractive.com>
 Kevin L. Mitchell <kevin.mitchell@rackspace.com>
 Kirill Shileev <kshileev@gmail.com>
 Koji Iida <iida.koji@lab.ntt.co.jp>
+Loganathan Parthipan <parthipan@hp.com>
 Lorin Hochstein <lorin@isi.edu>
 Lvov Maxim <usrleon@gmail.com>
 Mandell Degerness <mdegerne@gmail.com>

nova/image/glance.py

@@ -286,10 +286,28 @@ def delete(self, context, image_id):
         """Delete the given image.
 
         :raises: ImageNotFound if the image does not exist.
+        :raises: NotAuthorized if the user is not an owner.
 
         """
         # NOTE(vish): show is to check if image is available
-        self.show(context, image_id)
+        image_meta = self.show(context, image_id)
+
+        if FLAGS.use_deprecated_auth:
+            # NOTE(parthi): only allow image deletions if the user
+            # is a member of the project owning the image, in case of
+            # setup without keystone
+            # TODO Currently this access control breaks if
+            # 1. Image is not owned by a project
+            # 2. Deleting user is not bound a project
+            properties = image_meta['properties']
+            if (context.project_id and ('project_id' in properties)
+                and (context.project_id != properties['project_id'])):
+                raise exception.NotAuthorized(_("Not the image owner"))
+
+            if (context.project_id and ('owner_id' in properties)
+                and (context.project_id != properties['owner_id'])):
+                raise exception.NotAuthorized(_("Not the image owner"))
+
         try:
             result = self._get_client(context).delete_image(image_id)
         except glance_exception.NotFound:
@@ -329,6 +347,9 @@ def _is_image_available(context, image_meta):
 
         properties = image_meta['properties']
 
+        if context.project_id and ('owner_id' in properties):
+            return str(properties['owner_id']) == str(context.project_id)
+
         if context.project_id and ('project_id' in properties):
             return str(properties['project_id']) == str(context.project_id)
 

nova/tests/image/test_glance.py

@@ -272,6 +272,24 @@ def test_index_marker_and_limit(self):
             self.assertDictMatch(meta, expected)
             i = i + 1
 
+    def test_index_private_image(self):
+        fixture = self._make_fixture(name='test image')
+        fixture['is_public'] = False
+        properties = {'owner_id': 'proj1'}
+        fixture['properties'] = properties
+
+        image_id = self.service.create(self.context, fixture)['id']
+
+        proj = self.context.project_id
+        self.context.project_id = 'proj1'
+
+        image_metas = self.service.index(self.context)
+
+        self.context.project_id = proj
+
+        expected = [{'id': 'DONTCARE', 'name': 'test image'}]
+        self.assertDictListMatch(image_metas, expected)
+
     def test_detail_marker(self):
         fixtures = []
         ids = []
@@ -380,6 +398,32 @@ def test_delete(self):
         num_images = len(self.service.index(self.context))
         self.assertEquals(1, num_images)
 
+    def test_delete_not_by_owner(self):
+        # this test is only relevant for deprecated auth mode
+        self.flags(use_deprecated_auth=True)
+
+        fixture = self._make_fixture(name='test image')
+        properties = {'project_id': 'proj1'}
+        fixture['properties'] = properties
+
+        num_images = len(self.service.index(self.context))
+        self.assertEquals(0, num_images)
+
+        image_id = self.service.create(self.context, fixture)['id']
+        num_images = len(self.service.index(self.context))
+        self.assertEquals(1, num_images)
+
+        proj_id = self.context.project_id
+        self.context.project_id = 'proj2'
+
+        self.assertRaises(exception.NotAuthorized, self.service.delete,
+                          self.context, image_id)
+
+        self.context.project_id = proj_id
+
+        num_images = len(self.service.index(self.context))
+        self.assertEquals(1, num_images)
+
     def test_show_passes_through_to_client(self):
         fixture = self._make_fixture(name='image1', is_public=True)
         image_id = self.service.create(self.context, fixture)['id']