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
openstack · Sep 14, 2018 · 7c56588 · 7c56588
1 parent ffdd809
commit 7c56588
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 2 deletions.
diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
@@ -1167,8 +1167,10 @@ def _action_trigger_crash_dump(self, req, id, body):
 
 
 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
@@ -62,6 +62,20 @@
                 '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
@@ -182,6 +182,7 @@ def setUp(self):
         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 @@ def fake_get_all(context, search_opts=None,
         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
@@ -18,6 +18,7 @@
     "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
@@ -279,6 +279,7 @@ def setUp(self):
 "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
@@ -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.