openstack
/
nova
Code Issues Proposed changes

Merge "Remove db layer hard-code permission checks for quota_destroy_all_*"

This commit is contained in:
Jenkins 2015-06-10 18:44:09 +00:00 committed by Gerrit Code Review
parent b1ff74031e 7a853fb4dd
commit 58ab78ed13
4 changed files with 67 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nova/api/openstack/compute/contrib/quotas.py
View File

@ -220,6 +220,12 @@ class QuotaSetsController(wsgi.Controller):
raise webob.exc.HTTPNotFound()
try:
nova.context.authorize_project_context(context, id)
# 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)
if user_id:
QUOTAS.destroy_all_by_project_and_user(context,
id, user_id)

8
nova/api/openstack/compute/plugins/v3/quota_sets.py
View File

@ -172,22 +172,18 @@ class QuotaSetsController(wsgi.Controller):
# TODO(oomichi): Here should be 204(No Content) instead of 202 by v2.1
# +microversions because the resource quota-set has been deleted completely
# when returning a response.
@extensions.expected_errors(403)
@extensions.expected_errors(())
@wsgi.response(202)
def delete(self, req, id):
context = req.environ['nova.context']
authorize(context, action='delete')
authorize(context, action='delete', 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)
if user_id:
QUOTAS.destroy_all_by_project_and_user(context,
id, user_id)
else:
QUOTAS.destroy_all_by_project(context, id)
except exception.Forbidden:
raise webob.exc.HTTPForbidden()
class QuotaSets(extensions.V3APIExtensionBase):

2
nova/db/sqlalchemy/api.py
View File

@ -3648,7 +3648,6 @@ def reservation_rollback(context, reservations, project_id=None, user_id=None):
reservation_query.soft_delete(synchronize_session=False)
@require_admin_context
def quota_destroy_all_by_project_and_user(context, project_id, user_id):
session = get_session()
with session.begin():
@ -3671,7 +3670,6 @@ def quota_destroy_all_by_project_and_user(context, project_id, user_id):
soft_delete(synchronize_session=False)
@require_admin_context
def quota_destroy_all_by_project(context, project_id):
session = get_session()
with session.begin():

81
nova/tests/unit/api/openstack/compute/contrib/test_quotas.py
View File

@ -22,7 +22,6 @@ import webob
from nova.api.openstack.compute.contrib import quotas as quotas_v2
from nova.api.openstack.compute.plugins.v3 import quota_sets as quotas_v21
from nova.api.openstack import extensions
from nova import context as context_maker
from nova import exception
from nova import quota
from nova import test
@ -99,6 +98,9 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
self.ext_mgr = self.mox.CreateMock(extensions.ExtensionManager)
self.controller = self.plugin.QuotaSetsController(self.ext_mgr)
def _get_http_request(self, url=''):
return fakes.HTTPRequest.blank(url)
def test_format_quota_set(self):
quota_set = self.controller._format_quota_set('1234',
self.default_quotas)
@ -314,25 +316,16 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
body = {'quota_set': self.default_quotas}
self._quotas_update_bad_request_case(body)
def test_quotas_delete_as_unauthorized_user(self):
def test_quotas_delete(self):
if self._is_v20_api_test():
self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
self.mox.ReplayAll()
req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234')
self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
req, 1234)
def test_quotas_delete_as_admin(self):
if self._is_v20_api_test():
self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
context = context_maker.get_admin_context()
self.req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234')
self.req.environ['nova.context'] = context
req = self._get_http_request()
self.mox.StubOutWithMock(quota.QUOTAS,
"destroy_all_by_project")
quota.QUOTAS.destroy_all_by_project(context, 1234)
quota.QUOTAS.destroy_all_by_project(req.environ['nova.context'],
1234)
self.mox.ReplayAll()
res = self.controller.delete(self.req, 1234)
res = self.controller.delete(req, 1234)
self.mox.VerifyAll()
self.assertEqual(202, self.get_delete_status_int(res))
@ -443,6 +436,9 @@ class UserQuotasTestV21(BaseQuotaSetsTest):
super(UserQuotasTestV21, self).setUp()
self._setup_controller()
def _get_http_request(self, url=''):
return fakes.HTTPRequest.blank(url)
def _setup_controller(self):
self.ext_mgr = self.mox.CreateMock(extensions.ExtensionManager)
self.controller = self.plugin.QuotaSetsController(self.ext_mgr)
@ -509,25 +505,18 @@ class UserQuotasTestV21(BaseQuotaSetsTest):
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
req, 'update_me', body=body)
def test_user_quotas_delete_as_unauthorized_user(self):
self.setup_mock_for_update()
req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1')
self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
req, 1234)
def test_user_quotas_delete_as_admin(self):
def test_user_quotas_delete(self):
if self._is_v20_api_test():
self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
self.ext_mgr.is_loaded('os-user-quotas').AndReturn(True)
context = context_maker.get_admin_context()
url = '/v2/fake4/os-quota-sets/1234?user_id=1'
self.req = fakes.HTTPRequest.blank(url)
self.req.environ['nova.context'] = context
req = self._get_http_request(url)
self.mox.StubOutWithMock(quota.QUOTAS,
"destroy_all_by_project_and_user")
quota.QUOTAS.destroy_all_by_project_and_user(context, 1234, '1')
quota.QUOTAS.destroy_all_by_project_and_user(
req.environ['nova.context'], 1234, '1')
self.mox.ReplayAll()
res = self.controller.delete(self.req, 1234)
res = self.controller.delete(req, 1234)
self.mox.VerifyAll()
self.assertEqual(202, self.get_delete_status_int(res))
@ -569,6 +558,9 @@ class QuotaSetsTestV2(QuotaSetsTestV21):
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)
# NOTE: The following tests are tricky and v2.1 API does not allow
# this kind of input by strong input validation. Just for test coverage,
# we keep them now.
@ -622,6 +614,14 @@ class QuotaSetsTestV2(QuotaSetsTestV21):
self.assertRaises(webob.exc.HTTPNotFound, self.controller.delete,
req, 1234)
def test_quotas_delete_as_unauthorized_user(self):
if self._is_v20_api_test():
self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
self.mox.ReplayAll()
req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234')
self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
req, 1234)
class QuotaSetsTestV2WithoutServerGroupQuotas(QuotaSetsTestV2):
include_server_group_quotas = False
@ -665,6 +665,15 @@ class UserQuotasTestV2(UserQuotasTestV21):
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)
def test_user_quotas_delete_with_non_admin(self):
self.setup_mock_for_update()
req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1')
self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
req, '1234')
class UserQuotasTestV2WithoutServerGroupQuotas(UserQuotasTestV2):
include_server_group_quotas = False
@ -689,3 +698,21 @@ class UserQuotasTestV2WithoutServerGroupQuotas(UserQuotasTestV2):
req = fakes.HTTPRequest.blank(url, use_admin_context=True)
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
req, 'update_me', body=body)
class QuotaSetsPolicyEnforcementV21(test.NoDBTestCase):
def setUp(self):
super(QuotaSetsPolicyEnforcementV21, self).setUp()
self.controller = quotas_v21.QuotaSetsController()
self.req = fakes.HTTPRequest.blank('')
def test_delete_policy_failed(self):
rule_name = "os_compute_api:os-quota-sets:delete"
self.policy.set_rules({rule_name: "project_id:non_fake"})
exc = self.assertRaises(
exception.PolicyNotAuthorized,
self.controller.delete, self.req, fakes.FAKE_UUID)
self.assertEqual(
"Policy doesn't allow %s to be performed." % rule_name,
exc.format_message())