From 43626d293739dbe06c7fe26845d17c2e580e62dd Mon Sep 17 00:00:00 2001 From: jakedahn Date: Sun, 5 Aug 2012 14:32:07 -0700 Subject: [PATCH] Admin users should be restricted from seeing all snapshots by default. * Now to view all snapshots across all tenants you need to include the all_tenants=1 GET param in your api request. * Fixes remaining issues blocking bug #967882 Change-Id: I2a8338d77badc70201bb315198183f2091df43fb --- cinder/api/openstack/volume/snapshots.py | 6 ++- cinder/tests/api/openstack/fakes.py | 26 ++++++++++++ .../test_extended_snapshot_attributes.py | 2 +- .../api/openstack/volume/test_snapshots.py | 41 +++++++++++++++++-- cinder/volume/api.py | 13 ++++-- 5 files changed, 79 insertions(+), 9 deletions(-) diff --git a/cinder/api/openstack/volume/snapshots.py b/cinder/api/openstack/volume/snapshots.py index 3264c3ab2..9fe84a7fd 100644 --- a/cinder/api/openstack/volume/snapshots.py +++ b/cinder/api/openstack/volume/snapshots.py @@ -129,7 +129,11 @@ class SnapshotsController(object): """Returns a list of snapshots, transformed through entity_maker.""" context = req.environ['cinder.context'] - snapshots = self.volume_api.get_all_snapshots(context) + search_opts = {} + search_opts.update(req.GET) + + snapshots = self.volume_api.get_all_snapshots(context, + search_opts=search_opts) limited_list = common.limited(snapshots, req) res = [entity_maker(context, snapshot) for snapshot in limited_list] return {'snapshots': res} diff --git a/cinder/tests/api/openstack/fakes.py b/cinder/tests/api/openstack/fakes.py index 605d611ea..b6d2e34e8 100644 --- a/cinder/tests/api/openstack/fakes.py +++ b/cinder/tests/api/openstack/fakes.py @@ -239,3 +239,29 @@ def stub_volume_get_all(context, search_opts=None): def stub_volume_get_all_by_project(self, context, search_opts=None): return [stub_volume_get(self, context, '1')] + + +def stub_snapshot(id, **kwargs): + snapshot = { + 'id': id, + 'volume_id': 12, + 'status': 'available', + 'volume_size': 100, + 'created_at': None, + 'display_name': 'Default name', + 'display_description': 'Default description', + 'project_id': 'fake' + } + + snapshot.update(kwargs) + return snapshot + + +def stub_snapshot_get_all(self): + return [stub_snapshot(100, project_id='fake'), + stub_snapshot(101, project_id='superfake'), + stub_snapshot(102, project_id='superduperfake')] + + +def stub_snapshot_get_all_by_project(self, context): + return [stub_snapshot(1)] diff --git a/cinder/tests/api/openstack/volume/contrib/test_extended_snapshot_attributes.py b/cinder/tests/api/openstack/volume/contrib/test_extended_snapshot_attributes.py index 56e95e6fa..43490fbc1 100644 --- a/cinder/tests/api/openstack/volume/contrib/test_extended_snapshot_attributes.py +++ b/cinder/tests/api/openstack/volume/contrib/test_extended_snapshot_attributes.py @@ -51,7 +51,7 @@ def fake_snapshot_get(self, context, snapshot_id): return param -def fake_snapshot_get_all(self, context): +def fake_snapshot_get_all(self, context, search_opts=None): param = _get_default_snapshot_param() return [param] diff --git a/cinder/tests/api/openstack/volume/test_snapshots.py b/cinder/tests/api/openstack/volume/test_snapshots.py index 7dcdb0239..b224c4819 100644 --- a/cinder/tests/api/openstack/volume/test_snapshots.py +++ b/cinder/tests/api/openstack/volume/test_snapshots.py @@ -19,6 +19,7 @@ from lxml import etree import webob from cinder.api.openstack.volume import snapshots +from cinder import db from cinder import exception from cinder import flags from cinder.openstack.common import log as logging @@ -67,7 +68,7 @@ def stub_snapshot_get(self, context, snapshot_id): return param -def stub_snapshot_get_all(self, context): +def stub_snapshot_get_all(self, context, search_opts=None): param = _get_default_snapshot_param() return [param] @@ -77,9 +78,10 @@ class SnapshotApiTest(test.TestCase): super(SnapshotApiTest, self).setUp() self.controller = snapshots.SnapshotsController() - self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get) - self.stubs.Set(volume.api.API, "get_all_snapshots", - stub_snapshot_get_all) + self.stubs.Set(db, 'snapshot_get_all_by_project', + fakes.stub_snapshot_get_all_by_project) + self.stubs.Set(db, 'snapshot_get_all', + fakes.stub_snapshot_get_all) def test_snapshot_create(self): self.stubs.Set(volume.api.API, "create_snapshot", stub_snapshot_create) @@ -117,6 +119,7 @@ class SnapshotApiTest(test.TestCase): snapshot['display_description']) def test_snapshot_delete(self): + self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get) self.stubs.Set(volume.api.API, "delete_snapshot", stub_snapshot_delete) snapshot_id = UUID @@ -134,6 +137,7 @@ class SnapshotApiTest(test.TestCase): snapshot_id) def test_snapshot_show(self): + self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get) req = fakes.HTTPRequest.blank('/v1/snapshots/%s' % UUID) resp_dict = self.controller.show(req, UUID) @@ -149,6 +153,8 @@ class SnapshotApiTest(test.TestCase): snapshot_id) def test_snapshot_detail(self): + self.stubs.Set(volume.api.API, "get_all_snapshots", + stub_snapshot_get_all) req = fakes.HTTPRequest.blank('/v1/snapshots/detail') resp_dict = self.controller.detail(req) @@ -159,6 +165,33 @@ class SnapshotApiTest(test.TestCase): resp_snapshot = resp_snapshots.pop() self.assertEqual(resp_snapshot['id'], UUID) + def test_admin_list_snapshots_limited_to_project(self): + req = fakes.HTTPRequest.blank('/v1/fake/snapshots', + use_admin_context=True) + res = self.controller.index(req) + + self.assertTrue('snapshots' in res) + self.assertEqual(1, len(res['snapshots'])) + + def test_admin_list_snapshots_all_tenants(self): + req = fakes.HTTPRequest.blank('/v2/fake/snapshots?all_tenants=1', + use_admin_context=True) + res = self.controller.index(req) + self.assertTrue('snapshots' in res) + self.assertEqual(3, len(res['snapshots'])) + + def test_all_tenants_non_admin_gets_all_tenants(self): + req = fakes.HTTPRequest.blank('/v2/fake/snapshots?all_tenants=1') + res = self.controller.index(req) + self.assertTrue('snapshots' in res) + self.assertEqual(1, len(res['snapshots'])) + + def test_non_admin_get_by_project(self): + req = fakes.HTTPRequest.blank('/v2/fake/snapshots') + res = self.controller.index(req) + self.assertTrue('snapshots' in res) + self.assertEqual(1, len(res['snapshots'])) + class SnapshotSerializerTest(test.TestCase): def _verify_snapshot(self, snap, tree): diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 0d83d79b8..80e656f57 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -216,11 +216,18 @@ class API(base.Base): rv = self.db.snapshot_get(context, snapshot_id) return dict(rv.iteritems()) - def get_all_snapshots(self, context): + def get_all_snapshots(self, context, search_opts=None): check_policy(context, 'get_all_snapshots') - if context.is_admin: + + search_opts = search_opts or {} + + if (context.is_admin and 'all_tenants' in search_opts): + # Need to remove all_tenants to pass the filtering below. + del search_opts['all_tenants'] return self.db.snapshot_get_all(context) - return self.db.snapshot_get_all_by_project(context, context.project_id) + else: + return self.db.snapshot_get_all_by_project(context, + context.project_id) @wrap_check_policy def check_attach(self, context, volume): -- 2.45.2