From: git-harry Date: Wed, 13 Aug 2014 13:22:49 +0000 (+0100) Subject: Prevent tenant viewing volumes owed by another X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=5868e8f285d23b56ca6123dab760342c57bf8c80;p=openstack-build%2Fcinder-build.git 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 0890aeb40..24e2c9a28 100644 --- 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 0aebeaace..ddc286505 100644 --- 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 b75a88a37..7f254a16f 100644 --- 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 0e71cf83a..af6c3fe2f 100644 --- 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 bbfe45394..441fbd7d3 100644 --- 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 9b76efa3b..61b169054 100644 --- 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 2103ed1af..eb9736915 100644 --- 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 4cc95b0d5..741819602 100644 --- 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 6b2bcbef0..a30333701 100644 --- 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