openstack
/
nova
Code Issues Proposed changes

Remove db layer hard-code permission checks for flavor_access 

This patch removes the hard-code permission checks for db call
flavor_access.

There are elevated contexts in the code used to call service_create
aren't removed in this patch. The elevated context will be cleaned up
in last step. This is safe if we forget removing some hard-code
permission.

Partially implements bp v3-api-policy

Change-Id: I7195547099d9a8b6d7789885561bb2809234a155
This commit is contained in:
He Jie Xu 2015-03-02 15:46:53 +08:00
parent 4103a86f30
commit 50dc2957b2
4 changed files with 102 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
nova/api/openstack/compute/contrib/flavor_access.py
View File

@ -19,6 +19,7 @@ import webob
from nova.api.openstack import extensions
from nova.api.openstack import wsgi
from nova import context as nova_context
from nova import exception
from nova.i18n import _
from nova import objects
@ -47,7 +48,9 @@ class FlavorAccessController(object):
def index(self, req, flavor_id):
context = req.environ['nova.context']
authorize(context)
# NOTE(alex_xu): back-compatible with db layer hard-code admin
# permission checks.
nova_context.require_admin_context(context)
try:
flavor = objects.Flavor.get_by_flavor_id(context, flavor_id)
except exception.FlavorNotFound:
@ -103,7 +106,9 @@ class FlavorActionController(wsgi.Controller):
def _addTenantAccess(self, req, id, body):
context = req.environ['nova.context']
authorize(context, action="addTenantAccess")
# NOTE(alex_xu): back-compatible with db layer hard-code admin
# permission checks.
nova_context.require_admin_context(context)
self._check_body(body)
vals = body['addTenantAccess']
@ -126,7 +131,9 @@ class FlavorActionController(wsgi.Controller):
def _removeTenantAccess(self, req, id, body):
context = req.environ['nova.context']
authorize(context, action="removeTenantAccess")
# NOTE(alex_xu): back-compatible with db layer hard-code admin
# permission checks.
nova_context.require_admin_context(context)
self._check_body(body)
vals = body['removeTenantAccess']

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

@ -4673,7 +4673,6 @@ def _flavor_access_query(context, session=None):
read_deleted="no")
@require_admin_context
def flavor_access_get_by_flavor_id(context, flavor_id):
"""Get flavor access list by flavor id."""
instance_type_id_subq = \
@ -4684,7 +4683,6 @@ def flavor_access_get_by_flavor_id(context, flavor_id):
return access_refs
@require_admin_context
def flavor_access_add(context, flavor_id, project_id):
"""Add given tenant to the flavor access list."""
instance_type_id = _flavor_get_id_from_flavor(context, flavor_id)
@ -4700,7 +4698,6 @@ def flavor_access_add(context, flavor_id, project_id):
return access_ref
@require_admin_context
def flavor_access_remove(context, flavor_id, project_id):
"""Remove given tenant from the flavor access list."""
instance_type_id = _flavor_get_id_from_flavor(context, flavor_id)

124
nova/tests/unit/api/openstack/compute/contrib/test_flavor_access.py
View File

@ -132,7 +132,8 @@ class FlavorAccessTestV21(test.NoDBTestCase):
super(FlavorAccessTestV21, self).setUp()
self.flavor_controller = flavors_api.Controller()
self.req = FakeRequest()
self.context = self.req.environ['nova.context']
self.req.environ = {"nova.context": context.RequestContext('fake_user',
'fake')}
self.stubs.Set(db, 'flavor_get_by_flavor_id',
fake_get_flavor_by_flavor_id)
self.stubs.Set(db, 'flavor_get_all',
@ -163,25 +164,6 @@ class FlavorAccessTestV21(test.NoDBTestCase):
result = self.flavor_access_controller.index(self.req, '2')
self.assertEqual(result, expected)
def test_list_with_no_context(self):
req = fakes.HTTPRequest.blank(self._prefix + '/flavors/fake/flavors')
def fake_authorize(context, target=None, action=None):
raise exception.PolicyNotAuthorized(action='index')
if self.api_version == "2.1":
self.stubs.Set(flavor_access_v21,
'authorize',
fake_authorize)
else:
self.stubs.Set(flavor_access_v2,
'authorize',
fake_authorize)
self.assertRaises(exception.PolicyNotAuthorized,
self.flavor_access_controller.index,
req, 'fake')
def test_list_flavor_with_admin_default_proj1(self):
expected = {'flavors': [{'id': '0'}, {'id': '1'}]}
req = fakes.HTTPRequest.blank(self._prefix + '/fake/flavors',
@ -313,14 +295,6 @@ class FlavorAccessTestV21(test.NoDBTestCase):
result = add_access(req, '3', body=body)
self.assertEqual(result, expected)
def test_add_tenant_access_with_no_admin_user(self):
req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action',
use_admin_context=False)
body = {'addTenantAccess': {'tenant': 'proj2'}}
add_access = self._get_add_access()
self.assertRaises(exception.PolicyNotAuthorized,
add_access, req, '2', body=body)
def test_add_tenant_access_with_no_tenant(self):
req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action',
use_admin_context=True)
@ -365,14 +339,6 @@ class FlavorAccessTestV21(test.NoDBTestCase):
self.assertRaises(self.validation_ex,
remove_access, req, '2', body=body)
def test_remove_tenant_access_with_no_admin_user(self):
req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action',
use_admin_context=False)
body = {'removeTenantAccess': {'tenant': 'proj2'}}
remove_access = self._get_remove_access()
self.assertRaises(exception.PolicyNotAuthorized,
remove_access, req, '2', body=body)
class FlavorAccessTestV20(FlavorAccessTestV21):
api_version = "2.0"
@ -380,3 +346,89 @@ class FlavorAccessTestV20(FlavorAccessTestV21):
FlavorActionController = flavor_access_v2.FlavorActionController
_prefix = "/v2/fake"
validation_ex = exc.HTTPBadRequest
def setUp(self):
super(FlavorAccessTestV20, self).setUp()
self.req = FakeRequest()
self.req.environ = {"nova.context": context.get_admin_context()}
def test_remove_tenant_access_with_no_admin_user(self):
req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action',
use_admin_context=False)
body = {'removeTenantAccess': {'tenant': 'proj2'}}
remove_access = self._get_remove_access()
self.assertRaises(exception.AdminRequired,
remove_access, req, '2', body=body)
def test_add_tenant_access_with_no_admin_user(self):
req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action',
use_admin_context=False)
body = {'addTenantAccess': {'tenant': 'proj2'}}
add_access = self._get_add_access()
self.assertRaises(exception.AdminRequired,
add_access, req, '2', body=body)
def test_list_with_no_admin(self):
req = fakes.HTTPRequest.blank(self._prefix + '/flavors/fake/flavors')
self.assertRaises(exception.AdminRequired,
self.flavor_access_controller.index,
req, 'fake')
class FlavorAccessPolicyEnforcementV21(test.NoDBTestCase):
def setUp(self):
super(FlavorAccessPolicyEnforcementV21, self).setUp()
self.act_controller = flavor_access_v21.FlavorActionController()
self.access_controller = flavor_access_v21.FlavorAccessController()
self.req = fakes.HTTPRequest.blank('')
def test_add_tenant_access_policy_failed(self):
rule_name = "compute_extension:v3:os-flavor-access:add_tenant_access"
self.policy.set_rules({rule_name: "project:non_fake"})
exc = self.assertRaises(
exception.PolicyNotAuthorized,
self.act_controller._add_tenant_access, self.req, fakes.FAKE_UUID,
body={'addTenantAccess': {'tenant': fakes.FAKE_UUID}})
self.assertEqual(
"Policy doesn't allow %s to be performed." % rule_name,
exc.format_message())
def test_remove_tenant_access_policy_failed(self):
rule_name = ("compute_extension:v3:os-flavor-access:"
"remove_tenant_access")
self.policy.set_rules({rule_name: "project:non_fake"})
exc = self.assertRaises(
exception.PolicyNotAuthorized,
self.act_controller._remove_tenant_access, self.req,
fakes.FAKE_UUID,
body={'removeTenantAccess': {'tenant': fakes.FAKE_UUID}})
self.assertEqual(
"Policy doesn't allow %s to be performed." % rule_name,
exc.format_message())
def test_extend_create_policy_failed(self):
rule_name = "compute_extension:v3:os-flavor-access"
self.policy.set_rules({rule_name: "project:non_fake"})
self.act_controller.create(self.req, None, None)
def test_extend_show_policy_failed(self):
rule_name = "compute_extension:v3:os-flavor-access"
self.policy.set_rules({rule_name: "project:non_fake"})
self.act_controller.show(self.req, None, None)
def test_extend_detail_policy_failed(self):
rule_name = "compute_extension:v3:os-flavor-access"
self.policy.set_rules({rule_name: "project:non_fake"})
self.act_controller.detail(self.req, None)
def test_index_policy_failed(self):
rule_name = "compute_extension:v3:os-flavor-access"
self.policy.set_rules({rule_name: "project:non_fake"})
exc = self.assertRaises(
exception.PolicyNotAuthorized,
self.access_controller.index, self.req,
fakes.FAKE_UUID)
self.assertEqual(
"Policy doesn't allow %s to be performed." % rule_name,
exc.format_message())

10
nova/tests/unit/fake_policy.py
View File

@ -189,13 +189,11 @@ policy_data = """
"compute_extension:fixed_ips": "",
"compute_extension:v3:os-fixed-ips": "",
"compute_extension:flavor_access": "",
"compute_extension:flavor_access:addTenantAccess": "rule:admin_api",
"compute_extension:flavor_access:removeTenantAccess": "rule:admin_api",
"compute_extension:flavor_access:addTenantAccess": "",
"compute_extension:flavor_access:removeTenantAccess": "",
"compute_extension:v3:os-flavor-access": "",
"compute_extension:v3:os-flavor-access:remove_tenant_access":
"rule:admin_api",
"compute_extension:v3:os-flavor-access:add_tenant_access":
"rule:admin_api",
"compute_extension:v3:os-flavor-access:remove_tenant_access": "",
"compute_extension:v3:os-flavor-access:add_tenant_access": "",
"compute_extension:flavor_disabled": "",
"compute_extension:v3:os-flavor-disabled": "",
"compute_extension:flavor_rxtx": "",