Prevent tenant viewing volumes owed by another

Bug introduced by 0505bb268942534ad5d6ecd5e34a4d9b0e7f5c04 allows any
tenant to get the details of a volume belonging to any other tenant
if the UUID is known.

This commit allows only the tenant or an admin to get a volume.

Change-Id: I0268d374f7529d89068dcbf3c1cb9ab3d60d4115
Closes-Bug: #1356368

diff --git a/cinder/tests/api/fakes.py b/cinder/tests/api/fakes.py
index 0890aeb4010fca03b9eb02c80ecf4634f69e7c91..24e2c9a280bd3f44e7b7da57d9272461c8abee58 100644 (file)
--- a/cinder/tests/api/fakes.py
+++ b/cinder/tests/api/fakes.py
@@ -115,7 +115,7 @@ class HTTPRequest(webob.Request):
         out = os_wsgi.Request.blank(*args, **kwargs)
         out.environ['cinder.context'] = FakeRequestContext(
             'fake_user',
-            'fake',
+            'fakeproject',
             is_admin=use_admin_context)
         return out
 

diff --git a/cinder/tests/api/test_router.py b/cinder/tests/api/test_router.py
index 0aebeaacee5c4cb01ff794d994da491004a21f36..ddc2865059d3e4fa5bf4eb6b20e75fa4c4ce9e62 100644 (file)
--- a/cinder/tests/api/test_router.py
+++ b/cinder/tests/api/test_router.py
@@ -224,35 +224,35 @@ class VolumeRouterTestCase(test.TestCase):
         self.assertEqual(set(ids), set(['v1.0']))
 
     def test_volumes(self):
-        req = fakes.HTTPRequest.blank('/fake/volumes')
+        req = fakes.HTTPRequest.blank('/fakeproject/volumes')
         req.method = 'GET'
         req.content_type = 'application/json'
         response = req.get_response(self.app)
         self.assertEqual(200, response.status_int)
 
     def test_volumes_detail(self):
-        req = fakes.HTTPRequest.blank('/fake/volumes/detail')
+        req = fakes.HTTPRequest.blank('/fakeproject/volumes/detail')
         req.method = 'GET'
         req.content_type = 'application/json'
         response = req.get_response(self.app)
         self.assertEqual(200, response.status_int)
 
     def test_types(self):
-        req = fakes.HTTPRequest.blank('/fake/types')
+        req = fakes.HTTPRequest.blank('/fakeproject/types')
         req.method = 'GET'
         req.content_type = 'application/json'
         response = req.get_response(self.app)
         self.assertEqual(200, response.status_int)
 
     def test_snapshots(self):
-        req = fakes.HTTPRequest.blank('/fake/snapshots')
+        req = fakes.HTTPRequest.blank('/fakeproject/snapshots')
         req.method = 'GET'
         req.content_type = 'application/json'
         response = req.get_response(self.app)
         self.assertEqual(200, response.status_int)
 
     def test_snapshots_detail(self):
-        req = fakes.HTTPRequest.blank('/fake/snapshots/detail')
+        req = fakes.HTTPRequest.blank('/fakeproject/snapshots/detail')
         req.method = 'GET'
         req.content_type = 'application/json'
         response = req.get_response(self.app)

diff --git a/cinder/tests/api/v1/test_snapshot_metadata.py b/cinder/tests/api/v1/test_snapshot_metadata.py
index b75a88a37ccf61577ab56863563e5e81f1ca9ea2..7f254a16f6a377993f7c691956091822f90f1109 100644 (file)
--- a/cinder/tests/api/v1/test_snapshot_metadata.py
+++ b/cinder/tests/api/v1/test_snapshot_metadata.py
@@ -121,7 +121,8 @@ def return_volume(context, volume_id):
             'encryption_key_id': None,
             'volume_type_id': None,
             'migration_status': None,
-            'metadata': {}}
+            'metadata': {},
+            'project_id': context.project_id}
 
 
 def return_snapshot_nonexistent(context, snapshot_id):

diff --git a/cinder/tests/api/v1/test_volume_metadata.py b/cinder/tests/api/v1/test_volume_metadata.py
index 0e71cf83aefb50345b3493c3358e1f5fe7bcb969..af6c3fe2fc5eb4725b73e368d2044841e4e80b9b 100644 (file)
--- a/cinder/tests/api/v1/test_volume_metadata.py
+++ b/cinder/tests/api/v1/test_volume_metadata.py
@@ -106,7 +106,8 @@ def stub_max_volume_metadata():
 def return_volume(context, volume_id):
     return {'id': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
             'name': 'fake',
-            'metadata': {}}
+            'metadata': {},
+            'project_id': context.project_id}
 
 
 def return_volume_nonexistent(context, volume_id):

diff --git a/cinder/tests/api/v2/test_snapshot_metadata.py b/cinder/tests/api/v2/test_snapshot_metadata.py
index bbfe4539437ef52442875cd5caeb53a4090202f4..441fbd7d3cfe344754a4a336a9105f93ce1f6e93 100644 (file)
--- a/cinder/tests/api/v2/test_snapshot_metadata.py
+++ b/cinder/tests/api/v2/test_snapshot_metadata.py
@@ -121,7 +121,8 @@ def return_volume(context, volume_id):
             'encryption_key_id': None,
             'volume_type_id': None,
             'migration_status': None,
-            'metadata': {}}
+            'metadata': {},
+            'project_id': context.project_id}
 
 
 def return_snapshot_nonexistent(context, snapshot_id):

diff --git a/cinder/tests/api/v2/test_volume_metadata.py b/cinder/tests/api/v2/test_volume_metadata.py
index 9b76efa3b2813519a57cbb1048c679da48dd6264..61b169054a9e4ec59279c1ad2842590c0844b9de 100644 (file)
--- a/cinder/tests/api/v2/test_volume_metadata.py
+++ b/cinder/tests/api/v2/test_volume_metadata.py
@@ -107,7 +107,8 @@ def stub_max_volume_metadata():
 def return_volume(context, volume_id):
     return {'id': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
             'name': 'fake',
-            'metadata': {}}
+            'metadata': {},
+            'project_id': context.project_id}
 
 
 def return_volume_nonexistent(context, volume_id):

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index 2103ed1af96eb90b6c227e1e2c4b8f2467aba759..eb973691575516bdec5aba50c9c8ee2186d6db38 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -98,9 +98,9 @@ class VolumeApiTest(test.TestCase):
                          'description': 'Volume Test Desc',
                          'id': '1',
                          'links':
-                         [{'href': 'http://localhost/v2/fake/volumes/1',
+                         [{'href': 'http://localhost/v2/fakeproject/volumes/1',
                            'rel': 'self'},
-                          {'href': 'http://localhost/fake/volumes/1',
+                          {'href': 'http://localhost/fakeproject/volumes/1',
                            'rel': 'bookmark'}],
                          'metadata': {},
                          'name': 'Volume Test Name',
@@ -201,9 +201,9 @@ class VolumeApiTest(test.TestCase):
                          'encrypted': False,
                          'id': '1',
                          'links':
-                         [{'href': 'http://localhost/v2/fake/volumes/1',
+                         [{'href': 'http://localhost/v2/fakeproject/volumes/1',
                            'rel': 'self'},
-                          {'href': 'http://localhost/fake/volumes/1',
+                          {'href': 'http://localhost/fakeproject/volumes/1',
                            'rel': 'bookmark'}],
                          'metadata': {},
                          'name': 'Volume Test Name',
@@ -305,11 +305,11 @@ class VolumeApiTest(test.TestCase):
                 'size': 1,
                 'links': [
                     {
-                        'href': 'http://localhost/v2/fake/volumes/1',
+                        'href': 'http://localhost/v2/fakeproject/volumes/1',
                         'rel': 'self'
                     },
                     {
-                        'href': 'http://localhost/fake/volumes/1',
+                        'href': 'http://localhost/fakeproject/volumes/1',
                         'rel': 'bookmark'
                     }
                 ],
@@ -357,11 +357,11 @@ class VolumeApiTest(test.TestCase):
                 'size': 1,
                 'links': [
                     {
-                        'href': 'http://localhost/v2/fake/volumes/1',
+                        'href': 'http://localhost/v2/fakeproject/volumes/1',
                         'rel': 'self'
                     },
                     {
-                        'href': 'http://localhost/fake/volumes/1',
+                        'href': 'http://localhost/fakeproject/volumes/1',
                         'rel': 'bookmark'
                     }
                 ],
@@ -412,11 +412,11 @@ class VolumeApiTest(test.TestCase):
                 'size': 1,
                 'links': [
                     {
-                        'href': 'http://localhost/v2/fake/volumes/1',
+                        'href': 'http://localhost/v2/fakeproject/volumes/1',
                         'rel': 'self'
                     },
                     {
-                        'href': 'http://localhost/fake/volumes/1',
+                        'href': 'http://localhost/fakeproject/volumes/1',
                         'rel': 'bookmark'
                     }
                 ],
@@ -462,11 +462,11 @@ class VolumeApiTest(test.TestCase):
             'size': 1,
             'links': [
                 {
-                    'href': 'http://localhost/v2/fake/volumes/1',
+                    'href': 'http://localhost/v2/fakeproject/volumes/1',
                     'rel': 'self'
                 },
                 {
-                    'href': 'http://localhost/fake/volumes/1',
+                    'href': 'http://localhost/fakeproject/volumes/1',
                     'rel': 'bookmark'
                 }
             ],
@@ -575,11 +575,12 @@ class VolumeApiTest(test.TestCase):
                     'id': '1',
                     'links': [
                         {
-                            'href': 'http://localhost/v2/fake/volumes/1',
+                            'href': 'http://localhost/v2/fakeproject/volumes/'
+                                    '1',
                             'rel': 'self'
                         },
                         {
-                            'href': 'http://localhost/fake/volumes/1',
+                            'href': 'http://localhost/fakeproject/volumes/1',
                             'rel': 'bookmark'
                         }
                     ],
@@ -625,11 +626,12 @@ class VolumeApiTest(test.TestCase):
                     'size': 1,
                     'links': [
                         {
-                            'href': 'http://localhost/v2/fake/volumes/1',
+                            'href': 'http://localhost/v2/fakeproject/volumes/'
+                                    '1',
                             'rel': 'self'
                         },
                         {
-                            'href': 'http://localhost/fake/volumes/1',
+                            'href': 'http://localhost/fakeproject/volumes/1',
                             'rel': 'bookmark'
                         }
                     ],
@@ -731,7 +733,7 @@ class VolumeApiTest(test.TestCase):
         links = res_dict['volumes_links']
         self.assertEqual(links[0]['rel'], 'next')
         href_parts = urlparse.urlparse(links[0]['href'])
-        self.assertEqual('/v2/fake/volumes', href_parts.path)
+        self.assertEqual('/v2/fakeproject/volumes', href_parts.path)
         params = urlparse.parse_qs(href_parts.query)
         self.assertTrue('marker' in params)
         self.assertEqual('1', params['limit'][0])
@@ -820,7 +822,7 @@ class VolumeApiTest(test.TestCase):
         links = res_dict['volumes_links']
         self.assertEqual(links[0]['rel'], 'next')
         href_parts = urlparse.urlparse(links[0]['href'])
-        self.assertEqual('/v2/fake/volumes/detail', href_parts.path)
+        self.assertEqual('/v2/fakeproject/volumes/detail', href_parts.path)
         params = urlparse.parse_qs(href_parts.query)
         self.assertTrue('marker' in params)
         self.assertEqual('1', params['limit'][0])
@@ -900,7 +902,7 @@ class VolumeApiTest(test.TestCase):
             '''Verify next link and url.'''
             self.assertEqual(links[0]['rel'], 'next')
             href_parts = urlparse.urlparse(links[0]['href'])
-            self.assertEqual('/v2/fake/%s' % key, href_parts.path)
+            self.assertEqual('/v2/fakeproject/%s' % key, href_parts.path)
 
         # Verify both the index and detail queries
         api_keys = ['volumes', 'volumes/detail']
@@ -1083,11 +1085,11 @@ class VolumeApiTest(test.TestCase):
                 'size': 1,
                 'links': [
                     {
-                        'href': 'http://localhost/v2/fake/volumes/1',
+                        'href': 'http://localhost/v2/fakeproject/volumes/1',
                         'rel': 'self'
                     },
                     {
-                        'href': 'http://localhost/fake/volumes/1',
+                        'href': 'http://localhost/fakeproject/volumes/1',
                         'rel': 'bookmark'
                     }
                 ],
@@ -1124,11 +1126,11 @@ class VolumeApiTest(test.TestCase):
                 'size': 1,
                 'links': [
                     {
-                        'href': 'http://localhost/v2/fake/volumes/1',
+                        'href': 'http://localhost/v2/fakeproject/volumes/1',
                         'rel': 'self'
                     },
                     {
-                        'href': 'http://localhost/fake/volumes/1',
+                        'href': 'http://localhost/fakeproject/volumes/1',
                         'rel': 'bookmark'
                     }
                 ],

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 4cc95b0d5d7d0874f6cb09a4aa3b560ac4ad7eeb..74181960291f92d41ecbaa77c8950833cc7c81a7 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -604,6 +604,28 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.mox.UnsetStubs()
         self.volume.delete_volume(self.context, volume_id)
 
+    def test_get_volume_different_tenant(self):
+        """Test can't get volume of another tenant when viewable_admin_meta."""
+        volume = tests_utils.create_volume(self.context,
+                                           **self.volume_params)
+        volume_id = volume['id']
+        self.volume.create_volume(self.context, volume_id)
+
+        another_context = context.RequestContext('another_user_id',
+                                                 'another_project_id',
+                                                 is_admin=False)
+        self.assertNotEqual(another_context.project_id,
+                            self.context.project_id)
+
+        volume_api = cinder.volume.api.API()
+
+        self.assertRaises(exception.VolumeNotFound, volume_api.get,
+                          another_context, volume_id, viewable_admin_meta=True)
+        self.assertEqual(volume_id,
+                         volume_api.get(self.context, volume_id)['id'])
+
+        self.volume.delete_volume(self.context, volume_id)
+
     def test_delete_volume_in_error_extending(self):
         """Test volume can be deleted in error_extending stats."""
         # create a volume

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 6b2bcbef0f66156da1a407f23027d18cd971a3dc..a30333701708f869af6de1cabc2ca5d08ede185b 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -283,9 +283,13 @@ class API(base.Base):
 
     def get(self, context, volume_id, viewable_admin_meta=False):
         if viewable_admin_meta:
-            context = context.elevated()
-        rv = self.db.volume_get(context, volume_id)
+            ctxt = context.elevated()
+        else:
+            ctxt = context
+        rv = self.db.volume_get(ctxt, volume_id)
         volume = dict(rv.iteritems())
+        if not context.is_admin and volume['project_id'] != context.project_id:
+            raise exception.VolumeNotFound(volume_id=volume_id)
         check_policy(context, 'get', volume)
         return volume