From d635c418ed22f5888adac67a4245dfcd1e3629ec Mon Sep 17 00:00:00 2001
From: Nikhil Komawar <nikhil.komawar@rackspace.com>
Date: Fri, 9 Nov 2012 13:34:39 -0500
Subject: [PATCH] Fixes is_image_visible to not use deleted key

The 'is_image_visible' method is looking for deleted members while
they are already being filtered in the query. Registry while validating
all the members gets a KeyError as the key deleted does not exist.

Fixes Bug: 1077106

Change-Id: Ia53b6be3b2f740357df86e6c7a6a257a990c3aab
---
 glance/db/simple/api.py            |  9 ++++--
 glance/db/sqlalchemy/api.py        |  2 +-
 glance/tests/functional/db/base.py | 47 ++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py
index 5b26a32d7e..fa216d1135 100644
--- a/glance/db/simple/api.py
+++ b/glance/db/simple/api.py
@@ -207,8 +207,11 @@ def image_get(context, image_id, session=None, force_show_deleted=False):
             and (not image.get('is_public')) \
             and (image['owner'] is not None) \
             and (image['owner'] != context.owner):
-        LOG.info('Unable to get unowned image')
-        raise exception.Forbidden()
+        members = image_member_find(context, image_id=image_id,
+                                    member=context.owner)
+        if not members:
+            LOG.info('Unable to get unowned image')
+            raise exception.Forbidden()
 
     return image
 
@@ -467,7 +470,7 @@ def is_image_visible(context, image):
                                     image_id=image['id'],
                                     member=context.owner)
         if members:
-            return not members[0]['deleted']
+            return True
 
     # Private image
     return False
diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py
index 33fd834cf3..94a7e93f44 100644
--- a/glance/db/sqlalchemy/api.py
+++ b/glance/db/sqlalchemy/api.py
@@ -323,7 +323,7 @@ def is_image_visible(context, image):
                                     image_id=image['id'],
                                     member=context.owner)
         if members:
-            return not members[0]['deleted']
+            return True
 
     # Private image
     return False
diff --git a/glance/tests/functional/db/base.py b/glance/tests/functional/db/base.py
index 65420b3943..477747a304 100644
--- a/glance/tests/functional/db/base.py
+++ b/glance/tests/functional/db/base.py
@@ -24,6 +24,7 @@
 from glance.common import utils
 from glance import context
 from glance.openstack.common import timeutils
+from glance.openstack.common import uuidutils
 import glance.tests.functional.db as db_tests
 from glance.tests.unit import base
 
@@ -410,6 +411,52 @@ def test_image_get_all_limit_marker(self):
         images = self.db_api.image_get_all(self.context, limit=2)
         self.assertEquals(2, len(images))
 
+    def test_image_get_multiple_members(self):
+        TENANT1 = uuidutils.generate_uuid()
+        TENANT2 = uuidutils.generate_uuid()
+        ctxt1 = context.RequestContext(is_admin=False, tenant=TENANT1,
+                                       owner_is_tenant=True)
+        ctxt2 = context.RequestContext(is_admin=False, user=TENANT2,
+                                       owner_is_tenant=False)
+        UUIDX = uuidutils.generate_uuid()
+        #we need private image and context.owner should not match image owner
+        self.db_api.image_create(ctxt1, {'id': UUIDX,
+                                         'status': 'queued',
+                                         'is_public': False,
+                                         'owner': TENANT1})
+        values = {'image_id': UUIDX, 'member': TENANT2, 'can_share': False}
+        self.db_api.image_member_create(ctxt1, values)
+
+        image = self.db_api.image_get(ctxt2, UUIDX)
+        self.assertEquals(UUIDX, image['id'])
+
+    def test_is_image_visible(self):
+        TENANT1 = uuidutils.generate_uuid()
+        TENANT2 = uuidutils.generate_uuid()
+        ctxt1 = context.RequestContext(is_admin=False, tenant=TENANT1,
+                                       owner_is_tenant=True)
+        ctxt2 = context.RequestContext(is_admin=False, user=TENANT2,
+                                       owner_is_tenant=False)
+        UUIDX = uuidutils.generate_uuid()
+        #we need private image and context.owner should not match image owner
+        image = self.db_api.image_create(ctxt1, {'id': UUIDX,
+                                                 'status': 'queued',
+                                                 'is_public': False,
+                                                 'owner': TENANT1})
+
+        values = {'image_id': UUIDX, 'member': TENANT2, 'can_share': False}
+        self.db_api.image_member_create(ctxt1, values)
+
+        result = self.db_api.is_image_visible(ctxt2, image)
+        self.assertTrue(result)
+
+        # image should not be visible for a deleted memeber
+        members = self.db_api.image_member_find(ctxt1, image_id=UUIDX)
+        self.db_api.image_member_delete(ctxt1, members[0]['id'])
+
+        result = self.db_api.is_image_visible(ctxt2, image)
+        self.assertFalse(result)
+
     def test_image_tag_create(self):
         tag = self.db_api.image_tag_create(self.context, UUID1, 'snap')
         self.assertEqual('snap', tag)