From 7c56588647be64a2248b1f37d40369765bc6b977 Mon Sep 17 00:00:00 2001 From: Sam Morrison Date: Fri, 8 Dec 2017 10:15:53 +1100 Subject: [PATCH] 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 --- nova/api/openstack/compute/servers.py | 6 ++- nova/policies/servers.py | 14 +++++++ .../api/openstack/compute/test_serversV21.py | 38 +++++++++++++++++++ nova/tests/unit/fake_policy.py | 1 + nova/tests/unit/test_policy.py | 1 + ...e_all_filters-policy-3ddfe1885056f0ca.yaml | 5 +++ 6 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/add-server-use_all_filters-policy-3ddfe1885056f0ca.yaml diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 8736e00651b8..7d80fea1da14 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -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) diff --git a/nova/policies/servers.py b/nova/policies/servers.py index e140e13f9c2b..597f210c2a11 100644 --- a/nova/policies/servers.py +++ b/nova/policies/servers.py @@ -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, diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index d8e0723a3cce..bea58a5941fe 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -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, diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 587903cf6d7d..ab4dbb94a3b1 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -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": "", diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 0e4fc19e14aa..dc887727ed7c 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -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", diff --git a/releasenotes/notes/add-server-use_all_filters-policy-3ddfe1885056f0ca.yaml b/releasenotes/notes/add-server-use_all_filters-policy-3ddfe1885056f0ca.yaml new file mode 100644 index 000000000000..d15e3885fae4 --- /dev/null +++ b/releasenotes/notes/add-server-use_all_filters-policy-3ddfe1885056f0ca.yaml @@ -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.