Filtering images by user_id now

5 files changed, 101 insertions, 54 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 3a83f1c6a..0d51be336 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -447,7 +447,8 @@ class API(base.Base):
 
         :retval: A dict containing image metadata
         """
-        properties = {'instance_id': str(instance_id)}
+        properties = {'instance_id': str(instance_id),
+                      'user_id': str(context.user_id)}
         sent_meta = {'name': name, 'is_public': False,
                      'properties': properties}
         recv_meta = self.image_service.create(context, sent_meta)
diff --git a/nova/image/glance.py b/nova/image/glance.py
index b7bb88002..ec6e9e094 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -52,15 +52,28 @@ class GlanceImageService(service.BaseImageService):
         """
         Calls out to Glance for a list of images available
         """
-        return self.client.get_images()
+        # NOTE(sirp): We need to use get_images_detailed and not get_images
+        # here because we need `is_public` and properties included so we can
+        # filter by user
+        filtered = []
+        image_metas = self.client.get_images_detailed()
+        for image_meta in image_metas:
+            if self._is_image_available(context, image_meta):
+                meta = utils.subset_dict(image_meta, ('id', 'name'))
+                filtered.append(meta)
+        return filtered
 
     def detail(self, context):
         """
         Calls out to Glance for a list of detailed image information
         """
+        filtered = []
         image_metas = self.client.get_images_detailed()
-        translate = self._translate_to_base
-        return [translate(image_meta) for image_meta in image_metas]
+        for image_meta in image_metas:
+            if self._is_image_available(context, image_meta):
+                meta = self._translate_to_base(image_meta)
+                filtered.append(meta)
+        return filtered
 
     def show(self, context, image_id):
         """
@@ -145,3 +158,30 @@ class GlanceImageService(service.BaseImageService):
         Clears out all images
         """
         pass
+
+    @staticmethod
+    def _is_image_available(context, image_meta):
+        """
+        Images are always available if they are public or if the user is an
+        admin.
+
+        Otherwise, we filter by project_id (if present) and then fall-back to
+        images owned by user.
+        """
+        # FIXME(sirp): We should be filtering by user_id on the Glance side
+        # for security; however, we can't do that until we get authn/authz
+        # sorted out. Until then, filtering in Nova.
+        if image_meta['is_public'] or context.is_admin:
+            return True
+
+        properties = image_meta['properties']
+
+        if context.project_id and ('project_id' in properties):
+            return str(properties['project_id']) == str(project_id)
+
+        try:
+            user_id = properties['user_id']
+        except KeyError:
+            return False
+
+        return (str(user_id) == str(context.user_id))
diff --git a/nova/image/local.py b/nova/image/local.py
index 609d6c42a..1fb6e1f13 100644
--- a/nova/image/local.py
+++ b/nova/image/local.py
@@ -24,6 +24,7 @@ from nova import exception
 from nova import flags
 from nova import log as logging
 from nova.image import service
+from nova import utils
 
 
 FLAGS = flags.FLAGS
@@ -63,8 +64,12 @@ class LocalImageService(service.BaseImageService):
         return images
 
     def index(self, context):
-        return [dict(image_id=i['id'], name=i.get('name'))
-                for i in self.detail(context)]
+        filtered = []
+        image_metas = self.detail(context)
+        for image_meta in image_metas:
+            meta = utils.subset_dict(image_meta, ('id', 'name'))
+            filtered.append(meta)
+        return filtered
 
     def detail(self, context):
         images = []
diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py
index 57a9819e8..797bbef8f 100644
--- a/nova/tests/api/openstack/test_images.py
+++ b/nova/tests/api/openstack/test_images.py
@@ -44,18 +44,10 @@ FLAGS = flags.FLAGS
 
 
 class BaseImageServiceTests(object):
-
     """Tasks to test for all image services"""
 
     def test_create(self):
-
-        fixture = {'name': 'test image',
-                   'updated': None,
-                   'created': None,
-                   'status': None,
-                   'instance_id': None,
-                   'progress': None}
-
+        fixture = self._make_fixture('test image')
         num_images = len(self.service.index(self.context))
 
         id = self.service.create(self.context, fixture)['id']
@@ -65,14 +57,7 @@ class BaseImageServiceTests(object):
                           len(self.service.index(self.context)))
 
     def test_create_and_show_non_existing_image(self):
-
-        fixture = {'name': 'test image',
-                   'updated': None,
-                   'created': None,
-                   'status': None,
-                   'instance_id': None,
-                   'progress': None}
-
+        fixture = self._make_fixture('test image')
         num_images = len(self.service.index(self.context))
 
         id = self.service.create(self.context, fixture)['id']
@@ -85,14 +70,7 @@ class BaseImageServiceTests(object):
                           'bad image id')
 
     def test_update(self):
-
-        fixture = {'name': 'test image',
-                   'updated': None,
-                   'created': None,
-                   'status': None,
-                   'instance_id': None,
-                   'progress': None}
-
+        fixture = self._make_fixture('test image')
         id = self.service.create(self.context, fixture)['id']
 
         fixture['status'] = 'in progress'
@@ -102,20 +80,9 @@ class BaseImageServiceTests(object):
         self.assertEquals('in progress', new_image_data['status'])
 
     def test_delete(self):
-
-        fixtures = [
-                    {'name': 'test image 1',
-                     'updated': None,
-                     'created': None,
-                     'status': None,
-                     'instance_id': None,
-                     'progress': None},
-                    {'name': 'test image 2',
-                     'updated': None,
-                     'created': None,
-                     'status': None,
-                     'instance_id': None,
-                     'progress': None}]
+        fixture1 = self._make_fixture('test image 1')
+        fixture2 = self._make_fixture('test image 2')
+        fixtures = [fixture1, fixture2]
 
         num_images = len(self.service.index(self.context))
         self.assertEquals(0, num_images, str(self.service.index(self.context)))
@@ -133,6 +100,24 @@ class BaseImageServiceTests(object):
         num_images = len(self.service.index(self.context))
         self.assertEquals(1, num_images)
 
+    def test_index(self):
+        fixture = self._make_fixture('test image')
+        image_id = self.service.create(self.context, fixture)['id']
+        image_metas = self.service.index(self.context)
+        expected = [{'id': 'DONTCARE', 'name': 'test image'}]
+        self.assertDictListMatch(image_metas, expected)
+
+    @staticmethod
+    def _make_fixture(name):
+        fixture = {'name': 'test image',
+                   'updated': None,
+                   'created': None,
+                   'status': None,
+                   'is_public': True,
+                   'instance_id': None,
+                   'progress': None}
+        return fixture
+
 
 class LocalImageServiceTest(test.TestCase,
                             BaseImageServiceTests):
@@ -187,7 +172,7 @@ class GlanceImageServiceTest(test.TestCase,
         fakes.stub_out_compute_api_snapshot(self.stubs)
         service_class = 'nova.image.glance.GlanceImageService'
         self.service = utils.import_object(service_class)
-        self.context = context.RequestContext(None, None)
+        self.context = context.RequestContext(1, None)
         self.service.delete_all()
         self.sent_to_glance = {}
         fakes.stub_out_glance_add_image(self.stubs, self.sent_to_glance)
@@ -199,13 +184,15 @@ class GlanceImageServiceTest(test.TestCase,
     def test_create_with_instance_id(self):
         """Ensure instance_id is persisted as an image-property"""
         fixture = {'name': 'test image',
-                   'properties': {'instance_id': '42'}}
+                   'is_public': False,
+                   'properties': {'instance_id': '42', 'user_id': '1'}}
 
         image_id = self.service.create(self.context, fixture)['id']
 
         expected = {'id': image_id,
                     'name': 'test image',
-                    'properties': {'instance_id': '42'}}
+                    'is_public': False,
+                    'properties': {'instance_id': '42', 'user_id': '1'}}
         self.assertDictMatch(self.sent_to_glance['metadata'], expected)
 
         image_meta = self.service.show(self.context, image_id)
@@ -301,38 +288,46 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
         public_image = {'id': 123,
                         'name': 'public image',
                         'is_public': True,
-                        'status': 'active'}
+                        'status': 'active',
+                        'properties': {}}
         fixtures.append(public_image)
 
         queued_backup = {'id': 124,
                          'name': 'queued backup',
                          'is_public': False,
                          'status': 'queued',
-                         'instance_id': 42}
+                         'properties': {'instance_id': 42, 'user_id': 1}}
         fixtures.append(queued_backup)
 
         saving_backup = {'id': 125,
                          'name': 'saving backup',
                          'is_public': False,
                          'status': 'saving',
-                         'instance_id': 42,
-                         'progress': 0}
+                         'properties': {'instance_id': 42, 'user_id': 1}}
         fixtures.append(saving_backup)
 
         active_backup = {'id': 126,
                          'name': 'active backup',
                          'is_public': False,
                          'status': 'active',
-                         'instance_id': 42}
+                         'properties': {'instance_id': 42, 'user_id': 1}}
         fixtures.append(active_backup)
 
         killed_backup = {'id': 127,
                          'name': 'killed backup',
                          'is_public': False,
                          'status': 'killed',
-                         'instance_id': 42}
+                         'properties': {'instance_id': 42, 'user_id': 1}}
         fixtures.append(killed_backup)
 
+        someone_elses_backup = {'id': 127,
+                                'name': 'somone elses backup',
+                                'is_public': False,
+                                'status': 'active',
+                                'properties': {'instance_id': 43, 
+                                               'user_id': 2}}
+        fixtures.append(someone_elses_backup)
+
         base_attrs = {'created_at': cls.NOW_SERVICE_STR,
                       'updated_at': cls.NOW_SERVICE_STR,
                       'deleted_at': None,
diff --git a/nova/utils.py b/nova/utils.py
index 7b96a0daf..d114cb14f 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -629,3 +629,9 @@ def map_dict_keys(dict_, key_map):
         mapped_key = key_map[key] if key in key_map else key
         mapped[mapped_key] = value
     return mapped
+
+
+def subset_dict(dict_, keys):
+    """Return a dict that only contains a subset of keys"""
+    subset = partition_dict(dict_, keys)[0]
+    return subset