From f8088946b595184cc714b299bc8d7b0a475b1738 Mon Sep 17 00:00:00 2001 From: Monica Joshi Date: Mon, 27 Mar 2017 04:02:09 -0400 Subject: [PATCH] Fix for Group API update to include check policy All cinder APIs should do policy checks to enforce role based access controls. All group apis except groups update has this in place. This changeset adds policy check for the update groups api. Adds policy check for create_group_snapshot Adds policy check for reset_status Updates unit testcases Change-Id: I36d3c929709b82cf5f34f681a2e1c34bba9feef9 Closes-Bug: 1676278 --- cinder/group/api.py | 4 ++- cinder/tests/unit/group/test_groups_api.py | 30 +++++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/cinder/group/api.py b/cinder/group/api.py index e21dfa5a38d..160204a4dd4 100644 --- a/cinder/group/api.py +++ b/cinder/group/api.py @@ -551,6 +551,7 @@ class API(base.Base): self.volume_rpcapi.delete_group(context, group) + @wrap_check_policy def update(self, context, group, name, description, add_volumes, remove_volumes): """Update group.""" @@ -773,10 +774,10 @@ class API(base.Base): sort_dirs=sort_dirs) return groups + @wrap_check_policy def reset_status(self, context, group, status): """Reset status of generic group""" - check_policy(context, 'reset_status') if status not in c_fields.GroupStatus.ALL: msg = _("Group status: %(status)s is invalid, valid status " "are: %(valid)s.") % {'status': status, @@ -787,6 +788,7 @@ class API(base.Base): group.update(field) group.save() + @wrap_check_policy def create_group_snapshot(self, context, group, name, description): group.assert_not_frozen() options = {'group_id': group.id, diff --git a/cinder/tests/unit/group/test_groups_api.py b/cinder/tests/unit/group/test_groups_api.py index 9fe2a4a4a27..f5d531d09ff 100644 --- a/cinder/tests/unit/group/test_groups_api.py +++ b/cinder/tests/unit/group/test_groups_api.py @@ -55,6 +55,7 @@ class GroupAPITestCase(test.TestCase): mock_group_get.return_value = fake_group grp = self.group_api.get(self.ctxt, fake.GROUP_ID) self.assertEqual(fake_group, grp) + mock_policy.assert_called_once_with(self.ctxt, 'get', mock.ANY) @ddt.data(True, False) @mock.patch('cinder.objects.GroupList.get_all') @@ -72,9 +73,11 @@ class GroupAPITestCase(test.TestCase): grps = self.group_api.get_all(self.ctxt, filters={'all_tenants': True}) self.assertEqual(fake_groups, grps) + mock_policy.assert_called_once_with(self.ctxt, 'get_all') else: grps = self.group_api.get_all(self.user_ctxt) self.assertEqual(fake_groups_by_project, grps) + mock_policy.assert_called_once_with(self.user_ctxt, 'get_all') @mock.patch('cinder.volume.rpcapi.VolumeAPI.delete_group') @mock.patch('cinder.db.volume_get_all_by_generic_group') @@ -105,6 +108,7 @@ class GroupAPITestCase(test.TestCase): fake.GROUP_TYPE_ID, [fake.VOLUME_TYPE_ID], availability_zone = 'nova') + mock_policy.assert_called_with(self.ctxt, 'create') self.assertEqual(grp.obj_to_primitive(), ret_group.obj_to_primitive()) ret_group.host = "test_host@fakedrv#fakepool" @@ -114,6 +118,7 @@ class GroupAPITestCase(test.TestCase): mock_volume_get_all.assert_called_once_with(mock.ANY, ret_group.id) mock_volumes_update.assert_called_once_with(self.ctxt, []) mock_rpc_delete_group.assert_called_once_with(self.ctxt, ret_group) + mock_policy.assert_called_with(self.ctxt, 'delete', mock.ANY) @mock.patch('cinder.group.api.API._cast_create_group') @mock.patch('cinder.group.api.API.update_quota') @@ -143,6 +148,7 @@ class GroupAPITestCase(test.TestCase): mock_group_type_get.assert_called_once_with(self.ctxt, "fake-grouptype-name") + mock_policy.assert_called_with(self.ctxt, 'create') @mock.patch('cinder.group.api.API._cast_create_group') @mock.patch('cinder.group.api.API.update_quota') @@ -173,10 +179,12 @@ class GroupAPITestCase(test.TestCase): "fake-grouptype-name") mock_volume_types_get.assert_called_once_with(mock.ANY, volume_type_names) + mock_policy.assert_called_with(self.ctxt, 'create') @mock.patch('oslo_utils.timeutils.utcnow') @mock.patch('cinder.objects.Group') - def test_reset_status(self, mock_group, mock_time_util): + @mock.patch('cinder.group.api.check_policy') + def test_reset_status(self, mock_policy, mock_group, mock_time_util): mock_time_util.return_value = "time_now" self.group_api.reset_status(self.ctxt, mock_group, fields.GroupStatus.AVAILABLE) @@ -185,6 +193,8 @@ class GroupAPITestCase(test.TestCase): 'status': fields.GroupStatus.AVAILABLE} mock_group.update.assert_called_once_with(update_field) mock_group.save.assert_called_once_with() + mock_policy.assert_called_once_with(self.ctxt, + 'reset_status', mock.ANY) @mock.patch.object(GROUP_QUOTAS, "reserve") @mock.patch('cinder.objects.Group') @@ -219,6 +229,7 @@ class GroupAPITestCase(test.TestCase): "fake-grouptype-name", [fake.VOLUME_TYPE_ID], availability_zone='nova') + mock_policy.assert_called_with(self.ctxt, 'create') @mock.patch('cinder.volume.rpcapi.VolumeAPI.update_group') @mock.patch('cinder.db.volume_get_all_by_generic_group') @@ -286,6 +297,7 @@ class GroupAPITestCase(test.TestCase): mock_rpc_update_group.assert_called_once_with(self.ctxt, ret_group, add_volumes = vol1.id, remove_volumes = vol2.id) + mock_policy.assert_called_with(self.ctxt, 'update', mock.ANY) @mock.patch('cinder.objects.GroupSnapshot.get_by_id') @mock.patch('cinder.group.api.check_policy') @@ -295,6 +307,7 @@ class GroupAPITestCase(test.TestCase): grp_snap = self.group_api.get_group_snapshot( self.ctxt, fake.GROUP_SNAPSHOT_ID) self.assertEqual(fake_group_snap, grp_snap) + mock_policy.assert_called_with(self.ctxt, 'get_group_snapshot') @ddt.data(True, False) @mock.patch('cinder.objects.GroupSnapshotList.get_all') @@ -312,10 +325,14 @@ class GroupAPITestCase(test.TestCase): grp_snaps = self.group_api.get_all_group_snapshots( self.ctxt, filters={'all_tenants': True}) self.assertEqual(fake_group_snaps, grp_snaps) + mock_policy.assert_called_with(self.ctxt, + 'get_all_group_snapshots') else: grp_snaps = self.group_api.get_all_group_snapshots( self.user_ctxt) self.assertEqual(fake_group_snaps_by_project, grp_snaps) + mock_policy.assert_called_with(self.user_ctxt, + 'get_all_group_snapshots') @mock.patch('cinder.objects.GroupSnapshot') @mock.patch('cinder.group.api.check_policy') @@ -326,6 +343,7 @@ class GroupAPITestCase(test.TestCase): grp_snap_update) mock_group_snap.update.assert_called_once_with(grp_snap_update) mock_group_snap.save.assert_called_once_with() + mock_policy.assert_called_with(self.ctxt, 'update_group_snapshot') @mock.patch('cinder.volume.rpcapi.VolumeAPI.delete_group_snapshot') @mock.patch('cinder.volume.rpcapi.VolumeAPI.create_group_snapshot') @@ -365,9 +383,12 @@ class GroupAPITestCase(test.TestCase): ret_group_snap.id) mock_create_api.assert_called_once_with(self.ctxt, ret_group_snap) + mock_policy.assert_called_once_with(self.ctxt, 'create_group_snapshot', + mock.ANY) ret_group_snap.assert_not_frozen = mock.Mock(return_value=True) self.group_api.delete_group_snapshot(self.ctxt, ret_group_snap) mock_delete_api.assert_called_once_with(mock.ANY, ret_group_snap) + mock_policy.assert_called_with(self.ctxt, 'delete_group_snapshot') @mock.patch('cinder.objects.VolumeType.get_by_name_or_id') @mock.patch('cinder.db.group_volume_type_mapping_create') @@ -393,7 +414,6 @@ class GroupAPITestCase(test.TestCase): group_type_id = fake.GROUP_TYPE_ID, status = fields.GroupStatus.CREATING) mock_group_snap_get.return_value = grp_snap - vol1 = utils.create_volume( self.ctxt, availability_zone = 'nova', @@ -570,7 +590,9 @@ class GroupAPITestCase(test.TestCase): @mock.patch('oslo_utils.timeutils.utcnow') @mock.patch('cinder.objects.GroupSnapshot') - def test_reset_group_snapshot_status(self, mock_group_snapshot, + @mock.patch('cinder.group.api.check_policy') + def test_reset_group_snapshot_status(self, mock_policy, + mock_group_snapshot, mock_time_util): mock_time_util.return_value = "time_now" self.group_api.reset_group_snapshot_status( @@ -580,6 +602,8 @@ class GroupAPITestCase(test.TestCase): 'status': fields.GroupSnapshotStatus.ERROR} mock_group_snapshot.update.assert_called_once_with(update_field) mock_group_snapshot.save.assert_called_once_with() + mock_policy.assert_called_once_with(self.ctxt, + 'reset_group_snapshot_status') def test_create_group_from_src_frozen(self): service = utils.create_service(self.ctxt, {'frozen': True})