diff --git a/cinder/api/common.py b/cinder/api/common.py index 17e7dddb4..eb1f1cae3 100644 --- a/cinder/api/common.py +++ b/cinder/api/common.py @@ -25,7 +25,9 @@ import webob from cinder.api.openstack import wsgi from cinder.api import xmlutil +from cinder import exception from cinder.i18n import _ +import cinder.policy from cinder import utils @@ -78,6 +80,14 @@ def validate_key_names(key_names_list): return True +def validate_policy(context, action): + try: + cinder.policy.enforce_action(context, action) + return True + except exception.PolicyNotAuthorized: + return False + + def get_pagination_params(params, max_limit=None): """Return marker, limit, offset tuple from request. diff --git a/cinder/api/v2/types.py b/cinder/api/v2/types.py index d6e8ceaeb..8a334eba4 100644 --- a/cinder/api/v2/types.py +++ b/cinder/api/v2/types.py @@ -22,14 +22,11 @@ from cinder.api import common from cinder.api.openstack import wsgi from cinder.api.v2.views import types as views_types from cinder.api import xmlutil -from cinder import context as ctx from cinder import exception from cinder.i18n import _ from cinder import utils from cinder.volume import volume_types -import cinder.policy - def make_voltype(elem): elem.set('id') @@ -61,18 +58,6 @@ class VolumeTypesController(wsgi.Controller): _view_builder_class = views_types.ViewBuilder - def _validate_policy(self, context): - target = { - 'project_id': context.project_id, - 'user_id': context.user_id, - } - try: - action = 'volume_extension:access_types_extra_specs' - cinder.policy.enforce(context, action, target) - return True - except Exception: - return False - @wsgi.serializers(xml=VolumeTypesTemplate) def index(self, req): """Returns the list of volume types.""" @@ -85,9 +70,6 @@ class VolumeTypesController(wsgi.Controller): """Return a single volume type item.""" context = req.environ['cinder.context'] - if not context.is_admin and self._validate_policy(context): - context = ctx.get_admin_context() - # get default volume type if id is not None and id == 'default': vol_type = volume_types.get_default_volume_type() @@ -134,8 +116,6 @@ class VolumeTypesController(wsgi.Controller): # to filters. filters = {} context = req.environ['cinder.context'] - if not context.is_admin and self._validate_policy(context): - context = ctx.get_admin_context() if context.is_admin: # Only admin has query access to all volume types filters['is_public'] = self._parse_is_public( diff --git a/cinder/api/v2/views/types.py b/cinder/api/v2/views/types.py index 00591ae9a..fbf063111 100644 --- a/cinder/api/v2/views/types.py +++ b/cinder/api/v2/views/types.py @@ -26,9 +26,14 @@ class ViewBuilder(common.ViewBuilder): name=volume_type.get('name'), is_public=volume_type.get('is_public'), description=volume_type.get('description')) - if context.is_admin: - trimmed['qos_specs_id'] = volume_type.get('qos_specs_id') + if common.validate_policy( + context, + 'volume_extension:access_types_extra_specs'): trimmed['extra_specs'] = volume_type.get('extra_specs') + if common.validate_policy( + context, + 'volume_extension:access_types_qos_specs_id'): + trimmed['qos_specs_id'] = volume_type.get('qos_specs_id') return trimmed if brief else dict(volume_type=trimmed) def index(self, request, volume_types): diff --git a/cinder/tests/unit/api/v2/test_types.py b/cinder/tests/unit/api/v2/test_types.py index ddec2d39c..981ace027 100644 --- a/cinder/tests/unit/api/v2/test_types.py +++ b/cinder/tests/unit/api/v2/test_types.py @@ -16,10 +16,12 @@ import uuid from lxml import etree +import mock from oslo_utils import timeutils import six import webob +import cinder.api.common as common from cinder.api.v2 import types from cinder.api.v2.views import types as views_types from cinder import context @@ -322,6 +324,103 @@ class VolumeTypesApiTest(test.TestCase): ) self.assertDictMatch(expected_volume_type, output['volume_type']) + def test_view_builder_show_qos_specs_id_policy(self): + with mock.patch.object(common, + 'validate_policy', + side_effect=[False, True]): + view_builder = views_types.ViewBuilder() + now = timeutils.utcnow().isoformat() + raw_volume_type = dict( + name='new_type', + description='new_type_desc', + qos_specs_id='new_id', + is_public=True, + deleted=False, + created_at=now, + updated_at=now, + extra_specs={}, + deleted_at=None, + id=42, + ) + + request = fakes.HTTPRequest.blank("/v2") + output = view_builder.show(request, raw_volume_type) + + self.assertIn('volume_type', output) + expected_volume_type = dict( + name='new_type', + description='new_type_desc', + qos_specs_id='new_id', + is_public=True, + id=42, + ) + self.assertDictMatch(expected_volume_type, output['volume_type']) + + def test_view_builder_show_extra_specs_policy(self): + with mock.patch.object(common, + 'validate_policy', + side_effect=[True, False]): + view_builder = views_types.ViewBuilder() + now = timeutils.utcnow().isoformat() + raw_volume_type = dict( + name='new_type', + description='new_type_desc', + qos_specs_id='new_id', + is_public=True, + deleted=False, + created_at=now, + updated_at=now, + extra_specs={}, + deleted_at=None, + id=42, + ) + + request = fakes.HTTPRequest.blank("/v2") + output = view_builder.show(request, raw_volume_type) + + self.assertIn('volume_type', output) + expected_volume_type = dict( + name='new_type', + description='new_type_desc', + extra_specs={}, + is_public=True, + id=42, + ) + self.assertDictMatch(expected_volume_type, output['volume_type']) + + def test_view_builder_show_pass_all_policy(self): + with mock.patch.object(common, + 'validate_policy', + side_effect=[True, True]): + view_builder = views_types.ViewBuilder() + now = timeutils.utcnow().isoformat() + raw_volume_type = dict( + name='new_type', + description='new_type_desc', + qos_specs_id='new_id', + is_public=True, + deleted=False, + created_at=now, + updated_at=now, + extra_specs={}, + deleted_at=None, + id=42, + ) + + request = fakes.HTTPRequest.blank("/v2") + output = view_builder.show(request, raw_volume_type) + + self.assertIn('volume_type', output) + expected_volume_type = dict( + name='new_type', + description='new_type_desc', + qos_specs_id='new_id', + extra_specs={}, + is_public=True, + id=42, + ) + self.assertDictMatch(expected_volume_type, output['volume_type']) + def test_view_builder_list(self): view_builder = views_types.ViewBuilder() diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json index 07c1363f5..2ff533cbe 100644 --- a/cinder/tests/unit/policy.json +++ b/cinder/tests/unit/policy.json @@ -49,6 +49,8 @@ "volume_extension:volume_actions:upload_image": "", "volume_extension:types_manage": "", "volume_extension:types_extra_specs": "", + "volume_extension:access_types_qos_specs_id": "rule:admin_api", + "volume_extension:access_types_extra_specs": "rule:admin_api", "volume_extension:volume_type_access": "", "volume_extension:volume_type_access:addProjectAccess": "rule:admin_api", "volume_extension:volume_type_access:removeProjectAccess": "rule:admin_api", diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json index a7686a579..02af88bdf 100644 --- a/etc/cinder/policy.json +++ b/etc/cinder/policy.json @@ -26,6 +26,7 @@ "volume_extension:types_manage": "rule:admin_api", "volume_extension:types_extra_specs": "rule:admin_api", + "volume_extension:access_types_qos_specs_id": "rule:admin_api", "volume_extension:access_types_extra_specs": "rule:admin_api", "volume_extension:volume_type_access": "rule:admin_or_owner", "volume_extension:volume_type_access:addProjectAccess": "rule:admin_api",