Allow non-admin to list all tenants based on policy
Currently, running 'nova list --all-tenants' with a policy change: "compute:get_all_tenants": "role:special_role or is_admin:True" will not work as expected, The returned list of servers will not contain all instances of all tenants. We should support administrators who wish to enable this functionality in their policy.json. We need to fix this problem both in the v2 API and in the v2.1 as well. Deep in instance_get_all_by_filters_sort, there is a check which adds a filter of project_id or user_id if the context is NOT an admin context. So, the returned list will be a subset of all the instances in the database. To fix this scenario, the easy way is to call get_all with an elevated context to pass this check in instance_get_all_by_filters_sort. So in fixing the bug above, we need to fix the default policy so that all-tenants is available by default only to administrators. UpgradeImpact SecurityImpact DocImpact: --all-tenants will list all servers for non-admin APIImpact: --all-tenants will list all servers for non-admin Closes-Bug: #1464381 Change-Id: I6fe512ff00a0fde1c75d49efe8bfa5f3d2d34df6
This commit is contained in:

committed by
Davanum Srinivas (dims)

parent
045ee0336b
commit
55e63f83a7
@@ -10,7 +10,7 @@
|
||||
"compute:create:attach_volume": "",
|
||||
"compute:create:forced_host": "is_admin:True",
|
||||
"compute:get_all": "",
|
||||
"compute:get_all_tenants": "",
|
||||
"compute:get_all_tenants": "is_admin:True",
|
||||
"compute:start": "rule:admin_or_owner",
|
||||
"compute:stop": "rule:admin_or_owner",
|
||||
"compute:unlock_override": "rule:admin_api",
|
||||
@@ -185,6 +185,8 @@
|
||||
"network:delete_dns_domain": "",
|
||||
"network:attach_external_network": "rule:admin_api",
|
||||
|
||||
"os_compute_api:servers:detail:get_all_tenants": "is_admin:True",
|
||||
"os_compute_api:servers:index:get_all_tenants": "is_admin:True",
|
||||
"os_compute_api:servers:create_image:allow_volume_backed": "",
|
||||
"os_compute_api:servers:start": "rule:admin_or_owner",
|
||||
"os_compute_api:servers:stop": "rule:admin_or_owner",
|
||||
|
@@ -354,12 +354,14 @@ class ServersController(wsgi.Controller):
|
||||
except ValueError as err:
|
||||
raise exception.InvalidInput(six.text_type(err))
|
||||
|
||||
elevated = None
|
||||
if 'all_tenants' in search_opts:
|
||||
if is_detail:
|
||||
authorize(context, action="detail:get_all_tenants")
|
||||
else:
|
||||
authorize(context, action="index:get_all_tenants")
|
||||
del search_opts['all_tenants']
|
||||
elevated = context.elevated()
|
||||
else:
|
||||
if context.project_id:
|
||||
search_opts['project_id'] = context.project_id
|
||||
@@ -369,7 +371,7 @@ class ServersController(wsgi.Controller):
|
||||
limit, marker = common.get_limit_and_marker(req)
|
||||
sort_keys, sort_dirs = common.get_sort_params(req.params)
|
||||
try:
|
||||
instance_list = self.compute_api.get_all(context,
|
||||
instance_list = self.compute_api.get_all(elevated or context,
|
||||
search_opts=search_opts, limit=limit, marker=marker,
|
||||
want_objects=True, expected_attrs=['pci_devices'],
|
||||
sort_keys=sort_keys, sort_dirs=sort_dirs)
|
||||
|
@@ -203,11 +203,13 @@ class Controller(wsgi.Controller):
|
||||
except ValueError as err:
|
||||
raise exception.InvalidInput(six.text_type(err))
|
||||
|
||||
elevated = None
|
||||
if 'all_tenants' in search_opts:
|
||||
policy.enforce(context, 'compute:get_all_tenants',
|
||||
{'project_id': context.project_id,
|
||||
'user_id': context.user_id})
|
||||
del search_opts['all_tenants']
|
||||
elevated = context.elevated()
|
||||
else:
|
||||
if context.project_id:
|
||||
search_opts['project_id'] = context.project_id
|
||||
@@ -220,7 +222,7 @@ class Controller(wsgi.Controller):
|
||||
if self.ext_mgr.is_loaded('os-server-sort-keys'):
|
||||
sort_keys, sort_dirs = common.get_sort_params(req.params)
|
||||
try:
|
||||
instance_list = self.compute_api.get_all(context,
|
||||
instance_list = self.compute_api.get_all(elevated or context,
|
||||
search_opts=search_opts,
|
||||
limit=limit,
|
||||
marker=marker,
|
||||
|
@@ -845,6 +845,7 @@ class ServersControllerTest(ControllerTest):
|
||||
expected_attrs=None, sort_keys=None, sort_dirs=None):
|
||||
self.assertIsNotNone(filters)
|
||||
self.assertNotIn('project_id', filters)
|
||||
self.assertTrue(context.is_admin)
|
||||
return [fakes.stub_instance(100)]
|
||||
|
||||
self.stubs.Set(db, 'instance_get_all_by_filters_sort',
|
||||
|
@@ -822,6 +822,7 @@ class ServersControllerTest(ControllerTest):
|
||||
columns_to_join=None, use_slave=False):
|
||||
self.assertIsNotNone(filters)
|
||||
self.assertNotIn('project_id', filters)
|
||||
self.assertTrue(context.is_admin)
|
||||
return [fakes.stub_instance(100)]
|
||||
|
||||
self.stubs.Set(db, 'instance_get_all_by_filters',
|
||||
|
Reference in New Issue
Block a user