diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 5ad3ac7c9fd4..c53e47d86980 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -328,7 +328,8 @@ "os_compute_api:os-personality:discoverable": "", "os_compute_api:os-preserve-ephemeral-rebuild:discoverable": "", "os_compute_api:os-quota-sets:discoverable": "", - "os_compute_api:os-quota-sets:show": "", + "os_compute_api:os-quota-sets:show": "rule:admin_or_owner", + "os_compute_api:os-quota-sets:defaults": "", "os_compute_api:os-quota-sets:update": "rule:admin_api", "os_compute_api:os-quota-sets:delete": "rule:admin_api", "os_compute_api:os-quota-sets:detail": "rule:admin_api", diff --git a/nova/api/openstack/compute/contrib/quotas.py b/nova/api/openstack/compute/contrib/quotas.py index 59f532aa4201..bb41a14bc472 100644 --- a/nova/api/openstack/compute/contrib/quotas.py +++ b/nova/api/openstack/compute/contrib/quotas.py @@ -116,6 +116,15 @@ class QuotaSetsController(wsgi.Controller): def update(self, req, id, body): context = req.environ['nova.context'] authorize_update(context) + try: + # NOTE(alex_xu): back-compatible with db layer hard-code admin + # permission checks. This has to be left only for API v2.0 because + # this version has to be stable even if it means that only admins + # can call this method while the policy could be changed. + nova.context.require_admin_context(context) + except exception.AdminRequired: + raise webob.exc.HTTPForbidden() + project_id = id bad_keys = [] @@ -137,6 +146,9 @@ class QuotaSetsController(wsgi.Controller): user_id = params.get('user_id', [None])[0] try: + # NOTE(alex_xu): back-compatible with db layer hard-code admin + # permission checks. + nova.context.authorize_project_context(context, id) settable_quotas = QUOTAS.get_settable_quotas(context, project_id, user_id=user_id) except exception.Forbidden: @@ -199,8 +211,6 @@ class QuotaSetsController(wsgi.Controller): except exception.QuotaExists: objects.Quotas.update_limit(context, project_id, key, value, user_id=user_id) - except exception.AdminRequired: - raise webob.exc.HTTPForbidden() values = self._get_quotas(context, id, user_id=user_id) return self._format_quota_set(None, values) diff --git a/nova/api/openstack/compute/plugins/v3/quota_sets.py b/nova/api/openstack/compute/plugins/v3/quota_sets.py index b503170aebdf..cda254fd75b2 100644 --- a/nova/api/openstack/compute/plugins/v3/quota_sets.py +++ b/nova/api/openstack/compute/plugins/v3/quota_sets.py @@ -22,7 +22,6 @@ from nova.api.openstack.compute.schemas.v3 import quota_sets from nova.api.openstack import extensions from nova.api.openstack import wsgi from nova.api import validation -import nova.context from nova import exception from nova.i18n import _ from nova import objects @@ -83,37 +82,29 @@ class QuotaSetsController(wsgi.Controller): else: return {k: v['limit'] for k, v in values.items()} - @extensions.expected_errors(403) + @extensions.expected_errors(()) def show(self, req, id): context = req.environ['nova.context'] - authorize(context, action='show') + authorize(context, action='show', target={'project_id': id}) params = urlparse.parse_qs(req.environ.get('QUERY_STRING', '')) user_id = params.get('user_id', [None])[0] - try: - nova.context.authorize_project_context(context, id) - return self._format_quota_set(id, - self._get_quotas(context, id, user_id=user_id)) - except exception.Forbidden: - raise webob.exc.HTTPForbidden() + return self._format_quota_set(id, + self._get_quotas(context, id, user_id=user_id)) - @extensions.expected_errors(403) + @extensions.expected_errors(()) def detail(self, req, id): context = req.environ['nova.context'] - authorize(context, action='detail') + authorize(context, action='detail', target={'project_id': id}) user_id = req.GET.get('user_id', None) - try: - nova.context.authorize_project_context(context, id) - return self._format_quota_set(id, self._get_quotas(context, id, - user_id=user_id, - usages=True)) - except exception.Forbidden: - raise webob.exc.HTTPForbidden() + return self._format_quota_set(id, self._get_quotas(context, id, + user_id=user_id, + usages=True)) - @extensions.expected_errors((400, 403)) + @extensions.expected_errors(400) @validation.schema(quota_sets.update) def update(self, req, id, body): context = req.environ['nova.context'] - authorize(context, action='update') + authorize(context, action='update', target={'project_id': id}) project_id = id params = urlparse.parse_qs(req.environ.get('QUERY_STRING', '')) user_id = params.get('user_id', [None])[0] @@ -121,12 +112,8 @@ class QuotaSetsController(wsgi.Controller): quota_set = body['quota_set'] force_update = strutils.bool_from_string(quota_set.get('force', 'False')) - - try: - settable_quotas = QUOTAS.get_settable_quotas(context, project_id, - user_id=user_id) - except exception.Forbidden: - raise webob.exc.HTTPForbidden() + settable_quotas = QUOTAS.get_settable_quotas(context, project_id, + user_id=user_id) # NOTE(dims): Pass #1 - In this loop for quota_set.items(), we validate # min/max values and bail out if any of the items in the set is bad. @@ -155,8 +142,6 @@ class QuotaSetsController(wsgi.Controller): except exception.QuotaExists: objects.Quotas.update_limit(context, project_id, key, value, user_id=user_id) - except exception.AdminRequired: - raise webob.exc.HTTPForbidden() # Note(gmann): Removed 'id' from update's response to make it same # as V2. If needed it can be added with microversion. return self._format_quota_set(None, self._get_quotas(context, id, @@ -165,7 +150,7 @@ class QuotaSetsController(wsgi.Controller): @extensions.expected_errors(()) def defaults(self, req, id): context = req.environ['nova.context'] - authorize(context, action='show') + authorize(context, action='defaults', target={'project_id': id}) values = QUOTAS.get_defaults(context) return self._format_quota_set(id, values) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index be4ec4f6fd7b..1324fd6ef2aa 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -3060,8 +3060,6 @@ def quota_get(context, project_id, resource, user_id=None): @require_context def quota_get_all_by_project_and_user(context, project_id, user_id): - nova.context.authorize_project_context(context, project_id) - user_quotas = model_query(context, models.ProjectUserQuota, (models.ProjectUserQuota.resource, models.ProjectUserQuota.hard_limit)).\ @@ -3078,8 +3076,6 @@ def quota_get_all_by_project_and_user(context, project_id, user_id): @require_context def quota_get_all_by_project(context, project_id): - nova.context.authorize_project_context(context, project_id) - rows = model_query(context, models.Quota, read_deleted="no").\ filter_by(project_id=project_id).\ all() @@ -3093,8 +3089,6 @@ def quota_get_all_by_project(context, project_id): @require_context def quota_get_all(context, project_id): - nova.context.authorize_project_context(context, project_id) - result = model_query(context, models.ProjectUserQuota).\ filter_by(project_id=project_id).\ all() @@ -3102,7 +3096,6 @@ def quota_get_all(context, project_id): return result -@require_admin_context def quota_create(context, project_id, resource, limit, user_id=None): per_user = user_id and resource not in PER_PROJECT_QUOTAS quota_ref = models.ProjectUserQuota() if per_user else models.Quota() @@ -3118,7 +3111,6 @@ def quota_create(context, project_id, resource, limit, user_id=None): return quota_ref -@require_admin_context def quota_update(context, project_id, resource, limit, user_id=None): per_user = user_id and resource not in PER_PROJECT_QUOTAS model = models.ProjectUserQuota if per_user else models.Quota @@ -3224,7 +3216,6 @@ def quota_usage_get(context, project_id, resource, user_id=None): def _quota_usage_get_all(context, project_id, user_id=None): - nova.context.authorize_project_context(context, project_id) query = model_query(context, models.QuotaUsage, read_deleted="no").\ filter_by(project_id=project_id) result = {'project_id': project_id} diff --git a/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py b/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py index 877d91725b3b..e5cdb02e94a6 100644 --- a/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py +++ b/nova/tests/unit/api/openstack/compute/contrib/test_quotas.py @@ -177,49 +177,38 @@ class QuotaSetsTestV21(BaseQuotaSetsTest): self.assertEqual(res_dict, expected) - def test_quotas_show_as_admin(self): + def test_quotas_show(self): self.setup_mock_for_show() - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234', - use_admin_context=True) + req = self._get_http_request() res_dict = self.controller.show(req, 1234) ref_quota_set = quota_set('1234', self.include_server_group_quotas) self.assertEqual(res_dict, ref_quota_set) - def test_quotas_show_as_unauthorized_user(self): - self.setup_mock_for_show() - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234') - self.assertRaises(webob.exc.HTTPForbidden, self.controller.show, - req, 1234) - - def test_quotas_update_as_admin(self): + def test_quotas_update(self): self.setup_mock_for_update() self.default_quotas.update({ 'instances': 50, 'cores': 50 }) body = {'quota_set': self.default_quotas} - - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me', - use_admin_context=True) + req = self._get_http_request() res_dict = self.controller.update(req, 'update_me', body=body) self.assertEqual(body, res_dict) @mock.patch('nova.objects.Quotas.create_limit') - def test_quotas_update_with_good_data_as_admin(self, mock_createlimit): + def test_quotas_update_with_good_data(self, mock_createlimit): self.setup_mock_for_update() self.default_quotas.update({}) body = {'quota_set': self.default_quotas} - - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me', - use_admin_context=True) + req = self._get_http_request() self.controller.update(req, 'update_me', body=body) self.assertEqual(len(self.default_quotas), len(mock_createlimit.mock_calls)) @mock.patch('nova.api.validation.validators._SchemaValidator.validate') @mock.patch('nova.objects.Quotas.create_limit') - def test_quotas_update_with_bad_data_as_admin(self, mock_createlimit, + def test_quotas_update_with_bad_data(self, mock_createlimit, mock_validate): self.setup_mock_for_update() self.default_quotas.update({ @@ -227,15 +216,13 @@ class QuotaSetsTestV21(BaseQuotaSetsTest): 'cores': -50 }) body = {'quota_set': self.default_quotas} - - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me', - use_admin_context=True) + req = self._get_http_request() self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, 'update_me', body=body) self.assertEqual(0, len(mock_createlimit.mock_calls)) - def test_quotas_update_zero_value_as_admin(self): + def test_quotas_update_zero_value(self): self.setup_mock_for_update() body = {'quota_set': {'instances': 0, 'cores': 0, 'ram': 0, 'floating_ips': 0, @@ -250,27 +237,13 @@ class QuotaSetsTestV21(BaseQuotaSetsTest): body['quota_set']['server_groups'] = 10 body['quota_set']['server_group_members'] = 10 - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me', - use_admin_context=True) + req = self._get_http_request() res_dict = self.controller.update(req, 'update_me', body=body) self.assertEqual(body, res_dict) - def test_quotas_update_as_user(self): - self.setup_mock_for_update() - self.default_quotas.update({ - 'instances': 50, - 'cores': 50 - }) - body = {'quota_set': self.default_quotas} - - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me') - self.assertRaises(webob.exc.HTTPForbidden, self.controller.update, - req, 'update_me', body=body) - def _quotas_update_bad_request_case(self, body): self.setup_mock_for_update() - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me', - use_admin_context=True) + req = self._get_http_request() self.assertRaises(self.validation_error, self.controller.update, req, 'update_me', body=body) @@ -371,6 +344,9 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest): 'maximum': -1}, } + def _get_http_request(self, url=''): + return fakes.HTTPRequest.blank(url) + def test_quotas_update_exceed_in_used(self): patcher = mock.patch.object(quota.QUOTAS, 'get_settable_quotas') get_settable_quotas = patcher.start() @@ -378,8 +354,7 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest): body = {'quota_set': {'cores': 10}} get_settable_quotas.side_effect = self.fake_get_settable_quotas - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me', - use_admin_context=True) + req = self._get_http_request() self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, 'update_me', body=body) mock.patch.stopall() @@ -395,8 +370,7 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest): get_settable_quotas.side_effect = self.fake_get_settable_quotas _get_quotas.side_effect = self.fake_get_quotas - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me', - use_admin_context=True) + req = self._get_http_request() self.controller.update(req, 'update_me', body=body) mock.patch.stopall() @@ -443,21 +417,14 @@ class UserQuotasTestV21(BaseQuotaSetsTest): self.ext_mgr = self.mox.CreateMock(extensions.ExtensionManager) self.controller = self.plugin.QuotaSetsController(self.ext_mgr) - def test_user_quotas_show_as_admin(self): + def test_user_quotas_show(self): self.setup_mock_for_show() - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1', - use_admin_context=True) + req = self._get_http_request('/v2/fake4/os-quota-sets/1234?user_id=1') res_dict = self.controller.show(req, 1234) ref_quota_set = quota_set('1234', self.include_server_group_quotas) self.assertEqual(res_dict, ref_quota_set) - def test_user_quotas_show_as_unauthorized_user(self): - self.setup_mock_for_show() - req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1') - self.assertRaises(webob.exc.HTTPForbidden, self.controller.show, - req, 1234) - - def test_user_quotas_update_as_admin(self): + def test_user_quotas_update(self): self.setup_mock_for_update() body = {'quota_set': {'instances': 10, 'cores': 20, 'ram': 51200, 'floating_ips': 10, @@ -473,35 +440,17 @@ class UserQuotasTestV21(BaseQuotaSetsTest): body['quota_set']['server_group_members'] = 10 url = '/v2/fake4/os-quota-sets/update_me?user_id=1' - req = fakes.HTTPRequest.blank(url, use_admin_context=True) + req = self._get_http_request(url) res_dict = self.controller.update(req, 'update_me', body=body) self.assertEqual(body, res_dict) - def test_user_quotas_update_as_user(self): - self.setup_mock_for_update() - body = {'quota_set': {'instances': 10, 'cores': 20, - 'ram': 51200, 'floating_ips': 10, - 'fixed_ips': -1, 'metadata_items': 128, - 'injected_files': 5, - 'injected_file_content_bytes': 10240, - 'security_groups': 10, - 'security_group_rules': 20, - 'key_pairs': 100, - 'server_groups': 10, - 'server_group_members': 10}} - - url = '/v2/fake4/os-quota-sets/update_me?user_id=1' - req = fakes.HTTPRequest.blank(url) - self.assertRaises(webob.exc.HTTPForbidden, self.controller.update, - req, 'update_me', body=body) - def test_user_quotas_update_exceed_project(self): self.setup_mock_for_update() body = {'quota_set': {'instances': 20}} url = '/v2/fake4/os-quota-sets/update_me?user_id=1' - req = fakes.HTTPRequest.blank(url, use_admin_context=True) + req = self._get_http_request(url) self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, 'update_me', body=body) @@ -622,6 +571,23 @@ class QuotaSetsTestV2(QuotaSetsTestV21): self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete, req, 1234) + def test_quotas_show_as_unauthorized_user(self): + self.setup_mock_for_show() + req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234') + self.assertRaises(webob.exc.HTTPForbidden, self.controller.show, + req, 1234) + + def test_quotas_update_as_user(self): + self.default_quotas.update({ + 'instances': 50, + 'cores': 50 + }) + body = {'quota_set': self.default_quotas} + + req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me') + self.assertRaises(webob.exc.HTTPForbidden, self.controller.update, + req, 'update_me', body=body) + class QuotaSetsTestV2WithoutServerGroupQuotas(QuotaSetsTestV2): include_server_group_quotas = False @@ -653,6 +619,9 @@ class ExtendedQuotasTestV2(ExtendedQuotasTestV21): self.controller = self.plugin.QuotaSetsController(self.ext_mgr) self.mox.ResetAll() + def _get_http_request(self, url=''): + return fakes.HTTPRequest.blank(url, use_admin_context=True) + class UserQuotasTestV2(UserQuotasTestV21): plugin = quotas_v2 @@ -674,6 +643,27 @@ class UserQuotasTestV2(UserQuotasTestV21): self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete, req, '1234') + def test_user_quotas_show_as_unauthorized_user(self): + self.setup_mock_for_show() + req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1') + self.assertRaises(webob.exc.HTTPForbidden, self.controller.show, + req, 1234) + + def test_user_quotas_update_as_user(self): + body = {'quota_set': {'instances': 10, 'cores': 20, + 'ram': 51200, 'floating_ips': 10, + 'fixed_ips': -1, 'metadata_items': 128, + 'injected_files': 5, + 'injected_file_content_bytes': 10240, + 'key_pairs': 100, + 'security_groups': 10, + 'security_group_rules': 20}} + + url = '/v2/fake4/os-quota-sets/update_me?user_id=1' + req = fakes.HTTPRequest.blank(url) + self.assertRaises(webob.exc.HTTPForbidden, self.controller.update, + req, 'update_me', body=body) + class UserQuotasTestV2WithoutServerGroupQuotas(UserQuotasTestV2): include_server_group_quotas = False @@ -716,3 +706,44 @@ class QuotaSetsPolicyEnforcementV21(test.NoDBTestCase): self.assertEqual( "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) + + def test_defaults_policy_failed(self): + rule_name = "os_compute_api:os-quota-sets:defaults" + self.policy.set_rules({rule_name: "project_id:non_fake"}) + exc = self.assertRaises( + exception.PolicyNotAuthorized, + self.controller.defaults, self.req, fakes.FAKE_UUID) + self.assertEqual( + "Policy doesn't allow %s to be performed." % rule_name, + exc.format_message()) + + def test_show_policy_failed(self): + rule_name = "os_compute_api:os-quota-sets:show" + self.policy.set_rules({rule_name: "project_id:non_fake"}) + exc = self.assertRaises( + exception.PolicyNotAuthorized, + self.controller.show, self.req, fakes.FAKE_UUID) + self.assertEqual( + "Policy doesn't allow %s to be performed." % rule_name, + exc.format_message()) + + def test_detail_policy_failed(self): + rule_name = "os_compute_api:os-quota-sets:detail" + self.policy.set_rules({rule_name: "project_id:non_fake"}) + exc = self.assertRaises( + exception.PolicyNotAuthorized, + self.controller.detail, self.req, fakes.FAKE_UUID) + self.assertEqual( + "Policy doesn't allow %s to be performed." % rule_name, + exc.format_message()) + + def test_update_policy_failed(self): + rule_name = "os_compute_api:os-quota-sets:update" + self.policy.set_rules({rule_name: "project_id:non_fake"}) + exc = self.assertRaises( + exception.PolicyNotAuthorized, + self.controller.update, self.req, fakes.FAKE_UUID, + body={'quota_set': {}}) + self.assertEqual( + "Policy doesn't allow %s to be performed." % rule_name, + exc.format_message()) diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 7de20bbf26e9..2219fd5cb0f8 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -300,6 +300,7 @@ policy_data = """ "os_compute_api:os-quota-sets:update": "", "os_compute_api:os-quota-sets:delete": "", "os_compute_api:os-quota-sets:detail": "", + "os_compute_api:os-quota-sets:defaults": "", "compute_extension:quota_classes": "", "os_compute_api:os-quota-class-sets": "", "compute_extension:rescue": "",