Replace JSON Body middleware with flask-native func
Replace the JSON Body middleware with flask-native before-request function. The body filtering and storing data in request.environ['openstack.params'] was not used in the code base and has been dropped. Test Changes: * JSON Body middleware has been removed, no testing of the removed code * JSON Body Before Request Method has been implemented and associated testing (mirroring the JSON Body middleware code). * Test entry points no longer looks for JSON Body middleware. Change-Id: I84491865870b6bf2b8f094b524ee8b77510f0054 Partial-Bug: #1776504
This commit is contained in:
Morgan Fainberg
parent
35c9bb7eff
commit
ee9b035cf1
|
|
@ -13,66 +13,13 @@
|
|||
# under the License.
|
||||
|
||||
from oslo_log import log
|
||||
from oslo_serialization import jsonutils
|
||||
|
||||
from keystone.common import wsgi
|
||||
from keystone import exception
|
||||
|
||||
|
||||
LOG = log.getLogger(__name__)
|
||||
|
||||
|
||||
class JsonBodyMiddleware(wsgi.Middleware):
|
||||
"""Middleware to allow method arguments to be passed as serialized JSON.
|
||||
|
||||
Accepting arguments as JSON is useful for accepting data that may be more
|
||||
complex than simple primitives.
|
||||
|
||||
Filters out the parameters `self`, `context` and anything beginning with
|
||||
an underscore.
|
||||
|
||||
"""
|
||||
|
||||
def process_request(self, request):
|
||||
# Abort early if we don't have any work to do
|
||||
params_json = request.body
|
||||
if not params_json:
|
||||
return
|
||||
|
||||
# Reject unrecognized content types. Empty string indicates
|
||||
# the client did not explicitly set the header
|
||||
if request.content_type not in ('application/json', ''):
|
||||
e = exception.ValidationError(attribute='application/json',
|
||||
target='Content-Type header')
|
||||
return wsgi.render_exception(e, request=request)
|
||||
|
||||
params_parsed = {}
|
||||
try:
|
||||
params_parsed = jsonutils.loads(params_json)
|
||||
except ValueError:
|
||||
e = exception.ValidationError(attribute='valid JSON',
|
||||
target='request body')
|
||||
return wsgi.render_exception(e, request=request)
|
||||
finally:
|
||||
if not params_parsed:
|
||||
params_parsed = {}
|
||||
|
||||
if not isinstance(params_parsed, dict):
|
||||
e = exception.ValidationError(attribute='valid JSON object',
|
||||
target='request body')
|
||||
return wsgi.render_exception(e, request=request)
|
||||
|
||||
params = {}
|
||||
for k, v in params_parsed.items():
|
||||
if k in ('self', 'context'):
|
||||
continue
|
||||
if k.startswith('_'):
|
||||
continue
|
||||
params[k] = v
|
||||
|
||||
request.environ[wsgi.PARAMS_ENV] = params
|
||||
|
||||
|
||||
class NormalizingFilter(wsgi.Middleware):
|
||||
"""Middleware filter to handle URL normalization."""
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import werkzeug.wsgi
|
|||
|
||||
import keystone.api
|
||||
from keystone.common import wsgi as keystone_wsgi
|
||||
from keystone.server.flask.request_processing import json_body
|
||||
|
||||
# TODO(morgan): _MOVED_API_PREFIXES to be removed when the legacy dispatch
|
||||
# support is removed.
|
||||
|
|
@ -179,6 +180,11 @@ def application_factory(name='public'):
|
|||
# NOTE(morgan): The Flask App actually dispatches nothing until we migrate
|
||||
# some routers to Flask-Blueprints, it is simply a placeholder.
|
||||
app = flask.Flask(name)
|
||||
|
||||
# Add core before request functions
|
||||
app.before_request(json_body.json_body_before_request)
|
||||
|
||||
# Add core after request functions
|
||||
app.after_request(_add_vary_x_auth_token_header)
|
||||
|
||||
# NOTE(morgan): Configure the Flask Environment for our needs.
|
||||
|
|
|
|||
|
|
@ -54,9 +54,6 @@ _APP_MIDDLEWARE = (
|
|||
_Middleware(namespace='keystone.server_middleware',
|
||||
ep='url_normalize',
|
||||
conf={}),
|
||||
_Middleware(namespace='keystone.server_middleware',
|
||||
ep='json_body',
|
||||
conf={}),
|
||||
_Middleware(namespace='keystone.server_middleware',
|
||||
ep='osprofiler',
|
||||
conf={}),
|
||||
|
|
|
|||
|
|
@ -0,0 +1,54 @@
|
|||
# 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.
|
||||
|
||||
# Before request processing for JSON Body enforcement
|
||||
|
||||
import flask
|
||||
from werkzeug import exceptions as werkzeug_exceptions
|
||||
|
||||
from keystone import exception
|
||||
from keystone.i18n import _
|
||||
|
||||
|
||||
def json_body_before_request():
|
||||
"""Enforce JSON Request Body."""
|
||||
# TODO(morgan): Allow other content-types when OpenAPI Doc or improved
|
||||
# federation is implemented for known/valid paths. This function should
|
||||
# be removed long term.
|
||||
|
||||
# exit if there is nothing to be done, (no body)
|
||||
if not flask.request.get_data():
|
||||
return None
|
||||
|
||||
try:
|
||||
# flask does loading for us for json, use the flask default loader
|
||||
# in the case that the data is *not* json or a dict, we should see a
|
||||
# raise of werkzeug.exceptions.BadRequest, re-spin this to the keystone
|
||||
# ValidationError message (as expected by our contract)
|
||||
|
||||
# Explicitly check if the content is supposed to be json.
|
||||
if (flask.request.is_json
|
||||
or flask.request.headers['Content-Type'] == ''):
|
||||
json_decoded = flask.request.get_json(force=True)
|
||||
if not isinstance(json_decoded, dict):
|
||||
# In the case that the returned value was not a dict, force
|
||||
# a raise that will be caught the same way that a Decode error
|
||||
# would be handled.
|
||||
raise werkzeug_exceptions.BadRequest(
|
||||
_('resulting JSON load was not a dict'))
|
||||
else:
|
||||
raise exception.ValidationError(attribute='application/json',
|
||||
target='Content-Type header')
|
||||
|
||||
except werkzeug_exceptions.BadRequest:
|
||||
raise exception.ValidationError(attribute='valid JSON',
|
||||
target='request body')
|
||||
|
|
@ -26,6 +26,7 @@ from keystone.common import rbac_enforcer
|
|||
import keystone.conf
|
||||
from keystone import exception
|
||||
from keystone.server.flask import common as flask_common
|
||||
from keystone.server.flask.request_processing import json_body
|
||||
from keystone.tests.unit import rest
|
||||
|
||||
|
||||
|
|
@ -654,3 +655,44 @@ class TestKeystoneFlaskCommon(rest.RestfulTestCase):
|
|||
self.assertTrue(ref['links']['self'].startswith(
|
||||
'https://localhost/v3/%s' % view_arg)
|
||||
)
|
||||
|
||||
def test_json_body_before_req_func_valid_json(self):
|
||||
with self.test_request_context(
|
||||
headers={'Content-Type': 'application/json'},
|
||||
data='{"key": "value"}'):
|
||||
# No exception should be raised, everything is happy.
|
||||
json_body.json_body_before_request()
|
||||
|
||||
def test_json_body_before_req_func_invalid_json(self):
|
||||
with self.test_request_context(
|
||||
headers={'Content-Type': 'application/json'},
|
||||
data='invalid JSON'):
|
||||
# keystone.exception.ValidationError should be raised
|
||||
self.assertRaises(exception.ValidationError,
|
||||
json_body.json_body_before_request)
|
||||
|
||||
def test_json_body_before_req_func_no_content_type(self):
|
||||
# Unset
|
||||
with self.test_request_context(data='{"key": "value"}'):
|
||||
# No exception should be raised, everything is happy.
|
||||
json_body.json_body_before_request()
|
||||
|
||||
# Explicitly set to ''
|
||||
with self.test_request_context(
|
||||
headers={'Content-Type': ''}, data='{"key": "value"}'):
|
||||
# No exception should be raised, everything is happy.
|
||||
json_body.json_body_before_request()
|
||||
|
||||
def test_json_body_before_req_func_unrecognized_content_type(self):
|
||||
with self.test_request_context(
|
||||
headers={'Content-Type': 'unrecognized/content-type'},
|
||||
data='{"key": "value"'):
|
||||
# keystone.exception.ValidationError should be raised
|
||||
self.assertRaises(exception.ValidationError,
|
||||
json_body.json_body_before_request)
|
||||
|
||||
def test_json_body_before_req_func_unrecognized_conten_type_no_body(self):
|
||||
with self.test_request_context(
|
||||
headers={'Content-Type': 'unrecognized/content-type'}):
|
||||
# No exception should be raised, everything is happy.
|
||||
json_body.json_body_before_request()
|
||||
|
|
|
|||
|
|
@ -23,7 +23,6 @@ class TestEntryPoints(test.TestCase):
|
|||
'build_auth_context',
|
||||
'cors',
|
||||
'debug',
|
||||
'json_body',
|
||||
'request_id',
|
||||
'sizelimit',
|
||||
'url_normalize',
|
||||
|
|
|
|||
|
|
@ -103,57 +103,6 @@ class MiddlewareRequestTestBase(unit.TestCase):
|
|||
return self._do_middleware_response(*args, **kwargs).request
|
||||
|
||||
|
||||
class JsonBodyMiddlewareTest(MiddlewareRequestTestBase):
|
||||
|
||||
MIDDLEWARE_CLASS = middleware.JsonBodyMiddleware
|
||||
|
||||
def test_request_with_params(self):
|
||||
headers = {'Content-Type': 'application/json'}
|
||||
params = '{"arg1": "one", "arg2": ["a"]}'
|
||||
req = self._do_middleware_request(params=params,
|
||||
headers=headers,
|
||||
method='post')
|
||||
self.assertEqual({"arg1": "one", "arg2": ["a"]},
|
||||
req.environ[wsgi.PARAMS_ENV])
|
||||
|
||||
def test_malformed_json(self):
|
||||
headers = {'Content-Type': 'application/json'}
|
||||
self._do_middleware_response(params='{"arg1": "on',
|
||||
headers=headers,
|
||||
method='post',
|
||||
status=http_client.BAD_REQUEST)
|
||||
|
||||
def test_not_dict_body(self):
|
||||
headers = {'Content-Type': 'application/json'}
|
||||
resp = self._do_middleware_response(params='42',
|
||||
headers=headers,
|
||||
method='post',
|
||||
status=http_client.BAD_REQUEST)
|
||||
|
||||
self.assertIn('valid JSON object', resp.json['error']['message'])
|
||||
|
||||
def test_no_content_type(self):
|
||||
headers = {'Content-Type': ''}
|
||||
params = '{"arg1": "one", "arg2": ["a"]}'
|
||||
req = self._do_middleware_request(params=params,
|
||||
headers=headers,
|
||||
method='post')
|
||||
self.assertEqual({"arg1": "one", "arg2": ["a"]},
|
||||
req.environ[wsgi.PARAMS_ENV])
|
||||
|
||||
def test_unrecognized_content_type(self):
|
||||
headers = {'Content-Type': 'text/plain'}
|
||||
self._do_middleware_response(params='{"arg1": "one", "arg2": ["a"]}',
|
||||
headers=headers,
|
||||
method='post',
|
||||
status=http_client.BAD_REQUEST)
|
||||
|
||||
def test_unrecognized_content_type_without_body(self):
|
||||
headers = {'Content-Type': 'text/plain'}
|
||||
req = self._do_middleware_request(headers=headers)
|
||||
self.assertEqual({}, req.environ.get(wsgi.PARAMS_ENV, {}))
|
||||
|
||||
|
||||
class AuthContextMiddlewareTest(test_backend_sql.SqlTests,
|
||||
MiddlewareRequestTestBase):
|
||||
|
||||
|
|
|
|||
|
|
@ -196,5 +196,4 @@ keystone.server_middleware =
|
|||
request_id = oslo_middleware:RequestId
|
||||
build_auth_context = keystone.middleware:AuthContextMiddleware
|
||||
token_auth = keystone.middleware:TokenAuthMiddleware
|
||||
json_body = keystone.middleware:JsonBodyMiddleware
|
||||
debug = oslo_middleware:Debug
|
||||
|
|
|
|||
Loading…
Reference in New Issue