diff --git a/etc/manila/policy.json b/etc/manila/policy.json index fb6b05c5d3..9e2cf3332d 100644 --- a/etc/manila/policy.json +++ b/etc/manila/policy.json @@ -27,6 +27,10 @@ "share:update_share_metadata": "rule:default", "share:migrate": "rule:admin_api", + "share_type:index": "rule:default", + "share_type:show": "rule:default", + "share_type:default": "rule:default", + "share_instance:index": "rule:admin_api", "share_instance:show": "rule:admin_api", diff --git a/manila/api/v1/share_types.py b/manila/api/v1/share_types.py index 29ef942c10..7f719572d2 100644 --- a/manila/api/v1/share_types.py +++ b/manila/api/v1/share_types.py @@ -22,8 +22,11 @@ from manila.api.openstack import wsgi from manila.api.views import types as views_types from manila import exception from manila.i18n import _ +from manila import policy from manila.share import share_types +RESOURCE_NAME = 'share_type' + class ShareTypesController(wsgi.Controller): """The share types API controller for the OpenStack API.""" @@ -32,6 +35,9 @@ class ShareTypesController(wsgi.Controller): def index(self, req): """Returns the list of share types.""" + context = req.environ['manila.context'] + policy.check_policy(context, RESOURCE_NAME, 'index') + limited_types = self._get_share_types(req) req.cache_db_share_types(limited_types) return self._view_builder.index(req, limited_types) @@ -39,6 +45,7 @@ class ShareTypesController(wsgi.Controller): def show(self, req, id): """Return a single share type item.""" context = req.environ['manila.context'] + policy.check_policy(context, RESOURCE_NAME, 'show') try: share_type = share_types.get_share_type(context, id) @@ -53,6 +60,7 @@ class ShareTypesController(wsgi.Controller): def default(self, req): """Return default volume type.""" context = req.environ['manila.context'] + policy.check_policy(context, RESOURCE_NAME, 'default') try: share_type = share_types.get_default_share_type(context) diff --git a/manila/api/views/types.py b/manila/api/views/types.py index 4845da0bee..acd6732995 100644 --- a/manila/api/views/types.py +++ b/manila/api/views/types.py @@ -14,6 +14,7 @@ # under the License. from manila.api import common +from manila.share import share_types class ViewBuilder(common.ViewBuilder): @@ -22,11 +23,22 @@ class ViewBuilder(common.ViewBuilder): def show(self, request, share_type, brief=False): """Trim away extraneous share type attributes.""" + + extra_specs = share_type.get('extra_specs', {}) + required_extra_specs = share_type.get('required_extra_specs', {}) + + # Remove non-tenant-visible extra specs in a non-admin context + if not request.environ['manila.context'].is_admin: + extra_spec_names = share_types.get_tenant_visible_extra_specs() + extra_specs = self._filter_extra_specs(extra_specs, + extra_spec_names) + required_extra_specs = self._filter_extra_specs( + required_extra_specs, extra_spec_names) + trimmed = dict(id=share_type.get('id'), name=share_type.get('name'), - extra_specs=share_type.get('extra_specs'), - required_extra_specs=share_type.get( - 'required_extra_specs')) + extra_specs=extra_specs, + required_extra_specs=required_extra_specs) if brief: return trimmed else: @@ -38,3 +50,7 @@ class ViewBuilder(common.ViewBuilder): for share_type in share_types] return dict(volume_types=share_types_list, share_types=share_types_list) + + def _filter_extra_specs(self, extra_specs, valid_keys): + return {key: value for key, value in extra_specs.items() + if key in valid_keys} diff --git a/manila/common/constants.py b/manila/common/constants.py index 2b9ad93f22..ed75d3dc9f 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -118,6 +118,10 @@ class ExtraSpecs(object): DRIVER_HANDLES_SHARE_SERVERS, SNAPSHOT_SUPPORT, ) + # NOTE(cknight): Some extra specs are necessary parts of the Manila API and + # should be visible to non-admin users. This list matches the UNDELETABLE + # list today, but that may not always remain true. + TENANT_VISIBLE = UNDELETABLE BOOLEAN = ( DRIVER_HANDLES_SHARE_SERVERS, SNAPSHOT_SUPPORT, diff --git a/manila/share/share_types.py b/manila/share/share_types.py index ba757f185f..041cf7eb16 100644 --- a/manila/share/share_types.py +++ b/manila/share/share_types.py @@ -203,6 +203,10 @@ def get_undeletable_extra_specs(): return constants.ExtraSpecs.UNDELETABLE +def get_tenant_visible_extra_specs(): + return constants.ExtraSpecs.TENANT_VISIBLE + + def get_boolean_extra_specs(): return constants.ExtraSpecs.BOOLEAN diff --git a/manila/tests/api/v1/test_share_types.py b/manila/tests/api/v1/test_share_types.py index e4fb8fab2b..29fafcdb94 100644 --- a/manila/tests/api/v1/test_share_types.py +++ b/manila/tests/api/v1/test_share_types.py @@ -22,6 +22,7 @@ from manila.api.v1 import share_types as types from manila.api.views import types as views_types from manila.common import constants from manila import exception +from manila import policy from manila.share import share_types from manila import test from manila.tests.api import fakes @@ -75,12 +76,17 @@ class ShareTypesApiTest(test.TestCase): def setUp(self): super(ShareTypesApiTest, self).setUp() self.controller = types.ShareTypesController() + self.mock_object(policy, 'check_policy', + mock.Mock(return_value=True)) - def test_share_types_index(self): + @ddt.data(True, False) + def test_share_types_index(self, admin): self.mock_object(share_types, 'get_all_types', return_share_types_get_all_types) - req = fakes.HTTPRequest.blank('/v2/fake/types') + req = fakes.HTTPRequest.blank('/v2/fake/types', + use_admin_context=admin) + res_dict = self.controller.index(req) self.assertEqual(3, len(res_dict['share_types'])) @@ -89,11 +95,16 @@ class ShareTypesApiTest(test.TestCase): actual_names = map(lambda e: e['name'], res_dict['share_types']) self.assertEqual(set(actual_names), set(expected_names)) for entry in res_dict['share_types']: - self.assertEqual('value1', entry['extra_specs']['key1']) + if admin: + self.assertEqual('value1', entry['extra_specs'].get('key1')) + else: + self.assertIsNone(entry['extra_specs'].get('key1')) self.assertTrue('required_extra_specs' in entry) required_extra_spec = entry['required_extra_specs'].get( constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS, '') self.assertEqual('true', required_extra_spec) + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], types.RESOURCE_NAME, 'index') def test_share_types_index_no_data(self): self.mock_object(share_types, 'get_all_types', @@ -103,6 +114,8 @@ class ShareTypesApiTest(test.TestCase): res_dict = self.controller.index(req) self.assertEqual(0, len(res_dict['share_types'])) + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], types.RESOURCE_NAME, 'index') def test_share_types_show(self): self.mock_object(share_types, 'get_share_type', @@ -114,6 +127,8 @@ class ShareTypesApiTest(test.TestCase): self.assertEqual(2, len(res_dict)) self.assertEqual('1', res_dict['share_type']['id']) self.assertEqual('share_type_1', res_dict['share_type']['name']) + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], types.RESOURCE_NAME, 'show') def test_share_types_show_not_found(self): self.mock_object(share_types, 'get_share_type', @@ -122,6 +137,8 @@ class ShareTypesApiTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/fake/types/777') self.assertRaises(webob.exc.HTTPNotFound, self.controller.show, req, '777') + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], types.RESOURCE_NAME, 'show') def test_share_types_default(self): self.mock_object(share_types, 'get_default_share_type', @@ -133,6 +150,8 @@ class ShareTypesApiTest(test.TestCase): self.assertEqual(2, len(res_dict)) self.assertEqual('1', res_dict['share_type']['id']) self.assertEqual('share_type_1', res_dict['share_type']['name']) + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], types.RESOURCE_NAME, 'default') def test_share_types_default_not_found(self): self.mock_object(share_types, 'get_default_share_type', @@ -141,6 +160,8 @@ class ShareTypesApiTest(test.TestCase): req = fakes.HTTPRequest.blank('/v2/fake/types/default') self.assertRaises(webob.exc.HTTPNotFound, self.controller.default, req) + policy.check_policy.assert_called_once_with( + req.environ['manila.context'], types.RESOURCE_NAME, 'default') def test_view_builder_show(self): view_builder = views_types.ViewBuilder() diff --git a/manila/tests/policy.json b/manila/tests/policy.json index 6ebaee1aca..81600eef02 100644 --- a/manila/tests/policy.json +++ b/manila/tests/policy.json @@ -18,6 +18,10 @@ "share:extend": "", "share:shrink": "", + "share_type:index": "rule:default", + "share_type:show": "rule:default", + "share_type:default": "rule:default", + "share_instance:index": "rule:admin_api", "share_instance:show": "rule:admin_api", diff --git a/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py b/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py index aaad426e9a..600072a45d 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py @@ -66,6 +66,18 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesAdminTest): self.member_shares_client.get_share_type_extra_specs, st["share_type"]["id"]) + @test.attr(type=["gate", "smoke", ]) + def test_try_read_extra_specs_on_share_type_with_user(self): + st = self._create_share_type() + share_type = self.member_shares_client.get_share_type( + st['share_type']['id']) + # Verify a non-admin can only read the required extra-specs + expected_keys = ['driver_handles_share_servers', 'snapshot_support'] + actual_keys = share_type['share_type']['extra_specs'].keys() + self.assertEqual(sorted(expected_keys), sorted(actual_keys), + 'Incorrect extra specs visible to non-admin user; ' + 'expected %s, got %s' % (expected_keys, actual_keys)) + @test.attr(type=["gate", "smoke", ]) def test_try_update_extra_spec_with_user(self): st = self._create_share_type()