From 41e6f02bf253e82fcc4baf4b77795ad831b58b43 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Wed, 30 Aug 2017 19:46:39 -0700 Subject: [PATCH] Pecan: process filters at end of hook pipeline Separate user-applied filters from policy-enforcement filters processing and put it at the end of the pipeline so users can't put filters on the API request that impact the fields available to the hooks. This prevents a filter excluding the ID field from breaking pagination. Change-Id: I05f4582fb1e8809740d473e24fa54483e040a6c8 Closes-Bug: #1714131 --- neutron/pecan_wsgi/app.py | 6 ++- neutron/pecan_wsgi/hooks/__init__.py | 2 + .../pecan_wsgi/hooks/policy_enforcement.py | 4 +- neutron/pecan_wsgi/hooks/userfilters.py | 53 +++++++++++++++++++ .../functional/pecan_wsgi/test_controllers.py | 9 +++- 5 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 neutron/pecan_wsgi/hooks/userfilters.py diff --git a/neutron/pecan_wsgi/app.py b/neutron/pecan_wsgi/app.py index ffad7be7b61..279b3f31b9a 100644 --- a/neutron/pecan_wsgi/app.py +++ b/neutron/pecan_wsgi/app.py @@ -25,9 +25,13 @@ def versions_factory(global_config, **local_config): def v2_factory(global_config, **local_config): + # Processing Order: + # As request enters lower priority called before higher. + # Reponse from controller is passed from higher priority to lower. app_hooks = [ - hooks.ExceptionTranslationHook(), # priority 100 + hooks.UserFilterHook(), # priority 90 hooks.ContextHook(), # priority 95 + hooks.ExceptionTranslationHook(), # priority 100 hooks.BodyValidationHook(), # priority 120 hooks.OwnershipValidationHook(), # priority 125 hooks.QuotaEnforcementHook(), # priority 130 diff --git a/neutron/pecan_wsgi/hooks/__init__.py b/neutron/pecan_wsgi/hooks/__init__.py index 700b26dd92e..e557988dab9 100644 --- a/neutron/pecan_wsgi/hooks/__init__.py +++ b/neutron/pecan_wsgi/hooks/__init__.py @@ -21,6 +21,7 @@ from neutron.pecan_wsgi.hooks import policy_enforcement from neutron.pecan_wsgi.hooks import query_parameters from neutron.pecan_wsgi.hooks import quota_enforcement from neutron.pecan_wsgi.hooks import translation +from neutron.pecan_wsgi.hooks import userfilters ExceptionTranslationHook = translation.ExceptionTranslationHook @@ -31,3 +32,4 @@ PolicyHook = policy_enforcement.PolicyHook QuotaEnforcementHook = quota_enforcement.QuotaEnforcementHook NotifierHook = notifier.NotifierHook QueryParametersHook = query_parameters.QueryParametersHook +UserFilterHook = userfilters.UserFilterHook diff --git a/neutron/pecan_wsgi/hooks/policy_enforcement.py b/neutron/pecan_wsgi/hooks/policy_enforcement.py index fdcfef0b40e..62a3aad62ee 100644 --- a/neutron/pecan_wsgi/hooks/policy_enforcement.py +++ b/neutron/pecan_wsgi/hooks/policy_enforcement.py @@ -211,10 +211,8 @@ class PolicyHook(hooks.PecanHook): # This routine will remove the fields that were requested to the # plugin for policy evaluation but were not specified in the # API request - user_fields = request.params.getall('fields') return dict(item for item in data.items() - if (item[0] not in fields_to_strip and - (not user_fields or item[0] in user_fields))) + if item[0] not in fields_to_strip) def _exclude_attributes_by_policy(self, context, controller, resource, collection, data): diff --git a/neutron/pecan_wsgi/hooks/userfilters.py b/neutron/pecan_wsgi/hooks/userfilters.py new file mode 100644 index 00000000000..6280e2a57e0 --- /dev/null +++ b/neutron/pecan_wsgi/hooks/userfilters.py @@ -0,0 +1,53 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from pecan import hooks + + +class UserFilterHook(hooks.PecanHook): + + # we do this at the very end to ensure user-defined filters + # don't impact things like pagination and notification hooks + priority = 90 + + def after(self, state): + user_fields = state.request.params.getall('fields') + if not user_fields: + return + try: + data = state.response.json + except ValueError: + return + resource = state.request.context.get('resource') + collection = state.request.context.get('collection') + if collection not in data and resource not in data: + return + is_single = resource in data + key = resource if resource in data else collection + if is_single: + data[key] = self._filter_item( + state.response.json[key], user_fields) + else: + data[key] = [ + self._filter_item(i, user_fields) + for i in state.response.json[key] + ] + state.response.json = data + + def _filter_item(self, item, fields): + return { + field: value + for field, value in item.items() + if field in fields + } diff --git a/neutron/tests/functional/pecan_wsgi/test_controllers.py b/neutron/tests/functional/pecan_wsgi/test_controllers.py index c8daff5aad1..ef4d518fd95 100644 --- a/neutron/tests/functional/pecan_wsgi/test_controllers.py +++ b/neutron/tests/functional/pecan_wsgi/test_controllers.py @@ -524,8 +524,9 @@ class TestPaginationAndSorting(test_functional.PecanFunctionalTest): if limit and marker: links_key = '%s_links' % collection self.assertIn(links_key, list_resp) - list_resp_ids = [item['id'] for item in list_resp[collection]] - self.assertEqual(expected_list, list_resp_ids) + if not fields or 'id' in fields: + list_resp_ids = [item['id'] for item in list_resp[collection]] + self.assertEqual(expected_list, list_resp_ids) if fields: for item in list_resp[collection]: for field in fields: @@ -535,6 +536,10 @@ class TestPaginationAndSorting(test_functional.PecanFunctionalTest): self._test_get_collection_with_pagination([self.networks[0]['id']], limit=1) + def test_get_collection_with_pagination_fields_no_pk(self): + self._test_get_collection_with_pagination([self.networks[0]['id']], + limit=1, fields=['name']) + def test_get_collection_with_pagination_limit_over_count(self): expected_ids = [network['id'] for network in self.networks] self._test_get_collection_with_pagination(