Allow ability for non admin users to use all filters on server list.

Adds a new policy rule "os_compute_api:servers:allow_all_filters"
to control whether a user can use all filters when listing servers.

Closes-bug: #1737050

Change-Id: Ia5504da9a00bad689766aeda20255e10b7629f63
This commit is contained in:
Sam Morrison 2017-12-08 10:15:53 +11:00 committed by Matt Riedemann
parent ffdd809838
commit 7c56588647
6 changed files with 63 additions and 2 deletions

View File

@ -1167,8 +1167,10 @@ class ServersController(wsgi.Controller):
def remove_invalid_options(context, search_options, allowed_search_options):
"""Remove search options that are not valid for non-admin API/context."""
if context.is_admin:
"""Remove search options that are not permitted unless policy allows."""
if context.can(server_policies.SERVERS % 'allow_all_filters',
fatal=False):
# Only remove parameters for sorting and pagination
for key in ('sort_key', 'sort_dir', 'limit', 'marker'):
search_options.pop(key, None)

View File

@ -62,6 +62,20 @@ rules = [
'path': '/servers/detail'
}
]),
policy.DocumentedRuleDefault(
SERVERS % 'allow_all_filters',
base.RULE_ADMIN_API,
"Allow all filters when listing servers",
[
{
'method': 'GET',
'path': '/servers'
},
{
'method': 'GET',
'path': '/servers/detail'
}
]),
policy.DocumentedRuleDefault(
SERVERS % 'show',
RULE_AOO,

View File

@ -182,6 +182,7 @@ class ControllerTest(test.TestCase):
self.ips_controller = ips.IPsController()
policy.reset()
policy.init()
self.addCleanup(policy.reset)
fake_network.stub_out_nw_api_get_instance_nw_info(self)
@ -1244,6 +1245,43 @@ class ServersControllerTest(ControllerTest):
self.assertEqual(1, len(servers))
self.assertEqual(uuids.fake, servers[0]['id'])
def test_get_servers_admin_filters_as_user_with_policy_override(self):
"""Test getting servers by admin-only or unknown options when
context is not admin but policy allows.
"""
server_uuid = uuids.fake
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None):
self.assertIsNotNone(search_opts)
# Allowed by user
self.assertIn('name', search_opts)
self.assertIn('terminated_at', search_opts)
# OSAPI converts status to vm_state
self.assertIn('vm_state', search_opts)
# Allowed only by admins with admin API on
self.assertIn('ip', search_opts)
self.assertNotIn('unknown_option', search_opts)
return objects.InstanceList(
objects=[fakes.stub_instance_obj(100, uuid=server_uuid)])
rules = {
"os_compute_api:servers:index": "project_id:fake",
"os_compute_api:servers:allow_all_filters": "project_id:fake",
}
policy.set_rules(oslo_policy.Rules.from_dict(rules))
self.mock_get_all.side_effect = fake_get_all
query_str = ("name=foo&ip=10.*&status=active&unknown_option=meow&"
"terminated_at=^2016-02-01.*")
req = self.req('/fake/servers?%s' % query_str)
servers = self.controller.index(req)['servers']
self.assertEqual(len(servers), 1)
self.assertEqual(servers[0]['id'], server_uuid)
def test_get_servers_allows_ip(self):
"""Test getting servers by ip."""
def fake_get_all(context, search_opts=None,

View File

@ -18,6 +18,7 @@ policy_data = """
"context_is_admin": "role:admin or role:administrator",
"os_compute_api:servers:show:host_status": "",
"os_compute_api:servers:allow_all_filters": "",
"os_compute_api:servers:migrations:force_complete": "",
"os_compute_api:os-admin-actions:inject_network_info": "",
"os_compute_api:os-admin-actions:reset_network": "",

View File

@ -279,6 +279,7 @@ class RealRolePolicyTestCase(test.NoDBTestCase):
"os_compute_api:servers:create:forced_host",
"os_compute_api:servers:detail:get_all_tenants",
"os_compute_api:servers:index:get_all_tenants",
"os_compute_api:servers:allow_all_filters",
"os_compute_api:servers:show:host_status",
"os_compute_api:servers:migrations:force_complete",
"os_compute_api:servers:migrations:delete",

View File

@ -0,0 +1,5 @@
---
features:
- |
A new policy rule ``os_compute_api:servers:allow_all_filters`` has been
added to control whether a user can use all filters when listing servers.