diff --git a/manila/api/v1/share_snapshots.py b/manila/api/v1/share_snapshots.py index d074ba6e09..d54458fc0f 100644 --- a/manila/api/v1/share_snapshots.py +++ b/manila/api/v1/share_snapshots.py @@ -26,6 +26,7 @@ from manila.api.views import share_snapshots as snapshot_views from manila import db from manila import exception from manila.i18n import _ +from manila import policy from manila import share LOG = log.getLogger(__name__) @@ -63,6 +64,7 @@ class ShareSnapshotMixin(object): context = req.environ['manila.context'] LOG.info("Delete snapshot with id: %s", id, context=context) + policy.check_policy(context, 'share', 'delete_snapshot') try: snapshot = self.share_api.get_snapshot(context, id) @@ -145,6 +147,7 @@ class ShareSnapshotMixin(object): def update(self, req, id, body): """Update a snapshot.""" context = req.environ['manila.context'] + policy.check_policy(context, 'share', 'snapshot_update') if not body or 'snapshot' not in body: raise exc.HTTPUnprocessableEntity() diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index 48d2797ab8..369a956c33 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -52,6 +52,7 @@ class ShareMixin(object): def _delete(self, *args, **kwargs): return self.share_api.delete(*args, **kwargs) + @wsgi.Controller.authorize('get') def show(self, req, id): """Return data about the given share.""" context = req.environ['manila.context'] @@ -63,6 +64,7 @@ class ShareMixin(object): return self._view_builder.detail(req, share) + @wsgi.Controller.authorize def delete(self, req, id): """Delete a share.""" context = req.environ['manila.context'] @@ -97,6 +99,7 @@ class ShareMixin(object): return webob.Response(status_int=http_client.ACCEPTED) + @wsgi.Controller.authorize("get_all") def index(self, req): """Returns a summary list of shares.""" req.GET.pop('export_location_id', None) @@ -107,6 +110,7 @@ class ShareMixin(object): req.GET.pop('with_count', None) return self._get_shares(req, is_detail=False) + @wsgi.Controller.authorize("get_all") def detail(self, req): """Returns a detailed list of shares.""" req.GET.pop('export_location_id', None) @@ -518,6 +522,7 @@ class ShareMixin(object): return self._access_view_builder.list_view(req, access_rules) + @wsgi.Controller.authorize("extend") def _extend(self, req, id, body): """Extend size of a share.""" context = req.environ['manila.context'] @@ -533,6 +538,7 @@ class ShareMixin(object): return webob.Response(status_int=http_client.ACCEPTED) + @wsgi.Controller.authorize("shrink") def _shrink(self, req, id, body): """Shrink size of a share.""" context = req.environ['manila.context'] diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index a008d147ec..89564449d7 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -219,6 +219,7 @@ class ShareController(shares.ShareMixin, @wsgi.Controller.api_version('2.7') @wsgi.action('reset_status') + @wsgi.Controller.authorize('reset_status') def share_reset_status(self, req, id, body): return self._reset_status(req, id, body) @@ -464,6 +465,7 @@ class ShareController(shares.ShareMixin, return self._revert(req, id, body) @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("get_all") def index(self, req): """Returns a summary list of shares.""" if req.api_version_request < api_version.APIVersionRequest("2.35"): @@ -481,6 +483,7 @@ class ShareController(shares.ShareMixin, return self._get_shares(req, is_detail=False) @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("get_all") def detail(self, req): """Returns a detailed list of shares.""" if req.api_version_request < api_version.APIVersionRequest("2.35"): diff --git a/manila/share/api.py b/manila/share/api.py index 19b8cecd8e..5b4816a19c 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -1837,8 +1837,6 @@ class API(base.Base): def _get_all(self, context, search_opts=None, sort_key='created_at', sort_dir='desc', show_count=False): - policy.check_policy(context, 'share', 'get_all') - if search_opts is None: search_opts = {} @@ -2224,9 +2222,9 @@ class API(base.Base): def extend(self, context, share, new_size, force=False): if force: - policy.check_policy(context, 'share', 'force_extend') + policy.check_policy(context, 'share', 'force_extend', share) else: - policy.check_policy(context, 'share', 'extend') + policy.check_policy(context, 'share', 'extend', share) if share['status'] != constants.STATUS_AVAILABLE: msg_params = { @@ -2333,8 +2331,6 @@ class API(base.Base): resource=share) def shrink(self, context, share, new_size): - policy.check_policy(context, 'share', 'shrink') - status = six.text_type(share['status']).lower() valid_statuses = (constants.STATUS_AVAILABLE, constants.STATUS_SHRINKING_POSSIBLE_DATA_LOSS_ERROR) diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index ddc445aa01..664091bbc1 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -106,6 +106,7 @@ class ShareAPITest(test.TestCase): } CONF.set_default("default_share_type", None) + self.mock_object(policy, 'check_policy') def _process_expected_share_detailed_response(self, shr_dict, req_version): """Sets version based parameters on share dictionary.""" @@ -1692,6 +1693,10 @@ class ShareAPITest(test.TestCase): {'display_name~': search_opts['display_name~'], 'display_description~': search_opts['display_description~']}) + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], + 'share', + 'get_all') if use_admin_context: search_opts_expected.update({'fake_key': 'fake_value'}) search_opts_expected['host'] = search_opts['host'] @@ -1736,6 +1741,10 @@ class ShareAPITest(test.TestCase): search_opts_expected = {} + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], + 'share', + 'get_all') if use_admin_context: search_opts_expected.update({'fake_key': 'fake_value'}) search_opts_expected['host'] = search_opts['host'] @@ -1771,6 +1780,10 @@ class ShareAPITest(test.TestCase): } ] } + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], + 'share', + 'get_all') self.assertEqual(expected, res_dict) @ddt.data({'use_admin_context': False, 'version': '2.4'}, @@ -1857,10 +1870,14 @@ class ShareAPITest(test.TestCase): search_opts['export_location_id']) search_opts_expected['export_location_path'] = ( search_opts['export_location_path']) - if use_admin_context: search_opts_expected.update({'fake_key': 'fake_value'}) search_opts_expected['host'] = search_opts['host'] + + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], + 'share', + 'get_all') mock_get_all.assert_called_once_with( req.environ['manila.context'], sort_key=search_opts['sort_key'], @@ -1928,7 +1945,13 @@ class ShareAPITest(test.TestCase): def _list_detail_test_common(self, req, expected): self.mock_object(share_api.API, 'get_all', stubs.stub_share_get_all_by_project) + res_dict = self.controller.detail(req) + + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], + 'share', + 'get_all') self.assertDictListMatch(expected['shares'], res_dict['shares']) self.assertEqual(res_dict['shares'][0]['volume_type'], res_dict['shares'][0]['share_type']) @@ -1987,7 +2010,13 @@ class ShareAPITest(test.TestCase): req = fakes.HTTPRequest.blank( '/v2/fake/shares/detail', environ=env, version=share_replicas.MIN_SUPPORTED_API_VERSION) + res_dict = self.controller.detail(req) + + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], + 'share', + 'get_all') expected = { 'shares': [ { @@ -2072,6 +2101,7 @@ class ShareActionsTest(test.TestCase): super(ShareActionsTest, self).setUp() self.controller = shares.ShareController() self.mock_object(share_api.API, 'get', stubs.stub_share_get) + self.mock_object(policy, 'check_policy') @ddt.unpack @ddt.data( diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 15306fee0b..ff1fa15dad 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -240,9 +240,9 @@ class ShareAPITestCase(test.TestCase): self.mock_object(db_api, 'share_get_all_by_project', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[0])) ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True) + shares = self.api.get_all(ctx) - share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') + db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_1', filters={}, is_public=False @@ -253,9 +253,9 @@ class ShareAPITestCase(test.TestCase): ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True) self.mock_object(db_api, 'share_get_all', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES)) + shares = self.api.get_all(ctx, {'all_tenants': 1}) - share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') + db_api.share_get_all.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', filters={}) self.assertEqual(_FAKE_LIST_OF_ALL_SHARES, shares) @@ -264,9 +264,9 @@ class ShareAPITestCase(test.TestCase): ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True) self.mock_object(db_api, 'share_get_all', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES)) + shares = self.api.get_all(ctx, {'all_tenants': ''}) - share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') + db_api.share_get_all.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', filters={}) self.assertEqual(_FAKE_LIST_OF_ALL_SHARES, shares) @@ -275,9 +275,9 @@ class ShareAPITestCase(test.TestCase): ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True) self.mock_object(db_api, 'share_get_all_by_project', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[0])) + shares = self.api.get_all(ctx, {'all_tenants': 'false'}) - share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') + db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_1', filters={}, is_public=False @@ -311,10 +311,8 @@ class ShareAPITestCase(test.TestCase): exception.NotAuthorized, self.api.get_all, ctx, filters) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - mock.call(ctx, 'share', policy), - ]) + share_api.policy.check_policy.assert_called_once_with( + ctx, 'share', policy) def test_get_all_admin_filter_by_share_server_and_all_tenants(self): # NOTE(vponomaryov): if share_server_id provided, 'all_tenants' opt @@ -326,10 +324,8 @@ class ShareAPITestCase(test.TestCase): self.mock_object(db_api, 'share_get_all_by_project') shares = self.api.get_all( ctx, {'share_server_id': 'fake_server_3', 'all_tenants': 1}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - mock.call(ctx, 'share', 'list_by_share_server_id'), - ]) + share_api.policy.check_policy.assert_called_once_with( + ctx, 'share', 'list_by_share_server_id') db_api.share_get_all_by_share_server.assert_called_once_with( ctx, 'fake_server_3', sort_dir='desc', sort_key='created_at', filters={}, @@ -344,10 +340,9 @@ class ShareAPITestCase(test.TestCase): db_api, 'share_get_all_by_project', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[1::2])) expected_filters = {'display_name': 'bar'} + shares = self.api.get_all(ctx, {'display_name': 'bar'}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) + db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_2', filters=expected_filters, is_public=False @@ -377,9 +372,6 @@ class ShareAPITestCase(test.TestCase): expected_filters = copy.copy(search_opts) shares = self.api.get_all(ctx, search_opts) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', @@ -394,10 +386,9 @@ class ShareAPITestCase(test.TestCase): ctx = context.RequestContext('fake_uid', 'fake_pid_2', is_admin=True) self.mock_object(db_api, 'share_get_all_by_project', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[1:])) + shares = self.api.get_all(ctx, {'export_location_' + type: 'test'}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) + db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_2', @@ -409,10 +400,9 @@ class ShareAPITestCase(test.TestCase): ctx = context.RequestContext('fake_uid', 'fake_pid_2', is_admin=True) self.mock_object(db_api, 'share_get_all', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[:1])) + shares = self.api.get_all(ctx, {'name': 'foo', 'all_tenants': 1}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) + db_api.share_get_all.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', filters={}) self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[:1], shares) @@ -425,9 +415,7 @@ class ShareAPITestCase(test.TestCase): mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[0::2])) shares = self.api.get_all(ctx, {'status': constants.STATUS_AVAILABLE}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) + db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_2', filters=expected_filter, is_public=False @@ -442,9 +430,6 @@ class ShareAPITestCase(test.TestCase): expected_filter = {'status': constants.STATUS_ERROR} shares = self.api.get_all( ctx, {'status': constants.STATUS_ERROR, 'all_tenants': 1}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) db_api.share_get_all.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', filters=expected_filter) @@ -455,10 +440,9 @@ class ShareAPITestCase(test.TestCase): ctx = context.RequestContext('fake_uid', 'fake_pid_2', is_admin=False) self.mock_object(db_api, 'share_get_all_by_project', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[1:])) + shares = self.api.get_all(ctx, {'all_tenants': 1}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) + db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_2', filters={}, is_public=False @@ -472,11 +456,10 @@ class ShareAPITestCase(test.TestCase): mock.Mock(side_effect=[ _FAKE_LIST_OF_ALL_SHARES[1::2], _FAKE_LIST_OF_ALL_SHARES[2::4]])) + shares = self.api.get_all( ctx, {'name': 'bar', 'status': constants.STATUS_ERROR}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) + expected_filter_1 = {'status': constants.STATUS_ERROR} expected_filter_2 = {'status': constants.STATUS_AVAILABLE} @@ -491,11 +474,8 @@ class ShareAPITestCase(test.TestCase): # one item expected, two filtered shares = self.api.get_all( ctx, {'name': 'foo1', 'status': constants.STATUS_AVAILABLE}) + self.assertEqual(_FAKE_LIST_OF_ALL_SHARES[2::4], shares) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - mock.call(ctx, 'share', 'get_all'), - ]) db_api.share_get_all_by_project.assert_has_calls([ mock.call( ctx, sort_dir='desc', sort_key='created_at', @@ -514,9 +494,6 @@ class ShareAPITestCase(test.TestCase): self.mock_object(db_api, 'share_get_all_by_project', mock.Mock( return_value=_FAKE_LIST_OF_ALL_SHARES[1:])) shares = self.api.get_all(ctx, {'is_public': is_public}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_2', filters={}, is_public=True @@ -530,9 +507,6 @@ class ShareAPITestCase(test.TestCase): self.mock_object(db_api, 'share_get_all_by_project', mock.Mock( return_value=_FAKE_LIST_OF_ALL_SHARES[1:])) shares = self.api.get_all(ctx, {'is_public': is_public}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_2', filters={}, is_public=False @@ -545,17 +519,14 @@ class ShareAPITestCase(test.TestCase): is_admin=False) self.assertRaises(ValueError, self.api.get_all, ctx, {'is_public': is_public}) - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - ]) def test_get_all_with_sorting_valid(self): self.mock_object(db_api, 'share_get_all_by_project', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[0])) ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=False) + shares = self.api.get_all(ctx, sort_key='status', sort_dir='asc') - share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') + db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='asc', sort_key='status', project_id='fake_pid_1', filters={}, is_public=False @@ -572,8 +543,6 @@ class ShareAPITestCase(test.TestCase): ctx, sort_key=1, ) - share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') def test_get_all_sort_dir_invalid(self): self.mock_object(db_api, 'share_get_all_by_project', @@ -585,23 +554,18 @@ class ShareAPITestCase(test.TestCase): ctx, sort_dir=1, ) - share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') def _get_all_filter_metadata_or_extra_specs_valid(self, key): self.mock_object(db_api, 'share_get_all_by_project', mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES[0])) ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=False) search_opts = {key: {'foo1': 'bar1', 'foo2': 'bar2'}} + shares = self.api.get_all(ctx, search_opts=search_opts.copy()) + if key == 'extra_specs': - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - mock.call(ctx, 'share_types_extra_spec', 'index'), - ]) - else: share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') + ctx, 'share_types_extra_spec', 'index') db_api.share_get_all_by_project.assert_called_once_with( ctx, sort_dir='desc', sort_key='created_at', project_id='fake_pid_1', filters=search_opts, is_public=False) @@ -621,13 +585,8 @@ class ShareAPITestCase(test.TestCase): self.assertRaises(exception.InvalidInput, self.api.get_all, ctx, search_opts=search_opts) if key == 'extra_specs': - share_api.policy.check_policy.assert_has_calls([ - mock.call(ctx, 'share', 'get_all'), - mock.call(ctx, 'share_types_extra_spec', 'index'), - ]) - else: share_api.policy.check_policy.assert_called_once_with( - ctx, 'share', 'get_all') + ctx, 'share_types_extra_spec', 'index') def test_get_all_filter_by_invalid_metadata(self): self._get_all_filter_metadata_or_extra_specs_invalid(key='metadata')